Skip to content

fix(ci): repair two cross-merge breakages reding main's own tip - #7508

Merged
buluoray merged 1 commit into
mainfrom
fix/remove-unused-eslint-disable-securitypanel
Sep 1, 2026
Merged

fix(ci): repair two cross-merge breakages reding main's own tip#7508
buluoray merged 1 commit into
mainfrom
fix/remove-unused-eslint-disable-securitypanel

Conversation

@helenastafford

@helenastafford helenastafford commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator

Problem / Motivation

Two checks fail on main's current tip for every open PR, regardless of the PR's own diff:

  1. CI / Frontend Lint & Type Check: npx eslint src/ --max-warnings 659 measures 660 warnings on a clean origin/main checkout. The extra warning is Unused eslint-disable directive (no problems were reported from 'jsx-a11y/label-has-for') at website/src/pages/settings/SecurityPanel.tsx:2133 — the directive no longer suppresses anything, and eslint counts a stale directive as a warning.
  2. Backend Tests shard 3: test_security_posture.py::TestGateSideLogRedactorSpelling::test_the_census_holds_no_slack fails on a clean origin/main checkout — _BASELINE_LOG_SITE_CENSUS says dashboard/handlers/files.py has 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.py from 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 659 exits 0 (exactly 659) and test/test_security_posture.py passes 47/47; at the parent both fail.

Manual verification

Ran locally at this commit: eslint budget check → exit 0; npx tsc -b → exit 0; full test_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

  • At most two commits (one is the norm), with a Conventional Commits title (feat|fix|docs|refactor|perf|test|chore|ci|build|revert: ...)
  • Existing tests pass and new tests added for new functionality
  • Self-review completed; code follows project style guidelines
  • Documentation updated (if applicable)
  • No secrets, credentials, or internal references in the diff

Opened by Ember, helenars's AI agent, to unbreak CI for all open PRs.

@helenastafford
helenastafford requested a review from a team September 1, 2026 04:39
@helenastafford
helenastafford requested a review from a team as a code owner September 1, 2026 04:39
@github-actions github-actions Bot added the readiness: checking Automated validation is still running label Sep 1, 2026
@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

UX Review (Fable 5) — ✅ PASS

UX-level review of 2b0672d313a21396ce4b02fb209f80e0f0fc7fa9 — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

UX-Verdict: PASS

No user-visible change: a comment-only .tsx edit and a test baseline constant — nothing renders differently.

[UX-REVIEWED] 2b0672d

@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Design Review (Fable 5) — ✅ PASS

Design-level review of 2b0672d313a21396ce4b02fb209f80e0f0fc7fa9 — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Both hunks verified: the census test (test_the_census_holds_no_slack, test/test_security_posture.py:1046) exists precisely to force this downward ratchet, and CI's --max-warnings 659 budget (ci.yml:1300) makes removing the stale directive — rather than bumping the budget to 660 — the tightening-direction fix. Description matches the diff exactly; both repairs go the strict direction.

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

@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Opus 4.8 Review — ✅ no blocking findings

Reviewed 2b0672d313a21396ce4b02fb209f80e0f0fc7fa9 — this comment is updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] 2b0672d

Verdict parsed from the review's SHA-scoped output markers for commit 2b0672d313a21396ce4b02fb209f80e0f0fc7fa9.

False positive or not applicable? A repository writer can comment:
/ai-review override fable 2b0672d313a21396ce4b02fb209f80e0f0fc7fa9: <one-sentence reason>

@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of 2b0672d313a21396ce4b02fb209f80e0f0fc7fa9 and found no blocking issues.

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 2b0672d

False positive or not applicable? A repository writer can comment:
/ai-review override gpt 2b0672d313a21396ce4b02fb209f80e0f0fc7fa9: <one-sentence reason>

@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

First Principles Review (Fable 5) — ✅ PASS

Premise-level review of 2b0672d313a21396ce4b02fb209f80e0f0fc7fa9 — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Verification complete. Both hunks check out against the repo: the census test enforces exactness in both directions (test_security_posture.py:1031 fails on growth, :1056 fails on slack, and its failure message at :1060 demands exactly this decrement), and ci.yml:1300 pins the eslint ceiling at 659 with an explicit "do not raise it" invariant, making deletion the only legal repair. The diff contains nothing beyond the two declared hunks — no new surface, no riders. One residue worth flagging: SecurityPanel.tsx:1844 carries a byte-identical directive with the same justification comment; my grep found 2 such directives in the file pre-PR, and the author's own arithmetic (660 → exactly 659 after removing one) implies the rule does not fire on this label shape, which would make the twin stale too — hidden inside the 659 baseline as silent slack.

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 ships

Intent: make main's own tip pass its two ratchet gates again so open PRs can go green — a FIX.

  1. Backend Tests shard 3 passes: the log-site census entry for files.py drops 3→1 — justified (the failing test's own message demands exactly this; census ratchets downward by design)
  2. Frontend lint gate passes: a stale jsx-a11y/label-has-for disable comment is deleted — justified (pure deletion; the --max-warnings 659 ceiling at ci.yml:1300 carries a documented "do not raise it" invariant, so deletion is the only legal repair)

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

  • SecurityPanel.tsx:1844 holds a byte-identical directive over an identically-shaped nested-Checkbox label (grep jsx-a11y/label-has-for in that file: 2 pre-PR, 1 after). The author's own measurement — deleting one directive alone moves 660 to exactly 659 — implies the rule reports nothing on this label shape, so the twin is likely stale too, hiding as slack inside the 659 baseline that ci.yml:1297 calls "silent admission". One eslint run settles it; if stale, delete it and ratchet the budget to 658.

[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.
@helenastafford
helenastafford force-pushed the fix/remove-unused-eslint-disable-securitypanel branch from 54b9869 to 2b0672d Compare September 1, 2026 04:55
@helenastafford helenastafford changed the title fix(lint): remove unused eslint-disable pushing main over the warning budget fix(ci): repair two cross-merge breakages reding main's own tip Sep 1, 2026
@github-actions github-actions Bot added readiness: passed Eligible automated validation passed for the current revision and removed readiness: checking Automated validation is still running labels Sep 1, 2026

@buluoray buluoray left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@buluoray
buluoray merged commit c412c2f into main Sep 1, 2026
71 of 78 checks passed
@buluoray
buluoray deleted the fix/remove-unused-eslint-disable-securitypanel branch September 1, 2026 05:42
@github-actions github-actions Bot removed the readiness: passed Eligible automated validation passed for the current revision label Sep 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants