feat(core): accept Fusor JSON webhooks - #205
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (5)
📜 Recent review details🧰 Additional context used📓 Path-based instructions (4)**/*.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{js,jsx,ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{tsx,ts}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{test,spec}.{js,jsx,ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
🔇 Additional comments (1)
📝 WalkthroughWalkthroughFusor webhook delivery now uses schema v1 JSON envelopes with canonical raw-body preservation, CloudEvents ChangesFusor webhook delivery
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant Adapter
participant Spectrum
participant FusorDecoder
participant FusorCore
participant Provider
Client->>Adapter: POST Fusor JSON envelope
Adapter->>Spectrum: raw bytes and headers
Spectrum->>FusorDecoder: decode envelope
FusorDecoder-->>Spectrum: parsed request and rawBody
Spectrum->>FusorCore: processRequest
FusorCore->>Provider: verify and handle request
Provider-->>FusorCore: handler outcome
FusorCore-->>Spectrum: combined reply
Spectrum-->>Client: HTTP response
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
@spectrum-ts/core
@spectrum-ts/elysia
@spectrum-ts/express
@spectrum-ts/fastify
@spectrum-ts/hono
@spectrum-ts/imessage
@spectrum-ts/imessage-local
@spectrum-ts/slack
spectrum-ts
@spectrum-ts/telegram
@spectrum-ts/terminal
@spectrum-ts/whatsapp-business
commit: |
There was a problem hiding this comment.
Problem / solution review
Verdict: The problem is real, and this PR fixes the actual contract change — it is not papering over a deeper bug.
What problem is this trying to solve?
Fusor’s HTTP webhook fanout is moving from a protobuf envelope to a versioned JSON envelope so low-code tools (for example n8n) can read deliveries without a protobuf decoder.
That creates two concrete SDK problems:
- Both webhook kinds are now JSON. Native Spectrum webhooks were already JSON; Fusor used to be binary protobuf. The old “does the body start with
{?” check can no longer tell them apart. - Providers still need the exact original request bytes. Platform signature checks (
verify()) must run on the provider’s real body, not on JSON that was parsed and re-serialized along the way.
So this is a coordinated public-contract change, not a local SDK quirk.
Is the proposed fix the right solution?
Yes. The design matches the constraints:
- Explicit routing:
ce-type: dev.spctrm.fusor.deliveryselects the Fusor path; everything else stays on the native HMAC path. That is the right discriminator once both bodies are JSON. - Exact bytes preserved: The envelope carries
rawBodyBase64, and the SDK always hands those decoded bytes to providerverify(). The normalizedbody/bodyEncodingfields are for human/low-code use, not for signature reconstruction. That avoids the classic “re-stringify JSON and break HMAC” failure mode. - Transport split stays clean: HTTP JSON goes through
processRequest; WebSocket still uses protobuf/processEvent. The shared provider pipeline is reused without forcing one wire format onto both transports. - Hard cut of legacy protobuf HTTP bodies is intentional and called out as coordinated with the Fusor producer. That is a release-ordering choice, not a symptom patch.
Is there a deeper issue underneath?
Not really. Sharing one webhook endpoint for native Spectrum and Fusor deliveries is an existing product shape. Sniffing payload bytes was always a brittle stand-in for a real content type; switching to CloudEvents ce-type is an improvement, not a workaround.
The only operational risk is the hard cut itself: this SDK and the Fusor JSON fanout must ship together, or in-flight protobuf deliveries will start returning 400. The PR already documents that. A dual-accept migration window would be softer, but it is not required for correctness if the cutover is coordinated.
Bottom line
Ship this as the SDK half of the Fusor JSON webhook contract. The important invariant — exact provider bytes for verify(), with a clear header-based route into that path — is handled directly.
Sent by Cursor Automation: PR analyze
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/core/src/fusor/types.ts (1)
87-99: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winDoc says headers "are required" but the type keeps them optional.
The rewritten contract states
headersare required for routing/verification, butWebhookRawRequest.headersis still typedheaders?: Record<string, string>. Downstream (readWebhookInputinspectrum.ts) does handle a missingheadersgracefully viaraw.headers ?? {}, but it silently routes to the native path and then fails closed with a 400/401 — worth aligning the doc wording with the actual optionality instead of asserting a hard requirement the type doesn't enforce.📝 Suggested wording fix
- * `headers` are required for routing and verification. Fusor deliveries carry + * `headers` (typed optional, but effectively required) drive routing and + * verification. Fusor deliveries carry🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/fusor/types.ts` around lines 87 - 99, Align the WebhookRawRequest documentation with the optional headers?: Record<string, string> contract. Update the comment above WebhookRawRequest to state that headers are optional and describe the existing missing-header behavior handled by readWebhookInput, without changing the type or routing logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/core/src/fusor/webhook.ts`:
- Around line 86-100: The duplicate header normalization logic should use one
shared merge helper. In packages/core/src/fusor/webhook.ts:86-100, extract the
duplicate-value joining from normalizeHeaders into a shared helper; in
packages/core/src/fusor/parse.ts:54-71, replace the inline existing-value merge
with that helper while preserving lowercase names and RFC-compliant ", "
joining.
---
Outside diff comments:
In `@packages/core/src/fusor/types.ts`:
- Around line 87-99: Align the WebhookRawRequest documentation with the optional
headers?: Record<string, string> contract. Update the comment above
WebhookRawRequest to state that headers are optional and describe the existing
missing-header behavior handled by readWebhookInput, without changing the type
or routing logic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9cfaf56f-a390-4abb-a23a-51c1592afed4
📒 Files selected for processing (16)
docs/webhooks.mdx.velpackages/core/src/fusor/core.tspackages/core/src/fusor/parse.tspackages/core/src/fusor/types.tspackages/core/src/fusor/webhook.tspackages/core/src/spectrum.tspackages/core/src/webhook/types.tspackages/core/test/core/fusor/webhook.test.tspackages/core/test/webhook/spectrum.test.tspackages/elysia/test/elysia.test.tspackages/express/src/index.tspackages/express/test/express.test.tspackages/fastify/src/index.tspackages/fastify/test/fastify.test.tspackages/hono/test/hono.test.tspackages/test-support/src/fusor.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use explicit function parameter and return types when they improve clarity; prefer
unknownoverany; useas constfor immutable literal values; and rely on TypeScript narrowing instead of assertions.
Files:
packages/core/src/fusor/parse.tspackages/core/src/webhook/types.tspackages/express/src/index.tspackages/fastify/src/index.tspackages/elysia/test/elysia.test.tspackages/core/src/fusor/webhook.tspackages/fastify/test/fastify.test.tspackages/core/src/fusor/types.tspackages/hono/test/hono.test.tspackages/express/test/express.test.tspackages/core/test/webhook/spectrum.test.tspackages/core/src/spectrum.tspackages/test-support/src/fusor.tspackages/core/src/fusor/core.tspackages/core/test/core/fusor/webhook.test.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,jsx,ts,tsx}: Use meaningful variable names and extract magic numbers into descriptively named constants.
Use arrow functions for callbacks and short functions.
Preferfor...ofloops over.forEach()and indexedforloops.
Use optional chaining and nullish coalescing for safer property access.
Prefer template literals over string concatenation and use destructuring for object and array assignments.
Useconstby default,letonly when reassignment is needed, and never usevar.
Always await promises in async functions and use the returned value; prefer async/await over promise chains.
Handle async errors appropriately with try-catch blocks and do not use async functions as Promise executors.
Removeconsole.log,debugger, andalertstatements from production code.
ThrowErrorobjects with descriptive messages rather than strings or other values.
Use try-catch blocks meaningfully and do not catch errors solely to rethrow them.
Prefer early returns for error cases and to reduce nesting; use simple conditionals instead of nested ternaries.
Keep functions focused and within reasonable cognitive-complexity limits, extract complex conditions into named booleans, and group related code while separating concerns.
AvoiddangerouslySetInnerHTMLunless absolutely necessary; do not useeval()or assign directly todocument.cookie; validate and sanitize user input.
Avoid spread syntax in accumulators within loops, use top-level regex literals instead of creating them in loops, prefer specific imports over namespace imports, and avoid barrel files that re-export everything.
Files:
packages/core/src/fusor/parse.tspackages/core/src/webhook/types.tspackages/express/src/index.tspackages/fastify/src/index.tspackages/elysia/test/elysia.test.tspackages/core/src/fusor/webhook.tspackages/fastify/test/fastify.test.tspackages/core/src/fusor/types.tspackages/hono/test/hono.test.tspackages/express/test/express.test.tspackages/core/test/webhook/spectrum.test.tspackages/core/src/spectrum.tspackages/test-support/src/fusor.tspackages/core/src/fusor/core.tspackages/core/test/core/fusor/webhook.test.ts
**/*.{tsx,ts}
📄 CodeRabbit inference engine (AGENTS.md)
In Next.js, use Server Components for async data fetching instead of async Client Components.
Files:
packages/core/src/fusor/parse.tspackages/core/src/webhook/types.tspackages/express/src/index.tspackages/fastify/src/index.tspackages/elysia/test/elysia.test.tspackages/core/src/fusor/webhook.tspackages/fastify/test/fastify.test.tspackages/core/src/fusor/types.tspackages/hono/test/hono.test.tspackages/express/test/express.test.tspackages/core/test/webhook/spectrum.test.tspackages/core/src/spectrum.tspackages/test-support/src/fusor.tspackages/core/src/fusor/core.tspackages/core/test/core/fusor/webhook.test.ts
**/*.{test,spec}.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{test,spec}.{js,jsx,ts,tsx}: Write assertions insideit()ortest()blocks, avoid done callbacks in async tests, and do not commit tests containing.onlyor.skip.
Keep test suites reasonably flat and avoid excessivedescribenesting.
Files:
packages/elysia/test/elysia.test.tspackages/fastify/test/fastify.test.tspackages/hono/test/hono.test.tspackages/express/test/express.test.tspackages/core/test/webhook/spectrum.test.tspackages/core/test/core/fusor/webhook.test.ts
🪛 ast-grep (0.44.1)
packages/express/test/express.test.ts
[warning] 125-125: Express application should use Helmet
Context: express()
Note: [CWE-693] Protection Mechanism Failure (Express app without Helmet security headers).
(missing-helmet-typescript)
🔇 Additional comments (20)
packages/core/src/fusor/webhook.ts (2)
1-49: LGTM!Also applies to: 51-73, 81-84
75-79: 🗄️ Data Integrity & IntegrationNo issue:
processRequestonly consumeseventIdandplatform, so the extra envelope fields are intentionally not part ofFusorWebhookEvent.> Likely an incorrect or invalid review comment.packages/core/src/fusor/parse.ts (1)
26-28: LGTM!packages/core/src/webhook/types.ts (1)
7-10: LGTM!docs/webhooks.mdx.vel (3)
18-22: LGTM!
82-94: LGTM!
221-221: LGTM!packages/test-support/src/fusor.ts (1)
2-7: LGTM!Also applies to: 20-28, 70-89, 118-188
packages/fastify/src/index.ts (1)
8-8: LGTM!Also applies to: 89-89
packages/fastify/test/fastify.test.ts (1)
3-7: LGTM!Also applies to: 117-152
packages/hono/test/hono.test.ts (1)
3-7: LGTM!Also applies to: 109-147
packages/express/test/express.test.ts (2)
126-126: Static-analysis Helmet hint doesn't apply here.Flagged by static analysis as missing Helmet security headers, but this
express()instance is a short-lived, in-process test server on a random localhost port used only for this test — not a production-facing app.Source: Linters/SAST tools
4-8: LGTM!Also applies to: 122-158
packages/core/src/fusor/core.ts (1)
41-45: LGTM!Also applies to: 168-168, 324-346, 347-360, 364-370, 392-407
packages/core/src/spectrum.ts (2)
8-9: LGTM!Also applies to: 21-25, 109-118, 1009-1010, 1072-1094, 1147-1151, 1302-1313, 1323-1328
1314-1321: 🎯 Functional Correctness | ⚡ Quick winMalformed-envelope 400 returns an empty body, unlike every other 400/401 path here.
processWebhookEvent's poison branch andhandleSpectrumWebhook's malformed-payload/signature-failure branches all return a descriptive text body viaencodeText(...). The Fusor envelope decode-failure branch returnsbody: new Uint8Array(0)with no explanation, making it harder for API consumers to diagnose a bad envelope from the HTTP response alone.🛠️ Suggested fix for consistency
const event = decodeWebhookEvent(bodyBytes); if (!event) { return buildWebhookResult(asWeb, { status: 400, headers: {}, - body: new Uint8Array(0), + body: encodeText("malformed Fusor envelope"), }); }packages/core/test/core/fusor/webhook.test.ts (1)
6-8: LGTM!Also applies to: 22-22, 41-41, 79-79, 115-115, 141-141, 172-180, 198-204, 206-244, 246-299, 301-327, 329-345, 347-414, 416-426, 442-442, 466-466, 497-497, 531-531, 560-566, 584-584, 623-623, 665-665, 701-701, 729-729
packages/core/test/webhook/spectrum.test.ts (1)
1-6: LGTM!Also applies to: 199-210, 229-261, 273-276
packages/elysia/test/elysia.test.ts (1)
3-7: LGTM!Also applies to: 107-144
packages/express/src/index.ts (1)
10-10: LGTM!


Summary
ce-type: dev.spctrm.fusor.deliveryschemaVersion: 1JSON envelope and reject legacy protobuf or invalid payloads with 400rawBodyBase64bytes plus method, path, and lowercase headers into the existing Fusor provider pipelineWhy
Fusor customer webhook fanout is moving from protobuf to plain JSON so low-code platforms such as n8n can consume it. The SDK must accept the new public contract without reconstructing provider request bytes.
Impact
This is a hard cut: legacy Fusor protobuf webhook bodies now return 400. Publish this SDK change before deploying the coordinated
photon-hq/fusorbranchcodex/fanout-webhook-json. Native Spectrum webhooks and Fusor WebSocket events are unchanged. Package versions remain at 11.2.0 in source because the lockstep release workflow owns versioning.Validation
bun run checkbun run typecheck— 13 tasksbun run test— 32/32 Node+Bun tasksbun run build— 12/12 tasksgit diff --checkNeed help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by CodeRabbit
New Features
ce-type.Bug Fixes
Documentation
bodyEncoding.