[kayahk]

Why

An api_trigger fired one graph-walk run per webhook delivery, with no way to say which deliveries it wanted. Pointed at a GitHub hook subscribed to check_run — one delivery per CI job per state — a single push fans out into dozens of runs. On our deployment that was 3,291 runs in ~2.5 days, ~85% of them check_run, all against one task, arriving in ~20/minute bursts. The graph's github.pr_state condition can't help: it runs after the trigger, and by then a trigger_ai_agent node has already asked for a sandbox.

Closes #605.

What

An optional allow-list on the trigger config:

{ "target_task_id": "…", "github_events": ["pull_request:opened", "pull_request:synchronize"] }

• Entry is event or event:action, matched against X-GitHub-Event and the body's action.
• Empty/absent → every delivery fires, exactly as before. No migration; non-GitHub senders are unaffected.
• A delivery matching nothing is accepted and dropped: 202, no run, no stream entry, so the sender never retries and the response can't be used to probe the filter.
• Invalid entries ("", ":opened", "pull_request:", "a:b:c") are rejected at save time via the existing ErrNodeConfigInvalid path, and github_events is rejected on any trigger type other than api_trigger — nothing else consumes a delivery.
• Handler reads the body with io.LimitReader(…, 1<<20); only action is ever decoded, and a malformed body simply matches no event:action entry.

Kept GitHub-shaped deliberately (action + header, not a generic matcher): that pair is the discriminator every GitHub integration has, and an expression language would be far more surface for the same result.

Tests

• TestMatchesGithubEvents — 9 cases: no filter, event alone, event:action match/mismatch on both event and action, any-entry-wins, unparseable body, missing header.
• TestValidateTriggerConfig_GithubEvents — valid list accepted; the four malformed shapes rejected; rejected on a non-api_trigger type.
• TestE2EAutomation_WebhookGithubEventsFilter — real HTTP: a check_run/completed and a pull_request/labeled delivery both return 202 and leave zero runs and the task untouched; a pull_request/opened delivery then fires and assigns the task. Confirmed to fail without the fix (expected no runs for filtered deliveries, got 2).
• go build, go vet, golangci-lint (0 issues), go test -race clean; e2e run green including the pre-existing webhook and call_api suites.

Compatibility

Config is JSON and the field is omitempty, so existing automations keep their current behaviour byte-for-byte. The only signature change is HandleWebhookTrigger, which gains the delivery as a value — one caller.

[pullfrog]

ℹ️ The backend change is correct and well covered; one builder round-trip hazard worth a decision before merge.

Reviewed changes

• Delivery filter wired end to end — HandleWebhookTrigger takes a WebhookDelivery and drops non-matching deliveries with a nil return, so the handler still answers 202; empty/absent github_events fires everything as before.
• matchesGithubEvents matcher — empty patterns ⇒ match; entries are event or event:action; only the top-level action is decoded, so an unparseable body just has no action to match.
• Config validation — malformed entries ("", :opened, pull_request:, a:b:c) are rejected via ErrNodeConfigInvalid, and github_events is rejected on any non-api_trigger builtin trigger.
• Handler body read — bounded io.ReadAll(io.LimitReader(r.Body, 1<<20)) before dispatch; previously the body was ignored.
• Tests — TestMatchesGithubEvents, TestValidateTriggerConfig_GithubEvents, and the real-HTTP e2e TestE2EAutomation_WebhookGithubEventsFilter (zero runs for filtered deliveries, fires on a wanted one). I ran the two unit tests locally and they pass.

ℹ️ The builder's api_trigger form will silently wipe github_events

The new filter can only be set through the API/MCP — the web config panel has no field for it. The api_trigger branch of TriggerConfigForm saves only { target_task_id } (automation-node-config-panel.tsx:1192), the PATCH sends just that object (automation-api.ts:516), and UpdateNode replaces the node config wholesale (automation_service.go:313-317). So a github_events configured on the deployment is removed the next time anyone opens that node in the builder and clicks Save — silently restoring the exact runaway-run behavior this PR fixes. Worth either surfacing the field in the panel or preserving unrecognized keys on save.

Technical details

# Builder save replaces trigger config, dropping API-set fields

## Affected sites
- `apps/web/src/components/projects/automation/automation-node-config-panel.tsx:1188-1195` — api_trigger SaveButton calls `onSave({ target_task_id: … })` only.
- `apps/web/src/routes/_authenticated/projects/$projectId/automation/$automationId.tsx:732-734` — forwards that object as `config`.
- `apps/web/src/lib/automation-api.ts:516-532` — PATCH body is only `{ config }`.
- `services/api/internal/service/automation/automation_service.go:313-317` — `UpdateNode` validates and stores `*in.Config` as the whole config (`normalizeConfig` only maps empty → `{}`).
- `apps/web/src/lib/automation-api.ts:194-213` — `TriggerConfig` TS interface has no `github_events`, so a field can't round-trip through the panel.

