Skip to content

feat(surface,sdk,kernel): event triggers via webhook + inbox watcher (#301) - #317

Merged
kjgbot merged 5 commits into
mainfrom
feat/spec-E-triggers-301
Sep 11, 2026
Merged

kjgbot merged 5 commits into
mainfrom
feat/spec-E-triggers-301

Conversation

@kjgbot

@kjgbot kjgbot commented Sep 11, 2026 •

Copy link
Copy Markdown
Contributor

Closes #301.

Summary

Event triggers end-to-end via a generic HTTP webhook + inbox watcher:

  • Surface: flow.on(webhook('name', filter?), (f, event) => body)
  • Trigger executor: flows serve-webhook --data-dir <dir> --port <p> HTTP-mounts events to <data-dir>/inbox/<trigger>/<uuid>.json
  • Kernel: watcher in relayflowd picks up new inbox files and spawns runs of matching flows, passing parsed event as second body arg
  • Preflight: webhook('X') refused unless X is in flows.json executors array

Not in scope (deferred follow-ups)

Written by codex agent spec-E-triggers-v3 on finn-mini; head at 70ac3d5.

Test plan

  • linux-x64-artifact green
  • packed-consumer green

🤖 Generated with Claude Code


Note

Medium Risk
Changes event submission, dedupe/resume, and journal creation durability; filesystem inbox ack boundaries must stay correct to avoid lost or double-processed webhooks.

Overview
Adds webhook-triggered flows end-to-end: authored flows declare handlers with flow(...).on(webhook('name', filter?), body), the CLI exposes flows serve-webhook to accept POST JSON into <data-dir>/inbox/<name>/, and relayflowd runs a 1 Hz inbox watcher that loads triggers/<name>.json, submits events, drives or resumes the run, then archives files only after a durable receipt.

The engine gains submit_webhook_event so deduped inbox retries resume the existing run instead of returning no run; run.spawned journals now include the event payload (additive flattened shape). SqliteJournal::create wraps schema + meta/segment inserts in one transaction so SIGKILL between create and first append no longer yields journals that fail to open on resume.

Preflight refuses webhook names not listed in flows.json executors, wired into flows check and direct flows run on .flow.ts (plus Slack helper checks on that path). Tests cover unit ingress, kernel watcher behavior, and live daemon + SIGKILL replay scenarios.

Reviewed by Cursor Bugbot for commit 4e0a02a. Bugbot is set up for automated code reviews on this repo. Configure here.

@coderabbitai

coderabbitai Bot commented Sep 11, 2026 •

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: a2c68b1b-4107-44ad-a880-3006967d078d


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale Bugbot comment from a previous run.

Comment thread packages/sdk/src/cli/serve-webhook.ts
@kjgbot

kjgbot commented Sep 11, 2026

Copy link
Copy Markdown
Contributor Author

maintainability lens — FAIL

Review — MAINTAINABILITY lens on PR #317

Blockers

  1. EventRunSpawnedPayload silently changes the run.spawned payload for every event-triggered run, not just webhooks. wake.rs:251-262 unconditionally wraps RunSpawnedPayload with an event field. The doc comment claims compatibility for "normal run.spawned readers", but that's a claim about tolerant JSON parsers — not an invariant enforced anywhere. A future maintainer reading the struct sees "additive" and won't know that the journal-shape change flows through the shared submit path. Either scope the wrap to the webhook branch, or replace the comment with a link to the schema-evolution policy that makes this safe (RFC §1 puts the journal at the boundary — silent shape drift is exactly what that section warns against).

  2. The polling thread is unshutdownable and its errors are eprintln!'d into the void. server.rs:77-86 spawns without holding a JoinHandle; trigger_watcher.rs:126-136 prints errors and poll failed to stderr and loops forever. AGENTS.md rule 4 ("Fail closed. … no console.warn where an error belongs") and RFC gate 1's failure taxonomy both require declared failure kinds, not stderr. Six months from now a reader has no way to see how the watcher stops, back-pressures, or surfaces retained-inbox counts. At minimum: return errors through a channel that a supervisor can observe, hold the handle, add a shutdown path.

  3. Uneven failure handling breaks the module's own contract. trigger_watcher.rs:36-38 and 47-49 propagate ? on fs::read_dir(directory.path()) and file_type(), so one unreadable inbox subdir aborts the whole scan — but the doc comment above poll_once promises "Errors retain the offending file and do not starve other inboxes." A stranger will trust the comment, not read the loop. Wrap the inner iterators with the same per-entry error-collection the file loop uses.

Concerns

  • Implicit contract via Option<&dyn Fn> at wake.rs:56-64. inbox_resume.is_some() is doing double duty ("this is a webhook" + "run preflight_placement" + "resume dedup hits"). Name the mode with an enum, or split the shared body; the current shape leaves a future editor guessing which behaviors ride on the callback's presence.
  • Trigger-name regex duplicated across languages — trigger_watcher.rs::valid_name and serve-webhook.ts::NAME. Silent drift = events posted successfully but never matched. Add a lockstep note in both, or better, derive both from one artifact (issue flows: event triggers via webhook + inbox watcher — SURFACE §1 harness #301 territory).
  • flow(name) with no body defers "no direct-run body" errors to run time (flow.ts:66-75). The lazy-throw async closure is a footgun given Covenant 1 ("errors name the author's mistake in the author's vocabulary"). A stranger reading a bodiless flow('x') won't know it's only legal alongside .on(...).
  • sleep 2 in webhook-live.test.ts — timing-based, will flake on loaded CI.

Notes

  • EventRunSpawnedPayload's #[serde(flatten)] silently collides if event ever contains a spec/spec_hash key; test only proves current shape, not the collision guard.
  • directory() in serve-webhook.ts:113-116 has a TOCTOU window between mkdir and lstat; low risk but worth a line.
  • check-triggers.ts:23-30 — the ternary building report.kind is dense; a small helper would age better than the nested 'kind' in error && error.kind === 'config_invalid' check.

REVIEW_FAILED

@kjgbot

kjgbot commented Sep 11, 2026

Copy link
Copy Markdown
Contributor Author

history lens — PASS

Blockers: none identified in PR #317 at 70ac3d51792af0bd1d3483ebd0c5f822f5c192ea under the three permitted HISTORY criteria.

Notes

  • The event-submission change preserves the existing claim mechanism. kernel/relayflowd/src/engine/wake.rs:141–154 adds resumption of a previously claimed run for webhook retries; it does not replace journal recovery with deterministic code replay. The claim-release guard remains intact. I found no reintroduction of the wake-context scan-error swallowing documented in ops/DRIVE-LOG.md.
  • kernel/relayflowd/src/engine/wake.rs:254–265 adds event data alongside the existing run.spawned payload fields. This does not introduce a provider SDK, tenant awareness, or a new kernel step type.
  • The actual commit message—“feat(surface,sdk,kernel): event triggers via webhook and inbox watcher (flows: event triggers via webhook + inbox watcher — SURFACE §1 harness #301)”—matches the changed components and makes no claims about passing tests, mutation verification, or completed deployment.
  • packages/sdk/src/cli/direct-run.ts:43–48 adds declaration checks before daemon connection, consistent with the earlier preflight work.

Concerns, not blockers

I inspected the requested history and governing documents; tests were not executed for this review. The RFC-referenced ../relayflows-rewrite-0825/REWRITE-CHARTER.md was unavailable locally. The older gate reference in ops/NEXT.md does not affect this verdict.

REVIEW_PASSED

@kjgbot

kjgbot commented Sep 11, 2026

Copy link
Copy Markdown
Contributor Author

structure lens — MISSING

@kjgbot

kjgbot commented Sep 11, 2026

Copy link
Copy Markdown
Contributor Author

🎯 review-swarm: FAILED (M:fail H:pass S:missing)

Lens transcripts posted as sibling comments above.

@kjgbot
kjgbot force-pushed the feat/spec-E-triggers-301 branch from 70ac3d5 to 2b46eb3 Compare September 11, 2026 09:54

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale Bugbot comment from a previous run.

Comment thread packages/sdk/src/cli.ts Outdated
@kjgbot
kjgbot force-pushed the feat/spec-E-triggers-301 branch 3 times, most recently from 40a9b26 to 17c00e4 Compare September 11, 2026 13:52

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale Bugbot comment from a previous run.

Comment thread packages/sdk/src/cli/serve-webhook.ts
@kjgbot
kjgbot force-pushed the feat/spec-E-triggers-301 branch from 217dcd0 to a66485f Compare September 11, 2026 14:21
miyaontherelay and others added 4 commits September 11, 2026 16:46
#301)

Session-Id: 01a08f7e-5920-7321-9c9b-ed10eb8ac4b8

Session-Id: efeda5df-9b7c-48d4-b2ce-957f5bef0a82

Session-Id: efeda5df-9b7c-48d4-b2ce-957f5bef0a82

Session-Id: efeda5df-9b7c-48d4-b2ce-957f5bef0a82

Session-Id: efeda5df-9b7c-48d4-b2ce-957f5bef0a82

Session-Id: efeda5df-9b7c-48d4-b2ce-957f5bef0a82

Session-Id: efeda5df-9b7c-48d4-b2ce-957f5bef0a82
…riant

E's trigger check imports the authored module before daemon-attach because
trigger sources are only observable after `flow(...).on(webhook(...))`
has run — the CI failure on direct-input.test.ts:121 was the old
assertion catching this legitimate import. The load-bearing invariant is
that the authored BODY is not called before daemon availability; the
fixture now records import vs body separately and the test targets the
body-run marker.

Session-Id: efeda5df-9b7c-48d4-b2ce-957f5bef0a82

Session-Id: efeda5df-9b7c-48d4-b2ce-957f5bef0a82

Session-Id: efeda5df-9b7c-48d4-b2ce-957f5bef0a82

Session-Id: efeda5df-9b7c-48d4-b2ce-957f5bef0a82

Session-Id: efeda5df-9b7c-48d4-b2ce-957f5bef0a82

Session-Id: efeda5df-9b7c-48d4-b2ce-957f5bef0a82
Cursor Bugbot MED at packages/sdk/src/cli.ts:115: flows check on a .flow.ts
routed through checkAuthoredTriggers (webhook-trigger preflight) but silently
skipped checkHelperBody (added for f.slack in PR#314). A .flow.ts using
f.slack.post without SLACK_BOT_TOKEN passed check and only crashed at run.

Fix by folding the slack helper preflight into checkAuthoredTriggers on the
same loaded definition, merging diagnostics. Direct-run also inherits the
helper check because it calls the same function.

Regression test: a .flow.ts using f.slack.post with no SLACK_BOT_TOKEN must
produce helper_slack.credential_missing via flows check --json.

Session-Id: efeda5df-9b7c-48d4-b2ce-957f5bef0a82

Session-Id: efeda5df-9b7c-48d4-b2ce-957f5bef0a82

Session-Id: efeda5df-9b7c-48d4-b2ce-957f5bef0a82

Session-Id: efeda5df-9b7c-48d4-b2ce-957f5bef0a82

Session-Id: efeda5df-9b7c-48d4-b2ce-957f5bef0a82

Session-Id: efeda5df-9b7c-48d4-b2ce-957f5bef0a82
…ia mocked authored flows

direct-run-failure.test.ts mocks getDefinition to return {} to simulate
loader failures. Both preflightHelpers (slack) and checkMcpHeader
unguarded-read definition.header, causing the tests to trap on TypeError
instead of surfacing the mocked worker-error classification.

Session-Id: efeda5df-9b7c-48d4-b2ce-957f5bef0a82

Session-Id: efeda5df-9b7c-48d4-b2ce-957f5bef0a82

Session-Id: efeda5df-9b7c-48d4-b2ce-957f5bef0a82
@kjgbot
kjgbot force-pushed the feat/spec-E-triggers-301 branch from a66485f to ebfadb7 Compare September 11, 2026 14:46

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit ebfadb7. Configure here.

spec.validate().context("invalid run spec")?;
if inbox_resume.is_some() {
self.preflight_placement(&spec)?;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Event spawn skips workspace binding

Medium Severity

The new webhook submit path calls preflight_placement but never bind_local_workspaces before drive. run.start journals those local workspace pins right after register so a later resume cannot inherit a different daemon cwd. Event-triggered runs with workspace-required deterministic steps lose that binding.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit ebfadb7. Configure here.

SqliteJournal::create ran execute_batch(SCHEMA) in autocommit before the
meta INSERT transaction. SIGKILL in that microsecond window left the
file with schema but no meta row, so open()'s SELECT ... FROM meta
returned NoRows (surfaced as 'Query returned no rows' on resume). The
webhook-live SIGKILL-after-spawn-before-ack test reproduced this
deterministically on GH runners.

Fold DDL + meta/segment inserts into a single WAL transaction. Either
SIGKILL leaves an empty file (no tables) or a fully initialized
journal — nothing in between.

Session-Id: efeda5df-9b7c-48d4-b2ce-957f5bef0a82
@kjgbot
kjgbot merged commit f8b4be5 into main Sep 11, 2026
8 of 10 checks passed
@kjgbot
kjgbot deleted the feat/spec-E-triggers-301 branch September 11, 2026 15:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

flows: event triggers via webhook + inbox watcher — SURFACE §1 harness

2 participants