Skip to content

ci: run 'make check' instead of open-coding two of its five steps - #854

Merged
lmorchard merged 1 commit into
mainfrom
ci/run-make-check
Sep 16, 2026
Merged

lmorchard merged 1 commit into
mainfrom
ci/run-make-check

Conversation

@lmorchard

Copy link
Copy Markdown
Owner

make check is install-js check-message-types plus ruff, pyright, and
tsc --noEmit over static/. The lint-and-test job open-coded
uv run ruff check and uv run pyright directly — two of those five steps —
and skipped the rest.

Two gates that were running nowhere

Gate What it guards Where it ran
check-message-types drift in the generated WS types (message_types.py, message-types.js, websocket-messages.md, tui/src/types.generated.ts) nowhere
check-js tsc --noEmit over static/ nowhere

CLAUDE.md says the drift check is "wired into make check", which was true —
but nothing automated ran make check. The check-js gap is the same family
as the #825/#844 breakage that left the web client dead for three days with
make check green.

This is also the mechanism by which a local Node too old to run install-js
stayed invisible: CI was green because CI never ran the steps that need Node.

Change

lint-and-test sets up Node and runs make check as a single step, so CI runs
the same gate a human runs. pytest stays its own step. Both jobs now take the
Node version from .nvmrc instead of a hardcoded '22', so there is one
source of truth — and .nvmrc starts actually meaning something in CI.

Verification

  • make check passes in a fresh worktree (install-js, drift check, ruff,
    pyright, tsc --noEmit), leaving a clean tree.
  • Teeth-checked rather than assumed present: a deliberate TS2345 in
    static/lib/ made make exit 2 and name the error. The probe was a
    newly created file, removed afterwards, so no existing file was touched.
  • ci.yml parses and the step order is uv -> Python -> Node -> uv sync ->
    make check -> pytest.

Follow-up, not included

tui/ is still outside the gate (#725) — no test or typecheck step, and its
typecheck fails on main right now: SrvShellApproval was added to
tui/src/types.generated.ts by #850 with no matching dispatcher.ts case.
Since assertNever has an empty body and does not throw, shell-approval
messages silently no-op in the TUI. This PR does gate one TUI file as a side
effect, since check-message-types diffs types.generated.ts.

The lint-and-test job ran 'uv run ruff check' and 'uv run pyright' directly.
'make check' is 'install-js check-message-types' plus ruff, pyright, and
'tsc --noEmit' over static/ — so CI reproduced two of five steps and skipped
the rest. Two gates the docs describe as live were running nowhere:

- check-message-types, the drift guard on the generated WebSocket message
  types (message_types.py, message-types.js, websocket-messages.md and
  tui/src/types.generated.ts). CLAUDE.md says it is 'wired into make check',
  which was true — but nothing automated ran make check.
- check-js, the tsc --noEmit typecheck of static/. That is the same family as
  the #825/#844 breakage that left the web client dead for three days.

This is also why a local node too old to run install-js went unnoticed: CI was
green because CI never ran the steps that need node.

lint-and-test now sets up node and runs 'make check' as one step, so CI runs
the same gate a human runs. Both jobs take the version from .nvmrc rather than
a hardcoded '22', so there is one source of truth.

Verified: 'make check' passes in a fresh worktree, and a deliberate TS2345 in
static/lib/ makes it exit 2 — so the newly-wired JS typecheck has teeth rather
than just being present.
Copilot AI lite review requested due to automatic review settings September 16, 2026 00:37

Copilot AI 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.

🟢 Approval recommended

The complete check gate now runs in CI with centralized Node versioning.

Pull request overview

Updates CI to run the complete make check gate and centralize Node.js version selection through .nvmrc.

Changes:

  • Replaces open-coded Ruff/Pyright steps with make check.
  • Adds Node setup to the Python job.
  • Uses .nvmrc in both CI jobs.
File summaries
File Description
.github/workflows/ci.yml Runs the complete validation gate with consistent Node setup.
Review details
  • Files reviewed: 1/1 changed files
  • Comments generated: 0
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

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