fix(ci): normalize committed CRLF to LF and name the failure for what it is - #9799
Merged
Conversation
… it is Twenty files were committed with CRLF and the repo had no .gitattributes. The `changes` job runs `git diff --check`, and git's default core.whitespace is `blank-at-eol` with `cr-at-eol` off, so a carriage return at end-of-line counts as trailing whitespace. Any PR adding a line to one of those files failed CI pointing at lines whose visible content is spotless, with nothing in the log saying "CRLF" — it hit #9787 (run 30436384202) on two lines of inline-suggestion-anchor.ts. Convert all twenty to LF via `git add --renormalize`, byte-identical otherwise (`git diff -w` reports only the two new files below), and pin the repo with `* text=auto eol=lf`. `text=auto` rather than a bare `text` is load-bearing: five .ts files embed NUL bytes as untrusted-text and sanitizer fixtures, and auto-detection classifies them binary and passes them through untouched. The whitespace step now names CRLF before falling through to the generic check. .gitattributes stops CRLF entering through `git add`, but checkin filters don't run on blobs written directly (web editor, Contents API) or on a merge of a branch cut earlier, so the diagnostic still has a job. It is scoped to added lines, so it fires on exactly what `git diff --check` already flagged — a relabel, not a new failure mode. Closes #9798
Contributor
|
Warning ⏸️ LoopOver review result - manual review recommendedReview updated: 2026-07-29 10:26:39 UTC
Review summary Nits — 7 non-blocking
Decision drivers
Context & advisory signals — never blocks the verdict
Linked issue satisfactionAddressed Review context
Contributor next steps
Signal definitions
🧪 Chat with LoopOverAsk LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.
Full command reference: https://loopover.ai/docs/loopover-commands 🧪 Experimental — new and may change. Decision record
🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed 💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →. Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.
|
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
loopover-ui | 267a99c | Commit Preview URL Branch Preview URL |
Jul 29 2026, 09:45 AM |
JSONbored
marked this pull request as ready for review
July 29, 2026 09:42
Contributor
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #9799 +/- ##
==========================================
+ Coverage 90.33% 90.46% +0.12%
==========================================
Files 918 918
Lines 113936 113936
Branches 26975 27050 +75
==========================================
+ Hits 102926 103074 +148
+ Misses 9681 9533 -148
Partials 1329 1329
Flags with carried forward coverage won't be shown. Click here to find out more.
|
…engine suite `buildIssueQualityReport` is re-exported from the engine barrel (src/index.ts) but this package's own node:test suite never called it. `npm run engine:coverage` therefore saw the module load — top-level constants only — and reported it LF:313 LH:96 with BRF:0, i.e. no function-level data at all, while the root vitest suite covered the same file 87/87 lines and 143/143 branches. Two uploads disagreeing about the same file is invisible while patch coverage only reads changed lines, because nothing changes all 313 at once. Renormalizing the file's line endings did, and the merged report capped at ~57%: of the 217 lines the engine flag called uncovered, 84 were rescued by vitest and 133 were function-signature and closing-brace lines v8 never lists as statements, so nothing could rescue them. Cover it where this package is actually graded rather than leaning on the vitest duplicate — the outcome scripts/engine-coverage.ts (#9064) was written to encourage. The file now reports 313/313 lines and 163/163 branches under the engine model. 17 cases covering the lane arms (issue_discovery / direct_pr / split / unknown), every bounty lifecycle, duplicate and invalid labelling, self-solved loops, maintainer-authored vs maintainer-WIP, linkedPrs back-references for both open and merged PRs, collision risk tiers, sort order, the lifecycle and report caps, and the null-repo / unparseable-date degradation paths.
This was referenced Jul 29, 2026
JSONbored
added a commit
that referenced
this pull request
Jul 29, 2026
`notify.after_n_builds` fires on the Nth upload to ARRIVE, and arrival order does not match importance. validate-code's rees / control-plane / engine reports land in ~2 minutes; validate-tests is still producing the backend lcov that covers most of src/**. So codecov/patch posted a conclusion computed from a partial report — on #9799 it went pass, then fail, while validate-tests was still pending, with Codecov's own comment printing `backend | BASE 1 | HEAD 0`. That is not cosmetic: the gate closes a contributor PR on any red check, codecov/patch included, so a transient partial-report red can close a PR that has no defect. `require_ci_to_pass` gates the final verdict but does not suppress the interim status, and `wait_for_ci` (already on by default) did not prevent it either. Raising the count cannot fix this — the reasoning preserved on `after_n_builds` explains why a floor above the guaranteed minimum leaves codecov/patch permanently un-posted on PRs that produce fewer uploads — because a count cannot express "wait for the backend upload specifically". `manual_trigger` can: Codecov sends nothing until `codecovcli send-notifications` runs, and the new codecov-notify job runs it once, after every uploading job has finished. The notifier is deliberately unable to fail a PR. It is not in `validate`'s needs, and every failure path — a missing pipx, a failed install, a rejected notification — warns and exits 0. If it never runs, no status posts, the check set stays incomplete and the gate waits: it cannot merge unreviewed and cannot false-close, which is the whole point. `!cancelled()` rather than `always()` so a run superseded by a newer push does not notify on a stale sha, and the job skips entirely when neither upload job ran, since notifying on a commit Codecov has no reports for is an error rather than a verdict. Closes #9801 Co-authored-by: JSONbored <aetherealdev@gmail.com>
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
1 participant
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.
Summary
Twenty files were committed on
mainwith CRLF line endings, and the repo had no.gitattributes.The
changesjob runsgit diff --check, and git's defaultcore.whitespaceisblank-at-eolwithcr-at-eoloff — so a carriage return at end-of-line genuinely is trailing whitespace to it. AnyPR that added a line to one of those files failed CI pointing at lines whose visible content is
spotless, with nothing in the output ever saying "CRLF".
That is not hypothetical. It hit #9787 (run 30436384202):
Neither line has trailing whitespace. Diagnosing it takes cloning the branch and hexdumping the file.
Three changes:
All twenty files converted to LF via
git add --renormalize .. Content is otherwisebyte-identical —
git diff -w --staton this PR reports only the two new files:Independently confirmed per file by stripping CR from each
origin/mainblob and comparing SHAsagainst the normalized blob — all twenty identical.
The issue listed eleven; a repo-wide
git grep -lI $'\r' origin/mainfound nine more underreview-enrichment/. They are included:.gitattributesdeclareseol=lffor the whole tree, soleaving them CRLF would leave the repo contradicting its own attribute and hand the next
--renormalizea surprise diff.A root
.gitattributespinning the repo to LF. It is* text=auto eol=lf, deliberately not* text eol=lf— five.tsfiles embed NUL bytes on purpose as untrusted-text/sanitizer fixtures(
test/unit/mcp-untrusted-text.test.ts,src/services/maintainer-recap-routing.ts,review-enrichment/src/analyzers/ci-check-signals.ts,scripts/counterfactual-replay-core.ts,packages/loopover-engine/test/governor-ledger.test.ts).text=autoleaves git's binary detectionin charge, so those pass through untouched; forcing
textwould rewrite them. Verified: a scratchgit add --renormalize .staged exactly the twenty CRLF files and nothing else — all five NULfixtures and the three
apps/loopover-ui/public/image assets were left alone.The whitespace step now names CRLF before falling through to
git diff --check. Same input,different message:
src/review/inline-comment-label.ts:1: trailing whitespace.+/** Pure inline-comment severity/category label rendering (#2149 / #1958). */::error::CRLF line endings in added lines -- this repo is LF-only (see .gitattributes)src/review/inline-comment-label.ts (17 added line(s) ending in CRLF)Fix: git add --renormalize <file> && git commit --amend.gitattributesstops CRLF entering throughgit add, so this is unreachable from an ordinarycommit — I confirmed that by trying, and git normalized it away. It stays worth having because
checkin filters don't run on blobs written directly (the GitHub web editor, the Contents API) or on
a merge of a branch cut before
.gitattributeslanded. Both were exercised against a realunnormalized blob; the table above is that run's actual output. It is scoped to added lines, so
it fires on exactly what
git diff --checkwould have flagged anyway — a relabel, not a new failuremode.
Twenty-two files is over the usual budget, but twenty of them are line-ending-only with zero
hand-written change (see the
-wstat above). The reviewable surface is.gitattributesand theworkflow step: 50 lines.
Follow-up found while validating: the engine flag reported this diff at 30.67%
codecov/patchfailed onpackages/loopover-engine/src/signals/issue-quality-report.ts, and thecause was not a coverage gap — the root vitest suite already covers that file 87/87 lines and
143/143 branches.
buildIssueQualityReportis re-exported from the engine barrel but this package'sown
node:testsuite never called it, sonpm run engine:coveragesaw the module load (top-levelconstants only) and emitted
LF:313 LH:96 BRF:0— no function-level data at all.Two uploads disagreeing about the same file is invisible while patch coverage only reads changed
lines, because nothing changes all 313 at once. Renormalizing the line endings did. Of the 217 lines
the engine flag called uncovered, 84 are hit by vitest and resolve on union; the other 133 are
function-signature and closing-brace lines that v8 never lists as statements, so nothing could
rescue them — capping the merged report near 57% no matter how well the root suite tested it.
Fixed by covering the module in the suite that actually grades this package, which is the outcome
scripts/engine-coverage.ts(#9064) was written to encourage — "deepening the real suite" ratherthan "duplicating a thin vitest test."
BRF:0(no data)Engine suite: 867 → 884 tests, all passing.
Separately:
codecov/patchposts a red verdict mid-runWorth its own issue rather than a change here.
codecov.ymlsetsnotify.after_n_builds: 1, whichfires the verdict on the first upload to land — on this PR that was
rees, thenengine, bothfrom
validate-code, whilevalidate-tests(which carries thebackendupload) was still running.So a red
codecov/patchappeared computed from a partial report.That is not cosmetic: per the gate's disposition matrix any failed CI check is a CLOSE, so a
contributor PR can be auto-closed on a red status that would have gone green minutes later. The
config comment explains
after_n_buildsmust stay1(rees/control-plane uploads only exist on PRstouching them), so a higher count is not the fix — a count cannot express "wait for the backend
upload specifically." The supported mechanism is
notify.manual_triggerwith an explicit finalizestep once every upload has landed.
Closes #9798
Scope
type(scope): short summaryConventional Commit format, for examplefix(api): restore profile access checks.CONTRIBUTING.mdand does not reintroduce GitHub Pages, VitePress,site/, orCNAME.Closes #123) — a linked open issue is required for every contributor PR.Validation
git diff --checknpm run actionlintnpm run typechecknpm run test:coveragelocally;codecov/patchrequires ≥99% coverage of the lines AND branches you changed (aim for 100% on your diff so CI variance does not fail near the threshold). Global coverage is a non-blocking trend with a loose 90% backstop, not the gate.npm run test:workersnpm run build:mcpnpm run test:mcp-packnpm run ui:openapi:checknpm run ui:lintnpm run ui:typechecknpm run ui:buildnpm audit --audit-level=moderateIf any required check was skipped, explain why:
regress: the six unit suites covering the renormalized
src/review/**and engine files (60 passed),npm run rees:testfor the ninereview-enrichment/**files (1379 passed), the engine package suite(867 passed),
npm run test:coverage,npm run typecheck, andnpm run actionlintfor the workflowedit. The unchecked boxes are MCP/UI/workers/audit paths this diff does not touch — no UI, MCP,
worker, OpenAPI, or dependency file is in it — and CI runs them anyway.
Safety
UI Evidencesection below with JPG/JPEG or PNG screenshots arranged as organized, captioned, clickable thumbnails. SVG screenshots are not used as review evidence. Review-only screenshots or recordings are not committed to the repository.Auth/CORS/session, OpenAPI/MCP, and UI are all not applicable — this diff contains no runtime change
of any kind. No changelog edit.
UI Evidence
Not applicable — no visible UI, frontend, docs, or extension change. The only non-line-ending edits are
a root
.gitattributesand onerun:step in.github/workflows/ci.yml.Notes
git diff --checkwith noarguments against a shallow single-commit checkout, where working tree and index are always identical,
so it unconditionally passed. Fixing that false-green is what turned these pre-existing CRLF files
into a live trap — the files are old, the failure is new.
src/review/inline-comment-range.tsis 36 CRLF lines out of 54,test/unit/inline-comment-range.test.ts66 of 97. Which lines tripped the check depended on where inthe file you happened to edit, which is part of why it read as random.