Correct repo-gate-exclude-globs Description: eol Never Reads the Tracked List - #960
Merged
Merged
Conversation
…ked List Real finding from coderabbitai on PR #959 (the develop -> main promotion PR carrying #958's exclude-globs work), reproduced before the fix. `repo-gate-exclude-globs`'s description claimed it narrows what `sha-pin`, `eol`, and `eol-coverage` scan, but `check_eol()` never reads its `files` parameter at all: it only compares `.editorconfig` and `.gitattributes` content directly. Only `sha-pin` (via `workflow_files(files)`) and the tracked-path portion of `eol-coverage` (via its shebang scan) actually narrow with the exclude list. ## The fix `.github/workflows/validate-task.yml`: dropped `eol` from the description, per CodeRabbit's own proposed wording. ## Verified `python3 scripts/repo_gate.py`, `prose_lint.py --diff origin/develop`, and a YAML parse of the edited file all pass clean. The change is a one-line description string; no behavior changed.
Code Review by Qodo🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)
Great, no issues found!Qodo reviewed your code and found no material issues that require reviewTip of the day💡 Did you know, you can turn these tips off under Display preferences |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 10 included reviews per hour; 5 remain after this review. 📝 WalkthroughWalkthroughThe workflow input description now explains that ChangesRepository gate documentation
Estimated code review effort: 1 (Trivial) | ~2 minutes Merge Risk: ⚪ Minimal · up to This change corrects the documented scope of the repository gate without changing execution behavior; no actionable merge-blocking risk remains after normal checks and review. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
PR Summary by QodoFix validate-task exclude-globs input description (eol not affected)
AI Description
High-Level Assessment
Files changed (1)
|
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The change is a one-line description correction that aligns documented behavior with the actual repo-gate implementation and does not alter workflow behavior.
Pull request overview
Updates the reusable validation workflow’s repo-gate-exclude-globs input description so it accurately reflects which repo-gate checks are actually narrowed by the exclude list.
Changes:
- Removes the incorrect claim that
eolis narrowed byrepo-gate-exclude-globs. - Clarifies that narrowing applies to
sha-pinand the tracked-path portion ofeol-coverage(viarepo_gate.py --exclude).
File summaries
| File | Description |
|---|---|
| .github/workflows/validate-task.yml | Corrects the repo-gate-exclude-globs input description to match repo-gate behavior (exclude affects sha-pin and tracked-file eol-coverage, not eol). |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
ptr727
added a commit
that referenced
this pull request
Aug 23, 2026
Real, HIGH-severity finding from qodo-code-review on PR #959 (the develop -> main promotion PR carrying #958/#960's exclude-globs work). `tracked()` printed git's own stderr on a nonzero exit but still parsed and returned `result.stdout` regardless. `main()` only checks `if not files:`, so a failed `git ls-files` call that happened to emit any stdout before failing would be read as a successful, complete scan, letting every check run against a silently incomplete file list. ## The fix `tracked()` now returns `[]` unconditionally on a nonzero exit, after printing stderr, never falling through to parse stdout on that path. ## Verified Added `test_a_failed_call_is_never_trusted_even_with_nonempty_stdout` (a mocked nonzero exit carrying non-empty stdout, asserting `tracked()` still returns `[]`). Full test suite (807 tests), ruff check and format, mypy, `repo_gate.py` against this checkout, and `prose_lint.py --diff origin/develop` all pass clean. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Bug Fixes** - Improved repository checks to safely handle failures when retrieving tracked files. - Prevented partial or invalid file results from being processed after a command failure. - Added clearer failure details when command error output is unavailable. - **Tests** - Added regression coverage for failed file-list retrieval, including cases with partial output and missing error details. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Real finding from coderabbitai on PR #959 (the develop -> main promotion PR carrying #958's exclude-globs work), reproduced before the fix.
repo-gate-exclude-globs's description claimed it narrows whatsha-pin,eol, andeol-coveragescan, butcheck_eol()never reads itsfilesparameter at all: it only compares.editorconfigand.gitattributescontent directly. Onlysha-pin(viaworkflow_files(files)) and the tracked-path portion ofeol-coverage(via its shebang scan) actually narrow with the exclude list.The fix
.github/workflows/validate-task.yml: droppedeolfrom the description, per CodeRabbit's own proposed wording.Verified
python3 scripts/repo_gate.py,prose_lint.py --diff origin/develop, and a YAML parse of the edited file all pass clean. The change is a one-line description string; no behavior changed.🤖 Generated with Claude Code
Summary by CodeRabbit