Skip to content

Reconcile configuration/docs semantics and Go version guidance with current implementation#6998

Merged
lpcox merged 3 commits into
mainfrom
copilot/fix-sample-rate-documentation
Jun 4, 2026
Merged

Reconcile configuration/docs semantics and Go version guidance with current implementation#6998
lpcox merged 3 commits into
mainfrom
copilot/fix-sample-rate-documentation

Conversation

Copilot AI commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Nightly docs reconciliation flagged mismatches between documented config behavior and actual implementation, plus contributor guidance drift around Go version checks and make build behavior in restricted environments. This PR aligns docs and install-time version messaging with current code paths.

  • Configuration reference corrections

    • sample_rate docs now reflect actual support matrix: sample_rate (TOML) and sampleRate (JSON stdin), plus CLI --otlp-sample-rate.
    • rate_limit_threshold / rate_limit_cooldown are now documented as TOML config only (removed misleading “JSON file configs” wording).
    • Added explicit note that tool_response_filters intentionally uses snake_case JSON key (tool_response_filters) with no camelCase alias.
    • Added a worked example showing top-level guards definition paired with per-server guard assignment.
  • Contributor/build guidance alignment

    • Updated prerequisites text to “Go 1.25.0 or later.”
    • Added a concise note that make build runs go mod tidy first (relevant for network-restricted environments).
  • Install-time Go version check behavior

    • make install warning logic now compares required vs installed major.minor baseline (warn only when older), avoiding false warnings on valid patch releases (e.g., 1.25.10).
  • Agent/docs clarity updates

    • Clarified AGENTS.md wording for make agent-finished: it runs go test ./... + Rust guard tests; binary integration tests remain a separate make test-integration step.
    • Added customSchemas to the README gateway/config summary table for JSON stdin completeness.
# install target version gate now warns only when installed Go < required major.minor
if [ "$$INSTALLED_MAJOR" -lt "$$REQUIRED_MAJOR" ] || { \
   [ "$$INSTALLED_MAJOR" -eq "$$REQUIRED_MAJOR" ] && \
   [ "$$INSTALLED_MINOR" -lt "$$REQUIRED_MINOR" ]; \
}; then
  echo "⚠ Warning: Expected Go $(GO_VERSION) or later, but found $$INSTALLED_VERSION"
fi

Copilot AI linked an issue Jun 4, 2026 that may be closed by this pull request
Copilot AI changed the title [WIP] Fix documentation for sample_rate JSON availability Reconcile configuration/docs semantics and Go version guidance with current implementation Jun 4, 2026
Copilot finished work on behalf of lpcox June 4, 2026 16:17
Copilot AI requested a review from lpcox June 4, 2026 16:17
@lpcox lpcox marked this pull request as ready for review June 4, 2026 16:45
Copilot AI review requested due to automatic review settings June 4, 2026 16:45

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR reconciles documentation and contributor guidance with the gateway’s current configuration semantics, and adjusts the make install Go version warning to align with “Go 1.25.x or later” expectations without spurious patch-level mismatches.

Changes:

  • Updates configuration docs to accurately reflect JSON stdin vs TOML support (e.g., sample_rate/sampleRate, TOML-only rate limit fields, snake_case tool_response_filters).
  • Aligns contributor/agent documentation with current build/test behavior (make build runs go mod tidy; make agent-finished scope clarified).
  • Refines make install Go version warning logic to compare major.minor baselines rather than exact version strings.
Show a summary per file
File Description
README.md Adds customSchemas to the quick config field table for JSON stdin completeness.
Makefile Updates Go version warning logic to major.minor baseline comparisons.
docs/CONFIGURATION.md Corrects/clarifies config key support and adds worked guard assignment examples.
CONTRIBUTING.md Updates Go version guidance and documents make build / go mod tidy behavior.
AGENTS.md Clarifies what make agent-finished runs vs what is covered by integration tests.

Copilot's findings

Tip

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

  • Files reviewed: 5/5 changed files
  • Comments generated: 2

Comment thread Makefile Outdated
Comment on lines 290 to 297
INSTALLED_MAJOR=$$(echo "$$INSTALLED_VERSION" | cut -d. -f1); \
INSTALLED_MINOR=$$(echo "$$INSTALLED_VERSION" | cut -d. -f2); \
REQUIRED_MAJOR=$$(echo "$(GO_VERSION)" | cut -d. -f1); \
REQUIRED_MINOR=$$(echo "$(GO_VERSION)" | cut -d. -f2); \
if [ "$$INSTALLED_MAJOR" -lt "$$REQUIRED_MAJOR" ] || { [ "$$INSTALLED_MAJOR" -eq "$$REQUIRED_MAJOR" ] && [ "$$INSTALLED_MINOR" -lt "$$REQUIRED_MINOR" ]; }; then \
echo "⚠ Warning: Expected Go $(GO_VERSION) or later, but found $$INSTALLED_VERSION"; \
echo " Visit https://go.dev/dl/ to install Go $(GO_VERSION) or later"; \
fi; \
Comment thread README.md
| `payload_dir` / `payloadDir` | Directory for large payload storage (must be absolute path) |
| `payload_size_threshold` / `payloadSizeThreshold` | Size threshold in bytes for payload storage (default: `524288`) |
| `trusted_bots` / `trustedBots` | Additional bot usernames to treat as trusted with "approved" integrity. Additive to the built-in trusted bot list. Non-empty array when present. Example: `["my-bot[bot]"]` |
| `customSchemas` (JSON stdin top-level) | Map custom server `type` names to HTTPS JSON schema URLs for custom server validation |
@lpcox

lpcox commented Jun 4, 2026

Copy link
Copy Markdown
Collaborator

@copilot address review feedback

Copilot AI commented Jun 4, 2026

Copy link
Copy Markdown
Contributor Author

@copilot address review feedback

Addressed in 6892bd5.

  • make install now parses major/minor as leading digits so prerelease versions like 1.25rc1 won’t break integer comparisons.
  • README wording now explicitly distinguishes gateway-level fields from top-level JSON stdin fields so customSchemas placement is clear.

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.

📚 Documentation Reconciliation Report - 2026-06-03

3 participants