Skip to content

ci: run lint, test, and build on every PR - #46

Merged
ErnieAtLYD merged 1 commit into
mainfrom
ci/github-actions
Aug 6, 2026
Merged

ci: run lint, test, and build on every PR#46
ErnieAtLYD merged 1 commit into
mainfrom
ci/github-actions

Conversation

@ErnieAtLYD

Copy link
Copy Markdown
Owner

Why

There was no .github/workflows/. 159 tests across 13 files, and nothing ran them but a laptop. Every "tests pass" claim in this repo has been a local claim.

What

One job — checkout → npm ci → lint → test → build — on pull_request and pushes to main.

One job, not three. The whole suite is roughly four seconds of real work (vitest ~0.8s, next build ~2.5s). Splitting into parallel jobs would pay the checkout + npm ci cost three times to save nothing.

No secrets required. src/env.ts validates ANTHROPIC_API_KEY lazily:

let _env: z.infer<typeof envSchema> | null = null
export function getEnv() {
  if (!_env) _env = envSchema.parse(process.env)
  return _env
}

It only runs on the first getClient() call, not at import or build time — so a keyless build is a genuine check rather than one that passes by accident. Verified by moving .env.local aside and unsetting the var:

Keyless run Result
npm test 155 passed, 13 files
npm run build compiled + 8/8 static pages

(The 155 vs 159 gap is just this branch being cut from main before #45's four new tests.)

Node 24 to match the Vercel production runtime. The repo pins no engines field, so this workflow is the de facto version of record — worth keeping in step if prod moves.

Lint fails on errors only. eslint exits 0 on warnings by design, and there are 3 pre-existing ones (_bt/_bh in ReviewForm.tsx, plus a stray .remember/tmp/ scratch file that arguably shouldn't be linted at all). Adding --max-warnings=0 today would fail the run on day one; the comment in the file notes where to tighten once they're cleared.

concurrency with cancel-in-progress so a rapid second push doesn't leave an obsolete run burning minutes, and permissions: contents: read since nothing here needs write.

Note on merge order

This PR and #45 both branch from the same commit. Whichever lands second will be the first to actually get a CI run — #45's four new tests included.

The repo had no .github/workflows — 159 tests that only ever ran on one
laptop. Adds a single verify job (checkout → npm ci → lint → test → build)
on pull_request and pushes to main.

One job rather than three: the whole suite is ~4s of actual work, so
parallel jobs would pay 3x setup cost to save nothing.

Needs no secrets. src/env.ts validates ANTHROPIC_API_KEY lazily on the
first getClient() call rather than at import or build time, so a keyless
build genuinely exercises the build — verified locally by moving
.env.local aside and unsetting the var for both `npm test` and
`npm run build`.

Node 24 to match the Vercel production runtime. The repo pins no engines
field, so this file is the de facto version of record.

Lint fails on errors only; `eslint` exits 0 on warnings by design and
there are 3 pre-existing ones. Tighten with --max-warnings=0 once cleared.

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 4:39pm

@ErnieAtLYD
ErnieAtLYD merged commit ea46052 into main Aug 6, 2026
5 checks passed
@ErnieAtLYD
ErnieAtLYD deleted the ci/github-actions branch August 6, 2026 17:53
ErnieAtLYD added a commit that referenced this pull request Aug 7, 2026
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.


Claude-Session: https://claude.ai/code/session_01UHwboozu79gcGsbgy54YEF

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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