docs: reconcile AGENTS.md, config.example.toml, and CONTRIBUTING.md with implementation - #4638
Conversation
…ith implementation Agent-Logs-Url: https://github.com/github/gh-aw-mcpg/sessions/ad78068f-61ac-4c16-94b3-7ad5e2f8bfe8 Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Documentation reconciliation to align agent docs, example configuration, and contributing instructions with the current MCP Gateway implementation.
Changes:
- Update
AGENTS.mdproject structure and environment variable references. - Clarify
${VAR}expansion support notes inconfig.example.toml. - Adjust a
--payload-direxample inCONTRIBUTING.md.
Show a summary per file
| File | Description |
|---|---|
| AGENTS.md | Updates quick reference text, project structure listing, and environment variable documentation. |
| config.example.toml | Refines notes about where ${VAR} expansion is supported/unsupported in TOML config. |
| CONTRIBUTING.md | Updates an example CLI invocation for payload directory usage. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comments suppressed due to low confidence (1)
config.example.toml:193
- Same issue as above: the note states
[gateway.opentelemetry]and[gateway.tracing]support${VAR_NAME}expansion, but only thegateway.opentelemetryTOML key triggersexpandTracingVariables(...)duringLoadFromFile. Consider rewording to mention only[gateway.opentelemetry](or clarify that legacy[gateway.tracing]does not get expanded).
# - Use ${VAR_NAME} syntax to reference environment variables in JSON stdin format only
# - TOML does not support ${VAR_NAME} expansion in server args/env fields; use absolute paths instead
# - The [gateway.opentelemetry] and [gateway.tracing] sections do support ${VAR_NAME} expansion
- Files reviewed: 3/3 changed files
- Comments generated: 3
| # NOTE: TOML configuration does not support ${VAR} variable expansion. | ||
| # NOTE: TOML does not support ${VAR} expansion in server args/env fields. | ||
| # Use absolute paths directly in args — shell variables like ${PWD} will NOT be expanded. | ||
| # (The [gateway.opentelemetry] and [gateway.tracing] sections DO support ${VAR} expansion.) |
There was a problem hiding this comment.
The comment says both [gateway.opentelemetry] and [gateway.tracing] support ${VAR} expansion, but in TOML load path variable expansion is only applied when the gateway.opentelemetry key is used (it’s merged into Gateway.Tracing and then expandTracingVariables is called). If users configure the legacy gateway.tracing key directly, ${VAR} placeholders won’t be expanded. Please adjust this note to avoid implying ${VAR} works in [gateway.tracing] TOML configs (or clarify the limitation explicitly).
This issue also appears on line 191 of the same file.
| # (The [gateway.opentelemetry] and [gateway.tracing] sections DO support ${VAR} expansion.) | |
| # (${VAR} expansion is supported in [gateway.opentelemetry]; the legacy [gateway.tracing] | |
| # TOML section does not expand ${VAR} placeholders.) |
| - `MCP_GATEWAY_PORT` - Gateway listen port (overrides config `port` field; validated 1-65535) | ||
| - `MCP_GATEWAY_DOMAIN` - Gateway domain name (overrides config `domain` field) | ||
| - `MCP_GATEWAY_API_KEY` - Gateway API key (overrides config `api_key` field) |
There was a problem hiding this comment.
The new descriptions imply MCP_GATEWAY_PORT/DOMAIN/API_KEY override the config’s port/domain/api_key fields and that the port is validated 1–65535. In the current implementation these env vars are only checked by environment validation / helper functions and used by run_containerized.sh to build the --listen flag; the awmg binary itself doesn’t apply these env vars as config overrides (and doesn’t currently range-validate the port unless callers invoke GetGatewayPortFromEnv, which is unused). Please reword these entries to reflect actual behavior (e.g., “required by run_containerized.sh / referenced via ${MCP_GATEWAY_*} in stdin JSON config”).
| - `MCP_GATEWAY_PORT` - Gateway listen port (overrides config `port` field; validated 1-65535) | |
| - `MCP_GATEWAY_DOMAIN` - Gateway domain name (overrides config `domain` field) | |
| - `MCP_GATEWAY_API_KEY` - Gateway API key (overrides config `api_key` field) | |
| - `MCP_GATEWAY_PORT` - Gateway listen port; used by `run_containerized.sh` when building the `--listen` flag and may be referenced via `${MCP_GATEWAY_PORT}` in stdin JSON config | |
| - `MCP_GATEWAY_DOMAIN` - Gateway domain name; may be referenced via `${MCP_GATEWAY_DOMAIN}` in stdin JSON config | |
| - `MCP_GATEWAY_API_KEY` - Gateway API key; may be referenced via `${MCP_GATEWAY_API_KEY}` in stdin JSON config |
| - `GITHUB_PERSONAL_ACCESS_TOKEN` - GitHub auth | ||
| - `GITHUB_API_URL` - Explicit GitHub API endpoint (e.g., `https://copilot-api.mycompany.ghe.com`); used by proxy to set upstream target | ||
| - `GITHUB_SERVER_URL` - GitHub server URL; proxy auto-derives API endpoint: `*.ghe.com` → `copilot-api.*.ghe.com`, GHES → `<host>/api/v3`, `github.com` → `api.github.com` | ||
| - `DOCKER_API_VERSION` - Set by querying Docker daemon's current API version; falls back to `1.44` for all architectures if detection fails | ||
| - `ACTIONS_ID_TOKEN_REQUEST_URL` - GitHub Actions OIDC token endpoint URL; required for `github-oidc` auth type | ||
| - `ACTIONS_ID_TOKEN_REQUEST_TOKEN` - GitHub Actions OIDC request token; required for `github-oidc` auth type | ||
| - `MCP_GATEWAY_PORT` - Gateway listen port (overrides config `port` field; validated 1-65535) | ||
| - `MCP_GATEWAY_DOMAIN` - Gateway domain name (overrides config `domain` field) | ||
| - `MCP_GATEWAY_API_KEY` - Gateway API key (overrides config `api_key` field) |
There was a problem hiding this comment.
DOCKER_API_VERSION was removed from the env var list, but it’s still set/used by the repository’s startup scripts (run.sh / run_containerized.sh) to pin the Docker client API version. If this section is meant to cover operationally relevant env vars (not only those read by Go code), consider keeping DOCKER_API_VERSION here (or add a short note that it’s script-managed so agents don’t try to set it manually).
Nightly reconciliation found 8 discrepancies between documentation and actual implementation. All are documentation-only fixes — no code changes.
AGENTS.md
internal/guard/description listedAllowOnly(a policy key) instead of the actual exported types — corrected toNoopGuard, WasmGuard, WriteSinkGuardinternal/httputil/,internal/oidc/,internal/proxy/,internal/syncutil/,internal/tracing/to Project StructureDOCKER_API_VERSION: Removed — not referenced anywhere ininternal/**/*.goMCP_GATEWAY_PORT,MCP_GATEWAY_DOMAIN,MCP_GATEWAY_API_KEY(read byconfig_env.go/validation_env.go) andACTIONS_ID_TOKEN_REQUEST_URL/ACTIONS_ID_TOKEN_REQUEST_TOKEN(used inoidc/provider.go)make test-integration: Changed "requires build" → "auto-builds binary if not present" (Makefile lines 68–71 auto-build)config.example.toml
Overstated caveat said
${VAR}expansion is unsupported everywhere in TOML. Corrected to clarify it is unsupported in serverargs/envfields only —[gateway.opentelemetry]and[gateway.tracing]sections do expand${VAR}viaexpandTracingVariables().CONTRIBUTING.md
Changed
--payload-dir /tmp/payloadsexample to--payload-dir ./payloads.Warning
Firewall rules blocked me from connecting to one or more addresses (expand for details)
I tried to connect to the following addresses, but was blocked by firewall rules:
example.com/tmp/go-build1388902967/b513/launcher.test /tmp/go-build1388902967/b513/launcher.test -test.testlogfile=/tmp/go-build1388902967/b513/testlog.txt -test.paniconexit0 -test.timeout=10m0s -goversion go1.25.9 -c=4 -nolocalimports -importcfg /tmp/go-build1388902967/b495/importcfg -pack /tmp/go-build1388902967/b495/_testmain.go -W .cfg 649178/b314/ x_amd64/vet . --gdwarf2 --64 x_amd64/vet(dns block)invalid-host-that-does-not-exist-12345.com/tmp/go-build1388902967/b495/config.test /tmp/go-build1388902967/b495/config.test -test.testlogfile=/tmp/go-build1388902967/b495/testlog.txt -test.paniconexit0 -test.timeout=10m0s /tmp/go-build1388902967/b398/vet.cfg 1.80.0/encoding/-errorsas 64/src/debug/dwa-ifaceassert x_amd64/vet /tmp/go-build477/opt/hostedtoolcache/go/1.25.9/x64/pkg/tool/linux_amd64/vet -imultiarch x86_64-linux-gnu-bool x_amd64/vet --de�� _.a --debug-prefix-m-ifaceassert x_amd64/vet -I race/v1 -I x_amd64/vet(dns block)nonexistent.local/tmp/go-build1388902967/b513/launcher.test /tmp/go-build1388902967/b513/launcher.test -test.testlogfile=/tmp/go-build1388902967/b513/testlog.txt -test.paniconexit0 -test.timeout=10m0s -goversion go1.25.9 -c=4 -nolocalimports -importcfg /tmp/go-build1388902967/b495/importcfg -pack /tmp/go-build1388902967/b495/_testmain.go -W .cfg 649178/b314/ x_amd64/vet . --gdwarf2 --64 x_amd64/vet(dns block)slow.example.com/tmp/go-build1388902967/b513/launcher.test /tmp/go-build1388902967/b513/launcher.test -test.testlogfile=/tmp/go-build1388902967/b513/testlog.txt -test.paniconexit0 -test.timeout=10m0s -goversion go1.25.9 -c=4 -nolocalimports -importcfg /tmp/go-build1388902967/b495/importcfg -pack /tmp/go-build1388902967/b495/_testmain.go -W .cfg 649178/b314/ x_amd64/vet . --gdwarf2 --64 x_amd64/vet(dns block)this-host-does-not-exist-12345.com/tmp/go-build1388902967/b522/mcp.test /tmp/go-build1388902967/b522/mcp.test -test.testlogfile=/tmp/go-build1388902967/b522/testlog.txt -test.paniconexit0 -test.timeout=10m0s ut-1�� .cfg /tmp/go-build477-ifaceassert x_amd64/vet ctor --gdwarf2 E=3 x_amd64/vet .cfg�� 649178/b405/_pkg_.a otection x_amd64/vet --gdwarf-5 g/grpc/internal/-atomic -o x_amd64/vet(dns block)If you need me to access, download, or install something from one of these locations, you can either: