Webhooks cli #2669
Merged
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughAdds a full webhooks CLI subtree (list/add/update/delete/enable/disable + event list/resend/payload), manager and parsing helpers, shared interactive pickers/formatters, TUI Select/MultiSelect/TextInput behavior changes, prompt-trail clearing, platform error formatting, and comprehensive tests for commands and prompts. ChangesCLI command wiring
Webhooks lib & shared helpers
Commands (add/list/delete/enable/disable + events)
Update command
TUI / UI changes
Platform error handling
Tests
🎯 4 (Complex) | ⏱️ ~45 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ 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)
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 |
Contributor
|
View Vercel preview at instant-www-js-webhooks-cli-new-jsv.vercel.app. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
client/packages/cli/src/ui/index.ts (1)
196-197: ⚡ Quick winConsider simplifying the Promise resolution.
The current pattern
Promise.resolve().then(() => exp())works but could be more direct:Promise.resolve(exp())This handles both sync and async returns uniformly without the extra then() wrapper.
🤖 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 `@client/packages/cli/src/ui/index.ts` around lines 196 - 197, Replace the indirect Promise.resolve().then(() => (exp as () => string | Promise<string>)()) pattern with a direct Promise.resolve(exp()) to simplify handling of both synchronous and asynchronous returns; locate the occurrence where the variable exp (cast as () => string | Promise<string>) is invoked and change it to pass exp() directly into Promise.resolve so the call semantics remain the same but the wrapper then() is removed.
🤖 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 `@client/packages/cli/__tests__/webhooks.test.ts`:
- Around line 13-35: The test suite currently overrides fetchRecentEvents in the
module mock, causing the pagination tests to exercise the test-local
implementation instead of the real client/packages/cli/src/lib/webhooks.ts code;
update the mock so it does not replace fetchRecentEvents (e.g., spread the
original module and keep orig.fetchRecentEvents) and only mock
useWebhooksManager, getRemoteEtypes, buildWebhooksManager or other helpers that
need stubbing, ensuring the real fetchRecentEvents implementation (the one
exported as fetchRecentEvents) is exercised against state.manager for pagination
behavior.
---
Nitpick comments:
In `@client/packages/cli/src/ui/index.ts`:
- Around line 196-197: Replace the indirect Promise.resolve().then(() => (exp as
() => string | Promise<string>)()) pattern with a direct Promise.resolve(exp())
to simplify handling of both synchronous and asynchronous returns; locate the
occurrence where the variable exp (cast as () => string | Promise<string>) is
invoked and change it to pass exp() directly into Promise.resolve so the call
semantics remain the same but the wrapper then() is removed.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 18a28c42-c26c-42a3-ac93-14672a26939d
📒 Files selected for processing (16)
client/packages/cli/__tests__/multiSelect.test.tsclient/packages/cli/__tests__/select.test.tsclient/packages/cli/__tests__/webhooks.test.tsclient/packages/cli/src/commands/webhooks/add.tsclient/packages/cli/src/commands/webhooks/delete.tsclient/packages/cli/src/commands/webhooks/disable.tsclient/packages/cli/src/commands/webhooks/enable.tsclient/packages/cli/src/commands/webhooks/events/list.tsclient/packages/cli/src/commands/webhooks/events/payload.tsclient/packages/cli/src/commands/webhooks/events/resend.tsclient/packages/cli/src/commands/webhooks/list.tsclient/packages/cli/src/commands/webhooks/shared.tsclient/packages/cli/src/commands/webhooks/update.tsclient/packages/cli/src/index.tsclient/packages/cli/src/lib/webhooks.tsclient/packages/cli/src/ui/index.ts
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (3)
client/packages/cli/src/commands/webhooks/update.ts (1)
68-70: ⚡ Quick winNormalize and validate URL input before building update params.
Both flag-driven and interactive paths pass URL values through as-is. Trimming + local URL validation here gives consistent, earlier feedback instead of deferring to backend failure messages.
💡 Proposed refactor
+const normalizeWebhookUrl = (raw: string) => { + const value = raw.trim(); + if (!value) return undefined; + try { + // Throws on invalid absolute URL + new URL(value); + return value; + } catch { + return undefined; + } +}; ... - if (opts.url) params.url = opts.url; + if (opts.url) { + const url = normalizeWebhookUrl(opts.url); + if (!url) { + return yield* BadArgsError.make({ message: '--url must be a valid URL' }); + } + params.url = url; + } ... - if (opts.url) params.url = opts.url; + if (opts.url) { + const url = normalizeWebhookUrl(opts.url); + if (!url) { + return yield* BadArgsError.make({ message: '--url must be a valid URL' }); + } + params.url = url; + } ... - pending.url = yield* runUIEffect( + const nextUrl = yield* runUIEffect( new UI.TextInput({ prompt: 'Webhook URL:', defaultValue: seed, placeholder: seed, modifyOutput: UI.modifiers.piped([ UI.modifiers.topPadding, UI.modifiers.dimOnComplete, ]), }), ); + const normalized = normalizeWebhookUrl(nextUrl); + if (!normalized) { + return yield* BadArgsError.make({ message: '--url must be a valid URL' }); + } + pending.url = normalized;Also applies to: 89-91, 151-163
🤖 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 `@client/packages/cli/src/commands/webhooks/update.ts` around lines 68 - 70, Trim and validate any URL input before assigning it to params.url in the update flow: normalize opts.url (trim whitespace) and run a local URL validation routine (reject empty/invalid URLs) before setting params.url (same change must be applied where interactive answers are assigned to params.url in the other update branches at the locations corresponding to lines 89-91 and 151-163). If validation fails, surface a clear CLI error/exit instead of adding the raw value to params; keep the handling for optsEtypes and optsActions unchanged.client/packages/cli/src/commands/webhooks/shared.ts (2)
280-280: 💤 Low valueConsider improving error object formatting.
The fallback
err?.message ?? errwill render[object Object]for error objects that lack amessageproperty. UsingString(err)orerr instanceof Error ? err.message : String(err)would provide a more readable fallback.🔍 Suggested improvement
- return `${base}\n${chalk.red(` Payload error: ${err?.message ?? err}`)}`; + return `${base}\n${chalk.red(` Payload error: ${err instanceof Error ? err.message : String(err)}`)}`;🤖 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 `@client/packages/cli/src/commands/webhooks/shared.ts` at line 280, The error formatting currently uses `err?.message ?? err`, which can produce `[object Object]`; update the template to normalize the error by converting non-Error values to strings and using Error.message for Error instances (e.g., replace `err?.message ?? err` with `err instanceof Error ? err.message : String(err)`) so the returned string built from `base` and `err` is always readable.
204-204: 💤 Low valueConsider documenting the magic number
13.The subtraction of
13from terminal columns likely accounts for indentation and the "body: " prefix. A brief comment would improve clarity for future maintainers.📝 Suggested clarification
- const max = Math.max(20, (process.stdout.columns ?? 80) - 13); + // Reserve space for indentation (9) + "body: " (4) + const max = Math.max(20, (process.stdout.columns ?? 80) - 13);🤖 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 `@client/packages/cli/src/commands/webhooks/shared.ts` at line 204, The expression computing max uses a magic subtractor 13 which is unclear; update the code around the const max = Math.max(20, (process.stdout.columns ?? 80) - 13) in shared.ts to either add a short inline comment explaining that 13 accounts for the fixed prefix width (e.g., indentation + "body: " label) or replace the literal with a well-named constant (e.g., BODY_PREFIX_WIDTH = 13) and document that constant so future maintainers know why 13 is subtracted.
🤖 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.
Nitpick comments:
In `@client/packages/cli/src/commands/webhooks/shared.ts`:
- Line 280: The error formatting currently uses `err?.message ?? err`, which can
produce `[object Object]`; update the template to normalize the error by
converting non-Error values to strings and using Error.message for Error
instances (e.g., replace `err?.message ?? err` with `err instanceof Error ?
err.message : String(err)`) so the returned string built from `base` and `err`
is always readable.
- Line 204: The expression computing max uses a magic subtractor 13 which is
unclear; update the code around the const max = Math.max(20,
(process.stdout.columns ?? 80) - 13) in shared.ts to either add a short inline
comment explaining that 13 accounts for the fixed prefix width (e.g.,
indentation + "body: " label) or replace the literal with a well-named constant
(e.g., BODY_PREFIX_WIDTH = 13) and document that constant so future maintainers
know why 13 is subtracted.
In `@client/packages/cli/src/commands/webhooks/update.ts`:
- Around line 68-70: Trim and validate any URL input before assigning it to
params.url in the update flow: normalize opts.url (trim whitespace) and run a
local URL validation routine (reject empty/invalid URLs) before setting
params.url (same change must be applied where interactive answers are assigned
to params.url in the other update branches at the locations corresponding to
lines 89-91 and 151-163). If validation fails, surface a clear CLI error/exit
instead of adding the raw value to params; keep the handling for optsEtypes and
optsActions unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: f35d60bd-6799-407c-8acb-371d1933cb02
📒 Files selected for processing (8)
client/packages/cli/__tests__/webhooks.test.tsclient/packages/cli/src/commands/webhooks/add.tsclient/packages/cli/src/commands/webhooks/events/list.tsclient/packages/cli/src/commands/webhooks/events/payload.tsclient/packages/cli/src/commands/webhooks/events/resend.tsclient/packages/cli/src/commands/webhooks/shared.tsclient/packages/cli/src/commands/webhooks/update.tsclient/packages/cli/src/ui/index.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- client/packages/cli/src/commands/webhooks/events/list.ts
- client/packages/cli/src/commands/webhooks/events/resend.ts
- client/packages/cli/src/commands/webhooks/events/payload.ts
- client/packages/cli/src/commands/webhooks/add.ts
- client/packages/cli/tests/webhooks.test.ts
- client/packages/cli/src/ui/index.ts
drew-harris
reviewed
May 14, 2026
drew-harris
reviewed
May 14, 2026
drew-harris
reviewed
May 14, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
client/packages/cli/src/ui/lib.ts (1)
277-281: ⚡ Quick winRow count ignores line wrapping; wide prompts will under-erase.
this.text.match(/\n/g)only counts logical newlines, but the terminal actually consumes one row per visual line — i.e. wrapped lines occupy1 + floor((width-1)/columns)rows, which is precisely what the existingclear()helper computes. For prompts whose rendered lines exceedstdout.columns(common with long URLs, labels with timestamps, etc.),clearPromptTrailwill move the cursor up fewer rows than were actually written and leave residue on screen.Consider reusing the same row-counting logic as
clear()to stay consistent:♻️ Suggested refactor
- if (this.status === 'submitted' && this.text.trim().length > 0) { - promptTrailLineCount += (this.text.match(/\n/g) ?? []).length; - } + if (this.status === 'submitted' && this.text.trim().length > 0) { + const perLine = this.stdout.columns; + const lines = this.text.split(/\r?\n/); + // Drop trailing empty line produced by the forced trailing `\n`. + if (lines.length > 0 && lines[lines.length - 1] === '') lines.pop(); + for (const line of lines) { + promptTrailLineCount += perLine + ? 1 + Math.floor(Math.max(stringWidth(line) - 1, 0) / perLine) + : 1; + } + }🤖 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 `@client/packages/cli/src/ui/lib.ts` around lines 277 - 281, The prompt row counting currently uses this.text.match(/\n/g) which counts logical newlines only; change promptTrailLineCount logic in the prompt-tracking code (where it checks this.status === 'submitted') to compute visual rows per line using the same formula as clear(): split this.text by '\n' and for each non-empty rendered line add 1 + Math.floor((Math.max(0, line.length - 1)) / process.stdout.columns) (or reuse the clear() helper if available) so wrapped lines contribute the correct number of terminal rows based on process.stdout.columns.
🤖 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 `@client/packages/cli/src/commands/webhooks/shared.ts`:
- Around line 97-105: The code returns params.id without applying the same
eligibility checks used by the interactive picker, so change the explicit id
path to validate the provided id with the picker filter before accepting it:
when params.id is present, resolve the picker via params.picker (same one passed
to pickWebhook) and run its .filter (or call the same validation logic used by
pickWebhook) against the target id; if the filter rejects it, behave like the
picker branch (return BadArgsError.make with the same message or prompt
behavior, respecting GlobalOpts yes), otherwise return the id; apply the same
change to the other occurrence around the 318-329 block so both explicit --id
and interactive paths enforce the picker.filter rules (refer to params.id,
params.picker, pickWebhook, BadArgsError, and GlobalOpts/yes).
- Around line 161-162: The formatted timestamp helper fmtTime currently strips
the trailing 'Z' from toISOString(), which removes the timezone marker; update
fmtTime so it still removes fractional seconds but preserves the trailing 'Z'
(UTC marker). Locate fmtTime and change the string transforms so you replace 'T'
with a space and strip the fractional-second portion only (e.g.
replace(/\.\d+Z$/, 'Z')) instead of removing the final 'Z', returning something
like "YYYY-MM-DD HH:MM:SSZ" for non-null Date values.
In `@client/packages/cli/src/ui/lib.ts`:
- Around line 185-190: clearPromptTrail currently emits a malformed ANSI
sequence (missing leading ESC) and doesn't perform an erase, so it prints
literal characters like "[5A" and leaves trailing text; update clearPromptTrail
to use the existing sisteransi helpers (cursor and erase) instead of hand-rolled
escapes: move the cursor up by promptTrailLineCount using cursor.up or
cursor.move and then call erase.screen or erase.down (or the appropriate erase
function) to clear from the cursor to end of screen, and finally reset
promptTrailLineCount = 0; reference clearPromptTrail and promptTrailLineCount to
locate where to replace the manual template string with calls into the imported
cursor and erase utilities.
---
Nitpick comments:
In `@client/packages/cli/src/ui/lib.ts`:
- Around line 277-281: The prompt row counting currently uses
this.text.match(/\n/g) which counts logical newlines only; change
promptTrailLineCount logic in the prompt-tracking code (where it checks
this.status === 'submitted') to compute visual rows per line using the same
formula as clear(): split this.text by '\n' and for each non-empty rendered line
add 1 + Math.floor((Math.max(0, line.length - 1)) / process.stdout.columns) (or
reuse the clear() helper if available) so wrapped lines contribute the correct
number of terminal rows based on process.stdout.columns.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 50420ba1-d412-4e66-a1a3-7e21e83543ec
📒 Files selected for processing (5)
client/packages/cli/src/commands/webhooks/add.tsclient/packages/cli/src/commands/webhooks/shared.tsclient/packages/cli/src/commands/webhooks/update.tsclient/packages/cli/src/ui/index.tsclient/packages/cli/src/ui/lib.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- client/packages/cli/src/commands/webhooks/update.ts
- client/packages/cli/src/commands/webhooks/add.ts
- client/packages/cli/src/ui/index.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
client/packages/cli/src/commands/webhooks/update.ts (1)
68-75: ⚡ Quick winExtract shared flag-to-params construction.
The same
UpdateWebhookParamsbuild/validation block is duplicated across both non-interactive flows. A small helper would reduce drift risk (especially around URL normalization/validation).Refactor sketch
+const buildUpdateParams = Effect.fn(function* ({ + url, + etypes, + actions, +}: { + url?: string; + etypes?: readonly string[]; + actions?: readonly WebhookAction[]; +}) { + const params: UpdateWebhookParams<any> = {}; + if (url) { + const normalizedUrl = url.trim(); + const err = validateWebhookUrl(normalizedUrl); + if (err) return yield* BadArgsError.make({ message: err }); + params.url = normalizedUrl; + } + if (etypes) params.etypes = etypes; + if (actions) params.actions = actions; + return params; +}); ... - const params: UpdateWebhookParams<any> = {}; - ... + const params = yield* buildUpdateParams({ + url: opts.url, + etypes: optsEtypes, + actions: optsActions, + }); ... - const params: UpdateWebhookParams<any> = {}; - ... + const params = yield* buildUpdateParams({ + url: opts.url, + etypes: optsEtypes, + actions: optsActions, + });Also applies to: 93-100
🤖 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 `@client/packages/cli/src/commands/webhooks/update.ts` around lines 68 - 75, Extract the duplicated UpdateWebhookParams construction/validation into a small helper (e.g., buildUpdateWebhookParams) that accepts the same inputs used here (opts, optsEtypes, optsActions), runs validateWebhookUrl and normalizes/trims url, sets etypes/actions when present, and returns a tuple or object like { params, err } so both non-interactive flows can call it and do "if (err) return yield* BadArgsError.make({ message: err });". Ensure the helper performs the exact same logic now duplicated (use UpdateWebhookParams type, validateWebhookUrl, trim the url, assign params.url/params.etypes/params.actions) so callers simply consume returned params when err is null.
🤖 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 `@client/packages/cli/src/commands/webhooks/update.ts`:
- Around line 69-73: The webhook URL validation is using the raw input with
surrounding whitespace instead of the trimmed value; change the branches that
handle opts.url (the flag-driven block using validateWebhookUrl and assignments
to params.url) and the interactive URL validator to trim the input first, then
call validateWebhookUrl on the trimmed string and assign the trimmed value to
params.url (and any interactive variables) only after successful validation;
ensure calls to validateWebhookUrl everywhere (including the blocks referenced
around the validateWebhookUrl call and the interactive validator) receive
opts.url.trim() or the trimmed interactive input.
---
Nitpick comments:
In `@client/packages/cli/src/commands/webhooks/update.ts`:
- Around line 68-75: Extract the duplicated UpdateWebhookParams
construction/validation into a small helper (e.g., buildUpdateWebhookParams)
that accepts the same inputs used here (opts, optsEtypes, optsActions), runs
validateWebhookUrl and normalizes/trims url, sets etypes/actions when present,
and returns a tuple or object like { params, err } so both non-interactive flows
can call it and do "if (err) return yield* BadArgsError.make({ message: err
});". Ensure the helper performs the exact same logic now duplicated (use
UpdateWebhookParams type, validateWebhookUrl, trim the url, assign
params.url/params.etypes/params.actions) so callers simply consume returned
params when err is null.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 54740fb4-c99c-4e7a-873a-69b64d9ca1e6
📒 Files selected for processing (6)
client/packages/cli/src/commands/webhooks/add.tsclient/packages/cli/src/commands/webhooks/events/payload.tsclient/packages/cli/src/commands/webhooks/events/resend.tsclient/packages/cli/src/commands/webhooks/update.tsclient/packages/cli/src/index.tsclient/packages/cli/src/layer.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- client/packages/cli/src/layer.ts
- client/packages/cli/src/commands/webhooks/events/payload.ts
- client/packages/cli/src/commands/webhooks/add.ts
- client/packages/cli/src/commands/webhooks/events/resend.ts
nezaj
reviewed
May 14, 2026
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (3)
client/packages/cli/src/commands/webhooks/shared.ts (3)
295-296:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix the payload truncation hint command string.
Line 295 suggests
instant-cli webhooks events payload --isn ..., but the registered command path in this PR isinstant-cli webhook event payload, and including--webhook-idavoids reprompting.Suggested fix
- ` … ${omitted} more lines · run \`instant-cli webhooks events payload --isn ${e.isn}\` for full output`, + ` … ${omitted} more lines · run \`instant-cli webhook event payload --webhook-id ${params.webhookId} --isn ${e.isn}\` for full output`,🤖 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 `@client/packages/cli/src/commands/webhooks/shared.ts` around lines 295 - 296, The payload truncation hint uses the wrong command and omits the webhook id; update the hint text in shared.ts so it instructs users to run the registered command path "instant-cli webhook event payload" and include the webhook id flag to avoid reprompting (e.g., use --webhook-id <webhookId> and --isn <isn>), referencing the existing e.webhookId and e.isn variables used in the template string.
161-162:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winPreserve UTC marker when formatting timestamps.
Line 162 strips the trailing
Z, so rendered times look timezone-less even though they come from UTCtoISOString().Suggested fix
const fmtTime = (d: Date | null) => - d ? d.toISOString().replace('T', ' ').replace(/\..*$/, '') : 'n/a'; + d ? d.toISOString().replace('T', ' ').replace(/\.\d+Z$/, 'Z') : 'n/a';🤖 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 `@client/packages/cli/src/commands/webhooks/shared.ts` around lines 161 - 162, The timestamp formatter fmtTime currently removes the trailing Z from toISOString(), making UTC timestamps look timezone-less; update fmtTime (the const fmtTime function) to preserve the UTC marker by either adjusting the replace/regex to keep the trailing 'Z' (e.g., strip milliseconds but retain 'Z') or by appending 'Z' after removing milliseconds, so the output remains "YYYY-MM-DD hh:mm:ssZ" (or similar) to clearly indicate UTC.
97-105:⚠️ Potential issue | 🟠 Major | ⚡ Quick winApply picker eligibility checks to explicit
--idpaths.Line 97 returns
params.idimmediately, and Line 323 validates against all webhooks instead of filtered candidates. This lets explicit IDs bypasspicker.filterrules and diverges from interactive selection behavior.Suggested fix
export const resolveWebhookId = (params: { id: string | undefined; picker: PickerParams; flagName?: string; }) => Effect.gen(function* () { - if (params.id) return params.id; + if (params.id) { + const webhook = yield* resolveWebhook(params); + return webhook.id; + } const { yes } = yield* GlobalOpts; if (yes) { return yield* BadArgsError.make({ message: `Must specify ${params.flagName ?? '--id'}`, }); @@ export const resolveWebhook = (params: { id: string | undefined; picker: PickerParams; flagName?: string; }) => Effect.gen(function* () { if (params.id) { const all = yield* useWebhooksManager( (m) => m.list(), 'Error listing webhooks', ); - const found = all.find((w) => w.id === params.id); + const candidates = params.picker.filter + ? all.filter(params.picker.filter) + : all; + const found = candidates.find((w) => w.id === params.id); if (!found) { return yield* BadArgsError.make({ message: `No webhook found with id ${params.id}`, }); }Also applies to: 318-329
🤖 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 `@client/packages/cli/src/commands/webhooks/shared.ts` around lines 97 - 105, When an explicit params.id is provided the code currently returns it without enforcing the picker eligibility rules; update the logic so that if params.id exists you still run the picker.filter (the same candidate filtering used by pickWebhook) and validate that params.id is present in the filtered candidates before returning it, otherwise call BadArgsError.make with the same message; use the existing picker reference (params.picker / picker.filter) and the same BadArgsError.make / params.flagName flow so explicit --id paths behave identically to interactive picks.
🤖 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.
Duplicate comments:
In `@client/packages/cli/src/commands/webhooks/shared.ts`:
- Around line 295-296: The payload truncation hint uses the wrong command and
omits the webhook id; update the hint text in shared.ts so it instructs users to
run the registered command path "instant-cli webhook event payload" and include
the webhook id flag to avoid reprompting (e.g., use --webhook-id <webhookId> and
--isn <isn>), referencing the existing e.webhookId and e.isn variables used in
the template string.
- Around line 161-162: The timestamp formatter fmtTime currently removes the
trailing Z from toISOString(), making UTC timestamps look timezone-less; update
fmtTime (the const fmtTime function) to preserve the UTC marker by either
adjusting the replace/regex to keep the trailing 'Z' (e.g., strip milliseconds
but retain 'Z') or by appending 'Z' after removing milliseconds, so the output
remains "YYYY-MM-DD hh:mm:ssZ" (or similar) to clearly indicate UTC.
- Around line 97-105: When an explicit params.id is provided the code currently
returns it without enforcing the picker eligibility rules; update the logic so
that if params.id exists you still run the picker.filter (the same candidate
filtering used by pickWebhook) and validate that params.id is present in the
filtered candidates before returning it, otherwise call BadArgsError.make with
the same message; use the existing picker reference (params.picker /
picker.filter) and the same BadArgsError.make / params.flagName flow so explicit
--id paths behave identically to interactive picks.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 0a040c39-f65d-47a6-a9e0-7c4c5f07e4b9
📒 Files selected for processing (7)
client/packages/cli/__tests__/webhooks.test.tsclient/packages/cli/src/commands/webhooks/add.tsclient/packages/cli/src/commands/webhooks/list.tsclient/packages/cli/src/commands/webhooks/shared.tsclient/packages/cli/src/commands/webhooks/update.tsclient/packages/cli/src/index.tsclient/packages/cli/src/lib/webhooks.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- client/packages/cli/src/commands/webhooks/list.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@client/packages/cli/src/commands/webhooks/update.ts`:
- Around line 37-42: The code treats an explicit --url "" as omitted because it
checks url truthiness; change the presence check to detect undefined (e.g., if
(typeof url !== 'undefined' || url !== undefined)) so empty strings are
validated, and trim first then run validateWebhookUrl(trimmed) and return
BadArgsError.make({ message: err }) when validation fails; update both
occurrences that reference url, validateWebhookUrl, and params.url (the block
around the first check and the similar block at the later occurrence) to use
this undefined-only presence check and trimming before validation.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 4e7b2ef0-7cef-4316-b157-9086ab997f32
📒 Files selected for processing (8)
client/packages/cli/__tests__/webhooks.test.tsclient/packages/cli/src/commands/webhooks/add.tsclient/packages/cli/src/commands/webhooks/list.tsclient/packages/cli/src/commands/webhooks/shared.tsclient/packages/cli/src/commands/webhooks/update.tsclient/packages/cli/src/index.tsclient/packages/cli/src/lib/webhooks.tsclient/packages/cli/src/ui/lib.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- client/packages/cli/src/commands/webhooks/list.ts
- client/packages/cli/src/ui/lib.ts
- client/packages/cli/src/index.ts
- client/packages/cli/tests/webhooks.test.ts
- client/packages/cli/src/commands/webhooks/add.ts
- client/packages/cli/src/lib/webhooks.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds cli commands for webhooks:
And events:
Adds a new multiselect component with filter for selecting multiple options.
When you're navigating through the events list to select an event to resend or a payload to view, you can hit tab to see more info. We'll use your terminal size to compress what we show you.
GIFs of all of the commands
instant-cli webhook addinstant-cli webhook listinstant-cli webhook updateinstant-cli webhook disableinstant-cli webhook enableinstant-cli webhook deleteinstant-cli webhook event listinstant-cli webhook event resendinstant-cli webhook event payload