Fix webhook list empty URL column and truncate long event lists#134
Conversation
…ists The WorkOS API returns endpoint_url (not url) in webhook endpoint responses. The CLI was reading ep.url which resolved to undefined, causing an empty URL column in workos webhook list output. - Rename url to endpoint_url in WebhookEndpoint interface and emulator entity to match the real API response shape - Update webhook list command to read ep.endpoint_url - Truncate long event lists at 60 chars in table output - Update emulator routes to accept endpoint_url (with url fallback) - Update all related tests and mocks Closes #133 Co-Authored-By: nick.nisi@workos.com <nick@nisi.org>
📝 WalkthroughWalkthroughWebhook endpoint URL property renamed from Changes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Greptile SummaryThis PR fixes a bug where Confidence Score: 5/5Safe to merge — the rename is applied uniformly across all layers, backward-compat fallbacks are in place, and the previously flagged issues in prior review threads have been resolved. No P0 or P1 issues found. The No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["CLI: runWebhookList(apiKey)"] --> B["WorkOS SDK\nclient.webhooks.list()"]
B --> C["WebhookEndpoint\n{ endpoint_url, events, ... }"]
C --> D{joined events\n≤ 60 chars?}
D -- Yes --> E["Row: id, endpoint_url,\nfull events, created_at"]
D -- No --> F["Greedy pack visible[]\nwhile len ≤ 60"]
F --> G["suffix = ', … (+hidden more)'"]
G --> H["Row: id, endpoint_url,\ntruncated events, created_at"]
E --> I["formatTable() → console.log"]
H --> I
subgraph Emulator["Emulator (backwards-compat)"]
J["POST/PUT body\n{url} or {endpoint_url}"] --> K["endpoint_url ?? url"]
K --> L["WorkOSWebhookEndpoint\n{ endpoint_url }"]
M["seedFromConfig\n{url} or {endpoint_url}"] --> N["endpoint_url ?? url\n+ required check"]
N --> L
end
Reviews (4): Last reviewed commit: "fix: tighten seed validation for webhook..." | Re-trigger Greptile |
- Truncate events at whole-token boundaries and show hidden count instead of total (e.g. '… (+3 more)' not '… (+5)') - Add url fallback in seedFromConfig for legacy seed configs - Add regression tests for legacy url input on POST/PUT Co-Authored-By: nick.nisi@workos.com <nick@nisi.org>
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/emulate/workos/index.ts (1)
320-331:⚠️ Potential issue | 🟠 MajorTighten seed validation before insert.
if (!endpointUrl)still lets truthy non-string values through, so malformed seed configs can persist invalid webhook endpoints. Match the route handler’stypeofguard here before inserting.🔧 Proposed fix
- const endpointUrl = whConfig.endpoint_url ?? whConfig.url; - if (!endpointUrl) { + const endpointUrl = whConfig.endpoint_url ?? whConfig.url; + if (!endpointUrl || typeof endpointUrl !== 'string') { throw new Error('workos seed config: webhookEndpoints[].endpoint_url is required'); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/emulate/workos/index.ts` around lines 320 - 331, The current seed logic accepts truthy non-string endpoint values; before calling ws.webhookEndpoints.insert when building endpointUrl from whConfig.endpoint_url ?? whConfig.url, validate that the resolved endpointUrl is a string (e.g., typeof endpointUrl === 'string') and throw the same error if not; update the check around endpointUrl (and/or inspect whConfig.endpoint_url and whConfig.url individually) so only string endpoint URLs are inserted by ws.webhookEndpoints.insert.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/emulate/workos/index.ts`:
- Around line 320-331: The current seed logic accepts truthy non-string endpoint
values; before calling ws.webhookEndpoints.insert when building endpointUrl from
whConfig.endpoint_url ?? whConfig.url, validate that the resolved endpointUrl is
a string (e.g., typeof endpointUrl === 'string') and throw the same error if
not; update the check around endpointUrl (and/or inspect whConfig.endpoint_url
and whConfig.url individually) so only string endpoint URLs are inserted by
ws.webhookEndpoints.insert.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: c51edaf8-2f98-495f-b5ee-2ea1782d1753
📒 Files selected for processing (3)
src/commands/webhook.tssrc/emulate/workos/index.tssrc/emulate/workos/routes/webhook-endpoints.spec.ts
Original prompt from nick.nisi@workos.com
|
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
Co-Authored-By: nick.nisi@workos.com <nick@nisi.org>
Summary
Fixes #133.
workos webhook listshows an emptyURLcolumn because the WorkOS API returnsendpoint_urlin webhook endpoint responses, but the CLI readsep.url(which isundefined). Additionally, when a webhook is subscribed to many events, theEventscolumn joins all event names into a single cell that can exceed the terminal width.Changes:
WebhookEndpointinterface (src/lib/workos-client.ts): Renamedurl→endpoint_urlto match the real API response shape.webhook.tslist command: Readep.endpoint_urlinstead ofep.url; truncate events at whole-token boundaries with a hidden count suffix (e.g.event.a, event.b, … (+3 more)).url→endpoint_urlthroughout; emulator POST/PUT accept bothendpoint_url(matching real API) andurl(backwards-compat fallback).WorkOSSeedWebhookEndpointaccepts bothendpoint_urland deprecatedurl;seedFromConfigresolves with fallback, validates presence and type.endpoint_url; added regression tests for legacyurlinput on create and update, and for event truncation.Review & Testing Checklist for Human
GET /webhook_endpointsreturnsendpoint_url(noturl) — the fix is based on API docs + the issue reportworkos webhook listagainst a real environment with multiple webhooks subscribed to many events and confirm the URL column is populated and events are truncated cleanlyworkos emulate→ create/list/delete webhook endpoints)Notes
All 1618 tests pass, typecheck passes, build passes.
Link to Devin session: https://app.devin.ai/sessions/c4287ca8c287477290199d58c4329b6d
Requested by: @nicknisi