Repository navigation
feat(mcp): Add create_monitor tool for cron monitors - #1846
Chipagosfinest wants to merge 2 commits into
Conversation
Agents could create uptime and metric monitors but not cron monitors,
so they sent users to the UI. Add a catalog-only create_monitor tool
gated by the project-management skill that calls
POST /organizations/{org}/monitors/ with Sentry's ConfigValidator fields
(schedule, timezone, check-in margin, max runtime, failure/recovery
thresholds), and point find_monitors at it.
Fixes getsentry#1845
Co-Authored-By: Grok Bot <noreply@x.ai>
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
The exactly-one crontab/interval rule lived only in the handler, so the generated tool schema allowed both or neither. Replace the two optional fields with a required `schedule` discriminated union of strict crontab and interval variants, which generates a oneOf schema. Allow checkInMargin=0, which Sentry's MissedMarginField accepts and coerces to 1. Co-Authored-By: Grok Bot <noreply@x.ai>
There was a problem hiding this comment.
Thanks for checking the API so carefully. The validator, limit, and scope work is solid. One design problem needs fixing before this lands, plus a smaller follow-up:
- Create through the detectors endpoint, not the legacy monitors endpoint.
createMonitorposts toPOST /organizations/{org}/monitors/. On that path,MonitorValidator.createcallsensure_cron_detector(monitors/utils.py), which creates theDetectorwithoutcreated_by_id. That means every cron monitor an agent creates would have no creator. #1837 fixes this exact gap forcreate_uptime_monitor. The current cron UI posts to the detectors API instead (cronFormDataToEndpointPayloadinstatic/app/views/detectors/components/forms/cron/fields.tsx:type: 'monitor_check_in_failure', with the monitor fields indataSources[0], plusworkflowIds). That path goes throughMonitorIncidentDetectorValidator, and the base detectorcreatesetscreated_by_id=acting_user_id(request.user)and connects workflows.create_metric_monitoralready usesPOST /organizations/{org}/projects/{project}/detectors/withworkflowIds, so following that pattern also lets agents attach existing Alerts, the same way they can for metric monitors. To keep the currentMonitoroutput shape, re-read the monitor after the create, the same way #1837 does for uptime. Since #1839, cron reads go through the sharedgetMonitorintoolkit-coreapi-client/endpoints/monitors.ts, so rebase onto main and use that instead of adding another cron method toSentryApiService. - Tell the agent when the monitor was created disabled. On both create paths, if
quotas.backend.assign_seatdoesn't accept the monitor, Sentry still returns success but disables the monitor (monitor.status = DISABLED,detector.enabled = False). The output includesstatus, but the description gives the agent no reason to look at it, so it will probably report a working monitor. Add one line saying that ifstatusisdisabledand the caller didn't ask for that, there was no monitor seat, and the agent should tell the user. Add a test where the mocked response comes back disabled.
Also, CI hasn't run yet: Test, Warden, Token Cost, and Eval are all waiting for maintainer approval because this PR comes from a fork.
|
will get this over the finish line ty! |
|
I picked this up in #1861. It keeps both of your commits and adds the review fix on top: the monitor is now created through the detectors endpoint ( |
Agents could create uptime and metric monitors but not cron monitors,
so they sent users to the UI. Add a catalog-only create_monitor tool
gated by the project-management skill that calls
POST /organizations/{org}/monitors/ with Sentry's ConfigValidator fields
(schedule, timezone, check-in margin, max runtime, failure/recovery
thresholds), and point find_monitors at it.
Fixes #1845
Co-Authored-By: Grok Bot <noreply@x.ai>
Originally authored by Alec Gutman (@Chipagosfinest) in #1846.
The exactly-one crontab/interval rule lived only in the handler, so the generated tool schema allowed both or neither. Replace the two optional fields with a required `schedule` discriminated union of strict crontab and interval variants, which generates a oneOf schema. Allow checkInMargin=0, which Sentry's MissedMarginField accepts and coerces to 1. Co-Authored-By: Grok Bot <noreply@x.ai> Originally authored by Alec Gutman (@Chipagosfinest) in #1846.
Fixes #1845. This supersedes #1846: it keeps @Chipagosfinest's commits for the `create_monitor` tool and adds a fix on top, applied from that PR's review. `create_monitor` lets agents create cron monitors instead of sending users to the UI. It's catalog-only, gated by the `project-management` skill, and takes the same schedule, timezone, margin, runtime and threshold inputs as in #1846. What changed from #1846: - **It creates through the detectors endpoint.** It now calls `POST /organizations/{org}/projects/{project}/detectors/` with `type: monitor_check_in_failure`, the same payload the cron UI sends (`cronFormDataToEndpointPayload`). The legacy `POST /organizations/{org}/monitors/` path creates the detector through `ensure_cron_detector`, which never sets `created_by_id`, so monitors created through MCP would have had no creator. This is the same gap #1837 fixes for uptime monitors. The detector path also matches `create_metric_monitor`, so the tool now accepts optional `workflowIds` to connect existing Alerts. - **The output shape stays the same.** Sentry serializes the created Monitor as `dataSources[0].queryObj`, so the tool returns the same summary with no second request. The only addition is the connected `workflowIds`. The owner is sent on both the detector and the monitor, which matches what the legacy path copied over. - **Agents are told about seat-disabled monitors.** When the org has no monitor seat available, Sentry still returns success but disables the monitor. The description now tells the agent to report an unrequested `disabled` status to the user. I checked the request and response shapes against getsentry/sentry@e39c07b (`MonitorIncidentDetectorValidator`, `MonitorDataSourceValidator`, and the base detector `create`). Nothing has been run against a live org yet. <!-- junior-request-attribution:start --> via @dcramer. <!-- junior-request-attribution:end --> <!-- junior-session-footer:start --> <!-- junior-conversation-id:slack%3AC08J1NSPU6S%3A1793724602.686199 --> -- [View Session](https://junior-prod.sentry.dev/conversations/slack%3AC08J1NSPU6S%3A1793724602.686199) [[Sentry]](https://sentry.sentry.io/explore/conversations/slack%3AC08J1NSPU6S%3A1793724602.686199/?project=4510944073809921) <!-- junior-session-footer:end --> --------- Co-authored-by: sentry-junior[bot] <264270552+sentry-junior[bot]@users.noreply.github.com>
Summary
Fixes #1845 by adding a
create_monitorcatalog tool, so agents can create cron monitors directly instead of sending users to the UI. The tool is gated by theproject-managementskill, like the other monitor write tools.Key Changes
create_monitor(tools/catalog/create-monitor.ts). It takes a requiredschedule, which must be either{ type: 'crontab', crontab }or{ type: 'interval', value, unit }, plus optionaltimezone,checkInMargin,maxRuntime,failureIssueThreshold,recoveryThreshold,slug,statusandowner. It returns a structured summary that includes the monitor's web URL.SentryApiService.createMonitor, which callsPOST /organizations/{org}/monitors/.find_monitorsbefore retrying, because the create isn't idempotent (same ascreate_metric_monitor).find_monitorsnow points atcreate_monitor, and theproject-managementskill description mentions cron monitors. Definitions are regenerated.API notes (checked against getsentry/sentry@e39c07b)
OrganizationMonitorIndexEndpoint.postreturns 201 with aMonitorSerializerResponse, or 400 withvalidator.errors.MonitorValidatortakesproject,name,slug,statusandowner.ConfigValidatoris a plainSerializer, soconfigkeys stay snake_case:schedule_type,schedule(a crontab string or[n, unit]),checkin_margin,max_runtime,timezone,failure_issue_thresholdandrecovery_threshold. The input limits use the values inconstants.py.OrganizationAlertRulePermission. Its POST scope_map accepts any oforg:read,org:write,org:adminoralerts:write. After that,check_can_create_monitorrequiresalerts:write,org:writeororg:admin, or project-levelalerts:write. Soalerts:writeis enough, and org write/admin isn't needed. The tool declaresorg:read,project:readandalerts:write, the same ascreate_metric_monitor.Testing
pnpm run tsc,pnpm run lint(oxlint, oxfmt, ast-grep scan and tests, workspace deps),pnpm run check:generatedandpnpm --filter @sentry/mcp-core test(1950 passed, 6 skipped).pnpm run testrun on macOS has 21 failures, all inpackages/cli(zip-writer,time-range,car-extract,install-script). The same 21 also fail onmainat 462bc50, so they aren't related to this PR. They depend on the environment:zip-writerandtime-rangepass withTZ=UTC,car-extractneeds Xcodeactool, and theinstall-scriptmusl case is Linux-specific.checkInMargin: 0, owner format, input limits, forwarding of slug/status/owner, a nested 400 validation error reaching the agent, and project-constraint rejection with no POST sent.Review
Before opening this PR, I had Claude Opus 5.5, Codex GPT-6.1 Sol and Gemini 4 Argon review the diff against the API notes above. Changes made from that review: tighter
ownervalidation (user:IDorteam:IDwith a numeric ID), removal of an unusedisMutedclient parameter, the extra tests listed above, retry guidance, a note about the five-field crontab rule and an updated skill description. Not changed: droppingorg:read, to stay consistent with the other alert-write tools, and local timezone validation, since Sentry already validates it.I also ran an OpenRouter council (Grok 4.7, Kimi K3.1 Pro, DeepSeek V4 Pro) on the diff. All three said approve with nits and found no bugs. I applied two of their nits in a follow-up commit. First, the crontab-or-interval rule now lives in the input schema as a discriminated union with strict variants, so the generated tool schema shows it as a
oneOf. Second,checkInMarginnow accepts0, which Sentry'sMissedMarginFieldaccepts and coerces to 1. Codex GPT-6.1 Sol reviewed that change and approved with one test nit, which is now covered. Left for the maintainers: includingownerandisMutedin the output, and droppingorg:read(kept for consistency).