From 5b5dcc945186641786b28780d3c3dac210d37330 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Sun, 1 Nov 2026 16:17:52 +0000 Subject: [PATCH] ci: Run package checks from the test.yml matrix Move the @sentry/mcp-server clean-consumer typecheck from its own path-filtered workflow into `sentryCi.e2e: "test:package"`, matching @sentry/logger. The test.yml matrix already selects the package when it or a workspace dependency changes, and its `test` job is the required check. Document `package.json#sentryCi` in AGENTS.md, the quality checks guide, and the GitHub Actions guide. Add a workflow policy test that fails when a workflow runs on pull requests without a listed reason, pointing contributors to `sentryCi` instead. Fixes #1777 Co-Authored-By: BYK <126780+BYK@users.noreply.github.com> --- .github/workflows/mcp-server-package.yml | 115 ------------------ AGENTS.md | 3 +- docs/contributing/quality-checks.md | 8 ++ docs/operations/github-actions.md | 51 +++++++- package.json | 2 +- packages/mcp-server/package.json | 4 + packages/mcp-server/test/package/check.sh | 22 ++++ .../mcp-server/test/package/consumer/index.ts | 19 +++ .../test/package/consumer/package.json | 4 + .../test/package/consumer/tsconfig.json | 10 ++ scripts/workflow-policy.test.mjs | 102 ++++++++++++++++ 11 files changed, 221 insertions(+), 119 deletions(-) delete mode 100644 .github/workflows/mcp-server-package.yml create mode 100755 packages/mcp-server/test/package/check.sh create mode 100644 packages/mcp-server/test/package/consumer/index.ts create mode 100644 packages/mcp-server/test/package/consumer/package.json create mode 100644 packages/mcp-server/test/package/consumer/tsconfig.json create mode 100644 scripts/workflow-policy.test.mjs diff --git a/.github/workflows/mcp-server-package.yml b/.github/workflows/mcp-server-package.yml deleted file mode 100644 index 93dc4da2f..000000000 --- a/.github/workflows/mcp-server-package.yml +++ /dev/null @@ -1,115 +0,0 @@ -name: MCP Server Package - -on: - push: - branches: [main] - pull_request: - paths: - - ".github/workflows/mcp-server-package.yml" - - "packages/mcp-core/**" - - "packages/mcp-server/**" - - "packages/mcp-server-mocks/**" - - "packages/mcp-server-tsconfig/**" - - "package.json" - - "pnpm-lock.yaml" - - "pnpm-workspace.yaml" - -permissions: - contents: read - -jobs: - package-consumer: - name: Clean Consumer Typecheck - runs-on: ubuntu-latest - - steps: - - uses: actions/checkout@v4 - - - name: Setup Node.js - uses: actions/setup-node@v4 - with: - node-version: "22" - - # pnpm/action-setup@v4 - - uses: pnpm/action-setup@a7487c7e89a18df4991f7f222e4898a00d66ddda - name: Install pnpm - with: - run_install: false - - - name: Get pnpm store directory - shell: bash - run: | - echo "STORE_PATH=$(pnpm store path --silent)" >> $GITHUB_ENV - - - uses: actions/cache@v4 - name: Setup pnpm cache - with: - path: ${{ env.STORE_PATH }} - key: ${{ runner.os }}-pnpm-store-${{ hashFiles('**/pnpm-lock.yaml') }} - restore-keys: | - ${{ runner.os }}-pnpm-store- - - - name: Install dependencies - run: pnpm install --frozen-lockfile - - - name: Build stdio package and dependencies - run: pnpm --filter '@sentry/mcp-server...' --if-present run build - - - name: Pack stdio package - run: pnpm --dir packages/mcp-server pack --pack-destination "$RUNNER_TEMP" - - - name: Typecheck clean consumer - shell: bash - run: | - set -euo pipefail - - consumer="$RUNNER_TEMP/mcp-server-consumer" - mkdir -p "$consumer" - cd "$consumer" - - export npm_config_cache="$RUNNER_TEMP/npm-cache" - cat > package.json <<'EOF' - { - "private": true, - "type": "module" - } - EOF - - npm install --ignore-scripts --no-audit --no-fund "$RUNNER_TEMP"/sentry-mcp-server-*.tgz typescript@^5.8.3 @types/node@^22 - - cat > tsconfig.json <<'EOF' - { - "compilerOptions": { - "module": "NodeNext", - "moduleResolution": "NodeNext", - "noEmit": true, - "strict": true, - "target": "ES2022" - }, - "include": ["index.ts"] - } - EOF - - cat > index.ts <<'EOF' - import { - startStdio, - type StdioServerContext, - } from "@sentry/mcp-server/transports/stdio"; - - declare const server: Parameters[0]; - - const telemetryContext: StdioServerContext = { - experimentalMode: false, - mcpUrl: "https://example.com/mcp", - sentryHost: "sentry.io", - }; - - void startStdio(server, telemetryContext); - void startStdio(server, { - accessToken: "token", - constraints: {}, - sentryHost: "sentry.io", - }); - EOF - - npx tsc --noEmit diff --git a/AGENTS.md b/AGENTS.md index f50faf526..03fbefe9a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -12,6 +12,7 @@ Toolkit contains the Sentry CLI, the Sentry MCP server, and shared code. Product - **MCP tool count**: Target ≤20, hard limit 25 (AI agents have limited tool slots). - **Quality gate**: `pnpm run tsc && pnpm run lint && pnpm run test` must pass before committing. +- **Package CI**: Put checks for one workspace package in its `package.json` scripts and `sentryCi`, which `test.yml` runs per project. Do not add a workflow for them. See docs/operations/github-actions.md#package-ci-checks. ## Repository Structure @@ -73,7 +74,7 @@ toolkit/ - docs/operations/stdio-auth.md — Device code flow, token caching, client ID architecture - docs/operations/oauth-signout-playbook.md — Remote OAuth diagnostic runbook - docs/operations/embedded-agents.md — LLM provider configuration for AI-powered tools -- docs/operations/github-actions.md — MCP deployment workflows +- docs/operations/github-actions.md — CI matrix, `package.json#sentryCi`, and MCP deployment workflows - docs/operations/logging.md — Logging guidance - docs/operations/monitoring.md — Monitoring guidance - docs/operations/token-cost-tracking.md — Tool definition token cost tracking diff --git a/docs/contributing/quality-checks.md b/docs/contributing/quality-checks.md index 5077344f9..7e1a11663 100644 --- a/docs/contributing/quality-checks.md +++ b/docs/contributing/quality-checks.md @@ -12,6 +12,13 @@ Do not proceed if any check fails. `pnpm run lint` includes `oxfmt --check`. If it fails, run `pnpm run format`. +## Package CI Checks + +CI runs each workspace package's build, lint, typecheck, test, and E2E scripts +from `test.yml`. To add a check for one package, add a script and, if its name +is not a default, point `package.json#sentryCi` at it. Do not add a workflow. +See [Package CI checks](../operations/github-actions.md#package-ci-checks). + ## Tool Count Limits See [adding-tools.md](adding-tools.md#tool-count-limits) for current limits and guidance on when to add vs. combine tools. @@ -29,4 +36,5 @@ happy-path inline snapshot test. Review tool output snapshots against - [ ] Tool count within limits - [ ] New tools have tests with inline snapshots - [ ] Tool/skill definitions regenerated if tools changed (`pnpm run --filter @sentry/mcp-core generate-definitions`) +- [ ] Package-specific CI checks are in `package.json#sentryCi`, not a new workflow - [ ] Documentation updated if patterns changed diff --git a/docs/operations/github-actions.md b/docs/operations/github-actions.md index b0e0484bb..7eeb1cf8b 100644 --- a/docs/operations/github-actions.md +++ b/docs/operations/github-actions.md @@ -18,8 +18,9 @@ docs depend on `sentry`). Root-level changes and non-PR events check every enabled project. Each project has its own build, lint, typecheck, test, policy, and E2E steps when those scripts exist. The always-present `test` job checks discovery, installation, repository quality, and every selected project. -Package-specific exceptions live in `package.json#sentryCi`; the standalone -smoke-test suite remains in its own workflow. +Package-specific checks and exceptions live in `package.json#sentryCi`; see +[Package CI checks](#package-ci-checks). The standalone smoke-test suite remains +in its own workflow. ### deploy.yml @@ -98,6 +99,52 @@ Runs the label publisher's regression tests when its workflow or tests change. Covers label replacement, stale revisions, failed classifications, and concurrent label creation. +## Package CI checks + +Add a check for one workspace package to that package's `package.json`, not to +a new workflow. `scripts/ci-projects.mjs` reads each package's scripts and its +`sentryCi` object, and `test.yml` runs the result for every pull request that +changes the package or one of its workspace dependencies. A path-filtered +workflow for one package duplicates that selection and is not a required check. + +Each step runs a package script. Without a `sentryCi` override, the first +script that exists is used: + +| Field | Default scripts | Notes | +| ------------ | ------------------------------ | ------------------------------------------------------------------------------- | +| `prepare` | `ci:prepare` | Runs before the build. | +| `build` | `build` | | +| `lint` | `lint` | | +| `typecheck` | `typecheck`, `tsc`, `check` | | +| `test` | `test:ci`, `test:unit`, `test` | | +| `policy` | `ci:policy` | | +| `e2e` | `ci:e2e`, `test:e2e` | Runs on Node 24 after the other steps. | +| `npmRuntime` | none | Runs the script, packs the package, and runs its binary on Node 20, 22, and 24. | + +Set a field to a script name to override it, or to `false` to skip it. The +other fields are: + +- `coverage`, `junit`: report paths the job verifies, or `false`. +- `dependencies`: extra workspace packages whose changes select this one. +- `enabled`: `false` removes the package from the matrix. +- `roles`: names that `node scripts/ci-projects.mjs --roles NAME` resolves to + the package name and path, so a specialized workflow need not hard-code them. + +For example, `@sentry/logger` and `@sentry/mcp-server` check their packed +tarballs from a clean consumer project: + +```json +{ + "sentryCi": { "e2e": "test:package" }, + "scripts": { "test:package": "bash test/package/check.sh" } +} +``` + +A new workflow fits only when the matrix job cannot run the check: another +operating system, a deployment, secrets, or pull request comments and labels. +`scripts/workflow-policy.test.mjs` fails for a workflow that runs on pull +requests unless its list names that workflow and the reason. + ## Required Secrets The `production` environment is restricted to `main` and holds: diff --git a/package.json b/package.json index d2b3cfc4d..a46aba362 100644 --- a/package.json +++ b/package.json @@ -23,7 +23,7 @@ }, "scripts": { "docs:check": "node scripts/check-doc-links.mjs", - "test:ci-projects": "node --test scripts/ci-projects.test.mjs scripts/cli-nightly-version.test.mjs scripts/cli-nightly-watchdog.test.mjs scripts/cloudflare-deployment.test.mjs scripts/deploy-workflow.test.mjs scripts/migrate-cloudflare-token.test.mjs scripts/oxc-config.test.mjs scripts/cli-lint-rules.test.mjs", + "test:ci-projects": "node --test scripts/ci-projects.test.mjs scripts/cli-nightly-version.test.mjs scripts/cli-nightly-watchdog.test.mjs scripts/cloudflare-deployment.test.mjs scripts/deploy-workflow.test.mjs scripts/migrate-cloudflare-token.test.mjs scripts/oxc-config.test.mjs scripts/cli-lint-rules.test.mjs scripts/workflow-policy.test.mjs", "dev": "pnpm --filter '@sentry/mcp-cloudflare...' --if-present run build && dotenv -e .env -e .env.local -- pnpm --parallel --filter @sentry/mcp-cloudflare --filter @sentry/mcp-core --filter @sentry/mcp-server-mocks --if-present run dev", "dev:stdio": "pnpm --filter '@sentry/mcp-server...' --if-present run build && dotenv -e .env -e .env.local -- pnpm --parallel --filter @sentry/mcp-server --filter @sentry/mcp-core --filter @sentry/mcp-server-mocks --if-present run dev", "build": "dotenv -e .env -e .env.local -- pnpm -r --filter '!sentry' --filter '!sentry-cli-docs' --if-present run build", diff --git a/packages/mcp-server/package.json b/packages/mcp-server/package.json index a54ee8bed..e22d0a3e5 100644 --- a/packages/mcp-server/package.json +++ b/packages/mcp-server/package.json @@ -10,6 +10,9 @@ "publishConfig": { "access": "public" }, + "sentryCi": { + "e2e": "test:package" + }, "license": "FSL-1.1-ALv2", "author": "Sentry", "homepage": "https://github.com/getsentry/sentry-mcp", @@ -47,6 +50,7 @@ "start": "tsx src/index.ts", "test": "vitest run", "test:ci": "vitest run --reporter=default --reporter=junit --outputFile=tests.junit.xml", + "test:package": "bash test/package/check.sh", "test:watch": "vitest", "tsc": "tsc --noEmit" }, diff --git a/packages/mcp-server/test/package/check.sh b/packages/mcp-server/test/package/check.sh new file mode 100755 index 000000000..02f843258 --- /dev/null +++ b/packages/mcp-server/test/package/check.sh @@ -0,0 +1,22 @@ +#!/usr/bin/env bash +# Pack @sentry/mcp-server, install the tarball into a clean consumer project, +# and typecheck it against the stdio transport exports and declarations that +# npm users get. Build workspace dependencies first, for example with +# `pnpm --filter '@sentry/mcp-server^...' --if-present run build`. +set -euo pipefail + +package_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" +work_dir="$(mktemp -d)" +trap 'rm -rf "$work_dir"' EXIT + +cd "$package_dir" +pnpm run build +pnpm pack --pack-destination "$work_dir" + +cp -R test/package/consumer "$work_dir/consumer" +cd "$work_dir/consumer" +export npm_config_cache="$work_dir/npm-cache" +npm install --ignore-scripts --no-audit --no-fund --no-save \ + "$work_dir"/sentry-mcp-server-*.tgz typescript@^5.8.3 @types/node@^22 +npx tsc -p . +echo "Packed @sentry/mcp-server typechecks in a clean consumer." diff --git a/packages/mcp-server/test/package/consumer/index.ts b/packages/mcp-server/test/package/consumer/index.ts new file mode 100644 index 000000000..b28de4663 --- /dev/null +++ b/packages/mcp-server/test/package/consumer/index.ts @@ -0,0 +1,19 @@ +import { + startStdio, + type StdioServerContext, +} from "@sentry/mcp-server/transports/stdio"; + +declare const server: Parameters[0]; + +const telemetryContext: StdioServerContext = { + experimentalMode: false, + mcpUrl: "https://example.com/mcp", + sentryHost: "sentry.io", +}; + +void startStdio(server, telemetryContext); +void startStdio(server, { + accessToken: "token", + constraints: {}, + sentryHost: "sentry.io", +}); diff --git a/packages/mcp-server/test/package/consumer/package.json b/packages/mcp-server/test/package/consumer/package.json new file mode 100644 index 000000000..e986b24bb --- /dev/null +++ b/packages/mcp-server/test/package/consumer/package.json @@ -0,0 +1,4 @@ +{ + "private": true, + "type": "module" +} diff --git a/packages/mcp-server/test/package/consumer/tsconfig.json b/packages/mcp-server/test/package/consumer/tsconfig.json new file mode 100644 index 000000000..d0ddb92be --- /dev/null +++ b/packages/mcp-server/test/package/consumer/tsconfig.json @@ -0,0 +1,10 @@ +{ + "compilerOptions": { + "module": "NodeNext", + "moduleResolution": "NodeNext", + "noEmit": true, + "strict": true, + "target": "ES2022" + }, + "include": ["index.ts"] +} diff --git a/scripts/workflow-policy.test.mjs b/scripts/workflow-policy.test.mjs new file mode 100644 index 000000000..c592b7513 --- /dev/null +++ b/scripts/workflow-policy.test.mjs @@ -0,0 +1,102 @@ +import assert from "node:assert/strict"; +import { readdirSync, readFileSync } from "node:fs"; +import { describe, it } from "node:test"; + +// Every workflow that runs on pull requests, and why `test.yml` cannot run its +// checks. Package-specific checks (build, test, E2E, packed-tarball checks) +// belong in `package.json#sentryCi`, so `test.yml` runs them in its +// per-project matrix. See docs/operations/github-actions.md#package-ci-checks. +const PULL_REQUEST_WORKFLOWS = new Map([ + ["test.yml", "Builds and runs the per-project matrix."], + ["eval.yml", "Runs AI evaluations against model providers."], + [ + "install-scripts.yml", + "Installs the latest published CLI release on macOS and Windows.", + ], + ["local.yml", "Runs Sentry Local E2E tests on Linux, macOS, and Windows."], + ["pr-risk-jev.yml", "Labels pull requests by risk."], + ["pr-risk-labels-test.yml", "Tests the PR risk label publisher script."], + [ + "smoke-tests.yml", + "Starts the MCP Worker and publishes a smoke-test report.", + ], + ["token-cost.yml", "Reports MCP tool token costs against main."], +]); + +const PULL_REQUEST_EVENTS = [ + "pull_request", + "pull_request_target", + "merge_group", +]; + +const workflowsDirectory = new URL("../.github/workflows/", import.meta.url); + +/** Return the text of a workflow's top-level `on` trigger. */ +function workflowTriggers(source) { + const lines = source.split("\n"); + const start = lines.findIndex((line) => /^["']?on["']?:/.test(line)); + if (start === -1) { + return ""; + } + const end = lines.findIndex( + (line, index) => index > start && /^[^\s#]/.test(line), + ); + return lines.slice(start, end === -1 ? undefined : end).join("\n"); +} + +function runsOnPullRequests(source) { + const triggers = workflowTriggers(source); + return PULL_REQUEST_EVENTS.some((event) => + new RegExp(`(?:^|[\\s\\[,])${event}(?=[\\s:,\\]]|$)`, "m").test(triggers), + ); +} + +describe("workflow policy", () => { + it("detects pull request triggers in every YAML form", () => { + assert.equal(runsOnPullRequests("on: pull_request\n"), true); + assert.equal(runsOnPullRequests("on: [push, pull_request]\n"), true); + assert.equal( + runsOnPullRequests("on:\n pull_request:\n paths: [a]\njobs:\n"), + true, + ); + assert.equal(runsOnPullRequests("on:\n merge_group:\njobs:\n"), true); + assert.equal( + runsOnPullRequests("on:\n pull_request_target:\n types: [opened]\n"), + true, + ); + assert.equal( + runsOnPullRequests( + "on:\n push:\n branches: [main]\njobs:\n pull_request:\n", + ), + false, + ); + assert.equal( + runsOnPullRequests("on:\n workflow_run:\n workflows: [Test]\n"), + false, + ); + }); + + it("keeps package checks in the test.yml matrix", () => { + const workflows = readdirSync(workflowsDirectory).filter((file) => + /\.ya?ml$/.test(file), + ); + const pullRequestWorkflows = workflows.filter((file) => + runsOnPullRequests( + readFileSync(new URL(file, workflowsDirectory), "utf8"), + ), + ); + + for (const file of pullRequestWorkflows) { + assert.ok( + PULL_REQUEST_WORKFLOWS.has(file), + `.github/workflows/${file} runs on pull requests. Put package-specific checks in package.json#sentryCi so test.yml runs them (see docs/operations/github-actions.md#package-ci-checks). If the matrix cannot run this workflow, add it with the reason to PULL_REQUEST_WORKFLOWS in scripts/workflow-policy.test.mjs.`, + ); + } + for (const file of PULL_REQUEST_WORKFLOWS.keys()) { + assert.ok( + pullRequestWorkflows.includes(file), + `Remove ${file} from PULL_REQUEST_WORKFLOWS; it no longer runs on pull requests.`, + ); + } + }); +});