docs: add MCP server development blog post#334
Merged
Conversation
… new tabs Chronicle the 48-hour MCP server development sprint (v1.51.0–v1.51.5) with a corpus analysis callout featuring highlights from 10 real-world OpenAPI specs. Add JS override to open all external links in new tabs across the docs site. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds a new MCP Server blog post and mkdocs nav entry, injects an external-links script into the site template, and applies widespread small code edits across generators, CLI commands, joiner, parser, and internal MCP server code (mostly direct buffer writes, path sanitization, and lint suppressions). Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/blog/building-the-mcp-server.md`:
- Line 5: The metadata line uses the label "Commits:" but the value is "7 PRs,
8,274 lines of Go" — change the label to accurately reflect pull requests; for
example update the markdown line to use "Pull Requests:" or a neutral label like
"Changes:" so it reads "**Pull Requests:** 7 PRs, 8,274 lines of Go" (edit the
metadata line containing "**Published:** February 2026 | **Releases:** v1.51.0 –
v1.51.5 | **Commits:** 7 PRs, 8,274 lines of Go" to replace the highlighted
label).
- Line 45: The text incorrectly states “17 tools” for the Day 0 context; update
the sentence referencing the shared specInput type so it matches the Day 0
release count (15 tools) or make it explicitly versioned (e.g., “serves all 15
tools in the Day 0 release, later expanded to 17 in v1.51.3”); locate the phrase
mentioning the shared specInput type (the “specInput” type) and change the tool
count or add the version qualifier to keep chronology consistent.
In `@overrides/main.html`:
- Around line 33-44: The current external-link script listens for
DOMContentLoaded (which doesn't run on navigation.instant) and is placed before
bundle.js/{{ super() }}, so window.document$ isn't available for dynamic page
loads; move the external-link script to a new <script> block after {{ super() }}
(so bundle.js defines document$ first) and replace the DOMContentLoaded handler
with document$.subscribe(callback) that iterates anchors, constructs new
URL(a.href) inside try/catch, and sets target="_blank" and rel="noopener" for
external hostnames (same logic as the original function).
Fix 136 lint issues introduced by golangci-lint update:
- staticcheck QF1012: replace buf.WriteString(fmt.Sprintf(...)) with
fmt.Fprintf(&buf, ...) across generator, builder, and joiner packages
- gosec G602: use range variable instead of index in joiner loop
- gosec G703: add filepath.Clean + nolint for CLI output paths
- gosec G704: nolint for user-provided URLs in parser and MCP server
- gosec G705: nolint for CLI template execution (not a web server)
- gosec G115: nolint for OAS version byte conversion (values 0-9)
Address CodeRabbit review on blog post:
- Fix "Commits:" label to "Pull Requests:" in metadata
- Fix forward reference to "17 tools" in Day 0 section
- Fix external link script for navigation.instant compatibility
(move after {{ super() }}, use document$.subscribe())
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #334 +/- ##
=======================================
Coverage 84.81% 84.81%
=======================================
Files 190 190
Lines 26965 26966 +1
=======================================
+ Hits 22871 22872 +1
Misses 2791 2791
Partials 1303 1303
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
cmd/oastools/commands/overlay.go (1)
238-250:⚠️ Potential issue | 🔴 CriticalApply
filepath.Cleantojoincommand output file.The
joincommand writes user-specified output files without usingfilepath.Clean()(line 386 injoin.go), whileconvert,fix, andoverlayall apply the protection. Alignjoin.gowith the other commands:if writeErr := os.WriteFile(filepath.Clean(flags.Output), data, 0600); writeErr != nil {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/oastools/commands/overlay.go` around lines 238 - 250, The join command currently writes the user-specified output file without cleaning the path; update the write path in join.go to use filepath.Clean(flags.Output) when calling os.WriteFile so it matches convert/fix/overlay behavior (i.e., replace os.WriteFile(flags.Output, ...) with os.WriteFile(filepath.Clean(flags.Output), ...) and preserve any existing error handling/nolint annotations and the flags.Output and data variables used in that call).cmd/oastools/commands/convert.go (1)
201-205:⚠️ Potential issue | 🟡 MinorLog the cleaned output path for consistency with the written file location.
Line 205 logs
flags.Output(which may contain path traversal sequences like../) while the file is written tofilepath.Clean(flags.Output). For inputs like./foo/../bar.yaml, the logged path differs from the actual write location. Store the cleaned path and reuse it in the success message:🔧 Proposed fix
if flags.Output != "" { - if err := os.WriteFile(filepath.Clean(flags.Output), data, 0600); err != nil { //nolint:gosec // G703 - output path is user-provided CLI flag + cleanedOutput := filepath.Clean(flags.Output) + if err := os.WriteFile(cleanedOutput, data, 0600); err != nil { //nolint:gosec // G703 - output path is user-provided CLI flag return fmt.Errorf("writing output file: %w", err) } if !flags.Quiet { - Writef(os.Stderr, "\nOutput written to: %s\n", flags.Output) + Writef(os.Stderr, "\nOutput written to: %s\n", cleanedOutput) } }This pattern exists in
overlay.goandfix.goas well and should be applied consistently across all output commands.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/oastools/commands/convert.go` around lines 201 - 205, The success message logs flags.Output while the file is written to filepath.Clean(flags.Output), causing mismatch for paths with traversal; change the convert command to compute cleaned := filepath.Clean(flags.Output) once, use cleaned both when calling os.WriteFile(cleaned, ...) and when calling Writef(os.Stderr, "Output written to: %s", cleaned), and apply the same pattern used in overlay.go and fix.go to ensure logged path matches actual write location (update references to flags.Output in the os.WriteFile and Writef calls within the convert command).joiner/joiner.go (1)
345-353:⚠️ Potential issue | 🟡 Minor
os.Chmodon line 351 uses the rawoutputPathwithoutfilepath.Clean, inconsistent with line 345.Line 345 wraps the path in
filepath.Cleanto address gosec G703 (path traversal via taint analysis), but the immediately followingos.Chmodcall on line 351 operates on the original, uncleanedoutputPath. gosec G703 is "Path traversal via taint analysis", so the same rule would fire onos.Chmodwith a user-supplied path. This leaves line 351 unprotected and will likely produce a residual gosec finding onos.Chmod.Apply
filepath.Cleanconsistently, or add the samenolintannotation to line 351:🛡️ Proposed fix — consistent path sanitisation
- if err := os.WriteFile(filepath.Clean(outputPath), data, outputFileMode); err != nil { //nolint:gosec // G703 - output path is user-provided + cleanPath := filepath.Clean(outputPath) + if err := os.WriteFile(cleanPath, data, outputFileMode); err != nil { //nolint:gosec // G703 - output path is user-provided return fmt.Errorf("joiner: failed to write output file: %w", err) } // Explicitly set permissions to ensure they're correct even if file existed before // This handles the case where an existing file may have had different permissions - if err := os.Chmod(outputPath, outputFileMode); err != nil { + if err := os.Chmod(cleanPath, outputFileMode); err != nil { //nolint:gosec // G703 - output path is user-provided return fmt.Errorf("joiner: failed to set output file permissions: %w", err) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@joiner/joiner.go` around lines 345 - 353, The os.Chmod call uses the raw user-provided outputPath while the preceding os.WriteFile uses filepath.Clean(outputPath), leaving an inconsistent taint-sanitisation and a likely gosec G703 finding; update the permission-setting call to use the cleaned path (i.e., call filepath.Clean(outputPath) when invoking os.Chmod) or, if intentional, add the same nolint:gosec // G703 annotation to the os.Chmod line so both uses are treated consistently—refer to os.WriteFile, os.Chmod, outputPath, and outputFileMode to locate the lines to change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/blog/building-the-mcp-server.md`:
- Line 278: Replace the hyphen in the date range on the line containing
"Development window | 48 hours (Feb 13-15, 2026)" with an en dash so it reads
"Feb 13–15, 2026"; update the string exactly where "Feb 13-15, 2026" appears to
use the en dash character (U+2013) instead of the ASCII hyphen.
- Line 5: Update the PR count from "7 PRs" to "8 PRs" in the document: replace
the metadata entry that currently reads "**Releases:** v1.51.0 – v1.51.5 |
**Pull Requests:** 7 PRs" to use "8 PRs", and also update the "By the Numbers"
table cell that lists "7 PRs" so it reads "8 PRs" to match the eight PRs
referenced (`#310`, `#315`, `#318`, `#321`, `#324`, `#327`, `#329`, `#330`).
In `@generator/security_enforce.go`:
- Around line 43-51: The file uses an explicit-discard pattern in
writeSecurityRequirement (lines 31–34) but other generated writes use plain
fmt.Fprintf(&buf, ...), causing inconsistency; update each plain
fmt.Fprintf(&buf, ...) usage (the ones you changed at the locations that
generate file header and subsequent writes) to use the explicit discard form "_,
_ = fmt.Fprintf(&buf, ...)" so all bytes.Buffer writes follow the same
errcheck-friendly pattern and match writeSecurityRequirement.
In `@overrides/main.html`:
- Line 43: Update the anchor rel attribute assignment so it includes both
noopener and noreferrer: locate the code where a.setAttribute("rel", "noopener")
is used and change it to set rel to "noopener noreferrer" so that
target="_blank" links both block window.opener access and suppress the Referer
header/document.referrer exposure.
---
Outside diff comments:
In `@cmd/oastools/commands/convert.go`:
- Around line 201-205: The success message logs flags.Output while the file is
written to filepath.Clean(flags.Output), causing mismatch for paths with
traversal; change the convert command to compute cleaned :=
filepath.Clean(flags.Output) once, use cleaned both when calling
os.WriteFile(cleaned, ...) and when calling Writef(os.Stderr, "Output written
to: %s", cleaned), and apply the same pattern used in overlay.go and fix.go to
ensure logged path matches actual write location (update references to
flags.Output in the os.WriteFile and Writef calls within the convert command).
In `@cmd/oastools/commands/overlay.go`:
- Around line 238-250: The join command currently writes the user-specified
output file without cleaning the path; update the write path in join.go to use
filepath.Clean(flags.Output) when calling os.WriteFile so it matches
convert/fix/overlay behavior (i.e., replace os.WriteFile(flags.Output, ...) with
os.WriteFile(filepath.Clean(flags.Output), ...) and preserve any existing error
handling/nolint annotations and the flags.Output and data variables used in that
call).
In `@joiner/joiner.go`:
- Around line 345-353: The os.Chmod call uses the raw user-provided outputPath
while the preceding os.WriteFile uses filepath.Clean(outputPath), leaving an
inconsistent taint-sanitisation and a likely gosec G703 finding; update the
permission-setting call to use the cleaned path (i.e., call
filepath.Clean(outputPath) when invoking os.Chmod) or, if intentional, add the
same nolint:gosec // G703 annotation to the os.Chmod line so both uses are
treated consistently—refer to os.WriteFile, os.Chmod, outputPath, and
outputFileMode to locate the lines to change.
---
Duplicate comments:
In `@cmd/oastools/commands/fix.go`:
- Around line 346-351: Replace the ambiguous nolint tag and the uncleaned path
in the output message: update the os.WriteFile call's //nolint comment to target
the correct rule (use //nolint:gosec // G304 to match the other fix) and change
the success print to use the cleaned path (use filepath.Clean(flags.Output)
instead of flags.Output) so both the linter exception and the printed path are
consistent with the fix in convert.go; adjust the os.WriteFile/flags.Output and
Writef calls in this block accordingly.
In `@cmd/oastools/commands/overlay.go`:
- Around line 239-244: The write uses an ambiguous nolint and a message that
prints the raw flag instead of the cleaned path; update the os.WriteFile call to
use the same specific gosec nolint annotation you applied in convert.go (replace
the generic //nolint:gosec comment with the exact rule ID used there) on the
os.WriteFile(filepath.Clean(flags.Output), ...) line, and change the status
print to use the cleaned path (call filepath.Clean(flags.Output) when formatting
the "Output written to:" message via Writef) while keeping references to
flags.Output, os.WriteFile, filepath.Clean, and Writef to locate the change.
In `@docs/blog/building-the-mcp-server.md`:
- Line 5: Replace the redundant label "**Pull Requests:** 7 PRs" with a
non-redundant field such as "**PRs:** 7" and split the metrics into distinct
labeled fields (for example change the line containing "**Published:** February
2026 | **Releases:** v1.51.0 – v1.51.5 | **Pull Requests:** 7 PRs, 8,274 lines
of Go" to use "**PRs:** 7 | **Lines of Go:** 8,274" while preserving the
existing "**Published:**" and "**Releases:**" tokens).
Blog post: - Fix PR count from 7 to 8 (8 distinct PRs referenced) - Use en dash for date range (Feb 13–15) Security/consistency: - Add noreferrer to external link rel attribute - Use cleanedOutput variable consistently in convert, fix, join, overlay commands (log cleaned path, not raw flags.Output) - Add filepath.Clean to join.go output path (was missing) - Use cleaned path for os.Chmod in joiner.go (consistency with WriteFile) - Align fmt.Fprintf error discard pattern in security_enforce.go Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Owner
Author
Addressing outside-diff-range comments from CodeRabbit reviewAll 3 fixed in 515afa0:
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cmd/oastools/commands/join.go (1)
354-396:⚠️ Potential issue | 🟡 MinorMinor: pre-write diagnostic reports the raw path while the post-write confirmation reports the cleaned path.
Line 355 prints
flags.Outputverbatim ("Output: %s\n", flags.Output) before the write, while line 396 printscleanedOutput. For inputs like./output.yamlorpath//to/file.yamlthe two messages will disagree, which can be confusing.🛠️ Proposed fix — use `cleanedOutput` in the early diagnostic too
if flags.Output != "" { - Writef(os.Stderr, "Output: %s\n", flags.Output) + Writef(os.Stderr, "Output: %s\n", filepath.Clean(flags.Output)) } else {Alternatively, compute
cleanedOutputearlier (before the diagnostic block) and reuse it both here and in the write block.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/oastools/commands/join.go` around lines 354 - 396, The pre-write diagnostic prints flags.Output while the post-write confirmation prints cleanedOutput, causing inconsistent messages for equivalent paths; compute cleanedOutput (via filepath.Clean(flags.Output)) before the diagnostic block and use that cleanedOutput in the initial Writef call(s) (where flags.Output is currently used) so both the pre-write and post-write messages reference the same normalized path (affecting the code around flags.Output, cleanedOutput, Writef and the Write output block).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@cmd/oastools/commands/join.go`:
- Around line 354-396: The pre-write diagnostic prints flags.Output while the
post-write confirmation prints cleanedOutput, causing inconsistent messages for
equivalent paths; compute cleanedOutput (via filepath.Clean(flags.Output))
before the diagnostic block and use that cleanedOutput in the initial Writef
call(s) (where flags.Output is currently used) so both the pre-write and
post-write messages reference the same normalized path (affecting the code
around flags.Output, cleanedOutput, Writef and the Write output block).
---
Duplicate comments:
In `@generator/security_enforce.go`:
- Around line 43-51: The fmt.Fprintf call sites must use the explicit-discard
pattern to match the rest of the file: replace any plain fmt.Fprintf(...) uses
with "_, _ = fmt.Fprintf(...)" and ensure the same buffer reference style as
nearby helpers (e.g. use &buf when other calls in the generator file use &buf,
and use buf when writeSecurityRequirement(buf, ...) expects a pointer/variable),
so update the affected calls (the initial package header fprintf and the other
sites referenced around writeSecurityRequirement) to use "_, _ =
fmt.Fprintf(&buf, ...)" consistently.
This was referenced Feb 21, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
overrides/main.htmlto open all external links in new tabs withrel="noopener"— zero new dependencies, applies site-wideTest plan
make docs-buildpasses with no new warningstarget="_blank"andrel="noopener"🤖 Generated with Claude Code