ci: split checks into lint and test jobs, harden the workflow - #102
Merged
Conversation
WHAT:
- Split the single `verify` job into `lint` and `test`, so a formatting
failure and a test failure are distinguishable and run in parallel.
- Deny-by-default `permissions: {}` at workflow level, granting
`contents: read` per job.
- Pin actions to full commit SHAs with the version in a trailing comment.
- `persist-credentials: false` on checkout so the token is not left behind in
.git/config.
- Pin the runner to ubuntu-24.04 rather than ubuntu-latest, add per-job
timeouts, name the explicit pull_request types, and add workflow_dispatch.
- Document each job with its step list.
The `test` job builds before running jest. This is load-bearing, not
incidental: tests/integration/bundle-version-interpolation.test.ts asserts
against dist/proxy.js and skips itself when the bundle is missing, so a test
job without a build would report green while silently dropping it.
WHY: SHA-pinned actions and deny-by-default permissions remove a supply-chain
and token-scope risk that the tag-based version carried. Splitting the jobs
makes a failure attributable to the thing that failed.
Note: the required status check names change from `verify` to `Lint and
typecheck` and `Unit and integration tests`.
galatanovidiu
force-pushed
the
ci/house-style
branch
from
July 27, 2026 14:15
7375954 to
b5a8608
Compare
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.
Restructures
ci.yml. The workflow added in #101 already ran on pull requests, but it was a single job using floating action tags and no permission floor.Split into two jobs
lintandtestreplace the singleverify. A formatting failure and a test failure are now distinguishable at a glance instead of both showing as one red check, they run in parallel, and each can be required independently.Hardening
permissions: {}at workflow level, withcontents: readgranted per job. Deny by default, grant explicitly.@v4can be repointed at new code by whoever controls it; a digest cannot.persist-credentials: falseon checkout, so the job token is not written into.git/configwhere any later step could read it.ubuntu-24.04instead ofubuntu-latest— the runner image stops changing underneath us.timeout-minutes, so a hung job fails instead of burning the full 6-hour default.pull_requesttypes (opened,synchronize) andworkflow_dispatchfor manual runs.One step that is load-bearing, not stylistic
The
testjob builds before running jest.tests/integration/bundle-version-interpolation.test.tsasserts againstdist/proxy.jsand wraps itself indescribeIfBuilt— when the bundle is absent it skips silently. A test job without a build step would report green while quietly dropping that test. That is the same failure mode as thenpx jest tests/unit/command that was labelled "run all tests" while omitting the integration suites.Required check names change
From
verifytoLint and typecheckandUnit and integration tests.The
protect trunkruleset currently has norequired_status_checksrule, so nothing breaks on merge — but both names need adding afterwards for the gate to bite.Not included
.nvmrcexists, sonode-version: "22"is explicit, matching whatrelease.ymlpublishes with. Adding one would be a reasonable follow-up.