Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
Updates the ToolHive documentation to reflect user-facing features shipped in v0.12.3–v0.13.0, aligning guides and concept pages with current CRD/CLI capabilities (scaling, auth, composite workflows, and skills install).
Changes:
- Document tool annotation overrides for aggregated tools (vMCP).
- Rewrite scaling guidance to cover new
replicas/backendReplicasfields and session storage requirements. - Add docs for
forEachcomposite-tool steps and for installing skills viathv skill install(registry + git sources), plus cross-references between guides.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/toolhive/guides-vmcp/tool-aggregation.mdx | Adds annotation override documentation and example configuration. |
| docs/toolhive/guides-vmcp/scaling-and-performance.mdx | Updates horizontal scaling guidance and adds session storage + MCPServer scaling sections. |
| docs/toolhive/guides-vmcp/composite-tools.mdx | Documents forEach iteration steps and expands template context table. |
| docs/toolhive/guides-registry/skills.mdx | Adds CLI-based skill installation instructions and flags table. |
| docs/toolhive/guides-k8s/run-mcp-k8s.mdx | Adds cross-link to scaling guidance for MCPServer replicas. |
| docs/toolhive/guides-k8s/redis-session-storage.mdx | Adds scaling-related context and links session storage to horizontal scaling. |
| docs/toolhive/guides-k8s/auth-k8s.mdx | Updates upstream provider note to reflect multi-upstream support in VirtualMCPServer. |
| docs/toolhive/concepts/skills.mdx | Updates current status to reflect CLI install support and links to guide. |
| docs/toolhive/concepts/backend-auth.mdx | Replaces “single upstream” language with multi-upstream (vMCP) + clarifies MCPServer limitations. |
b9da28d to
de415e8
Compare
de415e8 to
bfecf66
Compare
bfecf66 to
7a9131f
Compare
7a9131f to
46d40ca
Compare
46d40ca to
d4c8244
Compare
danbarr
left a comment
There was a problem hiding this comment.
As noted on Copilot's review comment, the new content about scaling MCPServer resources is out of place in the vMCP scaling/performance guide.
It should live with the other MCPServer docs in the main operator section - either as a new guide if the amount of content warrants it, or in the existing "Run MCP servers in Kubernetes" guide.
d4c8244 to
2abd168
Compare
|
@danbarr - I think I addressed the feedback so it should be ready for another round 👍 |
The updated locations of the scaling info make sense to me now. I'll clear my "changes requested" review and leave it to the platform team to do their technical review. |
762b2d3 to
ef1ea56
Compare
yrobla
left a comment
There was a problem hiding this comment.
Scalability review comment — session storage warning behavior
yrobla
left a comment
There was a problem hiding this comment.
Remaining scalability review comments
Catch up documentation with features shipped in v0.12.3 through v0.13.0. Auto-generated CLI/CRD reference docs were already current; these changes cover manual doc updates verified against source code at each release tag. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Radoslav Dimitrov <radoslav@stacklok.com>
This reverts commit ef1ea56.
07c7cb9 to
7fef985
Compare
d1adca2 to
334e758
Compare
- Clarify SessionStorageWarning is advisory-only and the operator still applies the requested replica count - Correct condition type (SessionStorageWarning) vs reason (SessionStorageMissingForReplicas) distinction - Add warning that ClientIP session affinity fails silently behind NAT or shared egress IPs, with guidance to use None for stateless backends - Fix MCPServer horizontal scaling section: backend is a StatefulSet, not a Deployment; add architecture overview and common scaling configs - Note that SessionStorageWarning only fires for spec.replicas > 1, not backendReplicas - Add connection draining note: 30s grace/drain period, no preStop hook, override via podTemplateSpec - Add Redis address example comment to prompt users to update the value - Clarify maxParallel fan-out is per-pod, not distributed across replicas - Add tip on sizing workflow timeouts relative to maxIterations/maxParallel
334e758 to
63a7880
Compare
danbarr
left a comment
There was a problem hiding this comment.
Nice work bringing the docs up to date with v0.12.3-v0.13.0. The new scaling, forEach, and annotation overrides sections are well-structured with good examples, and the cross-references form a clear navigation path.
The only blocking issue is the malformed admonition noted inline.
Non-blocking nits
Em dashes - A few lines introduced by this PR use em dashes (— or —). Per our style, these should be hyphens with spaces (-). Spotted in:
backend-auth.mdx: "chained authentication—where"run-mcp-k8s.mdx: "same pod — subject to the same NAT"scaling-and-performance.mdx: "same IP — routing every client"tool-aggregation.mdx: "optional—only set"composite-tools.mdx: "originate from a single pod…" area
Title case - scaling-and-performance.mdx has title: Scaling and Performance in front matter. Should be sentence case: Scaling and performance. (Pre-existing, but since the file is already being touched.)
| :::note The `SessionStorageWarning` condition fires only when | ||
| `spec.replicas > 1`. Scaling only the backend (`backendReplicas > 1`) does not | ||
| trigger a warning, but backend client-IP affinity is still unreliable behind NAT | ||
| or shared egress IPs. ::: |
There was a problem hiding this comment.
Formatting issue, this note won't render right
| :::note The `SessionStorageWarning` condition fires only when | |
| `spec.replicas > 1`. Scaling only the backend (`backendReplicas > 1`) does not | |
| trigger a warning, but backend client-IP affinity is still unreliable behind NAT | |
| or shared egress IPs. ::: | |
| :::note | |
| The `SessionStorageWarning` condition fires only when `spec.replicas > 1`. | |
| Scaling only the backend (`backendReplicas > 1`) does not trigger a warning, but | |
| backend client-IP affinity is still unreliable behind NAT or shared egress IPs. | |
| ::: |
Summary
Catch up documentation with user-facing features shipped in ToolHive v0.12.3 through v0.13.0. Changes were verified against source code at each release tag.
replicasandbackendReplicasCRD fields, session storage config, and stdio transport limitationsthv skill installdocumentation (registry and git sources)Releases covered
Excluded (verified as not needing docs)
--publishflag (not present in CLI at v0.13.0)Test plan
npm run buildpasses with no broken links or MDX errors🤖 Generated with Claude Code