## Required outcome
- A `github_events` value set via the API survives a builder save of the trigger node (either the panel renders/edits it, or the save path preserves unknown keys).

## Suggested approach (optional)
- Add a `github_events` editor to the api_trigger form, or have the panel/E? merge the existing `node.config` with the fields it edits instead of sending a fresh object.

## Open questions for the human
- Is UI configuration of `github_events` intended for a later PR, and is API/MCP-only configuration acceptable for now?

  ^{｜ Fix it ➔ ｜ View workflow run ｜ Using deepseek-v4.1-flash (free via Pullfrog for OSS) ｜ 𝕏}

[kayahk]

Good catch — that was a real hole, fixed in ef63d8ff.

What changed: the api_trigger branch of TriggerConfigForm now saves {...config, target_task_id} instead of a fresh {target_task_id}. target_task_id still comes from the form, so clearing it still clears (the key is dropped from the JSON), but every field the form does not render survives the round trip. That covers github_events today and any future API-only field, without needing a UI control for it.

On the open question: UI editing of github_events is deliberately a later PR. The panel needs an event/action picker with the GitHub taxonomy, which is a bigger surface than this fix; preserving the stored value is the part that must not wait, since the failure mode (silent wipe → runaway runs) is invisible until the runs pile up again.

Not done, on purpose: I did not make UpdateNode merge configs server-side. Wholesale replace is the right contract for a PATCH that carries the whole config — merging would make it impossible to clear a field, and it would paper over the same class of bug for every other node type.

Also added github_events?: string[] to the TriggerConfig TS interface so the field is discoverable rather than only in the docs.

Verification: biome check clean on both files. The panel has no existing component test harness, and node_modules is not installed here, so this is type-checked through the editor's TS server rather than a tsc -p run — flagging that honestly rather than claiming a build I did not run. The backend test set from the original description is unchanged and still green.

[pullfrog]

✅ No new issues found.

Reviewed changes

The delta since the prior Pullfrog review (d69d7c6) is the follow-up commit ef63d8f, which closes the builder round-trip hazard flagged then.

• github_events survives a builder save — the api_trigger form now spreads the node's stored config before overriding target_task_id, so the whole-config PATCH carries an API/MCP-set filter through instead of dropping it.
• github_events declared on the web TriggerConfig — typed string[] with a doc comment stating it is API-only and preserved rather than overwritten.

  ^{｜ View workflow run ｜ Using deepseek-v4.1-flash (free via Pullfrog for OSS) ｜ 𝕏}

[pikann]

Hi @kayahk ,

Thank you so much for bringing this up and for your contribution!

The PR looks great! However, following the Paca design concept, we believe this feature would fit better inside the Github plugin repository: https://github.com/Paca-AI/paca-plugin-github

That repository already handles triggers for PR creation, state changes, and branch creation.

Feel free to open a PR over there to enhance it! Thanks again for your support and hard work.

[kayahk]

Following your guidance, I've moved the webhook-protection work to the GitHub plugin: Paca-AI/paca-plugin-github#27.

What it fixes there: createWebhook was subscribing every linked repo to push, pull_request and check_run, while handleWebhookEvent only acts on the first two — so every CI job's check_run delivery paid for a repository lookup plus an HMAC verification before being dropped in the default: case. The plugin now (a) stops requesting check_run and (b) drops any non-pull_request/push delivery before touching the database, which also covers hooks created by earlier versions that stay subscribed.

On this PR: you're right that the filtering concept belongs with the plugin. What's left here is generic — github_events filters any webhook sender, not just GitHub, and it lives in the trigger config where the automation author can see it. If you'd rather not carry that in the core, I'll close this PR and we keep just the plugin change; the one gap it leaves is that an automation can only react to PR activity through a plugin trigger, and per-action narrowing isn't expressible there yet (plugin trigger config is opaque to the core, which dispatches on eventTopic alone). Happy either way — just say which.

[kayahk]

Closing this in favour of the plugin, as suggested.

• Webhook protection is now fix(webhook): drop unhandled events before the repository lookup paca-plugin-github#27 (subscribed check_run removed; unhandled events dropped before the repository lookup and HMAC work).
• The remaining filtering gap is Automations cannot narrow GitHub PR triggers to specific actions paca-plugin-github#28.

For the record, nothing here was wrong — the change tested clean and the review's catch (a builder save wiping a config field the panel does not render) was real and fixed in ef63d8ff. It is simply the wrong layer for GitHub-specific semantics: github_events made the core understand GitHub's action taxonomy, and per the design guidance that belongs with the plugin. Thanks for the review.