← Back to PRs

#19767: cron: validate webhook delivery targets on create

by advaitpaliwal open 2026-02-18 04:29 View on GitHub →
docs app: macos app: web-ui gateway agents size: L trusted-contributor
## Summary - add coverage to reject invalid webhook `delivery.to` targets on `cron.add` - enforce the webhook URL validation path in create/update tests for cron jobs ## Validation - `pnpm test src/cron/service.get-job.test.ts src/cron/service.jobs.test.ts` ## Notes - backward compatibility for legacy `cron.webhook` config is intentionally not addressed in this PR. <!-- greptile_comment --> <h3>Greptile Summary</h3> This PR introduces per-job webhook delivery (`delivery.mode = "webhook"`) as a first-class delivery mode alongside `announce` and `none`, replacing the global `notify` boolean + `cron.webhook` config pattern. Changes span the full stack: - **New `webhook-url.ts` utility** validates URLs are HTTP(S) only, used consistently across service, gateway, and agent tool layers - **Service layer** (`service/jobs.ts`): `assertDeliverySupport` validates webhook URLs on both create and patch; webhook delivery is exempt from the `isolated`-only restriction, allowing main-session webhook jobs - **Gateway** (`server-cron.ts`): new `resolveCronWebhookTarget` resolves delivery URL from either the new `delivery.mode="webhook"` path or legacy `notify + cron.webhook` fallback with deduplicated deprecation warnings - **Protocol schema** (`schema/cron.ts`): `CronDeliverySchema` split into a `Type.Union` of three variants (noop, announce, webhook) with webhook requiring a non-empty `to` field; `notify` removed from job/add/patch schemas - **Agent tool** (`cron-tool.ts`): fail-fast webhook URL validation before gateway round-trip; delivery inference updated to skip infer for webhook mode - **UI**: delivery form now shows for all job types (not just isolated/agentTurn); announce option hidden when unsupported; `normalizeCronFormState` resets stale announce selection - **Swift/macOS**: `notify` removed; `delivery` type widened from `[String: AnyCodable]?` to `AnyCodable?` to accommodate the union schema - **Backward compatibility**: legacy stored jobs with `notify: true` still work via the `cron.webhook` config fallback, with deprecation logging The changes are well-tested with unit, service, e2e, and protocol conformance tests covering validation, normalization, and the legacy fallback path. <h3>Confidence Score: 4/5</h3> - This PR is safe to merge — validation is thorough and backward compatibility is maintained. - Score of 4 reflects clean, well-tested implementation with only a minor style concern: the agent tool's webhook URL fail-fast is limited to agentTurn payloads, though the gateway still validates at the service level. All core paths (create, update, gateway webhook delivery, legacy fallback) are covered by tests. - `src/agents/tools/cron-tool.ts` — webhook URL validation is gated by payload kind, missing fail-fast for systemEvent + webhook combinations. <sub>Last reviewed commit: 3981950</sub> <!-- greptile_other_comments_section --> **Context used:** - Context from `dashboard` - CLAUDE.md ([source](https://app.greptile.com/review/custom-context?memory=fd949e91-5c3a-4ab5-90a1-cbe184fd6ce8)) - Context from `dashboard` - AGENTS.md ([source](https://app.greptile.com/review/custom-context?memory=0d0c8278-ef8e-4d6c-ab21-f5527e322f13)) <!-- /greptile_comment -->

Most Similar PRs