Repository navigation
feat: scheduled tasks can run on a webhook - #321
Conversation
Port of t3code's webhook automations (pingdotgg/t3code#15085, #15088, and #16232's service shape, plus the redaction, max-age and outcome-header parts of #15487). Pathway Connect's managed tunnel already reaches the environment, so the relay forwarding and offline hold are left out. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 341f335556
ℹ️ 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".
| export const ScheduledTaskSchedule = Schema.Union([ | ||
| ScheduledTaskIntervalSchedule, | ||
| ScheduledTaskFixedTimeSchedule, | ||
| ScheduledTaskWebhookSchedule, |
There was a problem hiding this comment.
When a webhook task returned by scheduledTasks.list is opened on iOS, PathwayAdministrationScheduleDraft retains scheduleType == "webhook", but the editor only offers interval and fixed_time, and payload() serializes every non-interval value as fixed_time. Consequently, editing an unrelated field and saving silently converts the webhook into a 09:00 scheduled task, retires its URL, and may run its webhook-template prompt without a request. Preserve or disable webhook tasks in the iOS editor before exposing this schedule variant.
AGENTS.md reference: AGENTS.md:L74-L74
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 1ba7627. The iOS editor now keeps a webhook schedule and sends it back unchanged; the trigger is shown read-only. Builds already in the App Store are covered by the server: an upsert with creationSource: "mobile" can't change an existing webhook task's trigger.
| if (publicUrl !== null) { | ||
| yield* dependencies.secrets.set(CLOUD_MANAGED_ENDPOINT_URL, stringToBytes(publicUrl)); | ||
| } else { | ||
| yield* dependencies.secrets.remove(CLOUD_MANAGED_ENDPOINT_URL); | ||
| } |
There was a problem hiding this comment.
When Pathway Connect is linked, unlinked, or reconfigured while a scheduled-task subscription is active, this secret changes but nothing publishes to ScheduledTaskService's changesPubSub; subscribeList() only recomputes URLs after a task mutation or run transition. The open settings view can therefore keep showing a local URL after linking, or worse, continue offering a dead public URL after unlinking, including when another device changes the connection. Trigger a scheduled-task list refresh when the managed endpoint URL changes.
AGENTS.md reference: AGENTS.md:L78-L78
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 1ba7627. Applying or clearing the relay config and unlinking now call ScheduledTaskService.refreshWebhookAddresses, which publishes to changesPubSub so open lists re-read the public URL.
| webhook_secret = CASE | ||
| WHEN ${webhook.secretChanged ? 1 : 0} = 1 THEN excluded.webhook_secret | ||
| ELSE scheduled_tasks.webhook_secret |
There was a problem hiding this comment.
With overlapping edits from two clients, a stale signed-task save can commit a signature configuration without a secret. For example, client B reads the old secret and passes the pre-save validation, client A then disables signatures and clears the stored secret, and B finally saves an unrelated edit with secretChanged == false; this branch preserves A's now-null database value while schedule_json from B requires a signature. Every subsequent delivery is rejected until someone re-enters the secret, so validate and update the signature configuration and secret atomically or reject the stale save.
AGENTS.md reference: AGENTS.md:L78-L78
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 1ba7627. The upsert's DO UPDATE now has WHERE secretChanged OR webhook_secret IS NOT NULL with RETURNING, so a stale save that would leave a signature check with no secret fails with an error instead of committing.
| // Only the hand-off is uninterruptible; shutdown still stops the run. | ||
| Effect.interruptible, | ||
| Effect.forkIn(serviceScope), |
There was a problem hiding this comment.
After the route returns 202, a delivery exists only as this service-scoped fiber; its database row has already been recorded as accepted, but there is no durable pending state or startup recovery for fibers that have not dispatched. If the process crashes or is hard-stopped while a delivery waits on the semaphore, that request never reaches runTask, the sender will not retry its successful response, and the delivery remains permanently labeled accepted (shown as “Ran”). Persist and recover the pending handoff, or delay successful acknowledgement until it is durable.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 1ba7627. A delivery is now logged as queued (with its whole rendered prompt) before the 202 and only becomes accepted once its run starts. On startup the service dispatches every row still queued. Because the run is keyed by the delivery id, one that had already started won't start twice.
| export const untracedWebhookRequestsLayer = Layer.succeed(HttpMiddleware.TracerDisabledWhen)( | ||
| (request) => request.url.startsWith(`${ScheduledTaskService.WEBHOOK_ROUTE_PREFIX}/`), | ||
| ); |
There was a problem hiding this comment.
This predicate disables tracing for every /api/hooks/* request, but response logging is disabled only inside the matched POST/PUT/PATCH/GET handler. When HTTP logging is enabled—automatically for the dev server, or explicitly in production—an unsupported method such as OPTIONS/DELETE or a malformed path never enters that handler, so Effect's outer logger records the full path containing the secret token. Apply the logger suppression at the prefix middleware level so rejected webhook-shaped requests cannot leak credentials either.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 1ba7627. A global router middleware (unloggedWebhookRequestsLayer) now disables the response logger for every /api/hooks/* request before routing, so unmatched methods and paths are covered too.
- Deliveries are logged as queued until their run starts, and queued deliveries are dispatched again after a restart. - Linking or unlinking Pathway Connect refreshes webhook URLs in open lists. - A save that keeps a signed task's secret fails if a concurrent save cleared it. - The request logger is disabled for every /api/hooks/* request, matched or not. - iOS sends a webhook trigger back unchanged, and mobile saves cannot change it. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1ba7627dc4
ℹ️ 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".
| payload.endpoint?.providerKind === "cloudflare_tunnel" | ||
| ? normalizeSecureRelayUrl(payload.endpoint.httpBaseUrl) | ||
| : null; |
There was a problem hiding this comment.
When an environment is linked with managed tunnels from iOS, PathwayConnectionOnboardingModel.link forwards endpointRuntime but omits the endpoint returned by the relay. This branch therefore treats the active tunnel as having no public URL and removes CLOUD_MANAGED_ENDPOINT_URL, so locally connected clients and MCP only expose the bare/local webhook address even though Pathway Connect is active. Forward the relay endpoint from the iOS request as the web and startup-link flows now do.
AGENTS.md reference: AGENTS.md:L74-L74
Useful? React with 👍 / 👎.
| .handleRaw("webhookPost", handler) | ||
| .handleRaw("webhookPut", handler) | ||
| .handleRaw("webhookPatch", handler) | ||
| .handleRaw("webhookGet", handler); |
There was a problem hiding this comment.
When a webhook provider, uptime checker, or link preview sends HEAD to this URL, Effect's HttpRouter.asHttpEffect falls back to the registered GET route when no HEAD route exists (.repos/effect-smol/packages/effect/src/unstable/http/HttpRouter.ts, lines 196–198). The shared handler consequently calls triggerWebhook and launches an agent run even though the documented accepted methods exclude HEAD. Add an explicit HEAD rejection so metadata probes cannot execute tasks.
Useful? React with 👍 / 👎.
| // A queued delivery replays from this after a restart, so it is kept whole. | ||
| (input.outcome === "queued" | ||
| ? input.renderedPrompt | ||
| : input.renderedPrompt?.slice(0, WEBHOOK_DELIVERY_LOG_PROMPT_LIMIT)) ?? null |
There was a problem hiding this comment.
Every normally accepted delivery is initially inserted with outcome queued, so this branch stores its entire rendered prompt; markDelivery later changes only the outcome and error and never applies WEBHOOK_DELIVERY_LOG_PROMPT_LIMIT. Consequently the intended 64 KiB cap is bypassed for successful traffic, retaining up to 120,000 characters for each of 50 deliveries per task and returning that oversized value over the delivery-detail RPC. Keep the full prompt while recovery needs it, then truncate it when the delivery reaches a terminal outcome.
AGENTS.md reference: AGENTS.md:L13-L13
Useful? React with 👍 / 👎.
| if draft.originalWebhookSchedule != nil { | ||
| Section { LabeledContent("Trigger", value: "Webhook") } footer: { Text("Change a webhook trigger, its URL or its signature check in Pathway on desktop or web.") } |
There was a problem hiding this comment.
Although the editor now recognizes webhook tasks, PathwayAdministrationSchedulesView still adds a Run now context-menu action for every task and routes it to scheduledTasks.runNow. The server explicitly rejects that operation for webhook schedules, so selecting the action is guaranteed to surface an error; omit or replace it when the task's schedule type is webhook, as the web surfaces already do.
AGENTS.md reference: AGENTS.md:L74-L74
Useful? React with 👍 / 👎.
| WHERE task_id = ${id} | ||
| RETURNING task_id |
There was a problem hiding this comment.
For a webhook delivery, another client can delete and recreate the same task ID after runTask reads and checks the active task but before this update executes. Because markRunning filters only by ID, the RETURNING succeeds against the replacement row, after which the stale task's prompt, project, and thread are dispatched and completion state is written onto the replacement. Include the freshly read created_at in this update's predicate and treat a zero-row update as a skipped delivery.
AGENTS.md reference: AGENTS.md:L78-L78
Useful? React with 👍 / 👎.
| while (delivery.outcome === "queued") { | ||
| yield* Effect.yieldNow; | ||
| delivery = yield* read; |
There was a problem hiding this comment.
Several new webhook tests wait for asynchronous dispatch by repeatedly querying SQLite and yielding until the outcome changes. If dispatch stalls or a regression leaves the row queued, these tests never produce a focused failure and instead spin until the suite-level timeout; use a receipt, worker drain, or test-harness Deferred that completes at the delivery transition.
AGENTS.md reference: AGENTS.md:L113-L113
Useful? React with 👍 / 👎.
1 participant
Scheduled tasks could only run on a timer. Users want outside events (GitHub, Stripe, forms, Zapier) to start an agent run. This ports t3code's webhook automations: pingdotgg/t3code#15085 (server) and #15088 (web), shaped after #16232's service refactor. From #15487 it takes only the credential redaction, the max-age setting, the outcome header, and the new default prompt.
How it works
{type: "webhook"}. It gets a secret URL,/api/hooks/:taskId/:token, that accepts GET, POST, PUT and PATCH. A request starts a run through the existingrunTaskpath, with fire keytaskId:webhook:deliveryId, so the same delivery never runs twice.{{body.a.b}},{{headers.x}},{{query.x}},{{body}}, and{{request}}. The default prompt isHandle this webhook:\n{{body}}.404for an unknown task or a wrong tokenrotateWebhookToken,listWebhookDeliveries,getWebhookDelivery) need operate scope, and read-only sessions get task lists withoutwebhook.schedule_taskandupdate_scheduled_taskaccept the webhook trigger and returnwebhookUrl.docs/user/scheduled-tasks.md,docs/internals/webhook-tasks.md, and a glossary entry.Adapted for Pathway
infra/relay/src/environments/ManagedEndpointProvider.ts). So/api/hooks/*reaches the environment with the raw body intact. Cloudflare's own security settings (WAF, bot protection) aren't in the repo and could still challenge non-browser senders.RelayEnvironmentConfigRequestgains an optionalendpoint, which the server stores ascloud-managed-endpoint-urland clears on unlink. The web link flow and the CLI reconcile send it. Until an environment is re-linked,webhook.urlis null and clients resolvepathagainst the address they connect through.maxDeliveryAgeMinutesis checked when a queued delivery starts. Without the relay, age is always zero when a request arrives. A stale delivery is logged asexpiredinstead of being answered with410.WsRpcGroup(268 RPCs) made the Effect checker reportanyinrunServer's requirements (cli/server.ts). They are served by their own layer, like usage recovery and the Apple/Xcode groups, and left out of the core group.ForwardCompatibleArray, so clients skip trigger types they don't know.Known risks
fixed_time09:00 task and drops its token, and "Run now" shows an error. The follow-up iOS PR adds the editor and sendsendpointat link time. A server guard that refuses a trigger-type change unless the client asks for it explicitly would protect today's builds; I left it out of this PR.Testing
vp test runon the touched files passes 75/75.vp lintis clean.tsgo --noEmitis clean for web, contracts and client-runtime, and the server matches main (main's existingOrchestratorMcpServiceinstanceofwarning).PathwayOrchestrationInstructions.test.ts › distinguishes delegated subagentsfails on main too.🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.