feat: add observability tools domain - #11
Conversation
✅ Deploy Preview for ntl-mcp ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for netlify-mcp ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughSummary by CodeRabbitNew Features
WalkthroughThis PR adds an Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsStopped waiting for pipeline failures after 30000ms. One of your pipelines takes longer than our 30000ms fetch window to run, so review may not consider pipeline-failure results for inline comments if any failures occurred after the fetch window. Increase the timeout if you want to wait longer or run a Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@src/tools/index.ts`:
- Line 39: Before adding observabilityDomainTools to the listOfDomainTools
array, implement domain-aware redaction or selective logging to prevent
sensitive request telemetry from being captured. Identify the request/response
logging mechanism that processes tools from listOfDomainTools and add logic to
either redact sensitive identifiers (like client and network information) from
observability payloads or skip full payload/result logging specifically when
processing observability domain tools. This filtering should occur before the
observability tools are registered and start handling requests.
In `@src/tools/observability-tools/api-client.ts`:
- Around line 89-92: The authenticatedFetch call in the doPost method is sending
JSON data via POST without explicitly setting the Content-Type header, which can
cause backends to reject or mishandle the payload. Add a headers property to the
request options object passed to authenticatedFetch, setting Content-Type to
application/json to ensure proper handling of the JSON body by backend services.
- Around line 63-81: The parseFilters and parseSortBy functions silently return
undefined when JSON.parse fails, which causes caller constraints to be dropped
without notification. Instead of silently catching and returning undefined in
the catch blocks, throw an error with a descriptive message that indicates the
JSON parsing failure. This will force callers to explicitly handle invalid
filter or sort_by input rather than proceeding with incomplete constraints.
Update both function's catch blocks to throw an appropriate error rather than
return undefined.
- Around line 129-176: The dynamic path segments siteID and requestID are being
directly interpolated into URL paths without encoding, creating a security
vulnerability where crafted values could manipulate path semantics or redirect
to unintended routes. Fix this by wrapping siteID with encodeURIComponent() in
all functions (getCounts, getTopK, getTimeSeries, getLists, getAlerts) and
wrapping both siteID and requestID with encodeURIComponent() in the
getRequestDetail function before inserting them into the template literal URL
strings.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 080adbdb-d952-4db2-8eb4-21ace71abb78
📒 Files selected for processing (4)
src/tools/index.tssrc/tools/observability-tools/api-client.tssrc/tools/observability-tools/index.tssrc/tools/types.ts
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
netlify/blueprints(manual)
✅ Deploy Preview for mcp-isolate ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@src/tools/observability-tools/index.ts`:
- Around line 139-141: The callback function in the observability tools is
performing JSON stringification on the result from getRequestDetail and
returning it, but bindTools subsequently stringifies the return value again,
causing double JSON encoding. Remove the JSON.stringify() call wrapping the
result variable so that the callback returns the raw result object, allowing
bindTools to handle the single stringification step.
- Around line 127-132: The getRequestDetailSchema object has descriptions for
from_ts and to_ts indicating they should bracket the request (from_ts before,
to_ts after), but the schema does not enforce this constraint at validation
time. Add Zod schema validation (using refine or superRefine) to the
getRequestDetailSchema object to validate that from_ts is strictly less than
to_ts, ensuring invalid time windows are rejected during schema validation
rather than failing later in the API call.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f56eed3c-5a9f-4268-bfb8-4ac95b4d42e1
📒 Files selected for processing (2)
src/tools/observability-tools/api-client.tssrc/tools/observability-tools/index.ts
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
netlify/blueprints(manual)
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@src/tools/observability-tools/api-client.ts`:
- Line 68: The error messages at lines 68 and 80 in the API client parser are
including the full raw user input in the thrown Error messages, which could leak
sensitive request metadata if logged upstream. Replace the embedded raw variable
in both error messages with a fixed, generic error message that does not echo
back the user input. Instead of including the actual raw input value in the
error, use a descriptive message that only indicates the type error occurred
without exposing the full input details.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: efa728d1-9c86-4183-8a97-4863de7a2766
📒 Files selected for processing (1)
src/tools/observability-tools/api-client.ts
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
netlify/blueprints(manual)
| try { | ||
| return JSON.parse(raw) as QueryFilter[]; | ||
| } catch { | ||
| throw new Error(`Invalid filters: expected a JSON array, got ${raw}`); |
There was a problem hiding this comment.
Avoid echoing raw user input in parser errors.
Line 68 and Line 80 include full raw input in thrown errors. If those errors are logged upstream, filter values can leak sensitive request metadata. Prefer a fixed message (or a short redacted preview) instead of embedding full input.
Suggested minimal change
- throw new Error(`Invalid filters: expected a JSON array, got ${raw}`);
+ throw new Error('Invalid filters: expected a JSON array');
@@
- throw new Error(`Invalid sort_by: expected a JSON array, got ${raw}`);
+ throw new Error('Invalid sort_by: expected a JSON array');Also applies to: 80-80
🤖 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 `@src/tools/observability-tools/api-client.ts` at line 68, The error messages
at lines 68 and 80 in the API client parser are including the full raw user
input in the thrown Error messages, which could leak sensitive request metadata
if logged upstream. Replace the embedded raw variable in both error messages
with a fixed, generic error message that does not echo back the user input.
Instead of including the actual raw input value in the error, use a descriptive
message that only indicates the type error occurred without exposing the full
input details.
Adds an
observabilitytool domain to the MCP, 5 read-only tools over the existing Netlify observability API: counts, top-K breakdowns, time series, request logs, and request detail.