ci: run ci:local on every PR into main (#18 follow-up) - #23
Merged
Conversation
added 3 commits
July 4, 2026 20:16
The repo's quality gate (`npm run ci:local`) previously ran only post-merge, as the test job of npm-publish.yml (`on: push: [main]`). That let the #18 dashboard redesign merge with two failing layout tests and a ui-hardcode ratchet failure — nothing ran the gate in front of the PR. Add .github/workflows/ci.yml triggering on pull_request into main and running the identical gate (Node 24, root + dashboard `npm ci`, the same public VITE_INSFORGE_* build env as npm-publish.yml). Triggers on pull_request only — post-merge coverage on main stays in npm-publish.yml. To actually block merges, add "CI / ci:local" as a required status check in main's branch-protection rules.
`test/init-local-runtime-reinstall.test.js` asserts that installLocalTrackerApp copies dashboard/dist/index.html into the target app dir. That copy is guarded on the repo's dashboard/dist existing (init.js:1073), but ci:local ran `npm --prefix dashboard run build` LAST — so on a clean checkout the test ran before dist existed and failed with ENOENT. It only passed locally on stale build artifacts. Surfaced by the new PR CI workflow (first clean-env run of the gate). Move the dashboard build to the front so ci:local is self-contained on any fresh checkout. Verified: `rm -rf dashboard/dist && npm run ci:local` now builds first and the install test passes (exit 0).
`ci:local` is green on macOS (the maintainer's dev env) but has chronic Linux-only failures — inode reuse after unlink+recreate defeats the file-rotation detection exercised by rollout-parser tests, and npm-publish.yml's ubuntu `test` job has failed on every push to main for the same class of reason. Fixing those for a Linux runner is a separate cross-platform hardening effort. Running the PR gate on macOS gives a green, enforceable check now that still blocks the platform-independent regressions this gate exists for (a stale layout test fails on any OS).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a PR-triggered CI check so failing tests / validators block the merge button — closing the gap that let the #18 dashboard redesign merge with a red
ci:local.Root cause: the repo's gate (
npm run ci:local) only ran post-merge, as thetestjob ofnpm-publish.yml(on: push: branches: [main]). Nothing ran it in front of a PR, so two failing layout tests + a ui-hardcode ratchet failure sailed through review.This PR:
.github/workflows/ci.ymlpull_requestintomain(post-merge coverage stays innpm-publish.yml— no duplicate runs).npm cifor root +dashboard/, thennpm run ci:local.VITE_INSFORGE_*build env already committed innpm-publish.yml(not secrets — shipped in every bundle).permissions: contents: read; failure propagates (no|| true/continue-on-error).Test plan
pull_request→main, node = 24, gate =npm run ci:localci:localon GitHub and should pass (green)CI / ci:localas a required status check in branch protection to enforce blockingRefs #18