fix(ci): stop the path-filter comment self-triggering the to-do scanner - #6765
Conversation
The shared scanner keys on the uppercase marker wherever it appears in a comment, so prose that merely names the marker files an issue against itself. The explanatory comment on the todos-contract path filter did exactly that when it landed, producing ksail#6763 — an issue whose entire body was the comment block, with no work behind it. Spell the marker as "to-do" in that prose, matching the convention the shared workflow in devantler-tech/actions already applies to its own source for the same reason, and pin the trap with a test so it cannot come back silently. The guard is deliberately narrow. Filing issues from real markers is the scanner's job, so it asserts only that this one explanatory block does not spell the marker — it does not ban the marker repository-wide. The test assembles the marker at runtime rather than writing it out, because spelling it in the test file would reintroduce the defect being pinned, and it requires the comment block to be non-empty so deleting the comment cannot make it pass vacuously. Fixes #6763 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
✅MegaLinter analysis: Success✅ Linters with no issuesactionlint, bash-exec, git_diff, hadolint, jscpd, jsonlint, lychee, markdown-table-formatter, markdownlint, prettier, prettier, shellcheck, shfmt, stylelint, syft, trivy-sbom, trufflehog, v8r, v8r, yamllint Notices
See detailed reports in MegaLinter artifacts
|
CI has settled: 65 checks green. The single red is @coderabbitai full review |
|
✅ Action performedFull review finished. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 2 included reviews per hour; 0 remain after this review. 📜 Recent review details⏰ Context from checks skipped due to timeout. (6)
🧰 Additional context used📓 Path-based instructions (4)Use Go 1.26.1 or newer, matching the version declared in `go.mod`.📄 CodeRabbit inference engine (AGENTS.md) Files:
Generated files must not be hand-edited; run `make generate` as the canonical regeneration command.📄 CodeRabbit inference engine (AGENTS.md) Files:
Validate workflow changes with `mega-linter-runner -f go`; MegaLinter runs `actionlint` for GitHub Actions workflows.📄 CodeRabbit inference engine (AGENTS.md) Files:
Add regression tests for confident bug fixes and run flaky-test candidates repeatedly with `go test -run -count=10 ./...`.📄 CodeRabbit inference engine (AGENTS.md) Files:
🔇 Additional comments (2)
📝 WalkthroughWalkthroughThe workflow comment now uses lowercase, hyphenated “to-do” wording and explains why it avoids the uppercase scanner marker. A Go regression test reads the comment block above Merge Risk: ⚪ Minimal · up to This change narrowly prevents a CI comment from triggering the to-do scanner while preserving real marker handling, and adds a regression test; no actionable merge-blocking risk remains beyond normal checks and review. Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error)
✅ Passed checks (4 passed)
Full details: Linked Issues checkExplanation The changes do not implement the primary objectives in issue Resolution Implement the Full details: Docstring CoverageExplanation Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 1 files. (1 skipped: 1 unsupported.) Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Explains what the test pins — the bare reusable-workflow call, the immutable pin, and why the ignore pattern is asserted exactly rather than loosely. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The doc comment added in the previous commit opened with the function's own name, which embeds the uppercase marker the scanner keys on — the exact trap the sibling test in this file exists to pin. Reworded so the comment carries no marker, and said why it does not lead with the name. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@coderabbitai full review |
|
✅ Action performedFull review finished. |
Readiness record @
|
| State | ci.yaml filter comment |
TestCIFilterCommentDoesNotSpellTheScannerMarker |
|---|---|---|
| GREEN (as shipped) | says to-do |
PASS |
| RED (ablated) | marker restored | FAIL at todos_workflow_test.go:119 |
The ablation fixture was asserted to have actually built (git diff --numstat → 1+/1-, the reworded
line flipping back), and the failure was checked to be the intended assertion rather than a sibling
conjunct — it is the NotContains on the marker, reporting
comment must not spell the scanner marker: doing so files a spurious issue (ksail#6763). Reverted
afterwards and re-run green, so the tree is unmodified.
Census, with a positive control. Marker occurrences on comment lines: ci.yaml → 0,
todos_workflow_test.go → 0. The control (a fixture comment containing the marker) matched 1, so the
zero is a real absence rather than a broken pattern.
Worth recording explicitly, because a naive census reads as a failure here: the marker still appears
3× in ci.yaml and 2× in the test file — as job names, an echo string, and the Go test-function
identifier. None is a comment, and the scanner keys on the marker in comments, so those are
correctly untouched. The test scopes itself the same way: it walks only the contiguous comment block
immediately above the todos-contract: filter key, which is exactly the block that produced #6763.
The test also carries a proper guard against passing vacuously —
require.NotEmpty(t, block, "the todos-contract filter must keep its explanatory comment") — so
deleting the comment fails the test rather than silently satisfying it.
Pentad: checks green · 0 unresolved threads · 0 non-thread findings · no conflict ·
green_review=cr@88aea1426 (CodeRabbit completed 05:04:07Z, "No actionable comments were generated";
the only body section is the excluded informational 🔇 Additional comments).
Promoting and merging on that basis. Closes the untyped-residual issue #6763.
1 participant
Why
Our to-do scanner files a GitHub issue whenever it sees the uppercase marker in a comment. That is exactly what it is for — but it means a comment that merely talks about the scanner files an issue about itself. One landed yesterday and produced #6763: an issue whose entire body was a comment block, with no work behind it. It was also the only issue in the whole portfolio missing an Issue Type, so it showed up as triage debt on top of being noise.
What
Refers to the marker as "to-do" in that one explanatory comment — the same convention the shared workflow in
devantler-tech/actionsalready applies to its own source, for this exact reason — and adds a test so the trap cannot come back unnoticed.The guard is deliberately narrow: filing issues from real markers is the scanner's job, so it checks only that this one comment block stays clean rather than banning the marker repository-wide.
Fixes #6763