From 832d29ccfa4bd6e4882e078075ef1e7f4a61783a Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Tue, 20 Oct 2026 07:50:40 +0000 Subject: [PATCH 1/7] feat(scheduled-tasks): event filters for webhook deliveries Webhook tasks take optional filters ({ path, op, value? }) that every request must match before a run starts. Filters reuse the placeholder resolver, run after the signature and age checks, and a request that fails one is answered 202, logged as filtered with the failing filter, and never counts as a run. Web and mobile settings forms edit them; the MCP schedule tools accept them. Co-Authored-By: Claude Opus 5.5 --- .../SettingsScheduledTasksRouteScreen.tsx | 99 +++++++++++++ .../settings/scheduledTaskDraft.test.ts | 12 ++ .../features/settings/scheduledTaskDraft.ts | 14 +- .../src/mcp/toolkits/orchestrator/tools.ts | 4 +- .../scheduledTasks/ScheduledTaskService.ts | 17 ++- .../ScheduledTaskService.webhook.test.ts | 77 ++++++++++ .../src/scheduledTasks/webhookFilter.test.ts | 101 +++++++++++++ .../src/scheduledTasks/webhookFilter.ts | 51 +++++++ .../src/scheduledTasks/webhookRoute.test.ts | 1 + .../src/scheduledTasks/webhookTemplate.ts | 22 ++- .../settings/ScheduledTasksSettings.tsx | 133 +++++++++++++++++- .../scheduledTasksSettings.logic.test.ts | 20 +++ .../settings/scheduledTasksSettings.logic.ts | 12 +- docs/user/project-settings.md | 9 ++ .../src/scheduledTaskWebhook.test.ts | 32 ++++- .../src/scheduledTaskWebhook.ts | 79 ++++++++++- packages/contracts/src/scheduledTask.test.ts | 48 +++++++ packages/contracts/src/scheduledTask.ts | 58 ++++++++ 18 files changed, 774 insertions(+), 15 deletions(-) create mode 100644 apps/server/src/scheduledTasks/webhookFilter.test.ts create mode 100644 apps/server/src/scheduledTasks/webhookFilter.ts diff --git a/apps/mobile/src/features/settings/SettingsScheduledTasksRouteScreen.tsx b/apps/mobile/src/features/settings/SettingsScheduledTasksRouteScreen.tsx index 8913502da919..6597efc564ed 100644 --- a/apps/mobile/src/features/settings/SettingsScheduledTasksRouteScreen.tsx +++ b/apps/mobile/src/features/settings/SettingsScheduledTasksRouteScreen.tsx @@ -13,6 +13,7 @@ import type { } from "@t3tools/contracts"; import { MAX_WEBHOOK_DELIVERY_AGE_MINUTES, + MAX_WEBHOOK_FILTERS, resolveEnvironmentMachineKind, } from "@t3tools/contracts"; import type { MenuAction } from "@react-native-menu/menu"; @@ -24,7 +25,11 @@ import { } from "@t3tools/client-runtime/state/runtime"; import { DEFAULT_WEBHOOK_PROMPT, + EMPTY_WEBHOOK_FILTER_DRAFT, parseMaxDeliveryAge, + parseWebhookFilterDrafts, + WEBHOOK_FILTER_OPS, + type WebhookFilterDraft, } from "@t3tools/client-runtime/scheduled-task-webhook"; import { useEffect, useLayoutEffect, useRef, useState, type ReactNode } from "react"; import { useFocusEffect, useNavigation, usePreventRemove } from "@react-navigation/native"; @@ -222,6 +227,85 @@ function SelectRow(props: { ); } +function WebhookFilterRows(props: { + readonly filters: ReadonlyArray; + readonly disabled: boolean; + readonly onChange: (filters: ReadonlyArray) => void; +}) { + const update = (index: number, patch: Partial) => + props.onChange( + props.filters.map((filter, at) => (at === index ? { ...filter, ...patch } : filter)), + ); + return ( + <> + {props.filters.map((filter, index) => ( + // Rows have no identity of their own; removing one re-renders the rest in order. + // oxlint-disable-next-line react/no-array-index-key + + + Filter {index + 1} + props.onChange(props.filters.filter((_, at) => at !== index))} + className="active:opacity-70" + > + + + + update(index, { path })} + /> + option.op === filter.op)?.label ?? ""} + actions={WEBHOOK_FILTER_OPS.map((option) => ({ + id: option.op, + title: option.label, + state: option.op === filter.op ? ("on" as const) : undefined, + }))} + onSelect={(id) => { + const known = WEBHOOK_FILTER_OPS.find((option) => option.op === id); + if (known) update(index, { op: known.op }); + }} + /> + {filter.op === "exists" ? null : ( + update(index, { value })} + /> + )} + + ))} + {props.filters.length < MAX_WEBHOOK_FILTERS ? ( + props.onChange([...props.filters, EMPTY_WEBHOOK_FILTER_DRAFT])} + className="min-h-14 flex-row items-center gap-3 border-t border-border-subtle px-4 py-3 active:opacity-70" + > + Add filter + + + ) : null} + {props.filters.length > 0 ? ( + + Runs only when every filter matches. Other requests are logged as filtered. + + ) : null} + + ); +} + function PickerRow(props: { readonly label: string; readonly value: string; @@ -627,6 +711,14 @@ function TaskForm({ ); return; } + const parsedFilters = parseWebhookFilterDrafts(draft.schedule.filters); + if (draft.schedule.mode === "webhook" && "invalidIndex" in parsedFilters) { + Dialog.alert( + "Invalid filter", + `Filter ${parsedFilters.invalidIndex + 1} needs a path under body, headers or query, and a value its test accepts.`, + ); + return; + } if ( !draft.title.trim() || !draft.prompt.trim() || @@ -964,6 +1056,13 @@ function TaskForm({ setDraft({ ...draft, schedule: { ...draft.schedule, maxDeliveryAgeMinutes } }) } /> + + setDraft({ ...draft, schedule: { ...draft.schedule, filters } }) + } + /> ) : ( <> diff --git a/apps/mobile/src/features/settings/scheduledTaskDraft.test.ts b/apps/mobile/src/features/settings/scheduledTaskDraft.test.ts index 69532b3fd264..5a914444048c 100644 --- a/apps/mobile/src/features/settings/scheduledTaskDraft.test.ts +++ b/apps/mobile/src/features/settings/scheduledTaskDraft.test.ts @@ -65,6 +65,18 @@ describe("scheduleDraftForTask", () => { expect(saved).toEqual({ type: "webhook", signature, maxDeliveryAgeMinutes: null }); expect(saved?.type === "webhook" && saved.signature && "secret" in saved.signature).toBe(false); }); + + it("round-trips webhook filters and refuses an invalid one", () => { + const filters = [ + { path: "headers.x-github-event", op: "equals", value: "pull_request" }, + { path: "body.label.name", op: "matches", value: "^agent:" }, + ] as const; + const draft = scheduleDraftForTask({ schedule: { type: "webhook", signature: null, filters } }); + expect(scheduleFromDraft(draft)).toMatchObject({ filters }); + expect( + scheduleFromDraft({ ...draft, filters: [{ path: "", op: "exists", value: "" }] }), + ).toBeNull(); + }); }); describe("hasScheduledTaskDraftChanges", () => { diff --git a/apps/mobile/src/features/settings/scheduledTaskDraft.ts b/apps/mobile/src/features/settings/scheduledTaskDraft.ts index eff6674aacc9..1e51a5fa3853 100644 --- a/apps/mobile/src/features/settings/scheduledTaskDraft.ts +++ b/apps/mobile/src/features/settings/scheduledTaskDraft.ts @@ -9,7 +9,12 @@ import type { } from "@t3tools/contracts"; import { DEFAULT_SERVER_SETTINGS } from "@t3tools/contracts"; -import { parseMaxDeliveryAge } from "@t3tools/client-runtime/scheduled-task-webhook"; +import { + parseMaxDeliveryAge, + parseWebhookFilterDrafts, + webhookFilterDrafts, + type WebhookFilterDraft, +} from "@t3tools/client-runtime/scheduled-task-webhook"; import { resolveProjectSettings, type LegacyProjectSettingsFields, @@ -50,6 +55,7 @@ export type ScheduleDraft = { readonly intervalMinutes: string; readonly signature: ScheduledTaskWebhookSignature | null; readonly maxDeliveryAgeMinutes: string; + readonly filters: ReadonlyArray; }; export const DEFAULT_SCHEDULE: ScheduleDraft = { @@ -59,6 +65,7 @@ export const DEFAULT_SCHEDULE: ScheduleDraft = { intervalMinutes: "15", signature: null, maxDeliveryAgeMinutes: "", + filters: [], }; export function scheduleDraftForTask(task: Pick): ScheduleDraft { @@ -86,6 +93,7 @@ export function scheduleDraftForTask(task: Pick): Sch task.schedule.maxDeliveryAgeMinutes == null ? "" : String(task.schedule.maxDeliveryAgeMinutes), + filters: webhookFilterDrafts(task.schedule.filters), }; } } @@ -94,6 +102,8 @@ export function scheduleFromDraft(draft: ScheduleDraft): ScheduledTaskUpsertSche if (draft.mode === "webhook") { const maxDeliveryAgeMinutes = parseMaxDeliveryAge(draft.maxDeliveryAgeMinutes); if (maxDeliveryAgeMinutes === undefined) return null; + const parsedFilters = parseWebhookFilterDrafts(draft.filters); + if (!("filters" in parsedFilters)) return null; // No secret is sent, so the server keeps the stored one. return { type: "webhook", @@ -106,6 +116,7 @@ export function scheduleFromDraft(draft: ScheduleDraft): ScheduledTaskUpsertSche prefix: draft.signature.prefix, }, maxDeliveryAgeMinutes, + ...(parsedFilters.filters.length > 0 ? { filters: parsedFilters.filters } : {}), }; } if (draft.mode === "interval") { @@ -160,6 +171,7 @@ function draftSignature(draft: ScheduledTaskDraft): string { [...draft.schedule.weekdays].sort((a, b) => a - b), draft.schedule.intervalMinutes, draft.schedule.maxDeliveryAgeMinutes, + draft.schedule.filters, draft.workspace, draft.baseRef, draft.checkoutPath, diff --git a/apps/server/src/mcp/toolkits/orchestrator/tools.ts b/apps/server/src/mcp/toolkits/orchestrator/tools.ts index 893dd7da0fe2..fa5b29165fc3 100644 --- a/apps/server/src/mcp/toolkits/orchestrator/tools.ts +++ b/apps/server/src/mcp/toolkits/orchestrator/tools.ts @@ -116,7 +116,7 @@ const WorkflowStopTool = Tool.make("t3_workflow_stop", { export const ScheduleTaskTool = Tool.make("schedule_task", { description: - "Create persistent work in the app scheduler that runs even when no turn is active. Pass schedule as a STRUCTURED OBJECT, never JSON text. Timers: {type:'interval', everyMs:3600000} is hourly; {type:'fixed_time', timeOfDay:'09:00', weekdays:[1,2,3,4,5]} is weekday mornings; report the returned nextRunAt. Webhooks: {type:'webhook'} runs once per request to a generated URL. The run sees the request ONLY through prompt placeholders: {{body.path}} (e.g. {{body.action}}, {{body.release.tag_name}}), {{headers.name}}, {{query.name}}, {{body}}, or {{request}} (method, headers with credentials redacted, and body). Each value arrives framed as untrusted request data, and $skill, @file or /command mentions inside it do not act; without a signature, anyone with the URL can trigger a run. For a sender that signs requests, first call request_secret so the user enters the secret privately (never ask for it in chat or invent one), then set signature with the returned secretRef, e.g. GitHub: {type:'webhook', signature:{header:'x-hub-signature-256', encoding:'hex', prefix:'sha256=', secretRef}}. The result's webhookUrl is the public URL to give the user; if it is absent, this environment has no T3 Connect managed tunnel, so tell the user to enable T3 Connect remote access rather than sharing a path. Omit projectId for this thread's project. In this thread's project, runs post into THIS thread by default (bindToCurrentThread=true), which suits an orchestrator that sees every trigger, delegates work, and can dedupe against what is in flight; use false only when the user wants a fresh top-level thread per run. Elsewhere each run launches a fresh thread. Provider, model, and runtime settings inherit from this thread, or from the project default when there is no calling thread.", + "Create persistent work in the app scheduler that runs even when no turn is active. Pass schedule as a STRUCTURED OBJECT, never JSON text. Timers: {type:'interval', everyMs:3600000} is hourly; {type:'fixed_time', timeOfDay:'09:00', weekdays:[1,2,3,4,5]} is weekday mornings; report the returned nextRunAt. Webhooks: {type:'webhook'} runs once per request to a generated URL. The run sees the request ONLY through prompt placeholders: {{body.path}} (e.g. {{body.action}}, {{body.release.tag_name}}), {{headers.name}}, {{query.name}}, {{body}}, or {{request}} (method, headers with credentials redacted, and body). Each value arrives framed as untrusted request data, and $skill, @file or /command mentions inside it do not act; without a signature, anyone with the URL can trigger a run. For a sender that signs requests, first call request_secret so the user enters the secret privately (never ask for it in chat or invent one), then set signature with the returned secretRef, e.g. GitHub: {type:'webhook', signature:{header:'x-hub-signature-256', encoding:'hex', prefix:'sha256=', secretRef}}. To run only for some requests, add filters, all of which must match, e.g. filters:[{path:'headers.x-github-event', op:'equals', value:'pull_request'}, {path:'body.action', op:'in', value:['opened','reopened']}]; ops are equals, notEquals, in, startsWith, matches (regex) and exists. The result's webhookUrl is the public URL to give the user; if it is absent, this environment has no T3 Connect managed tunnel, so tell the user to enable T3 Connect remote access rather than sharing a path. Omit projectId for this thread's project. In this thread's project, runs post into THIS thread by default (bindToCurrentThread=true), which suits an orchestrator that sees every trigger, delegates work, and can dedupe against what is in flight; use false only when the user wants a fresh top-level thread per run. Elsewhere each run launches a fresh thread. Provider, model, and runtime settings inherit from this thread, or from the project default when there is no calling thread.", parameters: OrchestratorMcpScheduleTaskInput, success: OrchestratorMcpScheduleTaskResult, failure: OrchestratorMcpFailure, @@ -143,7 +143,7 @@ const ListScheduledTasksTool = Tool.make("list_scheduled_tasks", { const UpdateScheduledTaskTool = Tool.make("update_scheduled_task", { description: - "Update an existing scheduled task by scheduledTaskId (from list_scheduled_tasks). Only the provided fields change; omit a field to leave it as-is. Use enabled=false to pause a task without deleting it. Set bindToCurrentThread to move the task between posting into this thread and launching a fresh thread per run.", + "Update an existing scheduled task by scheduledTaskId (from list_scheduled_tasks). Only the provided fields change; omit a field to leave it as-is. Use enabled=false to pause a task without deleting it. Set bindToCurrentThread to move the task between posting into this thread and launching a fresh thread per run. A schedule replaces the whole trigger, so to change a webhook's filters pass its full schedule with the new filters.", parameters: OrchestratorMcpUpdateScheduledTaskInput, success: OrchestratorMcpScheduleTaskResult, failure: OrchestratorMcpFailure, diff --git a/apps/server/src/scheduledTasks/ScheduledTaskService.ts b/apps/server/src/scheduledTasks/ScheduledTaskService.ts index d3d2ef19c6ac..f66177144883 100644 --- a/apps/server/src/scheduledTasks/ScheduledTaskService.ts +++ b/apps/server/src/scheduledTasks/ScheduledTaskService.ts @@ -53,6 +53,7 @@ import { renderWebhookPrompt, type WebhookRequest, } from "./webhookTemplate.ts"; +import { describeFailedWebhookFilter, findFailedWebhookFilter } from "./webhookFilter.ts"; import { constantTimeEquals, verifyWebhookSignature } from "./webhookVerification.ts"; export const WEBHOOK_ROUTE_PREFIX = "/api/hooks"; @@ -114,6 +115,7 @@ export type WebhookDeliveryOutcome = | "disabled" | "rejected_signature" | "expired" + | "filtered" | "not_found" | "error"; @@ -121,7 +123,7 @@ export type WebhookTriggerResult = | { readonly _tag: "accepted"; readonly deliveryId: ScheduledTaskWebhookDeliveryId; - readonly outcome: "accepted" | "duplicate" | "prompt_too_long"; + readonly outcome: "accepted" | "duplicate" | "prompt_too_long" | "filtered"; } | { readonly _tag: "not_found" } | { readonly _tag: "rejected_signature" } @@ -995,6 +997,7 @@ export const layer = Layer.effect( prefix: input.schedule.signature.prefix, }, maxDeliveryAgeMinutes: input.schedule.maxDeliveryAgeMinutes ?? null, + ...(input.schedule.filters?.length ? { filters: input.schedule.filters } : {}), } : input.schedule; const webhook = @@ -1560,6 +1563,18 @@ export const layer = Layer.effect( return { _tag: "expired" as const }; } + // Filters: a request the task does not care about is accepted, so + // the sender does not retry it, and logged without starting a run. + const failedFilter = findFailedWebhookFilter(schedule.filters ?? [], request); + if (failedFilter !== null) { + yield* log("filtered", { + signatureVerified: signature !== null, + error: describeFailedWebhookFilter(failedFilter), + }); + yield* observe("filtered"); + return { _tag: "accepted" as const, deliveryId, outcome: "filtered" as const }; + } + const frameId = yield* crypto.randomBytes(6).pipe( Effect.map((bytes) => Buffer.from(bytes).toString("hex")), Effect.mapError((cause) => taskError("Could not generate frame id.", { cause })), diff --git a/apps/server/src/scheduledTasks/ScheduledTaskService.webhook.test.ts b/apps/server/src/scheduledTasks/ScheduledTaskService.webhook.test.ts index ae8594a03f0a..9d1bdcd6d1fd 100644 --- a/apps/server/src/scheduledTasks/ScheduledTaskService.webhook.test.ts +++ b/apps/server/src/scheduledTasks/ScheduledTaskService.webhook.test.ts @@ -953,3 +953,80 @@ it.effect("counts each handled request by what happened to it", () => }), ), ); + +it.effect("checks filters after the signature and max age, and runs only matching deliveries", () => + withService(({ service, launches }) => + Effect.gen(function* () { + const { task } = yield* service.upsert( + yield* webhookTaskInput({ + schedule: { + type: "webhook", + signature: { + header: "x-hub-signature-256", + encoding: "hex", + prefix: "sha256=", + secret: "s3cret", + }, + maxDeliveryAgeMinutes: 30, + filters: [ + { path: "headers.x-github-event", op: "equals", value: "pull_request" }, + { path: "body.action", op: "in", value: ["opened", "reopened"] }, + ], + }, + }), + ); + const delivery = ( + event: string, + action: string, + options: { readonly signed?: boolean; readonly receivedAt?: string } = {}, + ) => { + const body = new TextEncoder().encode( + JSON.stringify({ action, pull_request: { url: "https://github.com/org/repo/pull/45" } }), + ); + const signature = `sha256=${NodeCrypto.createHmac("sha256", "s3cret").update(body).digest("hex")}`; + return requestFor(task, { + headers: { + "content-type": "application/json", + "x-github-event": event, + ...(options.signed === false ? {} : { "x-hub-signature-256": signature }), + }, + body, + bodyText: new TextDecoder().decode(body), + ...(options.receivedAt === undefined + ? {} + : { relayDeliveryId: "old", receivedAt: options.receivedAt }), + }); + }; + const now = yield* DateTime.now; + const old = DateTime.formatIso(DateTime.subtract(now, { minutes: 31 })); + + // A request failing a filter is still refused first for its signature or age. + const unsigned = yield* service.triggerWebhook(delivery("push", "opened", { signed: false })); + assert.equal(unsigned._tag, "rejected_signature"); + const tooOld = yield* service.triggerWebhook(delivery("push", "opened", { receivedAt: old })); + assert.equal(tooOld._tag, "expired"); + + const wrongEvent = yield* service.triggerWebhook(delivery("push", "opened")); + const wrongAction = yield* service.triggerWebhook(delivery("pull_request", "closed")); + assert.deepInclude(wrongEvent, { _tag: "accepted", outcome: "filtered" }); + assert.deepInclude(wrongAction, { _tag: "accepted", outcome: "filtered" }); + assert.equal(yield* Queue.size(launches), 0); + const [listed] = (yield* service.list()).tasks; + assert.equal(listed?.runCount, 0); + assert.equal(listed?.lastRunStatus, "never"); + + const matched = yield* service.triggerWebhook(delivery("pull_request", "opened")); + assert.deepInclude(matched, { _tag: "accepted", outcome: "accepted" }); + assert.include((yield* Queue.take(launches)).initialMessage?.text ?? "", "pull/45"); + + const deliveries = (yield* service.listWebhookDeliveries({ id: task.id })).deliveries; + const filtered = deliveries + .filter((entry) => entry.outcome === "filtered") + .map((entry) => [entry.signatureVerified, entry.error]); + assert.sameDeepMembers(filtered, [ + [true, "Filter 1 (headers.x-github-event) did not match."], + [true, "Filter 2 (body.action) did not match."], + ]); + }), + ), +); diff --git a/apps/server/src/scheduledTasks/webhookFilter.test.ts b/apps/server/src/scheduledTasks/webhookFilter.test.ts new file mode 100644 index 000000000000..666253e798d6 --- /dev/null +++ b/apps/server/src/scheduledTasks/webhookFilter.test.ts @@ -0,0 +1,101 @@ +import { assert, describe, it } from "@effect/vitest"; +import type { ScheduledTaskWebhookFilter } from "@t3tools/contracts"; + +import { findFailedWebhookFilter, matchesWebhookFilter } from "./webhookFilter.ts"; +import type { WebhookRequest } from "./webhookTemplate.ts"; + +const request: WebhookRequest = { + method: "POST", + path: "/api/hooks/task-1", + query: "source=github", + headers: { "content-type": "application/json", "x-github-event": "pull_request" }, + bodyText: JSON.stringify({ + action: "labeled", + number: 45, + draft: false, + closed_at: null, + label: { name: "agent:review" }, + labels: ["bug"], + }), +}; + +const matches = (filter: ScheduledTaskWebhookFilter) => + findFailedWebhookFilter([filter], request) === null; + +describe("matchesWebhookFilter", () => { + it("compares scalars as strings", () => { + assert.isTrue(matches({ path: "body.action", op: "equals", value: "labeled" })); + assert.isTrue(matches({ path: "body.number", op: "equals", value: "45" })); + assert.isTrue(matches({ path: "body.draft", op: "equals", value: "false" })); + assert.isFalse(matches({ path: "body.action", op: "equals", value: "Labeled" })); + assert.isTrue(matches({ path: "body.action", op: "notEquals", value: "closed" })); + assert.isFalse(matches({ path: "body.action", op: "notEquals", value: "labeled" })); + assert.isTrue(matches({ path: "body.action", op: "in", value: ["opened", "labeled"] })); + assert.isFalse(matches({ path: "body.action", op: "in", value: [] })); + assert.isTrue(matches({ path: "body.label.name", op: "startsWith", value: "agent:" })); + assert.isFalse(matches({ path: "body.label.name", op: "startsWith", value: "review" })); + }); + + it("reads headers case-insensitively and query values by name", () => { + assert.isTrue(matches({ path: "headers.X-GitHub-Event", op: "equals", value: "pull_request" })); + assert.isTrue(matches({ path: "query.source", op: "equals", value: "github" })); + }); + + it("leaves regular expressions unanchored", () => { + assert.isTrue(matches({ path: "body.label.name", op: "matches", value: "review" })); + assert.isTrue( + matches({ path: "body.label.name", op: "matches", value: "^agent:(review|fix)$" }), + ); + assert.isFalse(matches({ path: "body.label.name", op: "matches", value: "^review" })); + }); + + it("lets objects and arrays satisfy only exists", () => { + for (const path of ["body.label", "body.labels"]) { + assert.isTrue(matches({ path, op: "exists" })); + assert.isFalse(matches({ path, op: "equals", value: "[object Object]" })); + assert.isFalse(matches({ path, op: "notEquals", value: "anything" })); + assert.isFalse(matches({ path, op: "in", value: ["bug"] })); + assert.isFalse(matches({ path, op: "startsWith", value: "" })); + assert.isFalse(matches({ path, op: "matches", value: "" })); + } + }); + + it("fails every op on a missing or null path, including notEquals", () => { + for (const path of [ + "body.missing", + "body.label.name.deeper", + "headers.x-none", + "body.closed_at", + ]) { + assert.isFalse(matches({ path, op: "exists" })); + assert.isFalse(matches({ path, op: "notEquals", value: "x" })); + assert.isFalse(matchesWebhookFilter({ path, op: "matches", value: "" }, undefined)); + } + }); +}); + +describe("findFailedWebhookFilter", () => { + it("passes with no filters and reports the first failing filter", () => { + assert.isNull(findFailedWebhookFilter([], request)); + assert.deepEqual( + findFailedWebhookFilter( + [ + { path: "headers.x-github-event", op: "equals", value: "pull_request" }, + { path: "body.action", op: "equals", value: "opened" }, + { path: "body.missing", op: "exists" }, + ], + request, + ), + { index: 1, path: "body.action" }, + ); + }); + + it("does not need a JSON body for header filters", () => { + assert.isNull( + findFailedWebhookFilter([{ path: "headers.x-github-event", op: "exists" }], { + ...request, + bodyText: "not json", + }), + ); + }); +}); diff --git a/apps/server/src/scheduledTasks/webhookFilter.ts b/apps/server/src/scheduledTasks/webhookFilter.ts new file mode 100644 index 000000000000..09047f6d1ead --- /dev/null +++ b/apps/server/src/scheduledTasks/webhookFilter.ts @@ -0,0 +1,51 @@ +import type { ScheduledTaskWebhookFilter } from "@t3tools/contracts"; + +import { webhookExpressionResolver, type WebhookRequest } from "./webhookTemplate.ts"; + +export interface FailedWebhookFilter { + readonly index: number; + readonly path: string; +} + +function scalarText(value: unknown): string | undefined { + if (typeof value === "string") return value; + if (typeof value === "number" || typeof value === "boolean") return String(value); + return undefined; +} + +/** Placeholders render null as missing, so filters treat it the same way. */ +export function matchesWebhookFilter(filter: ScheduledTaskWebhookFilter, value: unknown): boolean { + if (filter.op === "exists") return value !== undefined && value !== null; + const text = scalarText(value); + if (text === undefined) return false; + switch (filter.op) { + case "equals": + return text === filter.value; + case "notEquals": + return text !== filter.value; + case "in": + return filter.value.includes(text); + case "startsWith": + return text.startsWith(filter.value); + case "matches": + return new RegExp(filter.value).test(text); + } +} + +/** The first filter the request fails, or null when every filter matches. */ +export function findFailedWebhookFilter( + filters: ReadonlyArray, + request: WebhookRequest, +): FailedWebhookFilter | null { + if (filters.length === 0) return null; + const resolve = webhookExpressionResolver(request); + for (const [index, filter] of filters.entries()) { + if (!matchesWebhookFilter(filter, resolve(filter.path))) return { index, path: filter.path }; + } + return null; +} + +/** Filters are numbered from 1, in the order the settings form lists them. */ +export function describeFailedWebhookFilter(failed: FailedWebhookFilter): string { + return `Filter ${failed.index + 1} (${failed.path}) did not match.`; +} diff --git a/apps/server/src/scheduledTasks/webhookRoute.test.ts b/apps/server/src/scheduledTasks/webhookRoute.test.ts index 31261660c623..218281053704 100644 --- a/apps/server/src/scheduledTasks/webhookRoute.test.ts +++ b/apps/server/src/scheduledTasks/webhookRoute.test.ts @@ -222,6 +222,7 @@ describe("webhook route", () => { [{ _tag: "accepted", deliveryId, outcome: "accepted" }, 202, "accepted"], [{ _tag: "accepted", deliveryId, outcome: "duplicate" }, 202, "duplicate"], [{ _tag: "accepted", deliveryId, outcome: "prompt_too_long" }, 202, "prompt_too_long"], + [{ _tag: "accepted", deliveryId, outcome: "filtered" }, 202, "filtered"], [{ _tag: "not_found" }, 404, "not_found"], [{ _tag: "rejected_signature" }, 401, "rejected_signature"], [{ _tag: "disabled" }, 409, "disabled"], diff --git a/apps/server/src/scheduledTasks/webhookTemplate.ts b/apps/server/src/scheduledTasks/webhookTemplate.ts index b129704f0abe..5f3ef760ad8b 100644 --- a/apps/server/src/scheduledTasks/webhookTemplate.ts +++ b/apps/server/src/scheduledTasks/webhookTemplate.ts @@ -112,16 +112,17 @@ function formatWebhookRequest(request: WebhookRequest): string { ].join("\n"); } -export function renderWebhookPrompt( - template: string, +/** + * Resolves a placeholder expression (`body.action`, `headers.x-github-event`) + * against the request. Webhook filters share it, so both read the same value. + */ +export function webhookExpressionResolver( request: WebhookRequest, - frameId: string, -): RenderedWebhookPrompt { - const missing: string[] = []; +): (expression: string) => unknown { let body: { parsed: unknown } | undefined; const parsedBody = () => (body ??= { parsed: parseBody(request) }).parsed; - const resolve = (expression: string): unknown => { + return (expression) => { const [root, ...segments] = expression.split("."); switch (root) { case "request": @@ -143,6 +144,15 @@ export function renderWebhookPrompt( return undefined; } }; +} + +export function renderWebhookPrompt( + template: string, + request: WebhookRequest, + frameId: string, +): RenderedWebhookPrompt { + const missing: string[] = []; + const resolve = webhookExpressionResolver(request); const tag = `webhook-data-${frameId}`; let framed = false; diff --git a/apps/web/src/components/settings/ScheduledTasksSettings.tsx b/apps/web/src/components/settings/ScheduledTasksSettings.tsx index 98988c3d278c..d638fa614059 100644 --- a/apps/web/src/components/settings/ScheduledTasksSettings.tsx +++ b/apps/web/src/components/settings/ScheduledTasksSettings.tsx @@ -26,9 +26,16 @@ import type { ScheduledTaskWebhookDeliverySummary, ThreadId, } from "@t3tools/contracts"; -import { DEFAULT_WEBHOOK_PROMPT } from "@t3tools/client-runtime/scheduled-task-webhook"; +import { + DEFAULT_WEBHOOK_PROMPT, + EMPTY_WEBHOOK_FILTER_DRAFT, + parseWebhookFilterDrafts, + WEBHOOK_FILTER_OPS, + type WebhookFilterDraft, +} from "@t3tools/client-runtime/scheduled-task-webhook"; import { MAX_WEBHOOK_DELIVERY_AGE_MINUTES, + MAX_WEBHOOK_FILTERS, MIN_SCHEDULED_TASK_INTERVAL_MS, ProviderInstanceId, resolveEnvironmentMachineKind, @@ -144,8 +151,103 @@ const EMPTY_DRAFT: DraftState = { ...WEBHOOK_SIGNATURE_DEFAULTS, signatureSecret: "", maxDeliveryAgeMinutes: "", + filters: [], }; +function WebhookFiltersField({ + filters, + onChange, +}: { + filters: ReadonlyArray; + onChange: (filters: ReadonlyArray) => void; +}) { + const update = (index: number, patch: Partial) => + onChange(filters.map((filter, at) => (at === index ? { ...filter, ...patch } : filter))); + return ( +
+
+
+ +

+ Run only when every filter matches. Other requests are logged as filtered. +

+
+ +
+ {filters.map((filter, index) => ( + // Rows have no identity of their own; removing one re-renders the rest in order. + // oxlint-disable-next-line react/no-array-index-key +
+ update(index, { path: event.target.value })} + /> + + {filter.op === "exists" ? ( +
+ ) : ( + update(index, { value: event.target.value })} + /> + )} + +
+ ))} +
+ ); +} + function Field({ label, hint, @@ -237,10 +339,12 @@ const DELIVERY_OUTCOME_LABELS: RecordSignature verified ) : null}
- {selected.error ?

{selected.error}

: null} + {selected.error ? ( +

+ {selected.error} +

+ ) : null} {selected.renderedPrompt ?? "No run was started for this request."} @@ -798,6 +912,9 @@ function WebhookDeliveriesDialog({ + {delivery.outcome === "filtered" && delivery.error ? ( + {delivery.error} + ) : null} {delivery.missingFields.length > 0 ? ( {delivery.missingFields.length} empty placeholder @@ -1069,6 +1186,14 @@ function ScheduledTaskEditorForm({ reportFailure("Scheduled task is incomplete", "Add a title, prompt, project, and model."); return; } + const parsedFilters = parseWebhookFilterDrafts(draft.filters); + if (draft.scheduleMode === "webhook" && "invalidIndex" in parsedFilters) { + reportFailure( + "Invalid filter", + `Filter ${parsedFilters.invalidIndex + 1} needs a path under body, headers or query, and a value its test accepts.`, + ); + return; + } const schedule = scheduleFromDraft(draft); if (schedule === null) { reportFailure( @@ -1415,6 +1540,10 @@ function ScheduledTaskEditorForm({ } /> + setDraft((current) => ({ ...current, filters }))} + />
diff --git a/apps/web/src/components/settings/scheduledTasksSettings.logic.test.ts b/apps/web/src/components/settings/scheduledTasksSettings.logic.test.ts index 9c67ec46e8e0..a0ebe4e6bdef 100644 --- a/apps/web/src/components/settings/scheduledTasksSettings.logic.test.ts +++ b/apps/web/src/components/settings/scheduledTasksSettings.logic.test.ts @@ -218,6 +218,26 @@ describe("webhook scheduled tasks", () => { expect(scheduleFromDraft({ ...draft, maxDeliveryAgeMinutes: invalid })).toBeNull(); } }); + + it("round-trips filters and refuses to build a schedule with an invalid one", () => { + const filters = [ + { path: "headers.x-github-event", op: "equals", value: "pull_request" }, + { path: "body.action", op: "in", value: ["opened", "reopened"] }, + ] as const; + const draft = taskToDraft({ + ...webhookTask, + schedule: { type: "webhook", signature, filters }, + }); + expect(draft.filters.map((filter) => filter.op)).toEqual(["equals", "in"]); + expect(scheduleFromDraft(draft)).toMatchObject({ filters }); + expect(scheduleFromDraft({ ...draft, filters: [] })).not.toHaveProperty("filters"); + expect( + scheduleFromDraft({ + ...draft, + filters: [{ path: "body.action", op: "matches", value: "(unclosed" }], + }), + ).toBeNull(); + }); }); describe("scheduled task model defaults", () => { diff --git a/apps/web/src/components/settings/scheduledTasksSettings.logic.ts b/apps/web/src/components/settings/scheduledTasksSettings.logic.ts index 0767f99190c9..be0954ee34c4 100644 --- a/apps/web/src/components/settings/scheduledTasksSettings.logic.ts +++ b/apps/web/src/components/settings/scheduledTasksSettings.logic.ts @@ -9,7 +9,12 @@ import { type ProviderInteractionMode, type ServerSettings, } from "@t3tools/contracts"; -import { parseMaxDeliveryAge } from "@t3tools/client-runtime/scheduled-task-webhook"; +import { + parseMaxDeliveryAge, + parseWebhookFilterDrafts, + webhookFilterDrafts, + type WebhookFilterDraft, +} from "@t3tools/client-runtime/scheduled-task-webhook"; import { resolveProjectSettings, @@ -77,6 +82,7 @@ export interface DraftState { /** Write-only: empty keeps the secret already stored on the server. */ readonly signatureSecret: string; readonly maxDeliveryAgeMinutes: string; + readonly filters: ReadonlyArray; } export const WEBHOOK_SIGNATURE_DEFAULTS = { @@ -89,6 +95,8 @@ export function scheduleFromDraft(draft: DraftState): ScheduledTaskUpsertSchedul if (draft.scheduleMode === "webhook") { const maxDeliveryAgeMinutes = parseMaxDeliveryAge(draft.maxDeliveryAgeMinutes); if (maxDeliveryAgeMinutes === undefined) return null; + const parsedFilters = parseWebhookFilterDrafts(draft.filters); + if (!("filters" in parsedFilters)) return null; const secret = draft.signatureSecret.trim(); return { type: "webhook", @@ -101,6 +109,7 @@ export function scheduleFromDraft(draft: DraftState): ScheduledTaskUpsertSchedul } : null, maxDeliveryAgeMinutes, + ...(parsedFilters.filters.length > 0 ? { filters: parsedFilters.filters } : {}), }; } if (draft.scheduleMode === "interval") { @@ -161,6 +170,7 @@ export function taskToDraft(task: ScheduledTask): DraftState { schedule.type === "webhook" && schedule.maxDeliveryAgeMinutes != null ? String(schedule.maxDeliveryAgeMinutes) : "", + filters: webhookFilterDrafts(schedule.type === "webhook" ? schedule.filters : undefined), }; } diff --git a/docs/user/project-settings.md b/docs/user/project-settings.md index bb89de6717e8..09bb44f5e0a1 100644 --- a/docs/user/project-settings.md +++ b/docs/user/project-settings.md @@ -109,6 +109,15 @@ same secret in the repository's webhook settings with content type up on desktop or web; mobile keeps an existing signature check but can't turn one on. +To run only for some requests, add **Filters**. Each one tests a value named +like a placeholder, without the braces, such as `headers.x-github-event` equals +`pull_request` or `body.action` is one of `opened, reopened`. A task runs only +when every filter matches; other requests are answered normally, logged as +**Filtered** with the first filter that failed, and start nothing. Values +compare as text, and a value that is an object or a list only passes +**exists**. **Matches regex** is unanchored, so add `^` and `$` to match the +whole value. + On desktop and web, pick **Deliveries** from a task's menu to see recent requests and the prompt each one produced. diff --git a/packages/client-runtime/src/scheduledTaskWebhook.test.ts b/packages/client-runtime/src/scheduledTaskWebhook.test.ts index c2de7d240bc7..814c6c543aea 100644 --- a/packages/client-runtime/src/scheduledTaskWebhook.test.ts +++ b/packages/client-runtime/src/scheduledTaskWebhook.test.ts @@ -1,6 +1,10 @@ import { describe, expect, it } from "vite-plus/test"; -import { parseMaxDeliveryAge } from "./scheduledTaskWebhook.ts"; +import { + parseMaxDeliveryAge, + parseWebhookFilterDrafts, + webhookFilterDrafts, +} from "./scheduledTaskWebhook.ts"; describe("parseMaxDeliveryAge", () => { it("treats blank as no limit and rejects values the server would not accept", () => { @@ -11,3 +15,29 @@ describe("parseMaxDeliveryAge", () => { } }); }); + +describe("webhook filter drafts", () => { + it("round-trips saved filters through the form rows", () => { + const filters = [ + { path: "headers.x-github-event", op: "equals", value: "pull_request" }, + { path: "body.action", op: "in", value: ["opened", "reopened"] }, + { path: "query.source", op: "exists" }, + ] as const; + const drafts = webhookFilterDrafts(filters); + expect(drafts[1]).toEqual({ path: "body.action", op: "in", value: "opened, reopened" }); + expect(parseWebhookFilterDrafts(drafts)).toEqual({ filters }); + }); + + it("names the first row the server would refuse", () => { + expect( + parseWebhookFilterDrafts([ + { path: "body.action", op: "equals", value: "opened" }, + { path: "body.label.name", op: "matches", value: "(unclosed" }, + { path: "", op: "exists", value: "" }, + ]), + ).toEqual({ invalidIndex: 1 }); + expect(parseWebhookFilterDrafts([{ path: " ", op: "exists", value: "" }])).toEqual({ + invalidIndex: 0, + }); + }); +}); diff --git a/packages/client-runtime/src/scheduledTaskWebhook.ts b/packages/client-runtime/src/scheduledTaskWebhook.ts index 028591709659..9ef36c4e7a5a 100644 --- a/packages/client-runtime/src/scheduledTaskWebhook.ts +++ b/packages/client-runtime/src/scheduledTaskWebhook.ts @@ -1,4 +1,11 @@ -import { MAX_WEBHOOK_DELIVERY_AGE_MINUTES } from "@t3tools/contracts"; +import { + MAX_WEBHOOK_DELIVERY_AGE_MINUTES, + MAX_WEBHOOK_FILTERS, + ScheduledTaskWebhookFilter, + type ScheduledTaskWebhookFilterOp, +} from "@t3tools/contracts"; +import * as Option from "effect/Option"; +import * as Schema from "effect/Schema"; export const DEFAULT_WEBHOOK_PROMPT = "Handle this webhook:\n{{body}}"; @@ -10,3 +17,73 @@ export function parseMaxDeliveryAge(value: string): number | null | undefined { ? minutes : undefined; } + +/** One editable filter row. `in` takes a comma-separated list; `exists` ignores the value. */ +export interface WebhookFilterDraft { + readonly path: string; + readonly op: ScheduledTaskWebhookFilterOp; + readonly value: string; +} + +export const WEBHOOK_FILTER_OPS: ReadonlyArray<{ + readonly op: ScheduledTaskWebhookFilterOp; + readonly label: string; +}> = [ + { op: "equals", label: "equals" }, + { op: "notEquals", label: "does not equal" }, + { op: "in", label: "is one of" }, + { op: "startsWith", label: "starts with" }, + { op: "matches", label: "matches regex" }, + { op: "exists", label: "exists" }, +]; + +export const EMPTY_WEBHOOK_FILTER_DRAFT: WebhookFilterDraft = { + path: "", + op: "equals", + value: "", +}; + +export function webhookFilterDrafts( + filters: ReadonlyArray | undefined, +): ReadonlyArray { + return (filters ?? []).map((filter) => ({ + path: filter.path, + op: filter.op, + value: + filter.op === "exists" ? "" : filter.op === "in" ? filter.value.join(", ") : filter.value, + })); +} + +const decodeFilter = Schema.decodeUnknownOption(ScheduledTaskWebhookFilter); + +/** + * Checks each row with the server's own schema, so a draft that parses here + * also saves. `invalidIndex` names the first row that would not. + */ +export function parseWebhookFilterDrafts( + drafts: ReadonlyArray, +): + | { readonly filters: ReadonlyArray } + | { readonly invalidIndex: number } { + const filters: ScheduledTaskWebhookFilter[] = []; + for (const [index, draft] of drafts.entries()) { + const path = draft.path.trim(); + const filter = decodeFilter( + draft.op === "exists" + ? { path, op: draft.op } + : draft.op === "in" + ? { + path, + op: draft.op, + value: draft.value + .split(",") + .map((item) => item.trim()) + .filter((item) => item !== ""), + } + : { path, op: draft.op, value: draft.value }, + ); + if (Option.isNone(filter)) return { invalidIndex: index }; + filters.push(filter.value); + } + return filters.length > MAX_WEBHOOK_FILTERS ? { invalidIndex: MAX_WEBHOOK_FILTERS } : { filters }; +} diff --git a/packages/contracts/src/scheduledTask.test.ts b/packages/contracts/src/scheduledTask.test.ts index c3373d08abf4..8f98dc89ba48 100644 --- a/packages/contracts/src/scheduledTask.test.ts +++ b/packages/contracts/src/scheduledTask.test.ts @@ -50,3 +50,51 @@ describe("ScheduledTaskUpsertSchedule", () => { ).toThrow(); }); }); + +describe("webhook filters", () => { + const accepts = (filters: unknown) => { + try { + decodeUpsertSchedule({ type: "webhook", filters }); + return true; + } catch { + return false; + } + }; + + it("keeps valid filters", () => { + const filters = [ + { path: "headers.x-github-event", op: "equals", value: "pull_request" }, + { path: "body.action", op: "in", value: ["opened", "labeled"] }, + { path: "body.label.name", op: "matches", value: "^agent:(review|fix)$" }, + { path: "query.source", op: "exists" }, + ]; + expect(decodeUpsertSchedule({ type: "webhook", filters })).toEqual({ + type: "webhook", + filters, + }); + expect(decodeSchedule({ type: "webhook", signature: null, filters })).toMatchObject({ + filters, + }); + }); + + it("rejects invalid or oversized patterns at save time", () => { + expect(accepts([{ path: "body.action", op: "matches", value: "(unclosed" }])).toBe(false); + expect(accepts([{ path: "body.action", op: "matches", value: "a".repeat(257) }])).toBe(false); + }); + + it("rejects empty or unknown paths and values that do not fit the op", () => { + expect(accepts([{ path: "", op: "exists" }])).toBe(false); + expect(accepts([{ path: " ", op: "exists" }])).toBe(false); + expect(accepts([{ path: "request", op: "exists" }])).toBe(false); + expect(accepts([{ path: "header.x-github-event", op: "exists" }])).toBe(false); + expect(accepts([{ path: "body.action", op: "in", value: "opened" }])).toBe(false); + expect(accepts([{ path: "body.action", op: "equals" }])).toBe(false); + expect(accepts([{ path: "body.action", op: "contains", value: "x" }])).toBe(false); + }); + + it("allows at most 16 filters", () => { + const sixteen = Array.from({ length: 16 }, () => ({ path: "body.action", op: "exists" })); + expect(accepts(sixteen)).toBe(true); + expect(accepts([...sixteen, { path: "body.action", op: "exists" }])).toBe(false); + }); +}); diff --git a/packages/contracts/src/scheduledTask.ts b/packages/contracts/src/scheduledTask.ts index 70298d7c42da..5a52c1aa3e61 100644 --- a/packages/contracts/src/scheduledTask.ts +++ b/packages/contracts/src/scheduledTask.ts @@ -83,12 +83,68 @@ const WebhookMaxDeliveryAgeMinutes = Schema.Int.check( "Skip requests the relay held longer than this many minutes while the environment was offline. Null runs every request.", }); +export const MAX_WEBHOOK_FILTERS = 16; +export const MAX_WEBHOOK_FILTER_PATTERN_LENGTH = 256; + +const WebhookFilterPath = TrimmedNonEmptyString.check( + Schema.isPattern(/^(body|headers|query)(\.|$)/), +).annotate({ + description: + "Request value to test, in placeholder syntax without braces: headers.x-github-event, body.action, query.source.", +}); + +const WebhookFilterPattern = Schema.String.check( + Schema.isMaxLength(MAX_WEBHOOK_FILTER_PATTERN_LENGTH), + Schema.makeFilter( + (pattern: string) => { + try { + return RegExp(pattern) instanceof RegExp; + } catch { + return "an invalid regular expression"; + } + }, + { expected: "a valid regular expression" }, + ), +).annotate({ description: "Unanchored JavaScript regular expression." }); + +export const ScheduledTaskWebhookFilter = Schema.Union([ + Schema.Struct({ + path: WebhookFilterPath, + op: Schema.Literals(["equals", "notEquals", "startsWith"]), + value: Schema.String, + }), + Schema.Struct({ + path: WebhookFilterPath, + op: Schema.Literal("in"), + value: Schema.Array(Schema.String), + }), + Schema.Struct({ + path: WebhookFilterPath, + op: Schema.Literal("matches"), + value: WebhookFilterPattern, + }), + Schema.Struct({ path: WebhookFilterPath, op: Schema.Literal("exists") }), +]).annotate({ + description: + "Scalars compare as strings; objects and arrays only satisfy 'exists'. Every op but 'exists' fails when the path is missing.", +}); +export type ScheduledTaskWebhookFilter = typeof ScheduledTaskWebhookFilter.Type; +export type ScheduledTaskWebhookFilterOp = ScheduledTaskWebhookFilter["op"]; + +const ScheduledTaskWebhookFilters = Schema.Array(ScheduledTaskWebhookFilter) + .check(Schema.isMaxLength(MAX_WEBHOOK_FILTERS)) + .annotate({ + description: + "Run only when every filter matches the request; other requests are logged as filtered. Up to 16.", + }); + const ScheduledTaskWebhookSchedule = Schema.Struct({ type: Schema.Literal("webhook").annotate({ description: "Run when the task's webhook URL receives a request.", }), signature: Schema.NullOr(ScheduledTaskWebhookSignature), maxDeliveryAgeMinutes: Schema.optional(Schema.NullOr(WebhookMaxDeliveryAgeMinutes)), + filters: Schema.optional(ScheduledTaskWebhookFilters), }).annotate({ description: "Run on each request to the task's webhook URL. The prompt may use {{body.path}}, {{headers.name}}, {{query.name}}, {{body}} and {{request}} placeholders.", @@ -115,6 +171,7 @@ const ScheduledTaskUpsertWebhookSchedule = Schema.Struct({ description: "Signature check; omit or null to accept requests by URL token only.", }), maxDeliveryAgeMinutes: Schema.optional(Schema.NullOr(WebhookMaxDeliveryAgeMinutes)), + filters: Schema.optional(ScheduledTaskWebhookFilters), }).annotate({ description: "Run on each request to the task's webhook URL. The prompt may use {{body.path}}, {{headers.name}}, {{query.name}}, {{body}} and {{request}} placeholders.", @@ -255,6 +312,7 @@ export const ScheduledTaskWebhookDeliveryOutcome = Schema.Literals([ "disabled", "rate_limited", "expired", + "filtered", ]); export type ScheduledTaskWebhookDeliveryOutcome = typeof ScheduledTaskWebhookDeliveryOutcome.Type; From 33f064b13db3870fe8614322a9b37119f9d65ce2 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Tue, 20 Oct 2026 08:03:25 +0000 Subject: [PATCH 2/7] fix(scheduled-tasks): address webhook filter review - Drop the matches op: an attacker-controlled body against a backtracking regex could block the event loop. - A webhook save without filters keeps the stored filters; [] clears them. The web and mobile forms always send filters. - Filters run after the signature and age checks but before the rate slot, so ignored requests never take a run's slot, and filtered rows keep their own retention so they can't evict real deliveries. - Release the relay claim for a filtered delivery so a redelivery is evaluated again. - Decode delivery outcomes leniently and label unknown ones in the UI. - in values must be non-empty, comma-free and trimmed, so the forms' comma-separated editing round-trips them; in [] is rejected. - Forms report the shared filter error, including the 16-filter limit. Co-Authored-By: Claude Opus 5.5 --- .../SettingsScheduledTasksRouteScreen.tsx | 10 +- .../settings/scheduledTaskDraft.test.ts | 5 +- .../features/settings/scheduledTaskDraft.ts | 3 +- .../src/mcp/toolkits/orchestrator/tools.ts | 4 +- .../scheduledTasks/ScheduledTaskService.ts | 95 ++++++++++++------- .../ScheduledTaskService.webhook.test.ts | 86 +++++++++++++++++ .../src/scheduledTasks/webhookFilter.test.ts | 12 +-- .../src/scheduledTasks/webhookFilter.ts | 2 - .../settings/ScheduledTasksSettings.tsx | 35 ++++--- .../scheduledTasksSettings.logic.test.ts | 11 ++- .../settings/scheduledTasksSettings.logic.ts | 3 +- docs/user/project-settings.md | 4 +- .../src/scheduledTaskWebhook.test.ts | 15 ++- .../src/scheduledTaskWebhook.ts | 29 ++++-- packages/contracts/src/scheduledTask.test.ts | 38 ++++++-- packages/contracts/src/scheduledTask.ts | 33 +++---- 16 files changed, 258 insertions(+), 127 deletions(-) diff --git a/apps/mobile/src/features/settings/SettingsScheduledTasksRouteScreen.tsx b/apps/mobile/src/features/settings/SettingsScheduledTasksRouteScreen.tsx index 6597efc564ed..d35f88bf45d8 100644 --- a/apps/mobile/src/features/settings/SettingsScheduledTasksRouteScreen.tsx +++ b/apps/mobile/src/features/settings/SettingsScheduledTasksRouteScreen.tsx @@ -299,7 +299,8 @@ function WebhookFilterRows(props: { ) : null} {props.filters.length > 0 ? ( - Runs only when every filter matches. Other requests are logged as filtered. + Runs only when every filter matches. Other requests are logged as filtered. Separate “is + one of” values with commas; a value can't contain a comma. ) : null} @@ -712,11 +713,8 @@ function TaskForm({ return; } const parsedFilters = parseWebhookFilterDrafts(draft.schedule.filters); - if (draft.schedule.mode === "webhook" && "invalidIndex" in parsedFilters) { - Dialog.alert( - "Invalid filter", - `Filter ${parsedFilters.invalidIndex + 1} needs a path under body, headers or query, and a value its test accepts.`, - ); + if (draft.schedule.mode === "webhook" && "error" in parsedFilters) { + Dialog.alert("Invalid filter", parsedFilters.error); return; } if ( diff --git a/apps/mobile/src/features/settings/scheduledTaskDraft.test.ts b/apps/mobile/src/features/settings/scheduledTaskDraft.test.ts index 5a914444048c..ea9f3b365497 100644 --- a/apps/mobile/src/features/settings/scheduledTaskDraft.test.ts +++ b/apps/mobile/src/features/settings/scheduledTaskDraft.test.ts @@ -37,6 +37,7 @@ describe("scheduleDraftForTask", () => { type: "webhook", signature: null, maxDeliveryAgeMinutes: null, + filters: [], }); }); @@ -62,14 +63,14 @@ describe("scheduleDraftForTask", () => { const saved = scheduleFromDraft( scheduleDraftForTask({ schedule: { type: "webhook", signature } }), ); - expect(saved).toEqual({ type: "webhook", signature, maxDeliveryAgeMinutes: null }); + expect(saved).toEqual({ type: "webhook", signature, maxDeliveryAgeMinutes: null, filters: [] }); expect(saved?.type === "webhook" && saved.signature && "secret" in saved.signature).toBe(false); }); it("round-trips webhook filters and refuses an invalid one", () => { const filters = [ { path: "headers.x-github-event", op: "equals", value: "pull_request" }, - { path: "body.label.name", op: "matches", value: "^agent:" }, + { path: "body.label.name", op: "in", value: ["needs review", "agent:fix"] }, ] as const; const draft = scheduleDraftForTask({ schedule: { type: "webhook", signature: null, filters } }); expect(scheduleFromDraft(draft)).toMatchObject({ filters }); diff --git a/apps/mobile/src/features/settings/scheduledTaskDraft.ts b/apps/mobile/src/features/settings/scheduledTaskDraft.ts index 1e51a5fa3853..ac7ecd85293d 100644 --- a/apps/mobile/src/features/settings/scheduledTaskDraft.ts +++ b/apps/mobile/src/features/settings/scheduledTaskDraft.ts @@ -116,7 +116,8 @@ export function scheduleFromDraft(draft: ScheduleDraft): ScheduledTaskUpsertSche prefix: draft.signature.prefix, }, maxDeliveryAgeMinutes, - ...(parsedFilters.filters.length > 0 ? { filters: parsedFilters.filters } : {}), + // Always sent: the server keeps stored filters when the key is missing. + filters: parsedFilters.filters, }; } if (draft.mode === "interval") { diff --git a/apps/server/src/mcp/toolkits/orchestrator/tools.ts b/apps/server/src/mcp/toolkits/orchestrator/tools.ts index fa5b29165fc3..7e6536a014af 100644 --- a/apps/server/src/mcp/toolkits/orchestrator/tools.ts +++ b/apps/server/src/mcp/toolkits/orchestrator/tools.ts @@ -116,7 +116,7 @@ const WorkflowStopTool = Tool.make("t3_workflow_stop", { export const ScheduleTaskTool = Tool.make("schedule_task", { description: - "Create persistent work in the app scheduler that runs even when no turn is active. Pass schedule as a STRUCTURED OBJECT, never JSON text. Timers: {type:'interval', everyMs:3600000} is hourly; {type:'fixed_time', timeOfDay:'09:00', weekdays:[1,2,3,4,5]} is weekday mornings; report the returned nextRunAt. Webhooks: {type:'webhook'} runs once per request to a generated URL. The run sees the request ONLY through prompt placeholders: {{body.path}} (e.g. {{body.action}}, {{body.release.tag_name}}), {{headers.name}}, {{query.name}}, {{body}}, or {{request}} (method, headers with credentials redacted, and body). Each value arrives framed as untrusted request data, and $skill, @file or /command mentions inside it do not act; without a signature, anyone with the URL can trigger a run. For a sender that signs requests, first call request_secret so the user enters the secret privately (never ask for it in chat or invent one), then set signature with the returned secretRef, e.g. GitHub: {type:'webhook', signature:{header:'x-hub-signature-256', encoding:'hex', prefix:'sha256=', secretRef}}. To run only for some requests, add filters, all of which must match, e.g. filters:[{path:'headers.x-github-event', op:'equals', value:'pull_request'}, {path:'body.action', op:'in', value:['opened','reopened']}]; ops are equals, notEquals, in, startsWith, matches (regex) and exists. The result's webhookUrl is the public URL to give the user; if it is absent, this environment has no T3 Connect managed tunnel, so tell the user to enable T3 Connect remote access rather than sharing a path. Omit projectId for this thread's project. In this thread's project, runs post into THIS thread by default (bindToCurrentThread=true), which suits an orchestrator that sees every trigger, delegates work, and can dedupe against what is in flight; use false only when the user wants a fresh top-level thread per run. Elsewhere each run launches a fresh thread. Provider, model, and runtime settings inherit from this thread, or from the project default when there is no calling thread.", + "Create persistent work in the app scheduler that runs even when no turn is active. Pass schedule as a STRUCTURED OBJECT, never JSON text. Timers: {type:'interval', everyMs:3600000} is hourly; {type:'fixed_time', timeOfDay:'09:00', weekdays:[1,2,3,4,5]} is weekday mornings; report the returned nextRunAt. Webhooks: {type:'webhook'} runs once per request to a generated URL. The run sees the request ONLY through prompt placeholders: {{body.path}} (e.g. {{body.action}}, {{body.release.tag_name}}), {{headers.name}}, {{query.name}}, {{body}}, or {{request}} (method, headers with credentials redacted, and body). Each value arrives framed as untrusted request data, and $skill, @file or /command mentions inside it do not act; without a signature, anyone with the URL can trigger a run. For a sender that signs requests, first call request_secret so the user enters the secret privately (never ask for it in chat or invent one), then set signature with the returned secretRef, e.g. GitHub: {type:'webhook', signature:{header:'x-hub-signature-256', encoding:'hex', prefix:'sha256=', secretRef}}. To run only for some requests, add filters, all of which must match, e.g. filters:[{path:'headers.x-github-event', op:'equals', value:'pull_request'}, {path:'body.action', op:'in', value:['opened','reopened']}]; ops are equals, notEquals, in, startsWith and exists. The result's webhookUrl is the public URL to give the user; if it is absent, this environment has no T3 Connect managed tunnel, so tell the user to enable T3 Connect remote access rather than sharing a path. Omit projectId for this thread's project. In this thread's project, runs post into THIS thread by default (bindToCurrentThread=true), which suits an orchestrator that sees every trigger, delegates work, and can dedupe against what is in flight; use false only when the user wants a fresh top-level thread per run. Elsewhere each run launches a fresh thread. Provider, model, and runtime settings inherit from this thread, or from the project default when there is no calling thread.", parameters: OrchestratorMcpScheduleTaskInput, success: OrchestratorMcpScheduleTaskResult, failure: OrchestratorMcpFailure, @@ -143,7 +143,7 @@ const ListScheduledTasksTool = Tool.make("list_scheduled_tasks", { const UpdateScheduledTaskTool = Tool.make("update_scheduled_task", { description: - "Update an existing scheduled task by scheduledTaskId (from list_scheduled_tasks). Only the provided fields change; omit a field to leave it as-is. Use enabled=false to pause a task without deleting it. Set bindToCurrentThread to move the task between posting into this thread and launching a fresh thread per run. A schedule replaces the whole trigger, so to change a webhook's filters pass its full schedule with the new filters.", + "Update an existing scheduled task by scheduledTaskId (from list_scheduled_tasks). Only the provided fields change; omit a field to leave it as-is. Use enabled=false to pause a task without deleting it. Set bindToCurrentThread to move the task between posting into this thread and launching a fresh thread per run. A webhook schedule without filters keeps the stored filters; pass filters to replace them, or filters:[] to clear them.", parameters: OrchestratorMcpUpdateScheduledTaskInput, success: OrchestratorMcpScheduleTaskResult, failure: OrchestratorMcpFailure, diff --git a/apps/server/src/scheduledTasks/ScheduledTaskService.ts b/apps/server/src/scheduledTasks/ScheduledTaskService.ts index f66177144883..8d2d52e4847c 100644 --- a/apps/server/src/scheduledTasks/ScheduledTaskService.ts +++ b/apps/server/src/scheduledTasks/ScheduledTaskService.ts @@ -15,6 +15,7 @@ import { type ScheduledTaskListWebhookDeliveriesResult, type ScheduledTaskRotateWebhookTokenInput, type ScheduledTaskWebhookDeliveryOutcome, + type ScheduledTaskWebhookFilter, type ScheduledTaskListResult, type ScheduledTaskMutationResult, type ScheduledTaskRunNowInput, @@ -58,6 +59,8 @@ import { constantTimeEquals, verifyWebhookSignature } from "./webhookVerificatio export const WEBHOOK_ROUTE_PREFIX = "/api/hooks"; const WEBHOOK_DELIVERY_RETENTION = 50; +/** Kept apart from the rest, so a stream of filtered requests can't evict real deliveries. */ +const WEBHOOK_FILTERED_DELIVERY_RETENTION = 20; const WEBHOOK_DELIVERY_LOG_BODY_LIMIT = 64 * 1024; const WEBHOOK_DELIVERY_LOG_PROMPT_LIMIT = 64 * 1024; const WEBHOOK_MAX_QUEUED_PER_TASK = 20; @@ -134,6 +137,9 @@ export type WebhookTriggerResult = } | { readonly _tag: "expired" }; +const webhookFilters = (filters: ReadonlyArray | undefined) => + filters?.length ? { filters } : {}; + const decodeTask = Schema.decodeUnknownEffect(ScheduledTask); const decodeTaskId = Schema.decodeUnknownOption(ScheduledTaskId); const decodeScheduleJson = Schema.decodeUnknownEffect( @@ -997,7 +1003,14 @@ export const layer = Layer.effect( prefix: input.schedule.signature.prefix, }, maxDeliveryAgeMinutes: input.schedule.maxDeliveryAgeMinutes ?? null, - ...(input.schedule.filters?.length ? { filters: input.schedule.filters } : {}), + ...webhookFilters( + // Omitted keeps the stored filters, so a client that predates + // them can re-save a task without clearing them. + input.schedule.filters ?? + (existingTask?.schedule.type === "webhook" + ? existingTask.schedule.filters + : undefined), + ), } : input.schedule; const webhook = @@ -1301,15 +1314,17 @@ export const layer = Layer.effect( rendered_prompt = excluded.rendered_prompt, error = excluded.error `; + const filtered = input.outcome === "filtered" ? 1 : 0; yield* sql` DELETE FROM scheduled_task_webhook_deliveries WHERE task_id = ${input.taskId} + AND (outcome = 'filtered') = ${filtered} AND delivery_id NOT IN ( SELECT delivery_id FROM scheduled_task_webhook_deliveries - WHERE task_id = ${input.taskId} + WHERE task_id = ${input.taskId} AND (outcome = 'filtered') = ${filtered} -- rowid breaks timestamp ties in arrival order; delivery ids are random. ORDER BY received_at DESC, rowid DESC - LIMIT ${WEBHOOK_DELIVERY_RETENTION} + LIMIT ${filtered ? WEBHOOK_FILTERED_DELIVERY_RETENTION : WEBHOOK_DELIVERY_RETENTION} ) `; }), @@ -1520,6 +1535,42 @@ export const layer = Layer.effect( ...(details.error === undefined ? {} : { error: details.error }), }); + const signature = schedule.signature; + const signatureVerified = + signature === null || + (row.webhook_secret !== null && + verifyWebhookSignature({ + signature, + secret: row.webhook_secret, + headers: request.headers, + body: request.body, + })); + const maxAgeMinutes = schedule.maxDeliveryAgeMinutes ?? null; + const tooOld = + maxAgeMinutes !== null && + DateTime.toEpochMillis(now) - DateTime.toEpochMillis(receivedAt) > + maxAgeMinutes * 60_000; + + // Filters: checked after the signature and age, and before the rate + // limit, so requests the task ignores never take a run's slot. The + // request is accepted so the sender doesn't retry it, and its relay + // claim is released because nothing ran. + if (task.enabled && signatureVerified && !tooOld) { + const failedFilter = findFailedWebhookFilter(schedule.filters ?? [], request); + if (failedFilter !== null) { + yield* log("filtered", { + signatureVerified: signature !== null, + error: describeFailedWebhookFilter(failedFilter), + }); + yield* observe("filtered"); + return yield* releaseClaim({ + _tag: "accepted" as const, + deliveryId, + outcome: "filtered" as const, + }); + } + } + // Only the first rejected request in a window is logged, so a flood // cannot write rows or push the real deliveries out of the log. const slot = yield* takeRateSlot(task.id, DateTime.toEpochMillis(now)); @@ -1536,45 +1587,17 @@ export const layer = Layer.effect( yield* observe("disabled"); return { _tag: "disabled" as const }; } - const signature = schedule.signature; - if (signature !== null) { - const verified = - row.webhook_secret !== null && - verifyWebhookSignature({ - signature, - secret: row.webhook_secret, - headers: request.headers, - body: request.body, - }); - if (!verified) { - yield* log("rejected_signature"); - yield* observe("rejected_signature"); - return { _tag: "rejected_signature" as const }; - } + if (!signatureVerified) { + yield* log("rejected_signature"); + yield* observe("rejected_signature"); + return { _tag: "rejected_signature" as const }; } - const maxAgeMinutes = schedule.maxDeliveryAgeMinutes ?? null; - if ( - maxAgeMinutes !== null && - DateTime.toEpochMillis(now) - DateTime.toEpochMillis(receivedAt) > - maxAgeMinutes * 60_000 - ) { + if (tooOld) { yield* log("expired", { signatureVerified: signature !== null }); yield* observe("expired"); return { _tag: "expired" as const }; } - // Filters: a request the task does not care about is accepted, so - // the sender does not retry it, and logged without starting a run. - const failedFilter = findFailedWebhookFilter(schedule.filters ?? [], request); - if (failedFilter !== null) { - yield* log("filtered", { - signatureVerified: signature !== null, - error: describeFailedWebhookFilter(failedFilter), - }); - yield* observe("filtered"); - return { _tag: "accepted" as const, deliveryId, outcome: "filtered" as const }; - } - const frameId = yield* crypto.randomBytes(6).pipe( Effect.map((bytes) => Buffer.from(bytes).toString("hex")), Effect.mapError((cause) => taskError("Could not generate frame id.", { cause })), diff --git a/apps/server/src/scheduledTasks/ScheduledTaskService.webhook.test.ts b/apps/server/src/scheduledTasks/ScheduledTaskService.webhook.test.ts index 9d1bdcd6d1fd..325dd8d0b0ae 100644 --- a/apps/server/src/scheduledTasks/ScheduledTaskService.webhook.test.ts +++ b/apps/server/src/scheduledTasks/ScheduledTaskService.webhook.test.ts @@ -1030,3 +1030,89 @@ it.effect("checks filters after the signature and max age, and runs only matchin }), ), ); + +const pullRequestFilterInput = () => + webhookTaskInput({ + schedule: { + type: "webhook", + filters: [{ path: "headers.x-github-event", op: "equals", value: "pull_request" }], + }, + }); +const githubEvent = (event: string) => ({ + headers: { "content-type": "application/json", "x-github-event": event }, +}); + +it.effect("filtered deliveries take no rate slot and cannot evict real deliveries", () => + withService(({ service, launches }) => + Effect.gen(function* () { + const { task } = yield* service.upsert(yield* pullRequestFilterInput()); + const ignored = yield* Effect.forEach(Array.from({ length: 70 }), () => + service.triggerWebhook(requestFor(task, githubEvent("push"))), + ); + assert.isTrue( + ignored.every((result) => result._tag === "accepted" && result.outcome === "filtered"), + ); + const matched = yield* service.triggerWebhook(requestFor(task, githubEvent("pull_request"))); + assert.deepInclude(matched, { _tag: "accepted", outcome: "accepted" }); + yield* Queue.take(launches); + yield* Effect.forEach(Array.from({ length: 5 }), () => + service.triggerWebhook(requestFor(task, githubEvent("push"))), + ); + + const outcomes = (yield* service.listWebhookDeliveries({ id: task.id })).deliveries.map( + (delivery) => delivery.outcome, + ); + assert.equal(outcomes.filter((outcome) => outcome === "filtered").length, 20); + assert.equal(outcomes.filter((outcome) => outcome === "accepted").length, 1); + }), + ), +); + +it.effect("a filtered relay delivery releases its claim, so a redelivery is evaluated again", () => + withService(({ service, launches }) => + Effect.gen(function* () { + const { task } = yield* service.upsert(yield* pullRequestFilterInput()); + const held = { relayDeliveryId: "relay-filtered" }; + const first = yield* service.triggerWebhook( + requestFor(task, { ...held, ...githubEvent("push") }), + ); + const again = yield* service.triggerWebhook( + requestFor(task, { ...held, ...githubEvent("push") }), + ); + assert.deepInclude(first, { _tag: "accepted", outcome: "filtered" }); + assert.deepInclude(again, { _tag: "accepted", outcome: "filtered" }); + const ran = yield* service.triggerWebhook( + requestFor(task, { ...held, ...githubEvent("pull_request") }), + ); + assert.deepInclude(ran, { _tag: "accepted", outcome: "accepted" }); + yield* Queue.take(launches); + const replayed = yield* service.triggerWebhook( + requestFor(task, { ...held, ...githubEvent("pull_request") }), + ); + assert.deepInclude(replayed, { _tag: "accepted", outcome: "duplicate" }); + }), + ), +); + +it.effect("a save without filters keeps the stored ones and an empty list clears them", () => + withService(({ service }) => + Effect.gen(function* () { + const { task } = yield* service.upsert(yield* pullRequestFilterInput()); + const filters = task.schedule.type === "webhook" ? task.schedule.filters : undefined; + assert.lengthOf(filters ?? [], 1); + + const kept = yield* service.upsert( + yield* webhookTaskInput({ title: "Renamed", schedule: { type: "webhook" } }), + ); + assert.deepEqual( + kept.task.schedule.type === "webhook" && kept.task.schedule.filters, + filters, + ); + + const cleared = yield* service.upsert( + yield* webhookTaskInput({ schedule: { type: "webhook", filters: [] } }), + ); + assert.notProperty(cleared.task.schedule, "filters"); + }), + ), +); diff --git a/apps/server/src/scheduledTasks/webhookFilter.test.ts b/apps/server/src/scheduledTasks/webhookFilter.test.ts index 666253e798d6..b8d9d2070832 100644 --- a/apps/server/src/scheduledTasks/webhookFilter.test.ts +++ b/apps/server/src/scheduledTasks/webhookFilter.test.ts @@ -31,7 +31,6 @@ describe("matchesWebhookFilter", () => { assert.isTrue(matches({ path: "body.action", op: "notEquals", value: "closed" })); assert.isFalse(matches({ path: "body.action", op: "notEquals", value: "labeled" })); assert.isTrue(matches({ path: "body.action", op: "in", value: ["opened", "labeled"] })); - assert.isFalse(matches({ path: "body.action", op: "in", value: [] })); assert.isTrue(matches({ path: "body.label.name", op: "startsWith", value: "agent:" })); assert.isFalse(matches({ path: "body.label.name", op: "startsWith", value: "review" })); }); @@ -41,14 +40,6 @@ describe("matchesWebhookFilter", () => { assert.isTrue(matches({ path: "query.source", op: "equals", value: "github" })); }); - it("leaves regular expressions unanchored", () => { - assert.isTrue(matches({ path: "body.label.name", op: "matches", value: "review" })); - assert.isTrue( - matches({ path: "body.label.name", op: "matches", value: "^agent:(review|fix)$" }), - ); - assert.isFalse(matches({ path: "body.label.name", op: "matches", value: "^review" })); - }); - it("lets objects and arrays satisfy only exists", () => { for (const path of ["body.label", "body.labels"]) { assert.isTrue(matches({ path, op: "exists" })); @@ -56,7 +47,6 @@ describe("matchesWebhookFilter", () => { assert.isFalse(matches({ path, op: "notEquals", value: "anything" })); assert.isFalse(matches({ path, op: "in", value: ["bug"] })); assert.isFalse(matches({ path, op: "startsWith", value: "" })); - assert.isFalse(matches({ path, op: "matches", value: "" })); } }); @@ -69,7 +59,7 @@ describe("matchesWebhookFilter", () => { ]) { assert.isFalse(matches({ path, op: "exists" })); assert.isFalse(matches({ path, op: "notEquals", value: "x" })); - assert.isFalse(matchesWebhookFilter({ path, op: "matches", value: "" }, undefined)); + assert.isFalse(matchesWebhookFilter({ path, op: "startsWith", value: "" }, undefined)); } }); }); diff --git a/apps/server/src/scheduledTasks/webhookFilter.ts b/apps/server/src/scheduledTasks/webhookFilter.ts index 09047f6d1ead..7f46a15cc4fa 100644 --- a/apps/server/src/scheduledTasks/webhookFilter.ts +++ b/apps/server/src/scheduledTasks/webhookFilter.ts @@ -27,8 +27,6 @@ export function matchesWebhookFilter(filter: ScheduledTaskWebhookFilter, value: return filter.value.includes(text); case "startsWith": return text.startsWith(filter.value); - case "matches": - return new RegExp(filter.value).test(text); } } diff --git a/apps/web/src/components/settings/ScheduledTasksSettings.tsx b/apps/web/src/components/settings/ScheduledTasksSettings.tsx index d638fa614059..821f03097337 100644 --- a/apps/web/src/components/settings/ScheduledTasksSettings.tsx +++ b/apps/web/src/components/settings/ScheduledTasksSettings.tsx @@ -169,7 +169,8 @@ function WebhookFiltersField({

- Run only when every filter matches. Other requests are logged as filtered. + Run only when every filter matches. Other requests are logged as filtered. Separate “is + one of” values with commas; a value can't contain a comma.

{filters.map((filter, index) => ( + // Below md the row stacks: path and remove on one line, test and value on the next. // Rows have no identity of their own; removing one re-renders the rest in order. // oxlint-disable-next-line react/no-array-index-key -
+
update(index, { path: event.target.value })} /> - - {filter.op === "exists" ? ( -
- ) : ( - update(index, { value: event.target.value })} - /> - )} +
+ + {filter.op === "exists" ? ( +
+ ) : ( + update(index, { value: event.target.value })} + /> + )} +
))}
From f14571aa5cd1077b3e1505a2c4855ea5c6b2c7a3 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Tue, 20 Oct 2026 18:27:04 +0000 Subject: [PATCH 5/7] fix(web): webhook filter rows always stack and the task form never overflows Fieldsets default to min-width: min-content, so the Schedule segmented control widened the form past a 390px dialog and pushed the filter controls out of view; the fieldset now has min-w-0. Filter rows always stack (path and remove, then test and value) because the dialog stays 574px wide on large windows, and the test select fits "does not equal". Co-Authored-By: Claude Opus 5.5 --- .../settings/ScheduledTasksSettings.tsx | 51 ++++++++++--------- 1 file changed, 26 insertions(+), 25 deletions(-) diff --git a/apps/web/src/components/settings/ScheduledTasksSettings.tsx b/apps/web/src/components/settings/ScheduledTasksSettings.tsx index 3f3f88e1ac2b..5d8ea87b0fc9 100644 --- a/apps/web/src/components/settings/ScheduledTasksSettings.tsx +++ b/apps/web/src/components/settings/ScheduledTasksSettings.tsx @@ -186,27 +186,29 @@ function WebhookFiltersField({
{filters.map((filter, index) => ( - // Below md the row stacks: path and remove on one line, test and value on the next. + // Always stacked: the dialog stays narrow on wide windows, so a single line + // would cut off typical paths and values. // Rows have no identity of their own; removing one re-renders the rest in order. // oxlint-disable-next-line react/no-array-index-key -
- update(index, { path: event.target.value })} - /> - -
+
+
+ update(index, { path: event.target.value })} + /> + +
+
- {filter.op === "exists" ? ( -
- ) : ( + {filter.op === "exists" ? null : ( update(index, { value: event.target.value })} @@ -1286,7 +1286,8 @@ function ScheduledTaskEditorForm({ -
+ {/* Fieldsets default to min-width: min-content, which lets one wide control widen the form past the dialog. */} +
{!connected ? (

Reconnect this environment before saving.

) : null} From bd6d3b49e3b620878cf8f7b5100d070bf34844e2 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Tue, 20 Oct 2026 18:45:26 +0000 Subject: [PATCH 6/7] fix(web,mobile): webhook filter rows keep their identity when one is removed Rows were keyed by index, so removing the first of two filters handed the focused remove button to the next row. Draft rows now carry a client-only id, keep it through edits and reopen, and are removed by id. Layout classes move from the inputs and select trigger to wrapper elements, with an identical rendered layout. Co-Authored-By: Claude Opus 5.5 --- .../SettingsScheduledTasksRouteScreen.tsx | 11 ++-- .../settings/scheduledTaskDraft.test.ts | 2 +- .../features/settings/scheduledTaskDraft.ts | 2 +- .../settings/ScheduledTasksSettings.tsx | 60 +++++++++---------- .../scheduledTasksSettings.logic.test.ts | 5 +- .../src/scheduledTaskWebhook.test.ts | 25 ++++++-- .../src/scheduledTaskWebhook.ts | 22 +++++-- 7 files changed, 77 insertions(+), 50 deletions(-) diff --git a/apps/mobile/src/features/settings/SettingsScheduledTasksRouteScreen.tsx b/apps/mobile/src/features/settings/SettingsScheduledTasksRouteScreen.tsx index d35f88bf45d8..aadc124ba4a5 100644 --- a/apps/mobile/src/features/settings/SettingsScheduledTasksRouteScreen.tsx +++ b/apps/mobile/src/features/settings/SettingsScheduledTasksRouteScreen.tsx @@ -25,9 +25,10 @@ import { } from "@t3tools/client-runtime/state/runtime"; import { DEFAULT_WEBHOOK_PROMPT, - EMPTY_WEBHOOK_FILTER_DRAFT, + emptyWebhookFilterDraft, parseMaxDeliveryAge, parseWebhookFilterDrafts, + removeWebhookFilterDraft, WEBHOOK_FILTER_OPS, type WebhookFilterDraft, } from "@t3tools/client-runtime/scheduled-task-webhook"; @@ -239,9 +240,7 @@ function WebhookFilterRows(props: { return ( <> {props.filters.map((filter, index) => ( - // Rows have no identity of their own; removing one re-renders the rest in order. - // oxlint-disable-next-line react/no-array-index-key - + Filter {index + 1} props.onChange(props.filters.filter((_, at) => at !== index))} + onPress={() => props.onChange(removeWebhookFilterDraft(props.filters, filter.id))} className="active:opacity-70" > @@ -290,7 +289,7 @@ function WebhookFilterRows(props: { props.onChange([...props.filters, EMPTY_WEBHOOK_FILTER_DRAFT])} + onPress={() => props.onChange([...props.filters, emptyWebhookFilterDraft()])} className="min-h-14 flex-row items-center gap-3 border-t border-border-subtle px-4 py-3 active:opacity-70" > Add filter diff --git a/apps/mobile/src/features/settings/scheduledTaskDraft.test.ts b/apps/mobile/src/features/settings/scheduledTaskDraft.test.ts index 390465e2db36..bfe363cf0652 100644 --- a/apps/mobile/src/features/settings/scheduledTaskDraft.test.ts +++ b/apps/mobile/src/features/settings/scheduledTaskDraft.test.ts @@ -79,7 +79,7 @@ describe("scheduleDraftForTask", () => { filters: filters.slice(1), }); expect( - scheduleFromDraft({ ...draft, filters: [{ path: "", op: "exists", value: "" }] }), + scheduleFromDraft({ ...draft, filters: [{ id: "f1", path: "", op: "exists", value: "" }] }), ).toBeNull(); }); }); diff --git a/apps/mobile/src/features/settings/scheduledTaskDraft.ts b/apps/mobile/src/features/settings/scheduledTaskDraft.ts index fe8c62d3cd29..599fb4780e88 100644 --- a/apps/mobile/src/features/settings/scheduledTaskDraft.ts +++ b/apps/mobile/src/features/settings/scheduledTaskDraft.ts @@ -179,7 +179,7 @@ function draftSignature(draft: ScheduledTaskDraft): string { [...draft.schedule.weekdays].sort((a, b) => a - b), draft.schedule.intervalMinutes, draft.schedule.maxDeliveryAgeMinutes, - draft.schedule.filters, + draft.schedule.filters.map(({ path, op, value }) => [path, op, value]), draft.workspace, draft.baseRef, draft.checkoutPath, diff --git a/apps/web/src/components/settings/ScheduledTasksSettings.tsx b/apps/web/src/components/settings/ScheduledTasksSettings.tsx index 5d8ea87b0fc9..d5b3241140a2 100644 --- a/apps/web/src/components/settings/ScheduledTasksSettings.tsx +++ b/apps/web/src/components/settings/ScheduledTasksSettings.tsx @@ -28,8 +28,9 @@ import type { } from "@t3tools/contracts"; import { DEFAULT_WEBHOOK_PROMPT, - EMPTY_WEBHOOK_FILTER_DRAFT, + emptyWebhookFilterDraft, parseWebhookFilterDrafts, + removeWebhookFilterDraft, WEBHOOK_FILTER_OPS, type WebhookFilterDraft, } from "@t3tools/client-runtime/scheduled-task-webhook"; @@ -179,7 +180,7 @@ function WebhookFiltersField({ variant="outline" size="sm" disabled={filters.length >= MAX_WEBHOOK_FILTERS} - onClick={() => onChange([...filters, EMPTY_WEBHOOK_FILTER_DRAFT])} + onClick={() => onChange([...filters, emptyWebhookFilterDraft()])} > Add filter @@ -187,23 +188,23 @@ function WebhookFiltersField({
{filters.map((filter, index) => ( // Always stacked: the dialog stays narrow on wide windows, so a single line - // would cut off typical paths and values. - // Rows have no identity of their own; removing one re-renders the rest in order. - // oxlint-disable-next-line react/no-array-index-key -
+ // would cut off typical paths and values. The grid wrappers size each control + // without restyling it; a grid item stretches to its minmax(0, 1fr) track. +
- update(index, { path: event.target.value })} - /> +
+ update(index, { path: event.target.value })} + /> +
@@ -216,15 +217,13 @@ function WebhookFiltersField({ if (known) update(index, { op: known.op }); }} > - - - {WEBHOOK_FILTER_OPS.find((option) => option.op === filter.op)?.label} - - +
+ + + {WEBHOOK_FILTER_OPS.find((option) => option.op === filter.op)?.label} + + +
{WEBHOOK_FILTER_OPS.map((option) => ( @@ -234,13 +233,14 @@ function WebhookFiltersField({ {filter.op === "exists" ? null : ( - update(index, { value: event.target.value })} - /> +
+ update(index, { value: event.target.value })} + /> +
)}
diff --git a/apps/web/src/components/settings/scheduledTasksSettings.logic.test.ts b/apps/web/src/components/settings/scheduledTasksSettings.logic.test.ts index 2598f641ffc8..2b9264b9dd70 100644 --- a/apps/web/src/components/settings/scheduledTasksSettings.logic.test.ts +++ b/apps/web/src/components/settings/scheduledTasksSettings.logic.test.ts @@ -237,7 +237,10 @@ describe("webhook scheduled tasks", () => { // Removing every row is a change, so it clears the stored filters. expect(scheduleFromDraft({ ...draft, filters: [] })).toMatchObject({ filters: [] }); expect( - scheduleFromDraft({ ...draft, filters: [{ path: "body.action", op: "in", value: " , " }] }), + scheduleFromDraft({ + ...draft, + filters: [{ id: "f1", path: "body.action", op: "in", value: " , " }], + }), ).toBeNull(); }); }); diff --git a/packages/client-runtime/src/scheduledTaskWebhook.test.ts b/packages/client-runtime/src/scheduledTaskWebhook.test.ts index 76caa8bd5112..6e35d8446964 100644 --- a/packages/client-runtime/src/scheduledTaskWebhook.test.ts +++ b/packages/client-runtime/src/scheduledTaskWebhook.test.ts @@ -2,7 +2,9 @@ import { describe, expect, it } from "vite-plus/test"; import { parseMaxDeliveryAge, + emptyWebhookFilterDraft, parseWebhookFilterDrafts, + removeWebhookFilterDraft, webhookFilterDrafts, webhookFilterDraftsChanged, } from "./scheduledTaskWebhook.ts"; @@ -26,24 +28,25 @@ describe("webhook filter drafts", () => { { path: "query.source", op: "exists" }, ] as const; const drafts = webhookFilterDrafts(filters); - expect(drafts[1]).toEqual({ path: "body.action", op: "in", value: "opened, reopened" }); + expect(drafts[1]).toMatchObject({ path: "body.action", op: "in", value: "opened, reopened" }); expect(parseWebhookFilterDrafts(drafts)).toEqual({ filters }); }); it("names the first row the server would refuse", () => { expect( parseWebhookFilterDrafts([ - { path: "body.action", op: "equals", value: "opened" }, - { path: "body.action", op: "in", value: " , " }, - { path: "", op: "exists", value: "" }, + { id: "f1", path: "body.action", op: "equals", value: "opened" }, + { id: "f2", path: "body.action", op: "in", value: " , " }, + { id: "f3", path: "", op: "exists", value: "" }, ]), ).toEqual({ error: "Filter 2 needs a path under body, headers or query, and at least one value.", }); - expect(parseWebhookFilterDrafts([{ path: " ", op: "exists", value: "" }])).toEqual({ + expect(parseWebhookFilterDrafts([{ id: "f1", path: " ", op: "exists", value: "" }])).toEqual({ error: "Filter 1 needs a path under body, headers or query.", }); - const seventeen = Array.from({ length: 17 }, () => ({ + const seventeen = Array.from({ length: 17 }, (_, index) => ({ + id: `f${index}`, path: "body.action", op: "exists" as const, value: "", @@ -58,4 +61,14 @@ describe("webhook filter drafts", () => { expect(webhookFilterDraftsChanged(saved, [{ ...saved[0]!, value: "closed" }])).toBe(true); expect(webhookFilterDraftsChanged([], [])).toBe(false); }); + + it("removing the first row keeps the second row's id", () => { + const drafts = [ + ...webhookFilterDrafts([{ path: "body.action", op: "equals", value: "opened" }]), + emptyWebhookFilterDraft(), + ]; + expect(new Set(drafts.map((draft) => draft.id)).size).toBe(2); + const remaining = removeWebhookFilterDraft(drafts, drafts[0]!.id); + expect(remaining.map((draft) => draft.id)).toEqual([drafts[1]!.id]); + }); }); diff --git a/packages/client-runtime/src/scheduledTaskWebhook.ts b/packages/client-runtime/src/scheduledTaskWebhook.ts index 0b1dda8c8578..2b19d282dceb 100644 --- a/packages/client-runtime/src/scheduledTaskWebhook.ts +++ b/packages/client-runtime/src/scheduledTaskWebhook.ts @@ -23,6 +23,8 @@ export function parseMaxDeliveryAge(value: string): number | null | undefined { * lossless because list values can't contain commas; `exists` ignores the value. */ export interface WebhookFilterDraft { + /** Stable row identity for the form; never sent to the server. */ + readonly id: string; readonly path: string; readonly op: ScheduledTaskWebhookFilterOp; readonly value: string; @@ -39,16 +41,26 @@ export const WEBHOOK_FILTER_OPS: ReadonlyArray<{ { op: "exists", label: "exists" }, ]; -export const EMPTY_WEBHOOK_FILTER_DRAFT: WebhookFilterDraft = { - path: "", - op: "equals", - value: "", -}; +let lastWebhookFilterDraftId = 0; +const nextWebhookFilterDraftId = () => `filter-${++lastWebhookFilterDraftId}`; + +export function emptyWebhookFilterDraft(): WebhookFilterDraft { + return { id: nextWebhookFilterDraftId(), path: "", op: "equals", value: "" }; +} + +/** By id, so a removed row's identity never passes to the row after it. */ +export function removeWebhookFilterDraft( + drafts: ReadonlyArray, + id: string, +): ReadonlyArray { + return drafts.filter((draft) => draft.id !== id); +} export function webhookFilterDrafts( filters: ReadonlyArray | undefined, ): ReadonlyArray { return (filters ?? []).map((filter) => ({ + id: nextWebhookFilterDraftId(), path: filter.path, op: filter.op, value: From c5a96d132618f8be1166145cf8288e2f05bc3c14 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Tue, 20 Oct 2026 18:52:39 +0000 Subject: [PATCH 7/7] fix(web): the Schedule toggle wraps below its label on narrow screens At 390px with the toggle labels rendered at 16px, as under mobile text autosizing, the segmented control is 315px wide and the row overflowed the dialog. The row now wraps, so the toggle moves to its own line only when it can't sit beside the label; wider layouts are unchanged. Co-Authored-By: Claude Opus 5.5 --- apps/web/src/components/settings/ScheduledTasksSettings.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/apps/web/src/components/settings/ScheduledTasksSettings.tsx b/apps/web/src/components/settings/ScheduledTasksSettings.tsx index d5b3241140a2..75d3685d4916 100644 --- a/apps/web/src/components/settings/ScheduledTasksSettings.tsx +++ b/apps/web/src/components/settings/ScheduledTasksSettings.tsx @@ -1496,7 +1496,8 @@ function ScheduledTaskEditorForm({ minute.

) : null} -
+ {/* Wraps the toggle below the label when both don't fit, as on phones. */} +