feat: add google discovery api backends#126
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughThis PR adds Google Discovery backend support, shared media-artifact handling for non-inline HTTP-like responses, and the required config, auth, CLI, runtime, and test wiring across Caplets. ChangesGoogle Discovery backend
Sequence Diagram(s)sequenceDiagram
participant CLI
participant Engine
participant GoogleDiscoveryManager
participant GoogleAPI
CLI->>Engine: call_tool for Google Discovery caplet
Engine->>GoogleDiscoveryManager: callTool(api, toolName, args)
GoogleDiscoveryManager->>GoogleAPI: fetch discovery or API request
GoogleAPI-->>GoogleDiscoveryManager: discovery doc / HTTP response
GoogleDiscoveryManager-->>Engine: structured content
Engine-->>CLI: tool result
sequenceDiagram
participant HttpActionManager
participant readHttpLikeResponse
participant writeMediaArtifact
participant ArtifactStore
HttpActionManager->>readHttpLikeResponse: parse downstream Response
readHttpLikeResponse->>writeMediaArtifact: persist non-inline body bytes
writeMediaArtifact->>ArtifactStore: write artifact file and metadata
ArtifactStore-->>readHttpLikeResponse: MediaArtifact
readHttpLikeResponse-->>HttpActionManager: response envelope with body.artifact
Estimated code review effort🎯 5 (Critical) | ⏱️ ~110 minutes Possibly related PRs
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
Preview DeployedLanding: https://pr-126.preview.caplets.dev Built from commit 1a164d1 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
packages/core/src/media/input.ts (1)
50-61: 💤 Low valueArtifact file is read twice: once for hashing, once for bytes.
resolveMediaArtifactreads the entire file to compute the SHA-256 hash, then discards the bytes. Immediately after,readMediaFilereads the same file again to return the content. For large artifacts this doubles I/O.Consider having
resolveMediaArtifactoptionally return the bytes it already read, or deferring the hash computation to callers that need it.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/media/input.ts` around lines 50 - 61, The code reads the artifact file twice: once inside resolveMediaArtifact for hashing, and again via readMediaFile for the bytes. Modify resolveMediaArtifact to optionally return the file bytes it already read during hash computation along with the artifact metadata, then update the code in this section to use those returned bytes instead of calling readMediaFile separately. This eliminates the duplicate file I/O and improves performance for large artifacts.docs/plans/2026-06-16-google-discovery-api-backend-implementation.md (1)
1-2152: ⚡ Quick winPlease confirm this implementation plan is intended to be committed long-term.
This looks like a short-lived execution checklist; if it’s not explicitly required as a durable artifact, it should be removed after implementation (or repurposed into durable docs) to avoid stale guidance.
As per coding guidelines, “Avoid committing short-lived implementation plans or design specs unless explicitly requested; if needed, put them in
docs/plans/ordocs/specs/and delete or repurpose once superseded.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/plans/2026-06-16-google-discovery-api-backend-implementation.md` around lines 1 - 2152, The comment asks whether this implementation plan document should be committed as permanent reference material or removed after implementation completion. This is a documentation lifecycle decision, not a code defect. Decide whether to keep the plan at `docs/plans/2026-06-16-google-discovery-api-backend-implementation.md` as a durable architecture/implementation reference for future maintainers, or delete it after implementation is complete and migrate essential long-term guidance into `docs/architecture.md` and the public reference documentation in `apps/docs/`. If retaining the plan, add a note at the top clarifying its purpose (for example, "This document serves as both execution guidance and permanent architectural record for the Google Discovery API backend design decisions and implementation approach"). If treating it as temporary, file a cleanup task after implementation to remove the file and ensure all enduring patterns are documented in the appropriate permanent reference materials.Source: Coding guidelines
packages/core/src/caplet-source/parse.ts (1)
138-138: 💤 Low valueUnnecessary fallback
?? {}is inconsistent with other backends.The
CapletsConfigtype declaresgoogleDiscoveryApisas a required property (not optional), so the?? {}fallback is unnecessary and inconsistent with how the other backend records on lines 136-137 and 139-143 are accessed.♻️ Suggested fix for consistency
- ...Object.values(config.googleDiscoveryApis ?? {}), + ...Object.values(config.googleDiscoveryApis),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/caplet-source/parse.ts` at line 138, The line accessing config.googleDiscoveryApis contains an unnecessary `?? {}` fallback that is inconsistent with other backend record accesses. Since the CapletsConfig type declares googleDiscoveryApis as a required property (not optional), remove the `?? {}` fallback and change the expression to directly use Object.values(config.googleDiscoveryApis) to match the pattern used for the other backend records accessed nearby.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/core/src/http/response.ts`:
- Around line 89-95: The rejectOversizedContentLength function throws an error
when content length exceeds the limit but does not cancel the response body
stream first, leaving it unconsumed and degrading HTTP connection reuse. Before
throwing the responseExceededLimit error in rejectOversizedContentLength, call
response.body?.cancel() to properly clean up the stream. This aligns with the
established pattern already used in readInlineCandidate and readBoundedBytes
functions when they reject oversized content during streaming.
---
Nitpick comments:
In `@docs/plans/2026-06-16-google-discovery-api-backend-implementation.md`:
- Around line 1-2152: The comment asks whether this implementation plan document
should be committed as permanent reference material or removed after
implementation completion. This is a documentation lifecycle decision, not a
code defect. Decide whether to keep the plan at
`docs/plans/2026-06-16-google-discovery-api-backend-implementation.md` as a
durable architecture/implementation reference for future maintainers, or delete
it after implementation is complete and migrate essential long-term guidance
into `docs/architecture.md` and the public reference documentation in
`apps/docs/`. If retaining the plan, add a note at the top clarifying its
purpose (for example, "This document serves as both execution guidance and
permanent architectural record for the Google Discovery API backend design
decisions and implementation approach"). If treating it as temporary, file a
cleanup task after implementation to remove the file and ensure all enduring
patterns are documented in the appropriate permanent reference materials.
In `@packages/core/src/caplet-source/parse.ts`:
- Line 138: The line accessing config.googleDiscoveryApis contains an
unnecessary `?? {}` fallback that is inconsistent with other backend record
accesses. Since the CapletsConfig type declares googleDiscoveryApis as a
required property (not optional), remove the `?? {}` fallback and change the
expression to directly use Object.values(config.googleDiscoveryApis) to match
the pattern used for the other backend records accessed nearby.
In `@packages/core/src/media/input.ts`:
- Around line 50-61: The code reads the artifact file twice: once inside
resolveMediaArtifact for hashing, and again via readMediaFile for the bytes.
Modify resolveMediaArtifact to optionally return the file bytes it already read
during hash computation along with the artifact metadata, then update the code
in this section to use those returned bytes instead of calling readMediaFile
separately. This eliminates the duplicate file I/O and improves performance for
large artifacts.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 915083f3-b449-454f-b566-160a178b68a2
📒 Files selected for processing (66)
.changeset/google-discovery-media.mdCONTEXT.mdapps/docs/src/content/docs/capabilities.mdxapps/docs/src/content/docs/reference/caplet-files.mdxapps/docs/src/content/docs/reference/config.mdxapps/docs/src/content/docs/troubleshooting.mdxapps/landing/public/caplet-frontmatter.schema.jsonapps/landing/public/config.schema.jsondocs/adr/0002-media-artifacts-for-non-inline-results.mddocs/architecture.mddocs/plans/2026-06-16-google-discovery-api-backend-implementation.mddocs/specs/2026-06-16-google-discovery-api-backend.mdpackages/core/src/auth.tspackages/core/src/caplet-files-bundle.tspackages/core/src/caplet-sets.tspackages/core/src/caplet-source/parse.tspackages/core/src/cli.tspackages/core/src/cli/add.tspackages/core/src/cli/auth.tspackages/core/src/cli/commands.tspackages/core/src/cli/completion-discovery.tspackages/core/src/cli/doctor.tspackages/core/src/cli/inspection.tspackages/core/src/cli/setup-caplet.tspackages/core/src/cli/setup.tspackages/core/src/cloud/runtime-adapter.tspackages/core/src/config-runtime.tspackages/core/src/config.tspackages/core/src/config/paths.tspackages/core/src/engine.tspackages/core/src/google-discovery/index.tspackages/core/src/google-discovery/manager.tspackages/core/src/google-discovery/operations.tspackages/core/src/google-discovery/request.tspackages/core/src/google-discovery/schema.tspackages/core/src/google-discovery/types.tspackages/core/src/http-actions.tspackages/core/src/http/response.tspackages/core/src/media/artifacts.tspackages/core/src/media/index.tspackages/core/src/media/input.tspackages/core/src/native/service.tspackages/core/src/openapi.tspackages/core/src/registry.tspackages/core/src/remote-control/dispatch.tspackages/core/src/runtime-plan/planner.tspackages/core/src/runtime.tspackages/core/src/tools.tspackages/core/test/auth.test.tspackages/core/test/caplet-sets.test.tspackages/core/test/caplet-source.test.tspackages/core/test/cli-completion.test.tspackages/core/test/cli-remote.test.tspackages/core/test/cli.test.tspackages/core/test/config.test.tspackages/core/test/engine.test.tspackages/core/test/exposure-discovery.test.tspackages/core/test/fixtures/google-discovery/drive.discovery.jsonpackages/core/test/google-discovery.test.tspackages/core/test/http-actions.test.tspackages/core/test/media-artifacts.test.tspackages/core/test/native.test.tspackages/core/test/openapi.test.tspackages/core/test/tools.test.tsschemas/caplet.schema.jsonschemas/caplets-config.schema.json
| function rejectOversizedContentLength(response: Response, maxBytes: number): void { | ||
| const contentLength = response.headers.get("content-length"); | ||
| if (!contentLength) return; | ||
| const byteLength = Number.parseInt(contentLength, 10); | ||
| if (Number.isFinite(byteLength) && byteLength > maxBytes) { | ||
| throw responseExceededLimit(maxBytes); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify the preflight branch and whether it cancels the response body before throwing.
nl -ba packages/core/src/http/response.ts | sed -n '84,105p'
rg -n "rejectOversizedContentLength|body\\?\\.cancel\\(" packages/core/src/http/response.tsRepository: spiritledsoftware/caplets
Length of output: 112
🏁 Script executed:
#!/bin/bash
# View the relevant section with a different approach
cat -n packages/core/src/http/response.ts | sed -n '84,105p'Repository: spiritledsoftware/caplets
Length of output: 839
🏁 Script executed:
# Search for the function and body cancellation patterns
rg -n "rejectOversizedContentLength" packages/core/src/http/response.ts
rg -n "body.*cancel|cancel.*body" packages/core/src/http/response.tsRepository: spiritledsoftware/caplets
Length of output: 211
🏁 Script executed:
# Check the full file size to understand context better
wc -l packages/core/src/http/response.ts
head -n 100 packages/core/src/http/response.ts | tail -n 30Repository: spiritledsoftware/caplets
Length of output: 1000
🏁 Script executed:
# Check where rejectOversizedContentLength is called (line 23)
sed -n '15,35p' packages/core/src/http/response.tsRepository: spiritledsoftware/caplets
Length of output: 1064
🏁 Script executed:
# Get the full context of the readHttpLikeResponse function
sed -n '15,50p' packages/core/src/http/response.tsRepository: spiritledsoftware/caplets
Length of output: 1585
🏁 Script executed:
# Check if response.body is ever accessed after rejectOversizedContentLength throws
rg -n "readHttpLikeResponse|rejectOversizedContentLength" -A 5 packages/core/src/http/response.ts | head -40Repository: spiritledsoftware/caplets
Length of output: 1028
🏁 Script executed:
# Check the readBoundedBytes function to confirm it has reader.cancel() logic
sed -n '63,87p' packages/core/src/http/response.tsRepository: spiritledsoftware/caplets
Length of output: 693
🏁 Script executed:
# Verify if there are any tests or other handlers for unconsumed response bodies
rg -n "cancel|unconsumed|body" packages/core/src/http/response.ts | head -20Repository: spiritledsoftware/caplets
Length of output: 509
🏁 Script executed:
# Check if this pattern affects other files matching the guidelines
fd '\.ts$' packages/core/src --type f | head -5Repository: spiritledsoftware/caplets
Length of output: 230
Cancel the response body before throwing on oversized Content-Length.
At line 23, rejectOversizedContentLength(response, maxBytes) is called before any body reading, and throws without canceling the response body stream. This leaves the response unconsumed, which can degrade HTTP connection reuse under load. The pattern of canceling before throwing is already established in readInlineCandidate (line 57) and readBoundedBytes (line 80) when hitting size limits during streaming, so this preflight check should be consistent.
Suggested fix
function rejectOversizedContentLength(response: Response, maxBytes: number): void {
+ response.body?.cancel().catch(() => {});
const contentLength = response.headers.get("content-length");Alternatively, make it async:
-function rejectOversizedContentLength(response: Response, maxBytes: number): void {
+async function rejectOversizedContentLength(response: Response, maxBytes: number): Promise<void> {
const contentLength = response.headers.get("content-length");
if (!contentLength) return;
const byteLength = Number.parseInt(contentLength, 10);
if (Number.isFinite(byteLength) && byteLength > maxBytes) {
+ await response.body?.cancel().catch(() => {});
throw responseExceededLimit(maxBytes);
}
}And update the call at line 23 to await rejectOversizedContentLength(response, maxBytes);
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function rejectOversizedContentLength(response: Response, maxBytes: number): void { | |
| const contentLength = response.headers.get("content-length"); | |
| if (!contentLength) return; | |
| const byteLength = Number.parseInt(contentLength, 10); | |
| if (Number.isFinite(byteLength) && byteLength > maxBytes) { | |
| throw responseExceededLimit(maxBytes); | |
| } | |
| function rejectOversizedContentLength(response: Response, maxBytes: number): void { | |
| response.body?.cancel().catch(() => {}); | |
| const contentLength = response.headers.get("content-length"); | |
| if (!contentLength) return; | |
| const byteLength = Number.parseInt(contentLength, 10); | |
| if (Number.isFinite(byteLength) && byteLength > maxBytes) { | |
| throw responseExceededLimit(maxBytes); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/core/src/http/response.ts` around lines 89 - 95, The
rejectOversizedContentLength function throws an error when content length
exceeds the limit but does not cancel the response body stream first, leaving it
unconsumed and degrading HTTP connection reuse. Before throwing the
responseExceededLimit error in rejectOversizedContentLength, call
response.body?.cancel() to properly clean up the stream. This aligns with the
established pattern already used in readInlineCandidate and readBoundedBytes
functions when they reject oversized content during streaming.
There was a problem hiding this comment.
💡 Codex Review
caplets/packages/core/src/remote-control/dispatch.ts
Lines 182 to 183 in 10ee981
Remote-control auth login still starts from findAuthTarget, so Google Discovery targets without explicitly configured auth.scopes never load the Discovery document and never request the operation scopes that normal calls later require. In remote/cloud auth flows this can complete successfully but store credentials that immediately fail tokenBundleMissingScopes on the first Google API call, forcing users into a login loop; it should use the same resolved target path as local loginAuth.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
| Filename | Overview |
|---|---|
| packages/core/src/google-discovery/manager.ts | Core Google Discovery manager — two body-leak findings: response bodies are not consumed before throwing on 3xx/401/403 in both callTool and the shared fetchGoogleRequest helper, risking connection-pool exhaustion under repeated auth errors or upload redirects. |
| packages/core/src/google-discovery/schema.ts | Discovery-to-JSON-Schema converter — type any is incorrectly mapped to object, producing over-restrictive schemas for any Google API that uses an unconstrained value type such as Firestore or BigQuery. |
| packages/core/src/google-discovery/operations.ts | Operation extraction and input schema building — path parameters not explicitly marked required true in the Discovery document are missing from the inner schema required list, causing a runtime error instead of clean validation failure. |
| packages/core/src/google-discovery/request.ts | URL building and request initialization — URL validation, path-traversal prevention, reserved-expansion encoding, and query serialization all look correct. |
| packages/core/src/http/response.ts | New shared HTTP response reader — correctly streams to a size limit, inlines JSON and text and writes oversized or binary responses as media artifacts; body is always consumed via the reader. |
| packages/core/src/media/artifacts.ts | New media artifact writer — thorough path-containment, symlink-rejection, and permission hardening with 0o600 files. URI encoding and decoding validation is well-guarded. |
| packages/core/src/config.ts | Config validation extended with googleDiscoveryApis — duplicate-ID checks are now propagated through all backend chains, the mutual-exclusivity of discoveryPath and discoveryUrl is enforced, and header and URL constraints match the other backends. |
| packages/core/src/auth.ts | OAuth scope handling extended to accept resolvedScopes from Discovery operations — scope injection into authorization URLs, token refresh, and bundle-mismatch detection all updated consistently. |
| packages/core/src/engine.ts | Engine wired up with GoogleDiscoveryManager — dispatch chain, registry updates, and cache invalidation all correctly extended; selectHttpLikeOptions helper avoids duplication. |
Sequence Diagram
%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant MCP as MCP Client
participant Engine as CapletsEngine
participant GDM as GoogleDiscoveryManager
participant Cache as In-Memory Cache
participant Auth as genericOAuthHeaders
participant Google as Google API
MCP->>Engine: callTool(serverId, toolName, args)
Engine->>GDM: callTool(api, toolName, args)
GDM->>GDM: getOperation(api, toolName)
GDM->>Cache: get(api.server)
alt cache miss or expired
GDM->>Google: fetch(discoveryUrl)
Google-->>GDM: Discovery document JSON
GDM->>GDM: discoveryOperations(document)
GDM->>Cache: set(api.server, operations)
end
Cache-->>GDM: GoogleDiscoveryOperation
alt supportsMediaUpload and args.media present
GDM->>Auth: authHeaders(api, scopes)
Auth-->>GDM: Authorization header
GDM->>Google: single or multipart or resumable upload
Google-->>GDM: Response
else normal call
GDM->>Auth: authHeaders(api, scopes)
Auth-->>GDM: Authorization header
GDM->>Google: fetch(operationUrl, init)
Google-->>GDM: Response
end
GDM->>GDM: readHttpLikeResponse(response)
alt small JSON or text
GDM-->>Engine: inline body in result
else large or binary
GDM->>GDM: writeMediaArtifact(bytes)
GDM-->>Engine: artifact URI in result
end
Engine-->>MCP: CompatibilityCallToolResult
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant MCP as MCP Client
participant Engine as CapletsEngine
participant GDM as GoogleDiscoveryManager
participant Cache as In-Memory Cache
participant Auth as genericOAuthHeaders
participant Google as Google API
MCP->>Engine: callTool(serverId, toolName, args)
Engine->>GDM: callTool(api, toolName, args)
GDM->>GDM: getOperation(api, toolName)
GDM->>Cache: get(api.server)
alt cache miss or expired
GDM->>Google: fetch(discoveryUrl)
Google-->>GDM: Discovery document JSON
GDM->>GDM: discoveryOperations(document)
GDM->>Cache: set(api.server, operations)
end
Cache-->>GDM: GoogleDiscoveryOperation
alt supportsMediaUpload and args.media present
GDM->>Auth: authHeaders(api, scopes)
Auth-->>GDM: Authorization header
GDM->>Google: single or multipart or resumable upload
Google-->>GDM: Response
else normal call
GDM->>Auth: authHeaders(api, scopes)
Auth-->>GDM: Authorization header
GDM->>Google: fetch(operationUrl, init)
Google-->>GDM: Response
end
GDM->>GDM: readHttpLikeResponse(response)
alt small JSON or text
GDM-->>Engine: inline body in result
else large or binary
GDM->>GDM: writeMediaArtifact(bytes)
GDM-->>Engine: artifact URI in result
end
Engine-->>MCP: CompatibilityCallToolResult
Reviews (4): Last reviewed commit: "fix: finish google discovery review foll..." | Re-trigger Greptile
| function shouldSendDiscoveryAuth(api: GoogleDiscoveryApiConfig): boolean { | ||
| return Boolean( | ||
| api.discoveryUrl && | ||
| api.baseUrl && | ||
| new URL(api.discoveryUrl).origin === new URL(api.baseUrl).origin, | ||
| ); | ||
| } |
There was a problem hiding this comment.
Silent no-auth for remote private discovery documents
shouldSendDiscoveryAuth requires api.baseUrl to be explicitly set before it will attach auth headers to the discovery document fetch. If a user configures discoveryUrl with oauth2/oidc auth but does not set baseUrl (relying on the auto-derived value from rootUrl + servicePath), no auth header is sent and the discovery fetch will receive a 401/403, failing with a confusing "Google Discovery document request failed" error rather than an auth-related message.
Consider either (a) documenting this requirement clearly, or (b) sending auth headers whenever the auth type is not none and the discoveryUrl is set, regardless of baseUrl.
| } | ||
| } | ||
|
|
||
| for (const [api, rawValue] of Object.entries(config.googleDiscoveryApis)) { | ||
| const raw = rawValue as ConfigSchemaGoogleDiscoveryApiValue; | ||
| if (config.mcpServers[api]) { | ||
| ctx.addIssue({ | ||
| code: "custom", | ||
| path: ["googleDiscoveryApis", api], | ||
| message: `Caplet ID ${api} is already used by mcpServers`, | ||
| }); | ||
| } | ||
| if (config.openapiEndpoints[api]) { | ||
| ctx.addIssue({ | ||
| code: "custom", | ||
| path: ["googleDiscoveryApis", api], | ||
| message: `Caplet ID ${api} is already used by openapiEndpoints`, | ||
| }); | ||
| } | ||
| if (!SERVER_ID_PATTERN.test(api)) { | ||
| ctx.addIssue({ | ||
| code: "custom", | ||
| path: ["googleDiscoveryApis", api], | ||
| message: "Google Discovery API ID must match ^[a-zA-Z0-9_-]{1,64}$", | ||
| }); | ||
| } | ||
| if (Boolean(raw.discoveryPath) === Boolean(raw.discoveryUrl)) { | ||
| ctx.addIssue({ | ||
| code: "custom", | ||
| path: ["googleDiscoveryApis", api], | ||
| message: | ||
| "Google Discovery API must define exactly one discovery source: discoveryPath or discoveryUrl", | ||
| }); | ||
| } | ||
| if (raw.discoveryUrl && !isAllowedRemoteUrl(raw.discoveryUrl)) { | ||
| ctx.addIssue({ | ||
| code: "custom", | ||
| path: ["googleDiscoveryApis", api, "discoveryUrl"], | ||
| message: | ||
| "Google Discovery API discoveryUrl must use https except loopback development urls", | ||
| }); | ||
| } | ||
| if (raw.baseUrl && !isAllowedHttpBaseUrl(raw.baseUrl)) { | ||
| ctx.addIssue({ | ||
| code: "custom", | ||
| path: ["googleDiscoveryApis", api, "baseUrl"], | ||
| message: | ||
| "Google Discovery API baseUrl must use https except loopback development urls and must not include credentials, query, or fragment", | ||
| }); | ||
| } | ||
| if (raw.auth?.type === "headers") { | ||
| for (const headerName of Object.keys(raw.auth.headers)) { | ||
| const normalized = headerName.toLowerCase(); | ||
| if (!HEADER_NAME_PATTERN.test(headerName) || FORBIDDEN_HEADERS.has(normalized)) { | ||
| ctx.addIssue({ | ||
| code: "custom", | ||
| path: ["googleDiscoveryApis", api, "auth", "headers", headerName], | ||
| message: `header ${headerName} is not allowed`, | ||
| }); | ||
| } |
There was a problem hiding this comment.
Incomplete cross-backend duplicate Caplet ID checks
The new googleDiscoveryApis validation guards against ID collisions with mcpServers and openapiEndpoints, but not against graphqlEndpoints, httpApis, cliTools, or capletSets. A duplicate ID shared between googleDiscoveryApis and, for example, httpApis would silently produce two entries with the same server ID, breaking any lookup by ID. The existing backends appear to have the same gap, but since this is new code it's a good opportunity to be complete.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b137ec3386
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const started = await fetchGoogleRequest(api, operation, startUrl, { | ||
| method: operation.method.toUpperCase(), | ||
| headers, | ||
| body: JSON.stringify(args.body ?? {}), | ||
| redirect: "manual", | ||
| }); | ||
| const location = started.headers.get("location"); | ||
| if (!location) { | ||
| throw new CapletsError( | ||
| "DOWNSTREAM_PROTOCOL_ERROR", | ||
| "Google resumable upload missing Location", | ||
| ); | ||
| } | ||
| const uploadHeaders = new Headers(); | ||
| uploadHeaders.set("content-type", media.mimeType ?? "application/octet-stream"); | ||
| uploadHeaders.set("content-length", String(media.bytes.byteLength)); | ||
| uploadHeaders.set( | ||
| "content-range", | ||
| `bytes 0-${media.bytes.byteLength - 1}/${media.bytes.byteLength}`, | ||
| ); | ||
| return fetchGoogleRequest(api, operation, new URL(location), { | ||
| method: "PUT", | ||
| headers: uploadHeaders, | ||
| body: media.bytes, | ||
| redirect: "manual", | ||
| }); |
There was a problem hiding this comment.
Response body leak in resumable upload session start
The started response returned by fetchGoogleRequest has its body never consumed or cancelled. In Node.js's undici-backed fetch, an unconsumed body holds the underlying TCP connection open in-flight until the Response object is garbage-collected, preventing it from being returned to the connection pool. Every resumable upload call leaks one connection; under any meaningful upload load this will exhaust the pool and stall subsequent requests.
After reading the location header — whether or not it is present — the body must be explicitly discarded: started.body?.cancel().catch(() => {}) (fire-and-forget) or await started.body?.cancel() (eager). This applies to both the missing-location error path and the happy path.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8f39ac67f9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const response = await fetch(url, { ...init, signal: controller.signal }); | ||
| if (response.status >= 300 && response.status < 400) { | ||
| throw new CapletsError( | ||
| "DOWNSTREAM_PROTOCOL_ERROR", | ||
| "Google Discovery request returned a redirect", | ||
| { | ||
| server: requestApi.server, | ||
| status: response.status, | ||
| location: response.headers.get("location") ? "[REDACTED]" : undefined, | ||
| }, | ||
| ); | ||
| } | ||
| if (response.status === 401 || response.status === 403) { | ||
| throw googleAuthError(requestApi, response); | ||
| } |
There was a problem hiding this comment.
The 3xx and 401/403 error paths throw without consuming the response body. In Node.js's undici-backed fetch, an unconsumed body holds its TCP connection open until GC collects the
Response object. Under any meaningful request volume (repeated auth failures, occasional redirects) this slowly exhausts the connection pool. Add a fire-and-forget cancel before throwing, consistent with the fix needed in callResumableUpload.
| const response = await fetch(url, { ...init, signal: controller.signal }); | |
| if (response.status >= 300 && response.status < 400) { | |
| throw new CapletsError( | |
| "DOWNSTREAM_PROTOCOL_ERROR", | |
| "Google Discovery request returned a redirect", | |
| { | |
| server: requestApi.server, | |
| status: response.status, | |
| location: response.headers.get("location") ? "[REDACTED]" : undefined, | |
| }, | |
| ); | |
| } | |
| if (response.status === 401 || response.status === 403) { | |
| throw googleAuthError(requestApi, response); | |
| } | |
| const response = await fetch(url, { ...init, signal: controller.signal }); | |
| if (response.status >= 300 && response.status < 400) { | |
| response.body?.cancel().catch(() => {}); | |
| throw new CapletsError( | |
| "DOWNSTREAM_PROTOCOL_ERROR", | |
| "Google Discovery request returned a redirect", | |
| { | |
| server: requestApi.server, | |
| status: response.status, | |
| location: response.headers.get("location") ? "[REDACTED]" : undefined, | |
| }, | |
| ); | |
| } | |
| if (response.status === 401 || response.status === 403) { | |
| response.body?.cancel().catch(() => {}); | |
| throw googleAuthError(requestApi, response); | |
| } |
Summary
Adds first-class Google Discovery API Caplets alongside the existing MCP, OpenAPI, GraphQL, HTTP, CLI, and Caplet-set backends.
What changed
googleDiscoveryApisconfig andgoogleDiscoveryApiMarkdown Caplet frontmatter support, including generated JSON schemas and docs references.Validation
pnpm verifySummary by CodeRabbit
Release Notes
caplets add google-discoveryto create Discovery-backed caplets from local or remote inputs.googleDiscoveryApisconfiguration for managing multiple Discovery-backed caplets.