ci: add typecheck and smoke tests, fix .mcpb on Node < 22.7, group Dependabot by dangrondahl · Pull Request #87 · kosli-dev/mcp-server

dangrondahl · 2026-10-22T10:04:19Z

Gives more confidence before the next release by adding checks for things that so far were only tested by unit tests, or not at all. The new checks immediately turned up two bugs that were already on main, and both are fixed here: a mistyped test, and a .mcpb bundle that doesn't start on Node 22.0–22.6.

Fix: the `.mcpb` bundle didn't start on Node 22.0–22.6

pack-mcpb.sh deleted package.json from the bundle, so nothing declared dist/ as ESM. Node 22.7+ detects ESM from the syntax and prints a warning. Earlier versions parse the file as CommonJS and fail with Cannot use import statement outside a module. Both engines and manifest.json allow those versions.

The bundle now ships a one-line package.json with { "type": "module" }, and nothing from the repo's own package.json. I reproduced the failure on the official Node 22.6.0 binary, and confirmed the fixed bundle starts on 22.0.0, 22.6.0 and 22.7.0.

Type-check tests and scripts

tsconfig.json is src-only because it emits dist/, and vitest strips types without checking them. So a type error in test/ or scripts/ never failed anything. One had already slipped through: the fetch mock in test/client/kosli-client.test.ts was typed ReturnType<typeof vi.fn>, which doesn't match KosliClient's fetchFn. That is fixed here.

tsconfig.test.json and npm run typecheck cover the TypeScript in src, test and scripts. scripts/sync-version.mjs is plain JS and isn't covered. CI runs the typecheck after the build.

Stdio smoke test

scripts/smoke.ts (npm run smoke) starts a built server over real stdio and checks three things:

the version it advertises matches package.json
it lists exactly the three tools
the tools carry the right safety hints: execute_write_action is marked destructiveHint and not read-only, and execute_read_action and search_actions are marked readOnlyHint. These hints are what make an MCP client ask before a write, and no other test checks them.
search_actions finds list_environments

A failed tool call reports the server's own error message rather than a JSON parse error.

This covers what test/server.test.ts can't, because that test runs in memory: whether the bin starts, whether the JSON imports load, and whether a bundle's node_modules resolve.

The test is offline by default. It does one live read only when KOSLI_API_TOKEN and KOSLI_SMOKE_ORG are both set, and CI never sets them, so it's fork-safe.

CI now runs:

build
typecheck
test
smoke dist/
pack
unzip the .mcpb, run mcpb validate on its manifest, and smoke-test it
on the leg that sets consumer-floor in the matrix: switch to exactly Node 22.0.0 and smoke-test the bundle again

There's also a negative check: the smoke test must fail against a missing entry point. That way a later refactor can't turn the smoke steps into checks that always pass.

Some notes on those steps:

