diff --git a/.github/workflows/claude-review.yaml b/.github/workflows/claude-review.yaml new file mode 100644 index 0000000..02de60a --- /dev/null +++ b/.github/workflows/claude-review.yaml @@ -0,0 +1,271 @@ +# Automated PR review using Claude (anthropics/claude-code-action). +# +# Auth is OIDC federation via org-wide variables (ANTHROPIC_FEDERATION_RULE_ID, +# ANTHROPIC_ORGANIZATION_ID, ANTHROPIC_SERVICE_ACCOUNT_ID) — no API key secret is +# needed, which is also why this works on Dependabot-triggered runs, where regular +# Actions secrets are unavailable. +# +# Three cases: +# - Dependabot PRs: upgrade risk in changelogs and in how we actually use the dep. +# - update-catalog PRs: what changed in the generated catalog, especially removals. +# - Everything else: general code review against the invariants in CLAUDE.md. +# +# PRs from forks are skipped: they run without the credentials this needs, and on a +# public repo an unguarded trigger lets anyone spend budget. + +name: Claude PR Review + +on: + pull_request: + types: [opened, synchronize, reopened] + +concurrency: + group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true + +permissions: + contents: read + +jobs: + review-dependency-updates: + if: >- + github.event.pull_request.head.repo.full_name == github.repository && + github.event.pull_request.user.login == 'dependabot[bot]' + runs-on: ubuntu-latest + timeout-minutes: 15 + permissions: + contents: read + id-token: write + pull-requests: write + steps: + - name: Harden runner + uses: step-security/harden-runner@b09bb98e06d4d774595224525879c09bc6e98c40 # v2.20.1 + with: + egress-policy: audit + + - name: Checkout + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + with: + fetch-depth: 0 + + - name: Claude review (dependency updates) + uses: anthropics/claude-code-action@5ef2e550a465a721f4f45e4a7d3c340c873e1dcc # v1.0.190 + env: + GH_TOKEN: ${{ github.token }} + with: + anthropic_federation_rule_id: ${{ vars.ANTHROPIC_FEDERATION_RULE_ID }} + anthropic_organization_id: ${{ vars.ANTHROPIC_ORGANIZATION_ID }} + anthropic_service_account_id: ${{ vars.ANTHROPIC_SERVICE_ACCOUNT_ID }} + track_progress: true + use_sticky_comment: true + allowed_bots: 'dependabot[bot]' + claude_args: | + --max-turns 40 + --max-budget-usd 3 + --model claude-opus-5 + --allowedTools "mcp__github_inline_comment__create_inline_comment,Bash(gh pr comment:*),Bash(gh pr diff:*),Bash(gh pr view:*),Bash(git diff:*)" + prompt: | + REPO: ${{ github.repository }} + PR NUMBER: ${{ github.event.pull_request.number }} + + This PR updates a dependency. Please: + + 1. Identify what changed and whether it crosses a major version. + 2. Consider the changelog, release notes, and semver: look for breaking changes, + deprecations, and behaviour changes that could affect this codebase. + 3. Search the repo for how the dependency is actually used and flag anything that + might break. Specifics that matter here: + - `@modelcontextprotocol/sdk` — tool registration and annotations in `src/index.ts`. + - `vitest` — config and test APIs across `test/`. + - `typescript` — `strict: true` must not be weakened; ESM with `NodeNext` and + `.js` extensions on relative imports. + - `@types/node` — the package declares `engines: node >= 20`. Flag type + definitions that assume a newer runtime than we support. + - GitHub Actions — every action is pinned to a full commit SHA with a trailing + version comment. Confirm the bump preserved that style, and read the action's + release notes for breaking input or behaviour changes. + 4. Call out concrete risks and suggest follow-up checks or tests where useful. + + Note: The PR branch is already checked out in the current working directory. + Keep the review concise and scoped to this PR. + + Use `gh pr comment --edit-last --create-if-none` for top-level feedback, + so the summary replaces the previous run's summary instead of stacking. + Use `mcp__github_inline_comment__create_inline_comment` to highlight specific code issues. + Only post GitHub comments - don't submit review text as messages. + + review-catalog-update: + if: >- + github.event.pull_request.head.repo.full_name == github.repository && + github.event.pull_request.user.login == 'github-actions[bot]' && + github.event.pull_request.head.ref == 'chore/update-catalog' + runs-on: ubuntu-latest + timeout-minutes: 15 + permissions: + contents: read + id-token: write + pull-requests: write + steps: + - name: Harden runner + uses: step-security/harden-runner@b09bb98e06d4d774595224525879c09bc6e98c40 # v2.20.1 + with: + egress-policy: audit + + - name: Checkout + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + with: + fetch-depth: 0 + + - name: Claude review (catalog update) + uses: anthropics/claude-code-action@5ef2e550a465a721f4f45e4a7d3c340c873e1dcc # v1.0.190 + env: + GH_TOKEN: ${{ github.token }} + with: + anthropic_federation_rule_id: ${{ vars.ANTHROPIC_FEDERATION_RULE_ID }} + anthropic_organization_id: ${{ vars.ANTHROPIC_ORGANIZATION_ID }} + anthropic_service_account_id: ${{ vars.ANTHROPIC_SERVICE_ACCOUNT_ID }} + track_progress: true + use_sticky_comment: true + allowed_bots: 'github-actions[bot]' + claude_args: | + --max-turns 40 + --max-budget-usd 3 + --model claude-opus-5 + --allowedTools "mcp__github_inline_comment__create_inline_comment,Bash(gh pr comment:*),Bash(gh pr diff:*),Bash(gh pr view:*),Bash(git diff:*)" + prompt: | + REPO: ${{ github.repository }} + PR NUMBER: ${{ github.event.pull_request.number }} + + This PR regenerates `src/catalog.json` from Kosli's OpenAPI spec. The catalog is + generated, never hand-edited, and it is what `search_actions` exposes to the LLM — + so a silent loss here becomes a silently missing capability for users. + + Please summarise the diff in terms a reviewer can act on: + + 1. **Actions added** — list them by `id`, `method`, and `path`. + 2. **Actions removed** — list them, and say clearly that each removal needs + confirming against the live spec at https://app.kosli.com/api/v2/openapi.json + before merging. Removals have twice been the interesting case: once a stale + branch clobbered the catalog (#16), and once an endpoint was genuinely retired + upstream (#18). The diff alone cannot distinguish those. + 3. **Actions changed** — parameters added or removed, schemas altered, fields newly + marked `deprecated`. + 4. **Dangling references** — for anything removed or renamed, grep `src/`, `test/`, + and `README.md` for references that would now be stale. `src/hints.json` and + `test/fixtures/catalog-subset.json` are the likely places. + 5. **Sanity checks** — flag a suspiciously large drop in the number of actions, or + any `$ref` that survived into the catalog (the generator inlines them, so a + remaining `$ref` is a dangling pointer the LLM cannot follow). + + Note: The PR branch is already checked out in the current working directory. + `git diff origin/main...HEAD -- src/catalog.json` is the diff to read. + + Use `gh pr comment --edit-last --create-if-none` for top-level feedback, + so the summary replaces the previous run's summary instead of stacking. + Only post GitHub comments - don't submit review text as messages. + + review-general: + if: >- + github.event.pull_request.head.repo.full_name == github.repository && + github.event.pull_request.user.login != 'dependabot[bot]' && + github.event.pull_request.user.login != 'github-actions[bot]' + runs-on: ubuntu-latest + timeout-minutes: 15 + permissions: + contents: read + id-token: write + pull-requests: write + steps: + - name: Harden runner + uses: step-security/harden-runner@b09bb98e06d4d774595224525879c09bc6e98c40 # v2.20.1 + with: + egress-policy: audit + + - name: Checkout + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + with: + fetch-depth: 0 + + - name: Claude review (general) + uses: anthropics/claude-code-action@5ef2e550a465a721f4f45e4a7d3c340c873e1dcc # v1.0.190 + env: + GH_TOKEN: ${{ github.token }} + with: + anthropic_federation_rule_id: ${{ vars.ANTHROPIC_FEDERATION_RULE_ID }} + anthropic_organization_id: ${{ vars.ANTHROPIC_ORGANIZATION_ID }} + anthropic_service_account_id: ${{ vars.ANTHROPIC_SERVICE_ACCOUNT_ID }} + track_progress: true + use_sticky_comment: true + claude_args: | + --max-turns 40 + --max-budget-usd 5 + --model claude-opus-5 + --allowedTools "mcp__github_inline_comment__create_inline_comment,Bash(gh pr comment:*),Bash(gh pr diff:*),Bash(gh pr view:*),Bash(git diff:*)" + prompt: | + REPO: ${{ github.repository }} + PR NUMBER: ${{ github.event.pull_request.number }} + + Read CLAUDE.md in the repo root first. It documents the architecture, the build and + test commands, and a set of deliberate design decisions. Use it to judge whether this + PR follows established conventions. + + ## Phase 1: Architectural Analysis + + This is a catalog-driven MCP server. The design decisions below are intentional, and a + PR that quietly breaks one is the most valuable thing you can catch: + + - Exactly three MCP tools. New endpoints come from regenerating the catalog, never + from adding a tool per endpoint. + - `src/catalog.json` is generated by `scripts/generate-catalog.ts`, not hand-edited. + `$ref`s are inlined at generation time. + - `KosliClient` returns `{ error: true, status, statusText, message }` on non-2xx. + It does not throw. Tools stringify whatever they get. + - Responses are serialised with compact `JSON.stringify` to save tokens. No + pretty-printing. + - The `org` path parameter falls back to `config.org` in `KosliClient.buildUrl`. + - Every request carries `User-Agent: kosli-mcp-server`. + - ESM only: relative imports need `.js` extensions even though the source is `.ts`. + - TypeScript `strict: true` must not be weakened. + - No HTTP client library — `globalThis.fetch`, injectable as `fetchFn` for tests. + + Also check the read/write split: `execute_read_action` is annotated `readOnlyHint` + and `execute_write_action` `destructiveHint`. Changes that move an endpoint across + that boundary, or weaken an annotation, deserve scrutiny. + + ## Phase 2: Code Review + + Review this PR for: + + 1. **Code quality** – clarity and consistency with surrounding code. + 2. **Edge and corner cases** – missing null/empty checks, boundary conditions, error + paths, invalid inputs. Param serialisation differs between GET/DELETE + (querystring) and other methods (JSON body); both branches need to stay coherent. + 3. **Possible bugs** – logic errors, misuse of APIs, incorrect assumptions. + 4. **Readability and maintainability** – naming, structure, ease of future change. + 5. **Testing** – tests live under `test/` mirroring `src/`, use Vitest, and inject + `fetchFn` rather than hitting the network. `test/fixtures/catalog-subset.json` is + a hand-curated slice; prefer extending it over loading the full catalog. Flag new + behaviour that arrives without tests, and any test that would make a network call. + 6. **Security** – this server holds a Kosli API token and can perform destructive + writes. Watch for token leakage into logs or error messages, and for changes that + widen what a write action can reach. + 7. **Performance** – `searchActions` is deliberately dependency-free and O(n) over a + small catalog. Flag anything that makes it superlinear or pulls in a dependency. + + Also worth checking: commit messages and the PR title follow Conventional Commits, + and `manifest.json`'s version placeholder is not hand-edited (it is injected at pack + time from `package.json`). + + ## Phase 3: Acknowledge Good Work + + Briefly note genuinely good design, testing, or clean-ups in the PR. A good review + balances constructive criticism with recognition of quality work. + + Note: The PR branch is already checked out in the current working directory. + Be concise and actionable. Prefer inline suggestions where possible; add a short + summary at the end. + + Use `gh pr comment --edit-last --create-if-none` for top-level feedback, + so the summary replaces the previous run's summary instead of stacking. + Use `mcp__github_inline_comment__create_inline_comment` to highlight specific code issues. + Only post GitHub comments - don't submit review text as messages.