Skip to content

ci: split checks into lint and test jobs, harden the workflow - #102

Merged
galatanovidiu merged 1 commit into
trunkfrom
ci/house-style
Jul 27, 2026
Merged

ci: split checks into lint and test jobs, harden the workflow#102
galatanovidiu merged 1 commit into
trunkfrom
ci/house-style

Conversation

@galatanovidiu

@galatanovidiu galatanovidiu commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

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

lint and test replace the single verify. 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, with contents: read granted per job. Deny by default, grant explicitly.
  • Actions pinned to full commit SHAs, version in a trailing comment. A tag like @v4 can be repointed at new code by whoever controls it; a digest cannot.
  • persist-credentials: false on checkout, so the job token is not written into .git/config where any later step could read it.
  • ubuntu-24.04 instead of ubuntu-latest — the runner image stops changing underneath us.
  • Per-job timeout-minutes, so a hung job fails instead of burning the full 6-hour default.
  • Explicit pull_request types (opened, synchronize) and workflow_dispatch for manual runs.

One step that is load-bearing, not stylistic

The test job builds before running jest.

tests/integration/bundle-version-interpolation.test.ts asserts against dist/proxy.js and wraps itself in describeIfBuilt — 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 the npx jest tests/unit/ command that was labelled "run all tests" while omitting the integration suites.

Required check names change

From verify to Lint and typecheck and Unit and integration tests.

The protect trunk ruleset currently has no required_status_checks rule, so nothing breaks on merge — but both names need adding afterwards for the gate to bite.

Not included

  • No .nvmrc exists, so node-version: "22" is explicit, matching what release.yml publishes with. Adding one would be a reasonable follow-up.
  • No test matrix — this is a single-runtime Node package.

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 galatanovidiu changed the title ci: adopt the mcp-adapter workflow conventions ci: split checks into lint and test jobs, harden the workflow Jul 27, 2026
@galatanovidiu
galatanovidiu merged commit 550f38e into trunk Jul 27, 2026
2 checks passed
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