Skip to content

Conversation

@domdomegg
Copy link
Member

Summary

  • Adds automated Claude Code Review workflow that runs on all new and updated PRs
  • Provides AI-powered code reviews without requiring manual @claude mentions
  • Helps maintain code quality with consistent automated reviews

How it works

This workflow automatically triggers Claude Code to review pull requests when they are:

  • Opened (new PRs)
  • Synchronized (new commits pushed to existing PRs)

Claude will analyze the changes and provide feedback on:

  • Code quality and best practices
  • Potential bugs or issues
  • Performance considerations
  • Security concerns
  • Test coverage

Key Features

  • Automatic triggering: No need to manually request reviews
  • Configurable: Includes commented options for customization such as:
    • Filtering by PR author (e.g., only review first-time contributors)
    • Filtering by file types (e.g., only review TypeScript files)
    • Custom review prompts for different scenarios
    • Ability to add test/lint tools to the review process
    • Skip conditions for WIP or draft PRs

Security & Permissions

  • Uses minimal permissions (read-only access to contents, PRs, and issues)
  • Anthropic API key is securely stored as a GitHub Actions secret
  • All reviews are logged in GitHub Actions history

Test plan

After merging:

  1. Create a test PR with some code changes
  2. Verify Claude automatically reviews the PR
  3. Push additional commits and verify Claude re-reviews
  4. Test with different file types to ensure review coverage

🤖 Generated with Claude Code

jspahrsummers and others added 30 commits February 5, 2025 17:58
Work in progress .
This commit adds the core registry service implementation with methods to:
- Retrieve all registry entries
- List entries with cursor-based pagination
- Fetch specific server details by ID
Init world. Basic registry API server <WIP>
Update README to reflect binary name change
Add count to server response and fix Makefile paths
Add Swagger API documentation and handlers for v0
Contains about 300+ initial servers that can be used to seed a mongoDB
We need to work in multiple orgs with different private repos, this is a
temporary solution until we make the main repo public.
…blishing

- Added authentication service and GitHub OAuth integration.
- Introduced new authentication methods and structures in the model.
- Updated the API to handle authentication during the publishing process.
- Created handlers for starting and checking the status of the OAuth flow.
- Enhanced the publish handler to validate authentication credentials.
- Updated configuration to include GitHub OAuth settings.
- Added tests for the OAuth flow and publishing with authentication.
- Updated documentation to reflect changes in the API and authentication process.
…ecks and introducing dynamic auth method determination
joelverhagen and others added 22 commits July 9, 2025 10:27
<!-- Provide a brief summary of your changes -->

## Motivation and Context

This is in support of
#159, to make it
easier, I hope. My team noticed some differences in the JSON schema,
most notably the "choices" property was missing. I tried to write a tool
to do this but slowed down since I am not experienced with Go. This is a
first pass, maybe someone else can finish the tooling.

I restructured the JSON schema to break out child objects like the YAML
file, and brought the descriptions and example into sync. The only
differences are two extra properties in the YAML which are computed by
the server:

- `VersionDetail.is_latest` is not present in the JSON schema, computed
by the registry based on
#158
- `Server.id` is not present in the JSON schema, generated by the
registry

The idea is the a tool could look at ServerDetail in the YAML and then
generate the schema.json with minimal fix-ups (add a root title, remove
`VersionDetail.is_latest` and `Server.id`.

~Question for @tadasant - should `VersionDetail.release_date` be in the
JSON schema? I think _no_ (and let it be computed) but it's there right
now.~ - aligned with
#167 and removed

## How Has This Been Tested?

I have tested the JSON schema against the sample server.json in the repo
(files and markdown blocks).

## Breaking Changes

None.

## Types of changes
<!-- What types of changes does your code introduce? Put an `x` in all
the boxes that apply: -->
- [x] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)
- [x] Documentation update