Validating the bundled manifest, not the repo's copy, matters: the repo's manifest.json holds a placeholder version, and the real one is only injected at pack time.
The 22.0.0 step exists because the matrix floor is the dev floor (vitest needs a newer 22.x, see chore: pin the dev Node floor at 22.12, distinct from the consumer floor #47), so nothing tested the version users are promised. The smoke test doesn't need vitest, so it can run there. I rehearsed it locally on the real 22.0.0 binary: it fails on the old bundle and passes on the fixed one.
@anthropic-ai/mcpb is now a devDependency rather than an npx pin, so Dependabot keeps it up to date. It adds 49 dev-only packages. None of them ship (files: ["dist"]), and the server's own zod is unchanged at 4.3.6.

Release workflow: publish only after every check

release.yml used to run only build and test, then npm publish, and only then build the bundle. So a failed pack could leave a version on npm with no GitHub Release. It now runs the same checks as CI:

build, typecheck, test
smoke dist/
pack, mcpb validate, smoke the bundle
only then npm publish and the GitHub Release

npm publish can't be undone, so it stays last. This also checks the bundle under the npm@latest that the release job installs, which CI never uses.

Both jobs now have a timeout-minutes (15 for CI, 20 for release) instead of the 6-hour default.

Dependabot grouping and cooldown

Actions: all SHA bumps go in one group. They edit the same lines, so as separate PRs they conflict (ci: bump step-security/harden-runner from 2.21.1 to 2.22.2 #80 and ci: bump actions/setup-node from 7.0.0 to 7.1.0 #82 do today).
npm dev dependencies: minor and patch bumps are grouped.
@modelcontextprotocol/sdk: its minor and patch bumps get their own group, because it's the only runtime dependency.
Majors stay one PR each.
Cooldown: new releases wait 7 days before being proposed. Security updates aren't affected.

Docs

CONTRIBUTING.md gets a "Testing locally" section covering typecheck, smoke (offline and live), MCP Inspector, Claude Code against a checkout, and installing the .mcpb. README.md and CLAUDE.md are updated to match.

Verification

On Node 24 locally:

build, typecheck and npm test (110 tests) pass.
The smoke test passes against dist/ and against the unzipped bundle.
The smoke test exits 1 when the server fails to start.
mcpb validate passes on the bundled manifest, which carries version 0.5.0.
The live-API smoke test passed against a real org (with SDK 1.30.1).
The bundle smoke test passes on Node 22.0.0, 22.6.0 and 22.7.0.

Not run locally: the Node 22 matrix leg, which CI covers.

Not run: release.yml itself, which only runs on a tag. Its new steps are the same commands CI runs.

After merge, check the next Dependabot run is still green. cooldown support varies by ecosystem, and an unsupported key invalidates the whole config, not just that block.

Merge order for the open PRs

Suggested order once this lands: #47, then this PR, then #80 → #82 → #83 → #74 → #79 → #84 → #45. Before #79 (SDK), rerun the live smoke test on its branch. #47 conflicts with this PR on the matrix line: when resolving, change the include: entry to node-version: '22.12' as well. Merge #45 (TypeScript 7) last, after diffing dist/ output against 5.9.

claude · 2026-10-22T10:04:44Z

Claude finished @dangrondahl's task in 24s —— View job

All four open findings are resolved in the latest commits, and I found nothing new (0 new inline comments).

version.ts / sync-version.mjs comment: resolved. It now describes the { "type": "module" } stub, and src/version.ts was regenerated to match.
Typed annotations in smoke.ts, plus a mirror in test/server.test.ts: resolved. It uses a Map, so the checks are typed, and npm test now asserts the same hints.
CONTRIBUTING "first three" wording: resolved.
Negative smoke check: resolved. It requires the script's own not ok - line, which main().catch prints to stderr. The step captures with 2>&1, so the match works.

I couldn't run npm here.

… test Address review on #87: - Validate the manifest inside the unzipped .mcpb, which carries the version pack-mcpb.sh injects, rather than the repo's placeholder copy. - Assert readOnlyHint/destructiveHint on the three tools. Nothing else tests them, and they are what makes a client ask before a write. - Report a failed tool call's own message instead of a JSON parse error. - Connect inside the try so a failed handshake still closes the child. - Docs: typecheck covers the .ts files in scripts/, not sync-version.mjs.

The server sets it explicitly; !== true would also have passed if it were dropped. Addresses review on #87.

Address review on #87: - npx --no-install, so a missing devDependency fails instead of fetching and running whatever registry package is called mcpb. - Declare the consumer floor as a matrix include key, so the floor steps can't silently stop matching when the matrix changes; note that the client side runs on the floor too. - smoke.ts: build the child env from the SDK's getDefaultEnvironment (works on Windows, still copies no KOSLI_*), return unknown from callJson, and tolerate a result with no content array. - CLAUDE.md: the bundle now has a package.json stub with no version, which is why src/version.ts is still needed.

Address review on #87: - release.yml now typechecks, smoke-tests dist, and packs, validates and smoke-tests the .mcpb before npm publish, which can't be undone and previously ran before the bundle was even built. - timeout-minutes on the CI and release jobs, instead of the 6h default. - smoke.ts: rename hints to annotations; in this repo hints means src/hints.json.

dangrondahl · 2026-10-22T10:41:51Z

Replies to the latest review summary. Everything listed is in e155b19.

release.yml runs none of the new checks, and publishes before the bundle exists. Fixed here rather than as a follow-up, since release confidence is the point of this PR. The release job now runs build, typecheck, test, the dist/ smoke test, then pack, mcpb validate and the bundle smoke test, and only after all of that npm publish and the GitHub Release. A failed pack can no longer leave a version on npm with no Release. The bundle is also now checked under the npm@latest that this job installs. The release steps in CLAUDE.md are updated to match, with a note to keep publishing last.

No timeout-minutes. Added: 15 on the CI test job, and 20 on the release job.

hints naming in smoke.ts. Renamed to annotations, comment included.

ExperimentalWarning on 22.0.0. Expected, as you say. It's on stderr, not the protocol channel, and comes from the JSON imports on Node versions before JSON modules went stable. No change.

cooldown validity. A post-merge check, already listed in the PR description: confirm the Dependabot last-checked time keeps advancing after merge.

…llow-ups (#90) Closes #89. Supersedes #83 (folded in as its own commit, `b3d6841`). ## Why Every push re-ran a full Opus review. Each round found fresh nits instead of checking whether the last round's findings were fixed: 6 full runs on #87, 3 on #45 for an unchanged dependency diff. ## What changes - **Triggers:** adds `ready_for_review` and skips drafts. The fork guard stays on every job. - **Rebase detection:** on `synchronize`, a shell step compares the PR's own diff with the previous push's using `git patch-id`. A pure rebase onto newer `main` runs no review in any job. If the old SHA can't be fetched after a force-push, the step assumes the diff changed. - **Dependabot / catalog jobs:** run their full review again only when their diff changed. - **General job:** a full review on open. Later pushes get a Sonnet follow-up scoped to the new commits: `git diff BEFORE AFTER`, or `git range-diff` after a rebase so main's changes aren't included. It reports resolved / partially addressed / not addressed for earlier findings, resolves fixed threads, and flags only bugs, security issues and regressions. If the first full review was cancelled, it reviews the whole PR instead. - **Full review prompt:** adds a "what not to report" section, cuts Phase 3 to one sentence, and lets the reviewer read CI results with `gh pr checks` instead of running `npm`. - **Outdated rules in the prompt:** the general prompt now points at `CLAUDE.md` instead of restating it. The restated list said `org` falls back to `config.org`, the opposite of the rule, and left the version out of `User-Agent`. The dependency prompt now says tools are registered in `src/server.ts`. - `CLAUDE.md` documents the phasing. ## Testing - Rebase detection was tested in a throwaway repo: a pure rebase compared equal, a real change differed, and `range-diff` output was checked. - `actionlint` (with shellcheck) passes. ## Needs a manual review The action refuses to run when its workflow differs from the default branch (see `CLAUDE.md`, #33), so **this PR can't review itself**. It takes effect after merge; check the first follow-up run on the next PR. Points to look at: - **Model:** the follow-up uses `claude-sonnet-5-5`; the full reviews stay on `claude-opus-5`. - **Thread resolving:** not confirmed that the job's `pull-requests: write` is enough for the GraphQL `resolveReviewThread` mutation. If it isn't, threads stay open and the rest still works. - **`gh api` access:** the follow-up allows `Bash(gh api:*)`, as in the CLI repo's workflow. The token is limited to `contents: read` and `pull-requests: write`. - **Limits:** the follow-up is capped at 30 turns and $2; the full reviews keep 60.

tsconfig.json covers src only and vitest strips types, so type errors in test/ and scripts/ never failed anything; one in kosli-client.test.ts had gone unnoticed. tsconfig.test.json and npm run typecheck close that. scripts/smoke.ts starts a built server over real stdio and checks its version, tool list and search_actions. CI runs it against dist/ and against the unzipped .mcpb, and validates manifest.json with mcpb. It is offline unless KOSLI_API_TOKEN and KOSLI_SMOKE_ORG are set, so it is fork-safe.

Action SHA bumps edit the same workflow lines and conflict as separate PRs, so they become one group. Dev minor/patch bumps are grouped; the SDK, the only runtime dependency, gets its own group; majors stay ungrouped.

… test Address review on #87: - Validate the manifest inside the unzipped .mcpb, which carries the version pack-mcpb.sh injects, rather than the repo's placeholder copy. - Assert readOnlyHint/destructiveHint on the three tools. Nothing else tests them, and they are what makes a client ask before a write. - Report a failed tool call's own message instead of a JSON parse error. - Connect inside the try so a failed handshake still closes the child. - Docs: typecheck covers the .ts files in scripts/, not sync-version.mjs.

… 22.7 pack-mcpb.sh deleted package.json from the bundle, leaving nothing to declare dist/ as ESM. Node 22.7+ detects that from the syntax (with a warning); 22.0-22.6, which engines and the manifest both allow, parse it as CommonJS and fail on the first import. Reproduced on Node 22.6.0, and confirmed fixed on 22.0.0, 22.6.0 and 22.7.0.

…en broken - Run the bundle smoke test on exactly Node 22.0.0 on the 22 leg. The matrix floor is the dev floor, so nothing tested what engines promises. - Assert the smoke test fails against a missing entry point, so it can't silently turn into a step that always passes. - Move @anthropic-ai/mcpb into devDependencies so Dependabot tracks it, instead of an npx pin nothing updates.

The server sets it explicitly; !== true would also have passed if it were dropped. Addresses review on #87.

Address review on #87: - npx --no-install, so a missing devDependency fails instead of fetching and running whatever registry package is called mcpb. - Declare the consumer floor as a matrix include key, so the floor steps can't silently stop matching when the matrix changes; note that the client side runs on the floor too. - smoke.ts: build the child env from the SDK's getDefaultEnvironment (works on Windows, still copies no KOSLI_*), return unknown from callJson, and tolerate a result with no content array. - CLAUDE.md: the bundle now has a package.json stub with no version, which is why src/version.ts is still needed.

Address review on #87: - release.yml now typechecks, smoke-tests dist, and packs, validates and smoke-tests the .mcpb before npm publish, which can't be undone and previously ran before the bundle was even built. - timeout-minutes on the CI and release jobs, instead of the 6h default. - smoke.ts: rename hints to annotations; in this repo hints means src/hints.json.

… findings - smoke.ts read annotations through Object.fromEntries, which returns any, so typecheck didn't cover the four checks. A Map keeps them typed: a misspelt hint now fails typecheck. - test/server.test.ts gets the same annotation check, so a lost destructiveHint fails npm test, not only the smoke step. - The negative smoke step in ci.yml now requires smoke's own "not ok" line, so a crash in the smoke script itself can't pass for a caught failure. - sync-version.mjs (and the src/version.ts it generates) said pack-mcpb.sh deletes the bundle's package.json. It replaces it with a { "type": "module" } stub. - CONTRIBUTING.md said CI runs "the first three" steps; it now names them.

Release prep for **0.6.0**. A draft for now: it gets rebased once #97 (CodeQL) merges, then marked ready. `npm version minor --no-git-tag-version` moves `package.json`, `package-lock.json` and the generated `src/version.ts` together. `manifest.json` gets the version at pack time. Nothing else changes. ## What's in 0.6.0 (since 0.5.0) ### ⚠️ Changes that affect existing setups - **No configured org (#65).** `KOSLI_ORG` and the Desktop extension's *Organization* setting are removed. Every org-scoped call names its org through the execute tools' `org` input, so no request lands in an org the caller didn't name. An existing config that still sets `KOSLI_ORG` starts fine and logs that the value is ignored, but calls that relied on the default are now refused until an org is given. - **Node ≥ 22 (#38).** It was ≥ 20. ### Fixes - **Evidence downloads work again (#88, fixes #86).** Since 16 Oct, every `get_attestation_evidence` call failed: our token was forwarded to the pre-signed S3 URL. Text files come back as text, and binary files return a clear error. - **The `.mcpb` bundle starts on Node 22.0 – 22.6 (#87).** It didn't before. - **No known vulnerabilities in the bundled dependencies (#96).** `npm audit --omit=dev` is clean. ### Features - **`org` input on the execute tools (#65).** - **`User-Agent` carries the version (#41):** `kosli-mcp-server/<version>`. ### Dependencies - **Runtime:** `@modelcontextprotocol/sdk` 1.29.0 → 1.32.1 (#29, #71, #79). - **Dev:** TypeScript 7 (#45), with emitted output byte-identical to 5.9. vitest 5 (#93). - **Catalog:** regenerated from the OpenAPI spec (#50, #53, #60, #69, #78). ### Release confidence (CI only) Typecheck of tests and scripts, a stdio smoke test against `dist/` and the unzipped bundle (including Node 22.0.0), `mcpb validate`, and `release.yml` publishing only after every check (#87). There's also a phased Claude review (#90), Dependabot grouping with a 7-day cooldown, and CodeQL (#97). ## Checked locally on this branch - Build, typecheck, 122 tests. - The smoke test reports `server advertises version 0.6.0`, both from `dist/` and from the unzipped bundle. - `kosli-mcp-server-0.6.0.mcpb`: the manifest says 0.6.0 and `mcpb validate` passes. ## Before tagging (after merge) - [ ] Rebase onto `main` once #97 merges, then mark ready. - [ ] Install the 0.6.0 `.mcpb` in Claude Desktop, including an upgrade over 0.5.0 with an org configured. - [ ] Run `KOSLI_API_TOKEN=… KOSLI_SMOKE_ORG=<org> npm run smoke` on `main`. - [ ] Download one piece of evidence via `get_attestation_evidence`. - [ ] `git tag -a v0.6.0 -m "v0.6.0" <merge commit> && git push origin v0.6.0`. `release.yml` does the rest.