fix(ci): repair two cross-merge breakages reding main's own tip - #7508
Conversation
UX Review (Fable 5) — ✅ PASSUX-level review of UX-Verdict: PASS No user-visible change: a comment-only .tsx edit and a test baseline constant — nothing renders differently. [UX-REVIEWED] 2b0672d |
Design Review (Fable 5) — ✅ PASSDesign-level review of Both hunks verified: the census test ( Design-Verdict: PASS Both repairs tighten rather than loosen their ratchets — deleting the stale directive instead of bumping the budget, lowering the census as its own test demands. [DESIGN-REVIEWED] 2b0672d |
Opus 4.8 Review — ✅ no blocking findingsReviewed Verdict parsed from the review's SHA-scoped output markers for commit False positive or not applicable? A repository writer can comment: |
GPT 5.6 Review — ✅ no blocking findingsGPT 5.6 completed its review of This comment is updated in place on each push. Review detailsNo findings. False positive or not applicable? A repository writer can comment: |
First Principles Review (Fable 5) — ✅ PASSPremise-level review of Verification complete. Both hunks check out against the repo: the census test enforces exactness in both directions ( First-Principles-Verdict: PASS Two pure deletions, each demanded verbatim by a red gate on main's tip; nothing rides along and nothing new exists afterward. What this change shipsIntent: make main's own tip pass its two ratchet gates again so open PRs can go green — a FIX.
Both items unwedge a red build, which makes them derived; the description honestly declares the fix sits at symptom level relative to cross-merge ratchet drift and names what is left, and the gates themselves re-verify both numbers on this PR's own CI run. Watch
[FIRST-PRINCIPLES-REVIEWED] 2b0672d |
1. Remove the unused eslint-disable directive in SecurityPanel.tsx that pushed a clean main checkout to 660 eslint warnings against the Frontend Lint gate's --max-warnings 659 budget. 2. Ratchet _BASELINE_LOG_SITE_CENSUS for dashboard/handlers/files.py from 3 down to the code's actual 1 log site, fixing test_security_posture.py::TestGateSideLogRedactorSpelling:: test_the_census_holds_no_slack (Backend Tests shard 3). Both fail on a clean origin/main checkout, so every open PR inherits them through its merge ref regardless of its own diff.
54b9869 to
2b0672d
Compare
buluoray
left a comment
There was a problem hiding this comment.
Approving — verified both halves independently rather than on the title.
Census hunk. dashboard/handlers/files.py: 3 → 1 matches what the gate itself measures: the failure text on every affected PR reads "files.py: 1 sites, census says 3", so lowering the census to 1 is the tightening direction the assertion asks for, not a loosening. Main's 3eeda1250 (#7293) is what reduced the sites.
Lint hunk. The removed jsx-a11y/label-has-for directive was misplaced, which is why it went stale: eslint-disable-next-line applies to the line immediately after it, and that line was {needsAck && ( — not the <label> the comment describes. So it suppressed nothing and became an unused-directive warning. Removing it is −1 warning, and this PR's own Frontend Lint & Type Check is green, which is the empirical confirmation that the count is now within --max-warnings 659.
Also confirmed this PR is the one that should land: Backend Tests (3.10, 3), Frontend Lint & Type Check and Coverage Gate are all green here, and there are zero failing checks with all five review lanes clean.
Worth flagging for whoever merges: #7512 carries the byte-identical census hunk plus four more stale-directive removals in ChatPage.tsx/DevFleetPage.tsx. Whichever of the two lands first turns the other's census line into a conflict or a no-op. This one is further along (that one currently has PR Hygiene and Screenshot Evidence red), so merging this first and rebasing that one to keep only its extra eslint cleanup is the cheaper order.
Context on why this matters beyond the two gates: this drift is currently reding every open PR — it accounts for four of the five red checks on #7328 (three backend shards plus Coverage Gate, which fails closed downstream rather than on any coverage shortfall). Filed as #7490, with #7511 recording why the class surfaces on unrelated PRs instead of on main.
Problem / Motivation
Two checks fail on main's current tip for every open PR, regardless of the PR's own diff:
CI / Frontend Lint & Type Check:npx eslint src/ --max-warnings 659measures 660 warnings on a cleanorigin/maincheckout. The extra warning isUnused eslint-disable directive (no problems were reported from 'jsx-a11y/label-has-for')atwebsite/src/pages/settings/SecurityPanel.tsx:2133— the directive no longer suppresses anything, and eslint counts a stale directive as a warning.Backend Testsshard 3:test_security_posture.py::TestGateSideLogRedactorSpelling::test_the_census_holds_no_slackfails on a cleanorigin/maincheckout —_BASELINE_LOG_SITE_CENSUSsaysdashboard/handlers/files.pyhas 3 log sites, the code now has 1 (log sites were removed without ratcheting the census down).Why it matters
Repo-wide CI break: every PR based on current main reds both gates through no fault of its own (first observed on PR #7346, whose diff adds zero eslint warnings and touches neither file). Until this lands, no PR can go green.
What changed (motivation → approach → change)
Symptom: main's own tip fails two of its ratchet-style gates. Root cause: cross-merge drift — in each case one merged PR tightened a ratchet (lowered the eslint budget / the census counts a file's sites) while another independently made the tightened bound wrong (made a disable directive redundant / removed log sites), and neither failed alone because CI runs each PR against the merge ref it was created with. Change: two one-line ratchet repairs — delete the unused eslint-disable directive (a clean checkout then measures exactly 659 and the gate passes) and lower the census entry for
dashboard/handlers/files.pyfrom 3 to 1 (census ratchets downward by design; the test itself demands this repair in its failure message).Tests
The two failing gates are themselves the regression tests: at this commit
npx eslint src/ --max-warnings 659exits 0 (exactly 659) andtest/test_security_posture.pypasses 47/47; at the parent both fail.Manual verification
Ran locally at this commit: eslint budget check → exit 0;
npx tsc -b→ exit 0; fulltest_security_posture.py→ 47 passed; flake8/black/isort clean on the changed test file. At the parent commit both repaired gates fail with the exact CI errors.Why no screenshot: comment-only edit in a .tsx file plus a backend test baseline constant; no rendered output changes.
Related Issues
no linked issue: repo-wide CI breakage found while babysitting PR #7346; no tracking issue exists.
Pattern harvest
Rule candidate: lint
Pattern: cross-merge ratchet drift — one green PR tightens a budget/census while another independently invalidates the bound; main's tip breaks without either PR failing. A post-merge run of the ratchet gates on main's tip would catch it at the source.
Checklist
feat|fix|docs|refactor|perf|test|chore|ci|build|revert: ...)Opened by Ember, helenars's AI agent, to unbreak CI for all open PRs.