Repository navigation
ci(web): run lint, test and build on every pull request - #101
Merged
Merged
Conversation
Nothing in this repo was running any of these three commands. The checks were licenses, PR hygiene, gitleaks and actionlint, so a pull request could be green against a main that did not compile. That is not a hypothetical: #98 merged green and broke the production build, and it was found days later when the 1670 FleetOS host could not be rebuilt. All three run, not one of them, because each catches what the others miss. vitest run does not typecheck. tsc -b runs only inside npm run build. So a type error in a test file passes the suite and fails the production build, which is precisely the shape of the #99 break. npm ci rather than npm install, so CI installs exactly the lockfile and fails when package.json and the lockfile disagree, instead of quietly passing against a dependency tree nobody else has. The tree-clean check at the end catches a formatter or a generated file that rewrites something in place, which would otherwise pass here and surprise the next person. The break this gate catches was fixed first, in #100, so this does not fail on its own first run. Closes #99
Contributor
Author
|
Adds Every step was run locally in the same order first, including This closes #99 with the break already fixed in #100, so the gate did not fail on its own first run. Worth flagging for review: the job pins Node 22 LTS, and since the repo declares no |
Bugs5382
added a commit
that referenced
this pull request
Sep 25, 2026
Nothing in this repo was running any of these three commands. The checks were licenses, PR hygiene, gitleaks and actionlint, so a pull request could be green against a main that did not compile. That is not a hypothetical: #98 merged green and broke the production build, and it was found days later when the 1670 FleetOS host could not be rebuilt. All three run, not one of them, because each catches what the others miss. vitest run does not typecheck. tsc -b runs only inside npm run build. So a type error in a test file passes the suite and fails the production build, which is precisely the shape of the #99 break. npm ci rather than npm install, so CI installs exactly the lockfile and fails when package.json and the lockfile disagree, instead of quietly passing against a dependency tree nobody else has. The tree-clean check at the end catches a formatter or a generated file that rewrites something in place, which would otherwise pass here and surprise the next person. The break this gate catches was fixed first, in #100, so this does not fail on its own first run. Closes #99 Co-authored-by: Bugs5382 <12115015+Bugs5382@users.noreply.github.com>
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
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.
Closes #99. Second of two: #100 fixed the break, this is the gate that would
have caught it.
What and why
Nothing in this repo was running
lint,testorbuild. The checks werelicenses, PR hygiene, gitleaks and actionlint — so a pull request could be
green against a
mainthat did not compile.That is not hypothetical. #98 merged green and broke the production build; it
surfaced days later when the 1670 FleetOS host could not be rebuilt.
All three run, not one of them, because each catches what the others miss:
vitest rundoes not typecheck.tsc -bruns only insidenpm run build.So a type error in a test file passes the suite and fails the production
build. That is exactly the shape of #99.
Two smaller choices:
npm ci, notnpm install— installs exactly the lockfile and failswhen
package.jsonand the lockfile disagree, rather than quietly passingagainst a dependency tree nobody else has.
that rewrites something in place, which would otherwise pass and surprise
the next person.
Verification
How this was verified
Every step the workflow runs was run locally in the same order, including the
npm cithat CI will do:Merge order
After #100, which is already merged. The gate would otherwise fail on its own
first run.
One thing worth a second opinion
The job pins Node 22 LTS. The repo declares no
enginesrange, so that pinbecomes the de facto statement of what the project builds on — and it is not
what is on my workstation (24). If 22 is the wrong target, this is the line to
change.