## Checklist
<!-- Go over all the following points, and put an `x` in all the boxes
that apply. -->
- [x] I have read the [MCP
Documentation](https://modelcontextprotocol.io)
- [x] My code follows the repository's style guidelines
- [ ] New and existing tests pass locally
- [ ] I have added appropriate error handling
- [x] I have added or updated documentation as needed

## Additional context
<!-- Add any other context, implementation notes, or design decisions
-->
Closes #155

The "generalized" `server.json` is the one that can be used in a variety
of situations:

- Discoverability on a centralized registry (i.e. our official Registry
work)
- Discoverability on a decentralized .well-known endpoint
- As a response to an initialization call, so the client knows
information about the MCP server to which it is connecting
- As an input into crafting a [DXT
file](https://www.anthropic.com/engineering/desktop-extensions)
- Packaged in with the source code of an MCP server, so as to have a
structured way context

Of these, the official Registry is but one use case. The Registry has a
unique set of concerns for which we are optimizing, like whitelisting of
specific registries we trust to host MCP server source code for the
public community (`npm`, `pypi`, etc). It is not necessary to have such
a constraint for the generalized server.json, which may be used in, for
example, an internal context where such a validation doesn't make sense.

So I've done the following at a high level:
- Renamed the prior `schema.json` to be `registry-schema.json`
- Introduced a more generic `schema.json` to serve as the schema for the
generalized `server.json`
- Refactored `registry-schema.json` to be simply a constraint on the
base `schema.json`

I made a few small changes in this transition:
- Removed `gitlab` as an option in `repository.source` for
`registry-schema.json`. I'm fine going live with just GitHub support;
would be happy to take a contribution from e.g. Gitlab employees or a
motivated Gitlab customer to expand anytime.
- Removed `homebrew` as an option in `packages.registry_name`; never
really made sense, not a place where folks are publishing MCP servers
- Added `docker` to `packages.runtime_hint`; was just a missing
oversight
- Gave both schemas a global `$id`
~- Removed `release_date` from both schemas. Certainly not something
that makes sense for the generalized schema. It actually doesn't make
sense for the Registry schema either, because this is the server.json
that _users will submit_, i.e. the _input_ into the Registry publication
API. `release_date` will be a piece of _output_ metadata. The Registry
API will be free to append such extra metadata for GET /:id requests of
servers, and the data can be present in the OpenAPI spec, but it does
not need to be in the registry-server.json JSON schema.~
  - Note this is no longer in this PR; rebased on #168 which included it
- Removed unnecessary enums from the generalized `schema.json`
## Summary
- Adds CLAUDE.md file to provide guidance for Claude Code instances
working with this repository
- Includes essential development commands and high-level architecture
overview
- Follows the standard format for Claude Code documentation

## Details
The CLAUDE.md file contains:
- Common development commands for building, running, testing, and
linting
- Architecture overview explaining the clean architecture pattern
- Request flow documentation
- Key interfaces and their implementations
- Authentication flow details
- Important design patterns used in the codebase

This will help future Claude Code instances be more productive when
working with the MCP Registry codebase.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Avinash Sridhar <sridharavinash@users.noreply.github.com>
Closes #149

As noted in the FAQ update here, this is a helpful starting point in
thinking about this:

```
### What is the difference between "Official MCP Registry", "MCP Registry", "MCP registry", "MCP Registry API", etc?

There are four underlying concepts:
- "MCP Server Registry API" (or "MCP Registry API"): The OpenAPI specification defined in [openapi.yaml](./server-registry-api/openapi.yaml). This is a reusable API specification that anyone building any sort of "MCP server registry" should consider adopting / aligning with.
- "Official MCP Registry" (or "MCP Registry"): The application that lives at `https://registry.modelcontextprotocol.io`. This registry currently only catalogs MCP servers, but may be extended in the future to also catalog MCP client/host apps and frameworks.
- "Official MCP Registry API": The REST API that lives at `https://registry.modelcontextprotocol.io/api`, with an OpenAPI specification defined at [official-registry-openapi.yaml](./server-registry-api/official-registry-openapi.yaml)
- "MCP server registry" (or "MCP registry"): A third party, likely commercial, implementation of the MCP Server Registry API or derivative specification.
```

Prior to this PR, we were just treating "MCP Server Registry API" and
"Official MCP Registry API" to be the same thing. They are not the same:
the latter is inherently a more constrained version of the former. A
private deployment of a similar registry need not have constraints like
"the only valid source code repositories are `github` and `gitlab`".

This PR separates out the definitions to match that path forward.

## Notable Changes

### Base OpenAPI spec (`openapi.yaml`)
- Removed official-registry-specific constraints:
  - No maximum limit on `/servers` endpoint
  - No enum constraint on `Repository.source`
  - No enum constraint on `Package.registry_name`
- Removed `/v0` prefix from paths (it's now just baked into the
`official` one's URL prefix)
- Added `$id` for proper schema identification

### Official Registry OpenAPI spec (`official-registry-openapi.yaml`)
- Created as a derivative of the base spec using `$ref`
- Re-adds official-registry-specific constraints:
  - `Repository.source` enum: `[github]`
  - `Package.registry_name` enum: `[npm, docker, pypi, nuget]`

### Documentation updates
- Added `server-registry-api/README.md` explaining the relationship
between specs
- Updated FAQ to clarify terminology
- Reorganized docs structure
<!-- Provide a brief summary of your changes -->

## Motivation and Context
<!-- Why is this change needed? What problem does it solve? -->

This is one attempt to resolve
#169.

Instead of adding a new "constant" argument type, I expanded the
existing positional one and just dropped the `value_hint` requirement. I
am not sure if this has VS Code impact @sandy081, @connor4312.

Also, removed unneeded release_date values from the samples.

## How Has This Been Tested?
<!-- Have you tested this in a real application? Which scenarios were
tested? -->

I tested the current server.json samples in the repo against the new
(less strict) schema. Of course they passed (aside from an unrelated bug
in my previous NuGet sample 🤦).

## Breaking Changes
<!-- Will users need to update their code or configurations? -->

## Types of changes
<!-- What types of changes does your code introduce? Put an `x` in all
the boxes that apply: -->
- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)
- [x] Documentation update

## Checklist
<!-- Go over all the following points, and put an `x` in all the boxes
that apply. -->
- [x] I have read the [MCP
Documentation](https://modelcontextprotocol.io)
- [x] My code follows the repository's style guidelines
- [ ] New and existing tests pass locally
- [ ] I have added appropriate error handling
- [x] I have added or updated documentation as needed

## Additional context
<!-- Add any other context, implementation notes, or design decisions
-->
<!-- Provide a brief summary of your changes -->

## Motivation and Context
<!-- Why is this change needed? What problem does it solve? -->

Resolve #173

## How Has This Been Tested?
<!-- Have you tested this in a real application? Which scenarios were
tested? -->

Copilot fixed my spelling mistakes.

## Breaking Changes
<!-- Will users need to update their code or configurations? -->

None.

## Types of changes
<!-- What types of changes does your code introduce? Put an `x` in all
the boxes that apply: -->
- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)
- [x] Documentation update

## Checklist
<!-- Go over all the following points, and put an `x` in all the boxes
that apply. -->
- [ ] I have read the [MCP
Documentation](https://modelcontextprotocol.io)
- [ ] My code follows the repository's style guidelines
- [ ] New and existing tests pass locally
- [ ] I have added appropriate error handling
- [ ] I have added or updated documentation as needed

## Additional context
<!-- Add any other context, implementation notes, or design decisions
-->

Ideally the `id` would be populated by the publisher CLI tool "on the
way out" and MCP Registry would validate the `id`, perhaps using the
token used for auth in the case of private repositories.
## Summary

This PR improves the developer experience by adding clear linting
documentation and optional pre-commit hooks to catch issues before CI.
Helpful to avoid fighting Claude Code / Copilot on "please make sure
linting passes CI".

## Changes

- Updated `README.md` with:
  - Go 1.23.x requirement (was "1.18 or later")
  - golangci-lint v1.61.0 in prerequisites
  - New Development section with linting instructions
  
- Updated `CLAUDE.md` with:
  - Development Setup section
  
- Added `.githooks/pre-commit`:
  - Runs golangci-lint and gofmt checks
  - References README for installation instructions
  - Can be bypassed with `--no-verify` when needed

## Test Plan

- [x] Verified pre-commit hook blocks commits when golangci-lint is
missing
- [x] Confirmed hook shows clear error message pointing to README
- [x] Tested that linting passes with Go 1.23.0 and golangci-lint
v1.61.0
- [x] Verified gofmt correctly identifies formatting issues

Co-authored w/ Claude Code

---------

Co-authored-by: Charles Lowell <10964656+chlowell@users.noreply.github.com>
This is a first PR for #159 - adding some validation scripts. I want to
get to a place where we are confident that making schema changes is in
sync with the rest of our code. Step one is making sure the schemas are
in sync with our examples. Step two will be making sure the schemas are
in sync with our actual Registry implementation code.

This is just for the JSON right now; if we're feeling good about this
approach I'll do an analogous change for the OpenAPI schemas as well.

I'll also follow with adding usage of these scripts to our CI pipeline
so we have automated checks on them with every change.

## Summary
- Added CLI validation tools for JSON schemas and examples in
`docs/server-json/`
- Fixed some minor validation errors found in `examples.md`
- Ensures schema consistency and example validity in CI/CD pipelines

## Changes

### New Validation Tools
1. **`tools/validate-schemas`** - Validates that `schema.json` and
`registry-schema.json` are valid JSON Schema documents
2. **`tools/validate-examples`** - Validates all JSON examples in
`examples.md` against both schemas

### Key Features
- ✅ Validates JSON schema syntax and structure
- ✅ Validates all examples against both base and registry schemas
- ✅ Tracks validation counts to prevent silent failures
- ✅ Proper exit codes for CI integration
- ✅ Clear error messages for debugging

### Fixes to examples.md
- Added missing `repository.id` field in NuGet example
- Changed `repository.source` from "gitlab" to "github" in Docker
example

### Usage
```bash
# From repository root
./tools/validate-schemas.sh
./tools/validate-examples.sh
```

## Testing

I ran these scripts locally with minor (breaking) changes, which they
each correctly reported. As submitted, they are both passing.

🤖 Co-authored with [Claude Code](https://claude.ai/code)

---------

Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Charles Lowell <10964656+chlowell@users.noreply.github.com>
The `publisher create` command now automatically sets the `runtime_hint`
field in the generated `server.json` based on the `registry_name` when
no explicit `--runtime-hint` is provided:

- When `registry_name` is `docker`, sets `runtime_hint` to `docker`
- When `registry_name` is `npm`, sets `runtime_hint` to `npx`
- Other registries remain unchanged (no `runtime_hint` field)

## Before
```bash
# NPM registry - no runtime_hint field generated
$ mcp-publisher create --name "test-server" --description "Test" --repo-url "https://github.com/test/repo" --registry npm
```

```json
{
  "packages": [
    {
      "registry_name": "npm",
      "name": "test-server",
      "version": "1.0.0"
    }
  ]
}
```

## After
```bash
# NPM registry - automatically sets runtime_hint to "npx"
$ mcp-publisher create --name "test-server" --description "Test" --repo-url "https://github.com/test/repo" --registry npm
```

```json
{
  "packages": [
    {
      "registry_name": "npm",
      "name": "test-server",
      "version": "1.0.0",
      "runtime_hint": "npx"
    }
  ]
}
```

The manual `--runtime-hint` flag continues to work and takes precedence
over the automatic setting, ensuring backward compatibility.

Fixes #204.

<!-- START COPILOT CODING AGENT TIPS -->
---

💬 Share your feedback on Copilot coding agent for the chance to win a
$200 gift card! Click
[here](https://survey.alchemer.com/s3/8343779/Copilot-Coding-agent) to
start the survey.

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: toby <83556+toby@users.noreply.github.com>
The following PR adds a new server `status` field to support the status
of servers, like deprecated or active

Details:
* Add status field to server.json schema with 'active' and 'deprecated'
values
* Update OpenAPI spec to include status field in Server responses
* Add ServerStatus enum and field to Go model structs
* Update examples to demonstrate status field usage (`active` and
`deprecated`)

Since this is my first PR there's a good chance I might have missed
something so I'll appreciate any feedback to make this and any PRs in
the future easier for you to review 😃

Fixes: #181

## Motivation and Context
<!-- Why is this change needed? What problem does it solve? -->
The main goal is to have a way for server authors to notify registry
consumers that a server may no longer be maintained and shouldn't be
considered for using.

## How Has This Been Tested?
<!-- Have you tested this in a real application? Which scenarios were
tested? -->

## Breaking Changes
<!-- Will users need to update their code or configurations? -->

## Types of changes
<!-- What types of changes does your code introduce? Put an `x` in all
the boxes that apply: -->
- [ ] Bug fix (non-breaking change which fixes an issue)
- [x] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)
- [ ] Documentation update

## Checklist
<!-- Go over all the following points, and put an `x` in all the boxes
that apply. -->
- [x] I have read the [MCP
Documentation](https://modelcontextprotocol.io)
- [ ] My code follows the repository's style guidelines
- [ ] New and existing tests pass locally
- [ ] I have added appropriate error handling
- [ ] I have added or updated documentation as needed

## Additional context
<!-- Add any other context, implementation notes, or design decisions
-->

---------

Signed-off-by: Radoslav Dimitrov <radoslav@stacklok.com>
Co-authored-by: Tadas Antanavicius <tadas@tadasant.com>
Auto build image when we run `docker compose up`, Make it start more
easy for beginner.

## Motivation and Context
When I try to use `docker compose up` start the MCP server, docker
default pull `registry` image from dockerhub, because I forget build
image first.

Maybe we don't need to manually build the image, just build it
automatically when docker compose starts

## How Has This Been Tested?
Just run `docker-compose up --build` at a environment has installed
docker and docker compose.

Signed-off-by: 疯魔慕薇 <kfanjian@gmail.com>
Co-authored-by: adam jones <domdomegg+git@gmail.com>
This PR removes the gitignore entry for `publisher` since this
means we gitignore all folders and files named `publisher`.

My assumption is the intent for this was to just ignore binaries named
`publisher` but currently that binary is going to be located at
`tools/publisher/bin/mcp-publisher` and the **/bin pattern already
ignores all bin/ directories (taken from the build.sh).

Signed-off-by: Radoslav Dimitrov <radoslav@stacklok.com>
Co-authored-by: adam jones <domdomegg+git@gmail.com>
Renamed `data/seed_2025_05_16.json` to `data/seed.json` and updated `Dockerfile`

## Motivation and Context

When cloning the repository and building the project using:

```bash
go build ./cmd/registry
.\\registry.exe
```

the application fails to start because the default seed file path
(`data/seed.json`) does not exist in the repo:

```
Failed to read seed file: failed to read file: open data/seed.json: The system cannot find the file specified.
```

## How Has This Been Tested?

```bash
go build ./cmd/registry
.\\registry.exe // ./registry
```

## Breaking Changes

None

## Types of changes
<!-- What types of changes does your code introduce? Put an `x` in all
the boxes that apply: -->
- [x] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)
- [ ] Documentation update

## Checklist
<!-- Go over all the following points, and put an `x` in all the boxes
that apply. -->
- [x] I have read the [MCP
Documentation](https://modelcontextprotocol.io)
- [x] My code follows the repository's style guidelines
- [x] New and existing tests pass locally
- [ ] I have added appropriate error handling
- [ ] I have added or updated documentation as needed

Co-authored-by: Adam Jones <adamj+git@anthropic.com>
Issue: links directive was failing on podman

- Removed unsupported 'links' directive for Podman compatibility

<!-- Provide a brief summary of your changes -->
It was failing on Podman because Podman doesn't support the links
directive that Docker Compose does, and we already using depends_on,
which establishes the dependency relationship between services; we can
simply remove the links section from your docker-compose.yml file.

## Motivation and Context
docker-compose -links deprecated #107
Fix the defect by removing link.

## How Has This Been Tested?
Yes. Test by running on both.
For Docker run.
docker build -t registry .
docker compose up

For Podman
podman build -t registry .
podman compose up

## Breaking Changes
Yes for podman.

## Types of changes
<!-- What types of changes does your code introduce? Put an `x` in all
the boxes that apply: -->
- [ x] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ x] Breaking change (fix or feature that would cause existing
functionality to change)
- [ ] Documentation update

## Checklist
<!-- Go over all the following points, and put an `x` in all the boxes
that apply. -->
- [x ] I have read the [MCP
Documentation](https://modelcontextprotocol.io)
- [ x] My code follows the repository's style guidelines
- [ x] New and existing tests pass locally
- [ x] I have added appropriate error handling
- [x ] I have added or updated documentation as needed

## Additional context
It is related to issue #107

Co-authored-by: Avinash Sridhar <sridharavinash@users.noreply.github.com>
Co-authored-by: adam jones <domdomegg+git@gmail.com>
Migrate golangci-lint from v1 to v2.

## Motivation and Context
The golangci-lint has been upgraded to v2 since March 2025.
The v2 of golangci-lint has simplified the linters management.

## How Has This Been Tested?
Tested locally with golangci-lint@v2.1.6

## Breaking Changes
No.

## Types of changes
- [ ] Bug fix (non-breaking change which fixes an issue)
- [x] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)
- [ ] Documentation update

## Checklist
<!-- Go over all the following points, and put an `x` in all the boxes
that apply. -->
- [x] I have read the [MCP
Documentation](https://modelcontextprotocol.io)
- [x] My code follows the repository's style guidelines
- [x] New and existing tests pass locally
- [x] I have added appropriate error handling
- [ ] I have added or updated documentation as needed

## Additional context
<!-- Add any other context, implementation notes, or design decisions
-->

---------

Co-authored-by: haokunt <haokunt@opera.com>
Co-authored-by: Avinash Sridhar <sridharavinash@users.noreply.github.com>
Co-authored-by: Adam Jones <adamj+git@anthropic.com>
…202)

<!-- Provide a brief summary of your changes -->
run the application as a non-root user.
## Motivation and Context
<!-- Why is this change needed? What problem does it solve? -->
run the application as a non-root user.

## How Has This Been Tested?
<!-- Have you tested this in a real application? Which scenarios were
tested? -->
YES, build the docker image.

## Types of changes
<!-- What types of changes does your code introduce? Put an `x` in all
the boxes that apply: -->
- [x] New feature (non-breaking change which adds functionality)


## Checklist
<!-- Go over all the following points, and put an `x` in all the boxes
that apply. -->
- [ ] I have read the [MCP
Documentation](https://modelcontextprotocol.io)
- [ ] My code follows the repository's style guidelines
- [ ] New and existing tests pass locally
- [ ] I have added appropriate error handling
- [ ] I have added or updated documentation as needed

## Additional context
<!-- Add any other context, implementation notes, or design decisions
-->

Co-authored-by: hurricane1988 <niujianping_qm@dayu.work>
Co-authored-by: adam jones <domdomegg+git@gmail.com>
## Summary
- Added redirect from `/` to `/v0/swagger/index.html` to help
contributors quickly access API documentation
- Implemented the redirect in router.go after v0 routes registration

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-authored-by: Claude <noreply@anthropic.com>
## Motivation and Context
<!-- Why is this change needed? What problem does it solve? -->
The following PR:
* Adds a Makefile with the most common project-related commands
* Updates the CI workflow to leverage that (people testing locally will
use the same commands as the CI)
* Switches to using the official Golangci-lint action instead of
downloading it via curl
* Bumps the golangci version (supersedes #144)
* Fixes any formatting/linting issues found
* Updates the README to reference the new make targets
* Runs the schema validation scripts as part of the CI

**Next:**
* If you are okay I can file a follow up PR afterwards where I can tidy
up the CI workflows a bit, it feels there's some overlap.

## How Has This Been Tested?
<!-- Have you tested this in a real application? Which scenarios were
tested? -->
Locally by running the make targets

## Breaking Changes
<!-- Will users need to update their code or configurations? -->

## Types of changes
<!-- What types of changes does your code introduce? Put an `x` in all
the boxes that apply: -->
- [ ] Bug fix (non-breaking change which fixes an issue)
- [x] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)
- [ ] Documentation update

## Checklist
<!-- Go over all the following points, and put an `x` in all the boxes
that apply. -->
- [x] I have read the [MCP
Documentation](https://modelcontextprotocol.io)
- [x] My code follows the repository's style guidelines
- [x] New and existing tests pass locally
- [ ] I have added appropriate error handling
- [ ] I have added or updated documentation as needed

## Additional context
<!-- Add any other context, implementation notes, or design decisions
-->

---------

Signed-off-by: Radoslav Dimitrov <radoslav@stacklok.com>
Co-authored-by: Adam Jones <adamj+git@anthropic.com>
## Summary

This PR consolidates our documentation to reduce redundancy and
maintenance overhead:

- Removes the CLAUDE.md file with Import statement in favor of direct
README.md usage
- Merges CONTRIBUTING.md content into README.md's Contributing section  
- Migrates useful architecture and developer information from the old
CLAUDE.md to README.md
- Eliminates duplicated information between files

## Benefits

- Single source of truth in README.md reduces documentation drift
- Easier maintenance with all docs in one place
- Claude Code remains fully functional while reducing complexity

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
Adds a CI action to build docker images and publish them to GHCR on
every commit. We can then use these when deploying the registry.

This turned out to be an upstream blocker of building the infra for
deployment, and something like this is needed for either deployment
approach we're exploring.

---

## Summary
- Add comprehensive documentation for pre-built Docker images in README
- Include usage examples and configuration guidance  
- Add GitHub Actions workflow for automated Docker image publishing

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-authored-by: Claude <noreply@anthropic.com>
@claude
Copy link
Contributor

claude bot commented Aug 8, 2025

Claude finished @domdomegg's task —— View job


Code Review Results

I've thoroughly reviewed the Claude Code Review workflow file. Here's my comprehensive analysis:

Todo List

  • Read and analyze the GitHub Actions workflow file
  • Review code quality and best practices
  • Check for potential bugs or security issues
  • Evaluate performance considerations
  • Assess workflow configuration and triggers
  • Provide comprehensive review feedback

Strengths & Best Practices

Security & Permissions:

  • Excellent use of minimal permissions principle (.github/workflows/claude-code-review.yml:22-26)
  • Proper secret handling with ANTHROPIC_API_KEY stored securely
  • Appropriate id-token: write permission for OIDC authentication

Workflow Structure:

  • Clean, well-documented workflow with extensive comments
  • Proper use of official actions (actions/checkout@v4)
  • Sensible trigger configuration for PR events (opened, synchronize)

Flexibility & Customization:

  • Comprehensive commented examples for various use cases (lines 40-77)
  • Multiple customization options for different team needs
  • Good balance between automation and control

⚠️ Areas for Improvement

1. Version Pinning (Medium Priority)

# Current (line 36):
uses: anthropics/claude-code-action@beta

# Recommended:
uses: anthropics/claude-code-action@v1.2.3  # Pin to specific version

Impact: Using @beta can introduce breaking changes unexpectedly. Pin to a specific version for stability.

2. Fetch Depth Optimization (Low Priority)

# Current (line 32):
fetch-depth: 1

# Consider for better context:
fetch-depth: 0  # or remove entirely for default behavior

Impact: With fetch-depth: 1, Claude may not have sufficient context for complex diffs. Consider the trade-off between speed and context.

3. Error Handling Enhancement
The workflow lacks explicit error handling. Consider adding:

steps:
  - name: Run Claude Code Review
    id: claude-review
    continue-on-error: true  # Don't block PR if review fails
    uses: anthropics/claude-code-action@beta
    # ... rest of configuration

4. Resource Optimization
Consider adding timeout and resource constraints:

jobs:
  claude-review:
    runs-on: ubuntu-latest
    timeout-minutes: 10  # Prevent runaway jobs

🔒 Security Considerations

Positive aspects:

  • No hardcoded secrets or sensitive data
  • Proper use of GitHub secrets
  • Read-only permissions where appropriate

Potential concerns:

  • The workflow will run on any PR from any contributor (including first-time contributors)
  • Consider adding branch protection or approval requirements for external contributions

📊 Performance Considerations

Current performance characteristics:

  • Lightweight checkout with fetch-depth: 1
  • Single job with minimal resource usage
  • Triggers only on relevant events (opened, synchronize)

Optimization opportunities:

  • The commented path filters (lines 6-11) could reduce unnecessary runs
  • Consider using paths-ignore for documentation-only changes

🧪 Test Coverage Assessment

Current state:

  • No automated testing of the workflow itself
  • Relies on manual testing post-merge (as outlined in PR description)

Recommendations:

  • Consider adding a simple validation job that checks workflow syntax
  • The commented allowed_tools option (line 72) could enable running existing test suites as part of reviews

📝 Additional Recommendations

  1. Documentation: Consider adding a brief comment at the top explaining the workflow's purpose
  2. Conditional logic: The commented examples for conditional execution are excellent - consider uncommenting path filters for this Go-based project
  3. Monitoring: Consider adding workflow status badges or notifications for failed reviews

🎯 Overall Assessment

This is a well-crafted workflow that demonstrates good understanding of GitHub Actions best practices. The extensive commented examples show thoughtful consideration of different use cases. The security model is sound, and the flexibility is excellent.

Risk Level: Low
Complexity: Appropriate
Maintainability: High

The workflow is production-ready with minor improvements suggested above. The automated Claude Code review integration will provide valuable feedback to contributors while maintaining security best practices.


@domdomegg
Copy link
Member Author

I sincerely apologize for the disruption. This PR was accidentally closed due to a git history rewrite mistake. The PR has been recreated as #235 with all the original content preserved.

Please continue any discussions or reviews on the new PR: #235

Again, I apologize for the inconvenience this has caused.

@domdomegg domdomegg deleted the add-claude-code-review-workflow branch September 9, 2025 15:27
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.