Skip to content

ci: run actions on Node 24 natively, make lint warnings fail - #47

Merged
ErnieAtLYD merged 1 commit into
mainfrom
ci/tighten-lint
Aug 7, 2026
Merged

ci: run actions on Node 24 natively, make lint warnings fail#47
ErnieAtLYD merged 1 commit into
mainfrom
ci/tighten-lint

Conversation

@ErnieAtLYD

Copy link
Copy Markdown
Owner

Two follow-ups from the first CI run (#46).

1. Action versions: v4 → v7

Every run was annotated with:

Node.js 20 is deprecated. The following actions target Node.js 20 but are being forced to run on Node.js 24: actions/checkout@v4, actions/setup-node@v4

v7 of both runs on Node 24 natively, so the annotation goes away. Not urgent — GitHub was already force-running them on 24 — but it won't be force-run forever.

2. --max-warnings=0

Warnings now gate instead of being annotations you scroll past. Two things had to land first, and both are the actual content of this PR:

eslint now ignores .remember/**. This is why local lint reported 3 warnings and CI reported 2 — those are gitignored plugin scratch files, present on a laptop and never on a runner. Adding a zero-tolerance flag without this would have produced a lint that fails locally only, which is worse than no gate at all.

no-unused-vars configured with ignoreRestSiblings. The _bt/_bh bindings in ReviewForm.tsx weren't sloppiness — they're the destructure-to-omit idiom, unused by design:

posts.map(({ bodyText: _bt, bodyHtml: _bh, ...meta }) => meta)

Flagging that had pushed the call site into abbreviated names that obscured which fields were being dropped. The fix is the rule expressing the intent, not a workaround at each site — so the call site now reads with the real field names:

// Strip the heavy fields — /api/propose-courses only needs metadata,
// and its Zod schema would drop them anyway.
posts.map(({ bodyText, bodyHtml, ...meta }) => meta)

The flag lives in the lint script rather than the workflow step, so a local run fails exactly the way CI does.

Verification

Checked in both directions, because a gate that only proves the happy path proves nothing:

Probe Expected Result
const deadValue = 1 fails the gate ✗ "ESLint found too many warnings (maximum: 0)"
const { a, b, ...rest } = o still allowed ✓ passes, no false positive

That second row matters as much as the first — a gate that also broke the idiom it was meant to permit would just push everyone back to underscore-prefixing.

Full suite on this branch: 159 tests passed, lint clean, build green.

⚠️ On this PR's own CI check

GitHub Actions is in a major outage (started 15:22 UTC today; "workflow runs are still failing or delayed in starting, and some queued jobs may time out"). This PR's check will likely sit queued or get cancelled by timeout — that would be infrastructure, not this change.

Same thing already happened on main: the run for 9359e60 queued for 46 minutes and was cancelled without executing a single step, which is why main currently shows a red X. It needs a Re-run all jobs once Actions is healthy; there is nothing to fix in the code.

Everything above was verified locally on this exact commit.

Two follow-ups from the first CI run (#46).

1. actions/checkout and actions/setup-node bumped v4 → v7. v4 targets the
   Node 20 action runtime, which GitHub now force-runs on Node 24 and
   annotates with a deprecation warning on every run. v7 runs on 24
   natively; the annotation goes away.

2. --max-warnings=0 so warnings gate instead of scrolling past. Getting
   there needed two things first:

   - eslint now ignores .remember/** — plugin scratch files that are
     gitignored, so they exist on a laptop but never on a runner. That
     asymmetry is why local lint reported 3 warnings and CI reported 2;
     with a zero-tolerance flag it would have failed locally only.

   - no-unused-vars configured with ignoreRestSiblings. Destructuring to
     omit fields before a wire (`{ bodyText, bodyHtml, ...meta }`) leaves
     bindings unused by design; flagging it had pushed the call site into
     `_bt`/`_bh` naming that hid which fields were dropped. The rule now
     expresses the intent, so ReviewForm reads with the real field names.

The flag lives in the `lint` script rather than the workflow step so a
local run fails exactly the way CI does.

Verified both directions: an unused `deadValue` fails the gate, and a
rest-sibling omit still passes. Full suite green — 159 tests, build clean.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UHwboozu79gcGsbgy54YEF
@vercel

vercel Bot commented Aug 6, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
substack2eec Ready Ready Preview Aug 6, 2026 7:29pm

@ErnieAtLYD
ErnieAtLYD merged commit 8e00ab8 into main Aug 7, 2026
4 checks passed
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.

1 participant