Addressing PR comments by Claude · Pull Request #1176 · github/gh-aw-mcpg

Claude · 2026-02-20T18:58:21Z

Original prompt

This section details on the original issue you should resolve

<issue_title>[compliance] Compliance Gap: registry field rejected despite being valid per spec</issue_title>
<issue_description>## MCP Gateway Compliance Review — 2026-02-20

Summary

Found 2 compliance issues during daily review. One important issue (spec-valid field causes validation failure) and one minor gap (missing SHOULD feature).

Recent Changes Reviewed

Commit 4cd522d — Fix smoke-copilot workflow container tag mismatch (Fix smoke-copilot workflow container tag mismatch #1144)
Single grafted commit available in this environment; full history not accessible

Important Issues (spec-valid field rejected)

1. `registry` Field Defined in Spec but Rejected by Schema Validation

Specification Section: 4.1.2 Server Configuration Fields
Deep Link: https://github.com/github/gh-aw/blob/main/docs/src/content/docs/reference/mcp-gateway.md#412-server-configuration-fields

Requirement:

| registry | string | No | URI to the installation location when MCP is installed from a registry. This is an informational field used for documentation and tooling discovery. Applies to both stdio and HTTP servers. Example: "https://api.mcp.github.com/v0/servers/microsoft/markitdown" |

Current State:

The registry field is:

Not present in StdinServerConfig struct (internal/config/config_stdin.go)
Not present in ServerConfig struct (internal/config/config_core.go:88–112)
Not present in the pinned JSON schema at v0.41.1 for either stdioServerConfig or httpServerConfig
Not present in the latest JSON schema on main for either type either

Both stdioServerConfig and httpServerConfig in the JSON schema use "additionalProperties": false, so any configuration that includes registry will fail schema validation with an "additional property" error.

Gap:

The spec explicitly defines registry as a valid optional field for both stdio and http servers. Appendix A.5 includes complete examples of its use. However, the current implementation will reject any configuration containing this field.

Severity: Important (spec defines a valid optional field that the implementation actively rejects)

File References:

internal/config/config_stdin.go — StdinServerConfig struct (missing Registry field)
internal/config/config_core.go:88–112 — ServerConfig struct (missing Registry field)
internal/config/validation_schema.go:38 — pinned schema URL at v0.41.1
JSON schema at v0.41.1: stdioServerConfig.additionalProperties = false, no registry property
JSON schema at main: same — registry not present

Suggested Fix:

Add Registry field to StdinServerConfig:

// Registry is the URI to the installation location in an MCP registry (informational)
Registry string `json:"registry,omitempty"`

Add Registry field to ServerConfig:

// Registry is the URI to the installation location in an MCP registry (informational)
Registry string `toml:"registry" json:"registry,omitempty"`

Add registry property to the JSON schema (or open a PR in gh-aw to update the pinned schema), and update the schema pin version once merged.
Pass registry through in convertStdinConfig if it needs to be preserved in the internal config.

Estimated Effort: Small (1–2 hours)

Minor Suggestions (SHOULD improvements)

2. No Random API Key Generation When `apiKey` Is Not Configured

Specification Section: 7.3 Optimal Temporary API Key
Deep Link: https://github.com/github/gh-aw/blob/main/docs/src/content/docs/reference/mcp-gateway.md#73-optimal-temporary-api-key

Requirement:

The gateway SHOULD support temporary API keys:

Generate a random API key on startup if not provided

Include key in stdout configuration output

Current State:

In internal/cmd/root.go:380, apiKey is set to "" when cfg.Gateway.APIKey is empty. No random key is generated, and the stdout output configuration omits headers entirely when there is no API key (internal/cmd/root.go:406).

Gap:

Clients connecting to a gateway with no configured API key must know to send no Authorization header. If a random key were auto-generated, the gateway would always have an auth layer and clients would get the key from the stdout configuration output.

Severity: Minor (SHOULD, not MUST)

File References:

internal/cmd/root.go:380,406
internal/server/transport.go:62

Estimated Effort: Small (1–2 hours)

Compliance Status

Section	Requirement	Status
3.2.1 Containerization Requirement	stdio MUST be containerized	✅ Compliant
4.1.2 Server Configuration Fields — `registry`	Valid optional field	❌ Field rejected by schema
4.2 Variable Expression Rendering	Fail immediately on undefined variable	✅ Compliant
4.3 Configuration Validation	Unknown fields rejected, sche...

Fixes [compliance] Compliance Gap: registry field rejected despite being valid per spec #1162

- Add Registry field to StdinServerConfig struct for JSON stdin config - Add Registry field to ServerConfig struct for internal config and TOML - Pass registry field through in convertStdinServerConfig for both stdio and HTTP servers - Update fetchAndFixSchema to add registry property to stdioServerConfig and httpServerConfig - Add comprehensive tests for registry field in both JSON and TOML configs - Tests verify registry field is properly parsed and preserved for stdio, HTTP, and servers without registry Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>

lpcox · 2026-02-20T19:06:40Z

@claude[agent] please fix these linting errors

make lint
shell: /usr/bin/bash -e {0}
env:
GOTOOLCHAIN: local
Running linters...

github.com/github/gh-aw-mcpg/internal/cmd

[github.com/github/gh-aw-mcpg/internal/cmd]

Error: vet: internal/cmd/root_test.go:20:6: TestGetDefaultLogDir redeclared in this block
make: *** [Makefile:24: lint] Error 1

Remove duplicate TestGetDefaultLogDir from root_test.go since flags_logging_test.go already contains this test. This fixes the lint error: "TestGetDefaultLogDir redeclared in this block" Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>

Claude · 2026-02-20T19:09:14Z

@claude[agent] please fix these linting errors

make lint
...

Fixed in commit 3da8a16. Removed duplicate TestGetDefaultLogDir from root_test.go since it was already defined in flags_logging_test.go.

lpcox · 2026-02-20T19:17:48Z

@claude[agent] fix these linting errors

Run golangci/golangci-lint-action@ec5d184
with:
version: v2.8.0
args: --timeout=5m
install-mode: binary
github-token: ***
only-new-issues: false
skip-cache: false
skip-save-cache: false
problem-matchers: false
cache-invalidation-interval: 7
env:
GOTOOLCHAIN: local
prepare environment
Checking for go.mod: go.mod
Cache hit for: golangci-lint.cache-Linux-2929-1c48add4c8d3c1d0831358177434cd7599de72b3
Received 283194 of 283194 (100.0%), 5.9 MBs/sec
Cache Size: ~0 MB (283194 B)
/usr/bin/tar -xf /home/runner/work/_temp/f0885f59-95dc-4649-94f1-56926d806a73/cache.tzst -P -C /home/runner/work/gh-aw-mcpg/gh-aw-mcpg --use-compress-program unzstd
Cache restored successfully
Restored cache for golangci-lint from key 'golangci-lint.cache-Linux-2929-1c48add4c8d3c1d0831358177434cd7599de72b3' in 345ms
Finding needed golangci-lint version...
Installation mode: binary
Installing golangci-lint binary v2.8.0...
Downloading binary https://github.com/golangci/golangci-lint/releases/download/v2.8.0/golangci-lint-2.8.0-linux-amd64.tar.gz ...
/usr/bin/tar xz --overwrite --warning=no-unknown-keyword --overwrite -C /home/runner -f /home/runner/work/_temp/07f02d2e-b2bf-4383-a702-637fdaf48d7a
Installed golangci-lint into /home/runner/golangci-lint-2.8.0-linux-amd64/golangci-lint in 525ms
Prepared env in 871ms
run golangci-lint
Running [/home/runner/golangci-lint-2.8.0-linux-amd64/golangci-lint run --timeout=5m] in [/home/runner/work/gh-aw-mcpg/gh-aw-mcpg] ...
Error: internal/config/config_test.go:1496:4: QF1003: could use tagged switch on tt.expectedType (staticcheck)
if tt.expectedType == "http" {
^
1 issues:

staticcheck: 1

Error: issues found
Ran golangci-lint in 4448ms

Replace if-else chain with tagged switch statement for tt.expectedType check as suggested by staticcheck QF1003. This improves code readability and follows Go best practices. Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>

Claude · 2026-02-20T19:19:50Z

@claude[agent] fix these linting errors

Run golangci/golangci-lint-action@ec5d184
...

Fixed in commit e305e8d. Replaced if-else chain with a tagged switch statement on tt.expectedType as suggested by staticcheck QF1003.

Copilot

Pull request overview

This PR addresses a compliance gap where the registry field (defined in MCP Gateway spec section 4.1.2) was being rejected by JSON schema validation despite being a valid optional field for both stdio and HTTP server configurations.

Changes:

Added Registry field to StdinServerConfig and ServerConfig structs to store the registry URI
Patched JSON schema validation to accept registry property in stdioServerConfig and httpServerConfig definitions
Added comprehensive test coverage for registry field in both JSON (stdin) and TOML configuration formats

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
internal/config/validation_schema.go	Patches the fetched schema to add `registry` property definition to stdio and HTTP server configs
internal/config/config_stdin.go	Adds `Registry` field to `StdinServerConfig` and propagates it through conversion functions
internal/config/config_core.go	Adds `Registry` field to `ServerConfig` with TOML and JSON tags
internal/config/config_test.go	Adds integration tests for registry field parsing and preservation in both JSON and TOML configs
internal/cmd/root_test.go	Removes test for `getDefaultLogDir` function that was moved to a different file

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Initial plan

07a6d77

Claude AI assigned Claude and lpcox Feb 20, 2026

Claude started work on behalf of lpcox February 20, 2026 18:58 View session

Claude AI changed the title ~~[WIP] Fix compliance issue with registry field validation~~ Add registry field support per MCP Gateway spec 4.1.2 Feb 20, 2026

Claude AI requested a review from lpcox February 20, 2026 19:04

Claude finished work on behalf of lpcox February 20, 2026 19:04

Claude started work on behalf of lpcox February 20, 2026 19:07 View session

Claude AI changed the title ~~Add registry field support per MCP Gateway spec 4.1.2~~ Addressing PR comments Feb 20, 2026

Claude finished work on behalf of lpcox February 20, 2026 19:09

Claude started work on behalf of lpcox February 20, 2026 19:18 View session

Claude finished work on behalf of lpcox February 20, 2026 19:20

lpcox marked this pull request as ready for review February 20, 2026 19:21

Copilot AI review requested due to automatic review settings February 20, 2026 19:21

lpcox merged commit 014a712 into main Feb 20, 2026
11 checks passed

lpcox deleted the claude/fix-registry-field-validation branch February 20, 2026 19:22

Copilot started reviewing on behalf of lpcox February 20, 2026 19:22 View session

Copilot AI reviewed Feb 20, 2026

View reviewed changes

This was referenced Feb 20, 2026

Smoke Test: Copilot - 22237868615 #1177

Closed

Smoke Test: Copilot - 22237911772 #1180

Closed

Smoke Test: Copilot - 22238104045 #1183

Closed

Smoke Test: Copilot - 22238167309 #1187

Closed

Smoke Test: Copilot - 22238648584 #1188

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Addressing PR comments#1176

Addressing PR comments#1176
lpcox merged 4 commits intomainfrom
claude/fix-registry-field-validation

Claude AI commented Feb 20, 2026 •

edited

Loading

Uh oh!

lpcox commented Feb 20, 2026

Uh oh!

Claude AI commented Feb 20, 2026

Uh oh!

lpcox commented Feb 20, 2026

Uh oh!

Claude AI commented Feb 20, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

Claude AI commented Feb 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Recent Changes Reviewed

Important Issues (spec-valid field rejected)

1. registry Field Defined in Spec but Rejected by Schema Validation

Minor Suggestions (SHOULD improvements)

2. No Random API Key Generation When apiKey Is Not Configured

Compliance Status

Uh oh!

lpcox commented Feb 20, 2026

github.com/github/gh-aw-mcpg/internal/cmd

[github.com/github/gh-aw-mcpg/internal/cmd]

Uh oh!

Claude AI commented Feb 20, 2026

Uh oh!

lpcox commented Feb 20, 2026

Uh oh!

Claude AI commented Feb 20, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Claude AI commented Feb 20, 2026 •

edited

Loading

1. `registry` Field Defined in Spec but Rejected by Schema Validation

2. No Random API Key Generation When `apiKey` Is Not Configured