feat(control-plane): proxy webhook subscription create/delete to relayfile-cloud#345
feat(control-plane): proxy webhook subscription create/delete to relayfile-cloud#345khaliqgant wants to merge 2 commits into
Conversation
…yfile-cloud Local control-plane now proxies webhook_subscription create/delete to relayfile-cloud; TS client + generated OpenAPI types updated for the inbound integration bridge. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 32 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (6)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds support for managing server-side webhook subscriptions by introducing new endpoints to the control plane daemon, updating the OpenAPI specification, and extending the TypeScript client. The review feedback suggests propagating the request context (r.Context()) instead of using context.Background() in the Go handler to support request cancellation. Additionally, the reviewer points out a type mismatch in the TypeScript client where deleteWebhookSubscription is typed to return Promise<void> instead of Promise<{ ok: boolean }>, and provides suggestions to align the client implementation and its unit tests with the actual daemon response.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| if err := commandClient.client.postJSON( | ||
| context.Background(), | ||
| fmt.Sprintf("/v1/workspaces/%s/webhooks", url.PathEscape(workspaceID)), |
There was a problem hiding this comment.
Use r.Context() instead of context.Background() to ensure that the request context is propagated. This allows the outgoing HTTP request to be cancelled if the client disconnects or times out.
| if err := commandClient.client.postJSON( | |
| context.Background(), | |
| fmt.Sprintf("/v1/workspaces/%s/webhooks", url.PathEscape(workspaceID)), | |
| if err := commandClient.client.postJSON( | |
| r.Context(), | |
| fmt.Sprintf("/v1/workspaces/%s/webhooks", url.PathEscape(workspaceID)), |
There was a problem hiding this comment.
Fixed in d422ef6 — webhook subscription creation now passes r.Context() through to the upstream relayfile-cloud request.
| if err := commandClient.client.deleteJSON( | ||
| context.Background(), | ||
| fmt.Sprintf("/v1/workspaces/%s/webhooks/%s", url.PathEscape(workspaceID), url.PathEscape(subscriptionID)), |
There was a problem hiding this comment.
Use r.Context() instead of context.Background() to propagate the request context and support cancellation.
| if err := commandClient.client.deleteJSON( | |
| context.Background(), | |
| fmt.Sprintf("/v1/workspaces/%s/webhooks/%s", url.PathEscape(workspaceID), url.PathEscape(subscriptionID)), | |
| if err := commandClient.client.deleteJSON( | |
| r.Context(), | |
| fmt.Sprintf("/v1/workspaces/%s/webhooks/%s", url.PathEscape(workspaceID), url.PathEscape(subscriptionID)), |
There was a problem hiding this comment.
Fixed in d422ef6 — webhook subscription deletion now passes r.Context() through to the upstream relayfile-cloud request.
| deleteWebhookSubscription(subscriptionId: string, workspace?: string): Promise<void> { | ||
| return this.request<void>({ | ||
| method: 'DELETE', | ||
| path: '/v1/integrations/webhook-subscriptions', | ||
| body: { subscriptionId, ...(workspace ? { workspace } : {}) }, | ||
| }); | ||
| } |
There was a problem hiding this comment.
The deleteWebhookSubscription method is typed to return Promise<void>, but the control plane server actually returns {"ok": true} (as defined in the OpenAPI spec and implemented in control_plane.go). To align with the OpenAPI schema and the actual daemon response, the return type should be Promise<{ ok: boolean }>.
| deleteWebhookSubscription(subscriptionId: string, workspace?: string): Promise<void> { | |
| return this.request<void>({ | |
| method: 'DELETE', | |
| path: '/v1/integrations/webhook-subscriptions', | |
| body: { subscriptionId, ...(workspace ? { workspace } : {}) }, | |
| }); | |
| } | |
| deleteWebhookSubscription(subscriptionId: string, workspace?: string): Promise<{ ok: boolean }> { | |
| return this.request<{ ok: boolean }>({ | |
| method: 'DELETE', | |
| path: '/v1/integrations/webhook-subscriptions', | |
| body: { subscriptionId, ...(workspace ? { workspace } : {}) }, | |
| }); | |
| } |
| const rawRequest = vi.spyOn(client as unknown as { rawRequest: (opts: unknown) => Promise<unknown> }, 'rawRequest'); | ||
| rawRequest.mockResolvedValueOnce({ subscriptionId: 'whsub_123' }).mockResolvedValueOnce(undefined); |
There was a problem hiding this comment.
Update the mock for rawRequest to resolve with { ok: true } instead of undefined for the delete call, matching the actual daemon's response.
| const rawRequest = vi.spyOn(client as unknown as { rawRequest: (opts: unknown) => Promise<unknown> }, 'rawRequest'); | |
| rawRequest.mockResolvedValueOnce({ subscriptionId: 'whsub_123' }).mockResolvedValueOnce(undefined); | |
| const rawRequest = vi.spyOn(client as unknown as { rawRequest: (opts: unknown) => Promise<unknown> }, 'rawRequest'); | |
| rawRequest.mockResolvedValueOnce({ subscriptionId: 'whsub_123' }).mockResolvedValueOnce({ ok: true }); |
| }) | ||
| ).resolves.toEqual({ subscriptionId: 'whsub_123' }); | ||
|
|
||
| await expect(client.deleteWebhookSubscription('whsub_123', 'demo')).resolves.toBeUndefined(); |
There was a problem hiding this comment.
Update the assertion to expect { ok: true } instead of undefined to match the actual response of the daemon.
| await expect(client.deleteWebhookSubscription('whsub_123', 'demo')).resolves.toBeUndefined(); | |
| await expect(client.deleteWebhookSubscription('whsub_123', 'demo')).resolves.toEqual({ ok: true }); |
Relayfile Eval ReviewRun: Passed: 4 | Needs human: 0 | Reviewable: 0 | Missing output: 0 | Failed: 0 | Skipped: 0 Human Review CasesNo reviewable human-review cases captured Relayfile output. |
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| "publishConfig": { | ||
| "access": "public", | ||
| "provenance": true | ||
| }, |
There was a problem hiding this comment.
🟡 Scoped npm package loses public access configuration, breaking publishing workflow
The publish configuration is removed (publishConfig at packages/client/package.json:32-35) unlike every other scoped package in the monorepo, so npm publish will default to restricted access for this @relayfile/client package and fail in CI.
Impact: Automated or manual npm publishing of this package will fail or produce a private package instead of a public one.
All other monorepo packages retain publishConfig with access: public
Every other package in packages/ (core, file-observer, local-mount, cli, sdk/typescript, mount-*) retains publishConfig: { access: "public", provenance: true }. Scoped packages on npm default to restricted access without this setting. The removal appears accidental — this PR's stated purpose is adding webhook subscription support, not changing publishing behavior.
Prompt for agents
The publishConfig block was removed from packages/client/package.json but all other scoped packages in the monorepo retain it. Restore the publishConfig section with access: public and provenance: true to match the convention used by packages/core/package.json, packages/cli/package.json, packages/sdk/typescript/package.json, etc. This ensures the scoped @relayfile/client package can be published publicly via npm.
Was this helpful? React with 👍 or 👎 to provide feedback.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b787e29e80
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| createWebhookSubscription(input: WebhookSubscriptionRequestBody): Promise<WebhookSubscriptionResult> { | ||
| return this.request<WebhookSubscriptionResult>({ | ||
| method: 'POST', | ||
| path: '/v1/integrations/webhook-subscriptions', | ||
| body: input, | ||
| }); |
There was a problem hiding this comment.
When this new method is used against any older relayfile daemon that still passes ensureReady() (the client still accepts API v1 and MIN_RELAYFILE_VERSION 0.10.17), the daemon will not have registered /v1/integrations/webhook-subscriptions, so callers get a generic non-JSON 404/DAEMON_UNAVAILABLE instead of the existing actionable version-incompatible error. Please bump the control-plane/minimum daemon version or feature-gate this endpoint before exposing the client method.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in d422ef6 — the control-plane client now negotiates API v2, and the daemon advertises v2 support for the webhook subscription endpoint while still accepting v1 clients.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
3 issues found across 7 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="cmd/relayfile-cli/control_plane.go">
<violation number="1" location="cmd/relayfile-cli/control_plane.go:231">
P1: The unbind handler (`handleControlPlaneUnbind`) only removes the local relay integration binding and does not clean up the corresponding relayfile-cloud webhook subscription. This creates a lifecycle leak: after unbind, the cloud subscription remains active and can continue delivering inbound webhooks even though the local binding no longer exists. You should persist the subscription ID on the binding so that unbind can proxy a DELETE to `/v1/workspaces/{workspaceID}/webhooks/{subscriptionID}` before (or as part of) removing the local binding.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| mux.HandleFunc("/v1/integrations/bindings", withControlPlaneVersion(handleControlPlaneListBindings)) | ||
| mux.HandleFunc("/v1/integrations/unbind", withControlPlaneVersion(handleControlPlaneUnbind)) | ||
| mux.HandleFunc("/v1/integrations/writeback-secret", withControlPlaneVersion(handleControlPlaneWritebackSecret)) | ||
| mux.HandleFunc("/v1/integrations/webhook-subscriptions", withControlPlaneVersion(handleControlPlaneWebhookSubscription)) |
There was a problem hiding this comment.
P1: The unbind handler (handleControlPlaneUnbind) only removes the local relay integration binding and does not clean up the corresponding relayfile-cloud webhook subscription. This creates a lifecycle leak: after unbind, the cloud subscription remains active and can continue delivering inbound webhooks even though the local binding no longer exists. You should persist the subscription ID on the binding so that unbind can proxy a DELETE to /v1/workspaces/{workspaceID}/webhooks/{subscriptionID} before (or as part of) removing the local binding.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At cmd/relayfile-cli/control_plane.go, line 231:
<comment>The unbind handler (`handleControlPlaneUnbind`) only removes the local relay integration binding and does not clean up the corresponding relayfile-cloud webhook subscription. This creates a lifecycle leak: after unbind, the cloud subscription remains active and can continue delivering inbound webhooks even though the local binding no longer exists. You should persist the subscription ID on the binding so that unbind can proxy a DELETE to `/v1/workspaces/{workspaceID}/webhooks/{subscriptionID}` before (or as part of) removing the local binding.</comment>
<file context>
@@ -210,6 +228,7 @@ func newControlPlaneHandler() http.Handler {
mux.HandleFunc("/v1/integrations/bindings", withControlPlaneVersion(handleControlPlaneListBindings))
mux.HandleFunc("/v1/integrations/unbind", withControlPlaneVersion(handleControlPlaneUnbind))
mux.HandleFunc("/v1/integrations/writeback-secret", withControlPlaneVersion(handleControlPlaneWritebackSecret))
+ mux.HandleFunc("/v1/integrations/webhook-subscriptions", withControlPlaneVersion(handleControlPlaneWebhookSubscription))
return mux
}
</file context>
There was a problem hiding this comment.
Thanks for flagging this. I am not expanding this PR to make unbind proxy cloud subscription deletion: the local unbind path currently only owns local binding removal and does not carry the authenticated cloud workspace/session context needed to delete relayfile-cloud webhooks safely. The binding already stores subscriptionId for that follow-up lifecycle work, but tying teardown into unbind is a cross-repo behavior change tracked separately. This PR stays scoped to exposing create/delete webhook subscription proxy operations and the typed client API.
There was a problem hiding this comment.
The unbind cleanup point is fair in general, but it’s out of scope for this PR. This change only adds the webhook-subscription proxy operations and typed client API; the unbind teardown is a separate cross-repo behavior change.
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
1 participant
Part of the inbound integration bridge (provider → relay agent, server-side, no client process). Companion PRs: relaycast (ingress), relaycast-cloud (secret+expose), relay (CLI subscribe), relayfile (control-plane proxy), relayfile-cloud (snapshot enrichment). Cross-repo — review/merge together.
Local control-plane proxies webhook_subscription create/delete to relayfile-cloud; TS client + generated OpenAPI types updated.
Tests:
@relayfile/client(10 passing) + typecheck +go test ./cmd/relayfile-cli -run TestControlPlaneCloudIntegrationConformance.Review status
3-way fresh-eyes review completed. Fixed: fail-closed HMAC secret normalization, 5xx-on-transient (no silent loss), require-dedupe-key (no double-inject). Verified correct: exactly-once node delivery, snapshot bounds, signing order.
unsubscribemust delete the relayfile-cloud webhook_subscription (persist its id on the binding) — currently unsubscribe does not stop inbound delivery (cross-repo: relay CLI + relayfile daemon)cast.agentrelay.comtoRELAYFILE_WEBHOOK_HOST_ALLOWLISTfor the preview/prod host🤖 Generated with Claude Code