Add static docs site and agent-friendly CLI output#10
Conversation
📝 Walkthrough<review_stack_artifact> </review_stack_artifact> ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/cli/src/commands/setup.ts (1)
283-304:⚠️ Potential issue | 🟠 Major | ⚡ Quick winNon-interactive “current target unreachable” output still ignores installed server state.
You added
serverInstalledfor interactive recovery here, but the JSON/non-TTY path still callscurrentTargetBrokenOutput(...), which always emitsinstall-server. This makes automation output inconsistent and can incorrectly suggest reinstalling an already installed server.Suggested fix
- if (flags.json || !process.stdin.isTTY) { - await printData(currentTargetBrokenOutput(target, readiness), flags.json ? 'json' : 'human') + if (flags.json || !process.stdin.isTTY) { + const serverInstalled = await isServerInstalled() + await printData(currentTargetBrokenOutput(target, readiness, serverInstalled), flags.json ? 'json' : 'human') return }-function currentTargetBrokenOutput(target: Target, readiness: Readiness): Record<string, unknown> { +function currentTargetBrokenOutput(target: Target, readiness: Readiness, serverInstalled: boolean): Record<string, unknown> { + const serverAction = installedServerAction(serverInstalled) return { state: 'current-target-unreachable', message: `Beeper CLI is set up for ${target.name ?? target.id}, but it is not reachable.`, target: publicTarget(target), readiness, recommendedAction: action('retry-current', `beeper setup -t ${target.id}`), availableActions: [ action('retry-current', `beeper setup -t ${target.id}`), action('use-desktop', 'beeper setup --desktop'), - action('install-server', 'beeper setup --server --install --yes'), + serverAction, action('connect-remote', 'beeper setup --remote <url>'), ], } }🤖 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/cli/src/commands/setup.ts` around lines 283 - 304, The non-interactive path always outputs "install-server" because currentTargetBrokenOutput is called without the actual installed state; update the flow so the result of isServerInstalled() (serverInstalled) is used to determine the JSON/non-TTY output—either by passing serverInstalled into currentTargetBrokenOutput or by adding logic in the non-interactive branch to emit "use-installed-server" when serverInstalled is true (identify the isServerInstalled() call and the currentTargetBrokenOutput(...) invocation to modify). Ensure the JSON/TTY output mirrors the interactive prompt choice text for option 3 instead of always suggesting installation.
🤖 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/npm/scripts/build.ts`:
- Around line 132-137: The milestone logic in the download progress handler
(variables percent, nextLoggedPercent, downloaded, total, function logStep) can
skip intermediate thresholds when percent jumps past multiple milestones; change
the check to a loop that while percent >= nextLoggedPercent (or percent === 100)
emits logStep for each milestone and increments nextLoggedPercent by 25 each
iteration (ensuring the final 100% is emitted with milestone 100), so every 25%
milestone is logged even if a single data event advances progress across several
thresholds.
- Around line 113-118: The download function follows redirects recursively
without a cap; add a maxRedirects parameter (e.g., maxRedirects = 10) and a
currentRedirects counter to the download function signature, increment it on
each redirect and if it exceeds the limit reject with a clear error instead of
recursing forever; update the redirect branch that calls download(nextURL,
destination) to call download(nextURL, destination, currentRedirects + 1,
maxRedirects) (or pass a default when initiating), and ensure all caller sites
provide the new parameter or rely on the default.
---
Outside diff comments:
In `@packages/cli/src/commands/setup.ts`:
- Around line 283-304: The non-interactive path always outputs "install-server"
because currentTargetBrokenOutput is called without the actual installed state;
update the flow so the result of isServerInstalled() (serverInstalled) is used
to determine the JSON/non-TTY output—either by passing serverInstalled into
currentTargetBrokenOutput or by adding logic in the non-interactive branch to
emit "use-installed-server" when serverInstalled is true (identify the
isServerInstalled() call and the currentTargetBrokenOutput(...) invocation to
modify). Ensure the JSON/TTY output mirrors the interactive prompt choice text
for option 3 instead of always suggesting installation.
🪄 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: 33f5d751-b2d8-4103-bce8-43f1439e85eb
📒 Files selected for processing (5)
packages/cli/README.mdpackages/cli/src/commands/setup.tspackages/cli/src/commands/watch.tspackages/cli/test/cli-smoke.tspackages/npm/scripts/build.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: test (ubuntu-latest / bun 1.3.10)
- GitHub Check: test (macos-latest / bun 1.3.10)
🧰 Additional context used
🪛 ESLint
packages/cli/src/commands/setup.ts
[error] 416-416: Do not use useless undefined.
(unicorn/no-useless-undefined)
[error] 641-641: Expected blank line before this statement.
(@stylistic/padding-line-between-statements)
[error] 656-656: Expected blank line before this statement.
(@stylistic/padding-line-between-statements)
[error] 672-672: Expected blank line before this statement.
(@stylistic/padding-line-between-statements)
🔇 Additional comments (4)
packages/cli/src/commands/watch.ts (1)
17-18: LGTM!packages/cli/README.md (1)
2418-2419: LGTM!packages/cli/src/commands/setup.ts (1)
106-107: LGTM!Also applies to: 254-275, 347-351, 406-435, 513-530, 626-690
packages/cli/test/cli-smoke.ts (1)
3-3: LGTM!Also applies to: 238-262
|
@indent review |
|
batuhan
changed the title
![indent[bot]](https://avatars.githubusercontent.com/in/1426746?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
packages/cli/src/commands/setup.ts (1)
170-173:⚠️ Potential issue | 🟠 Major | ⚡ Quick winJSON fallback still hardcodes
install-serverfor broken current target.The non-interactive/JSON branch still emits
install-serverinavailableActionsinstead of preferringuse-installed-serverwhen a local server is already present, which conflicts with this PR’s fallback objective.Suggested fix
- await printData(currentTargetBrokenOutput(target, readiness), flags.json ? 'json' : 'human') + const serverInstalled = await isServerInstalled() + await printData(currentTargetBrokenOutput(target, readiness, serverInstalled), flags.json ? 'json' : 'human') return } -function currentTargetBrokenOutput(target: Target, readiness: Readiness): Record<string, unknown> { +function currentTargetBrokenOutput(target: Target, readiness: Readiness, serverInstalled: boolean): Record<string, unknown> { return { @@ availableActions: [ action('retry-current', `beeper setup -t ${target.id}`), action('use-desktop', 'beeper setup --desktop'), - action('install-server', 'beeper setup --server --install --yes'), + installedServerAction(serverInstalled), action('connect-remote', 'beeper setup --remote <url>'), ], } }Also applies to: 708-720
packages/cli/src/commands/auth/logout.ts (1)
12-19:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winSkip the env-token hard error in dry-run mode.
Line 12’s guard runs before the dry-run return, so
auth logout --dry-runcan error even though it performs no mutation. Move the dry-run branch earlier (or gate the throw with!flags['dry-run']).Proposed fix
const { flags } = await this.parse(AuthLogout) ensureWritable(flags) const target = await resolveTarget({ target: flags.target, baseURL: flags['base-url'] }) + const token = target.auth?.accessToken + if (flags['dry-run']) { + await printDryRun('auth.logout', { target: target.id, baseURL: target.baseURL, hadToken: Boolean(token), revokeToken: Boolean(token) }, flags.json ? 'json' : 'human') + return + } if (process.env.BEEPER_ACCESS_TOKEN && !target.auth?.accessToken) { throw new Error('auth logout cannot clear BEEPER_ACCESS_TOKEN from the environment; unset it in the calling process.') } - const token = target.auth?.accessToken - if (flags['dry-run']) { - await printDryRun('auth.logout', { target: target.id, baseURL: target.baseURL, hadToken: Boolean(token), revokeToken: Boolean(token) }, flags.json ? 'json' : 'human') - return - }🤖 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/cli/src/commands/auth/logout.ts` around lines 12 - 19, The guard that throws when process.env.BEEPER_ACCESS_TOKEN is set runs before the dry-run branch, causing `auth logout --dry-run` to error; to fix, ensure the dry-run short-circuit happens before that throw (or wrap the throw in a `if (!flags['dry-run'])` check). Concretely, in the logout command move the `if (flags['dry-run']) { await printDryRun('auth.logout', ...) ; return }` block to run before checking `process.env.BEEPER_ACCESS_TOKEN && !target.auth?.accessToken`, or add `&& !flags['dry-run']` to that throw condition so `auth.logout --dry-run` does not throw while still preserving the original guard for real runs.packages/cli/src/commands/send/voice.ts (1)
23-25:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
send voice --dry-runshould not require writable mode.The command checks writability before the dry-run early return, so read-only mode can prevent preview-only execution.
Proposed fix
async run(): Promise<void> { const { flags } = await this.parse(SendVoice) - ensureWritable(flags) + if (!flags['dry-run']) ensureWritable(flags) const client = await createClient(flags)Also applies to: 39-42
🤖 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/cli/src/commands/send/voice.ts` around lines 23 - 25, The command enforces writable mode before checking dry-run; update SendVoice command so that the dry-run path bypasses writability: parse flags via this.parse(SendVoice), check flags.dryRun (or flags["dry-run"]) and return the preview path before calling ensureWritable(flags) or createClient(flags), and apply the same change to the other occurrence that currently calls ensureWritable before the dry-run check; keep references to ensureWritable, createClient and SendVoice when locating the code to reorder the checks.packages/cli/src/commands/accounts/add.ts (1)
30-33:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
--dry-runis blocked by the write guard.
ensureWritable(flags)runs before the dry-run early return, so read-only mode prevents previewing this command. Gate the write check when--dry-runis set.Proposed fix
async run(): Promise<void> { const { args, flags } = await this.parse(AccountsAdd) - ensureWritable(flags) const client = await createClient(flags) @@ if (flags['dry-run']) { await printDryRun('accounts.add', { @@ return } + + ensureWritable(flags) const step = await client.bridges.loginSessions.create(accountType.id, {Also applies to: 70-84
🤖 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/cli/src/commands/accounts/add.ts` around lines 30 - 33, The current run() in AccountsAdd calls ensureWritable(flags) before handling the --dry-run flag, which prevents previewing in read-only mode; modify run() (and any other locations in the same file around the 70-84 block) to check for flags['dry-run'] (or flags.dryRun) first and early-return/preview when set, and only call ensureWritable(flags) for non-dry-run flows; keep the same behavior and messaging but gate the write-guard behind the dry-run check so ensureWritable is skipped during dry runs.
🟡 Minor comments (25)
packages/cli/src/commands/chats/notify-anyway.ts-5-5 (1)
5-5:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDrop unused
printSuccessimport.
printSuccessisn’t used and currently causes a lint error.Suggested fix
-import { printData, printDryRun, printSuccess } from '../../lib/output.js' +import { printData, printDryRun } from '../../lib/output.js'🤖 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/cli/src/commands/chats/notify-anyway.ts` at line 5, Remove the unused import symbol printSuccess from the import list in the notify-anyway module to resolve the lint error: update the import statement that currently reads import { printData, printDryRun, printSuccess } from '../../lib/output.js' to only import the symbols actually used (printData, printDryRun) so that printSuccess is no longer referenced in the file.packages/cli/src/commands/chats/remind.ts-5-5 (1)
5-5:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove unused
printDataimport.
printDatais no longer used in this file and currently triggers lint failure.Suggested fix
-import { printData, printDryRun, printSuccess } from '../../lib/output.js' +import { printDryRun, printSuccess } from '../../lib/output.js'🤖 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/cli/src/commands/chats/remind.ts` at line 5, The import list includes an unused symbol printData in the import statement that causes a lint error; remove printData from the import (keep printDryRun and printSuccess) in the top-level import line where printData is referenced so the file no longer imports an unused identifier (or alternatively use printData where intended), ensuring the import matches actual usage in the remind.ts module.packages/cli/src/commands/chats/draft.ts-5-5 (1)
5-5:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove unused
printSuccessimport.
printSuccessis unused and currently fails lint.Suggested fix
-import { printData, printDryRun, printSuccess } from '../../lib/output.js' +import { printData, printDryRun } from '../../lib/output.js'🤖 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/cli/src/commands/chats/draft.ts` at line 5, The import list in draft.ts includes an unused symbol printSuccess which triggers the linter; remove printSuccess from the import statement (leave printData and printDryRun intact) in the import from '../../lib/output.js' so the file only imports the symbols actually used.packages/cli/src/lib/output.ts-102-102 (1)
102-102:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove unnecessary fallback object in spread.
The
?? {}fallback is redundant in object spread and currently hits lint.Suggested fix
- writeJSON(jsonPayload({ message: opts.message, detail: opts.detail, entity: opts.entity, ...(opts.data ?? {}) }), format) + writeJSON(jsonPayload({ message: opts.message, detail: opts.detail, entity: opts.entity, ...opts.data }), format)🤖 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/cli/src/lib/output.ts` at line 102, The object spread uses a redundant fallback; update the call to writeJSON/jsonPayload to spread opts.data directly instead of using the unnecessary "...(opts.data ?? {})" fallback—i.e., change the spread to "...(opts.data)" within the jsonPayload invocation so writeJSON(jsonPayload({ message: opts.message, detail: opts.detail, entity: opts.entity, ...(opts.data) }), format) and remove the "?? {}" portion.packages/cli/src/lib/output.ts-281-281 (1)
281-281:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse strict null checks in
printText.The nullish check uses
==, which violates the configured lint rules.Suggested fix
- if (item == null) continue + if (item === null || item === undefined) continue🤖 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/cli/src/lib/output.ts` at line 281, In function printText replace the loose nullish check on the variable item (currently using ==) with a strict check that explicitly tests for null and undefined so lint rules pass; locate the line "if (item == null) continue" in printText and change it to an explicit null and undefined check for item (i.e., check both null and undefined) so the branch behavior remains the same but uses strict comparison.packages/cli/src/commands/chats/mark-unread.ts-5-5 (1)
5-5:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove unused
printSuccessimport.
printSuccessis no longer used and currently triggers lint failure.Suggested fix
-import { printData, printDryRun, printSuccess } from '../../lib/output.js' +import { printData, printDryRun } from '../../lib/output.js'🤖 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/cli/src/commands/chats/mark-unread.ts` at line 5, Remove the unused import to fix the lint error: in the import statement that currently imports printData, printDryRun, printSuccess (from '../../lib/output.js') drop printSuccess so only printData and printDryRun are imported; update the import line where printSuccess is referenced to ensure no other code uses it (e.g., check for any occurrences of printSuccess in mark-unread.ts) and run linter to confirm the error is resolved.packages/cli/src/commands/chats/archive.ts-5-5 (1)
5-5:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDrop unused
printSuccessimport.
printSuccessis unused in this command and should be removed.Suggested fix
-import { printData, printDryRun, printSuccess } from '../../lib/output.js' +import { printData, printDryRun } from '../../lib/output.js'🤖 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/cli/src/commands/chats/archive.ts` at line 5, The import list includes an unused symbol printSuccess from the module imported in the top-level import statement (the line that currently reads import { printData, printDryRun, printSuccess } from '../../lib/output.js'); remove printSuccess from that import so only printData and printDryRun are imported, and run a quick lint/type check to ensure no other references to printSuccess remain in the chats/archive command.packages/cli/src/commands/chats/mark-read.ts-5-5 (1)
5-5:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove unused
printSuccessfrom imports.
printSuccessis not used and currently triggersno-unused-vars.Suggested fix
-import { printData, printDryRun, printSuccess } from '../../lib/output.js' +import { printData, printDryRun } from '../../lib/output.js'🤖 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/cli/src/commands/chats/mark-read.ts` at line 5, The import list in mark-read.ts includes an unused symbol printSuccess which triggers no-unused-vars; remove printSuccess from the import statement that currently reads import { printData, printDryRun, printSuccess } from '../../lib/output.js' so the import only includes the actually used symbols (printData, printDryRun) and save the file.packages/cli/src/commands/send/file.ts-33-37 (1)
33-37:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd required blank line before the final statement.
This block currently violates
@stylistic/padding-line-between-statements.Suggested fix
if (flags['dry-run']) { await printDryRun('send.file', request, flags.json ? 'json' : 'human') return } + await printData(await sendMessage(client, request), flags.json ? 'json' : 'human')🤖 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/cli/src/commands/send/file.ts` around lines 33 - 37, Add a blank line between the dry-run early-return block and the final await so the file complies with padding-line-between-statements: after the if (flags['dry-run']) { ... return } block (which calls printDryRun) insert one empty line before the final await printData(await sendMessage(client, request), flags.json ? 'json' : 'human'); ensure the surrounding code using sendMessage, printData and printDryRun remains unchanged.packages/cli/src/commands/chats/description.ts-5-5 (1)
5-5:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove the unused
printSuccessimport.
printSuccessisn’t referenced and should be dropped to satisfy lint rules.Suggested fix
-import { printData, printDryRun, printSuccess } from '../../lib/output.js' +import { printData, printDryRun } from '../../lib/output.js'🤖 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/cli/src/commands/chats/description.ts` at line 5, Remove the unused import symbol printSuccess from the import statement that currently reads import { printData, printDryRun, printSuccess } from '../../lib/output.js'; update the import to only include referenced symbols (printData, printDryRun) so the unused printSuccess is eliminated and lint errors are resolved.packages/cli/src/commands/resolve/account.ts-25-26 (1)
25-26:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
--pick 0is incorrectly treated as “not provided”.Using a truthy check lets
0bypass validation and fall into default selection logic.Suggested fix
- const selected = flags.pick ? candidates[flags.pick - 1] : candidates.length === 1 ? candidates[0] : undefined - if (flags.pick && !selected) throw notFound(`--pick ${flags.pick} is outside the ${candidates.length} matching accounts`, { selector: args.selector, pick: flags.pick, count: candidates.length }) + const pick = flags.pick + const selected = pick !== undefined ? candidates[pick - 1] : candidates.length === 1 ? candidates[0] : undefined + if (pick !== undefined && !selected) throw notFound(`--pick ${pick} is outside the ${candidates.length} matching accounts`, { selector: args.selector, pick, count: candidates.length })🤖 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/cli/src/commands/resolve/account.ts` around lines 25 - 26, The code treats flags.pick==0 as "not provided" due to a truthy check; update the selection logic to explicitly detect when a pick was provided (e.g., use typeof flags.pick !== 'undefined' or flags.pick != null) and validate it as an integer in range 1..candidates.length before indexing candidates (use flags.pick - 1). Replace the current ternary that references flags.pick with this explicit check and ensure the notFound error (and its message) is thrown when the provided pick is outside the valid range.packages/cli/src/commands/resolve/contact.ts-33-34 (1)
33-34:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
--pick 0bypasses candidate range validation.Because selection is guarded by truthiness,
0behaves like “unset” instead of invalid input.Suggested fix
- const selected = flags.pick ? candidates[flags.pick - 1] : candidates.length === 1 ? candidates[0] : undefined - if (flags.pick && !selected) throw notFound(`--pick ${flags.pick} is outside the ${candidates.length} matching contacts`, { selector: args.selector, pick: flags.pick, count: candidates.length }) + const pick = flags.pick + const selected = pick !== undefined ? candidates[pick - 1] : candidates.length === 1 ? candidates[0] : undefined + if (pick !== undefined && !selected) throw notFound(`--pick ${pick} is outside the ${candidates.length} matching contacts`, { selector: args.selector, pick, count: candidates.length })🤖 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/cli/src/commands/resolve/contact.ts` around lines 33 - 34, The validation incorrectly treats flags.pick as truthy so a value of 0 bypasses range checking; update the logic around selected and the subsequent throw to explicitly detect when flags.pick is provided (e.g., flags.pick !== undefined && flags.pick !== null), compute the index as flags.pick - 1 to set selected from candidates, and validate that flags.pick is between 1 and candidates.length (throw notFound using the existing notFound, args.selector, and flags.pick if out of range); ensure selected remains undefined when no pick was provided and keep references to selected, flags.pick, candidates, notFound, and args.selector.packages/cli/src/commands/targets/start.ts-20-27 (1)
20-27:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAddress padding-line lint failures in the new dry-run branches.
Line 20 and Line 24/27 need the blank-line separation required by
@stylistic/padding-line-between-statements; this will likely fail lint as-is.🤖 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/cli/src/commands/targets/start.ts` around lines 20 - 27, The new branches around launchDesktopApp/printSuccess and the subsequent conditionals violate padding-line rules; add the required blank lines to separate statement groups: insert a blank line after the "return" following printSuccess (the launchDesktopApp/printSuccess block) and ensure there's a blank line before the if that checks "if (!target.managed || target.type !== 'server')" and before "if (flags['dry-run'])" so each logical statement group (launchDesktopApp/printSuccess/return, the non-managed/type check, and the dry-run branch) is separated; update the block containing launchDesktopApp, printSuccess, the return, the non-managed/type if, and the flags['dry-run'] if to include those blank lines.packages/cli/src/lib/command.ts-148-158 (1)
148-158:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix switch-case lint violations in
errorCode.Line 151–157 currently violates
unicorn/switch-case-braces(and ordering if your lint config enforces it), which can block CI.Proposed fix
function errorCode(code: number, isBug: boolean): string { if (isBug) return 'internal_error' switch (code) { - case ExitCodes.Usage: return 'usage_error' - case ExitCodes.AuthRequired: return 'auth_required' - case ExitCodes.NotReady: return 'not_ready' - case ExitCodes.NotFound: return 'not_found' - case ExitCodes.Ambiguous: return 'ambiguous_selector' - case ExitCodes.CommandNotFound: return 'command_not_found' - default: return 'runtime_error' + case ExitCodes.Ambiguous: { return 'ambiguous_selector' } + case ExitCodes.AuthRequired: { return 'auth_required' } + case ExitCodes.CommandNotFound: { return 'command_not_found' } + case ExitCodes.NotFound: { return 'not_found' } + case ExitCodes.NotReady: { return 'not_ready' } + case ExitCodes.Usage: { return 'usage_error' } + default: { return 'runtime_error' } } }🤖 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/cli/src/lib/command.ts` around lines 148 - 158, The switch in function errorCode violates unicorn/switch-case-braces—wrap each case body in braces and use explicit breaks (or returns inside braces) to satisfy the rule; update the cases for ExitCodes.Usage, ExitCodes.AuthRequired, ExitCodes.NotReady, ExitCodes.NotFound, ExitCodes.Ambiguous, and ExitCodes.CommandNotFound inside errorCode to use { return '...'; } for each case (and reorder cases if your linter enforces a specific order) so the switch follows the lint rule and CI will pass.packages/cli/src/commands/chats/unpin.ts-5-5 (1)
5-5:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove unused
printSuccessimport.
printSuccessis unused and should be dropped to satisfy lint.Suggested fix
-import { printData, printDryRun, printSuccess } from '../../lib/output.js' +import { printData, printDryRun } from '../../lib/output.js'🤖 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/cli/src/commands/chats/unpin.ts` at line 5, The import list includes an unused symbol printSuccess which triggers a lint error; remove printSuccess from the import statement that currently imports printData, printDryRun, printSuccess so the import only includes the used symbols (printData and printDryRun).packages/cli/src/commands/chats/unmute.ts-5-5 (1)
5-5:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove unused
printSuccessimport.
printSuccessis not used and triggers the reported ESLint error.Suggested fix
-import { printData, printDryRun, printSuccess } from '../../lib/output.js' +import { printData, printDryRun } from '../../lib/output.js'🤖 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/cli/src/commands/chats/unmute.ts` at line 5, The import statement currently brings in printData, printDryRun and printSuccess but printSuccess is unused; edit the import in the module to remove the unused symbol (remove printSuccess from the import list in the import { printData, printDryRun, printSuccess } line) and run linting to confirm no remaining references to printSuccess in the file (check functions or usages around unmute command handlers).packages/cli/src/commands/chats/pin.ts-5-5 (1)
5-5:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove unused
printSuccessimport.
printSuccessis unused in this command and should be removed.Suggested fix
-import { printData, printDryRun, printSuccess } from '../../lib/output.js' +import { printData, printDryRun } from '../../lib/output.js'🤖 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/cli/src/commands/chats/pin.ts` at line 5, The import list in the chats pin command currently includes an unused symbol `printSuccess`; remove `printSuccess` from the import statement that reads `import { printData, printDryRun, printSuccess } from '../../lib/output.js'` so only used functions (e.g., `printData` and `printDryRun`) are imported to eliminate the unused import and lint warnings.packages/cli/src/commands/chats/focus.ts-5-5 (1)
5-5:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove unused
printSuccessimport.
printSuccessis not used in this command and is currently reported by ESLint.Suggested fix
-import { printData, printDryRun, printSuccess } from '../../lib/output.js' +import { printData, printDryRun } from '../../lib/output.js'🤖 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/cli/src/commands/chats/focus.ts` at line 5, Remove the unused printSuccess import from the import list in the focus command; edit the import line that currently reads "import { printData, printDryRun, printSuccess } from '../../lib/output.js'" and drop printSuccess so only used symbols (printData, printDryRun) are imported to satisfy ESLint.packages/cli/src/commands/chats/avatar.ts-5-5 (1)
5-5:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove unused
printSuccessimport.
printSuccessis not referenced and is flagged by ESLint.Suggested fix
-import { printData, printDryRun, printSuccess } from '../../lib/output.js' +import { printData, printDryRun } from '../../lib/output.js'🤖 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/cli/src/commands/chats/avatar.ts` at line 5, The import statement in avatar.ts includes an unused symbol printSuccess which ESLint flags; remove printSuccess from the import list (leave printData and printDryRun intact) so the import becomes only the used symbols and run lint to confirm no other unused imports remain.packages/cli/src/commands/chats/unarchive.ts-5-5 (1)
5-5:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove unused
printSuccessimport.
printSuccessis unused in this file and currently lint-failing.Suggested fix
-import { printData, printDryRun, printSuccess } from '../../lib/output.js' +import { printData, printDryRun } from '../../lib/output.js'🤖 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/cli/src/commands/chats/unarchive.ts` at line 5, The import list in unarchive.ts unnecessarily includes printSuccess which is not used and causes a linter error; remove printSuccess from the import statement that currently reads "import { printData, printDryRun, printSuccess } from '../../lib/output.js'" so it only imports the used symbols (printData, printDryRun).packages/cli/src/commands/send/text.ts-36-36 (1)
36-36:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd blank line before this statement.
The project's ESLint configuration requires a blank line between the preceding statement block and this line.
📏 Proposed fix
if (flags['dry-run']) { await printDryRun('send.text', request, flags.json ? 'json' : 'human') return } + await printData(await sendMessage(client, request), flags.json ? 'json' : 'human')🤖 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/cli/src/commands/send/text.ts` at line 36, Add a blank line before the statement that awaits printData(await sendMessage(client, request), flags.json ? 'json' : 'human'); specifically insert a newline separating the preceding block from the call that uses sendMessage and printData (which reference client, request, and flags.json) so the ESLint rule requiring a blank line before that statement is satisfied.packages/cli/src/commands/targets/remove.ts-7-7 (1)
7-7:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove unused import.
printDatais imported but never used in this file.🧹 Proposed fix
-import { printData, printDryRun, printSuccess } from '../../lib/output.js' +import { printDryRun, printSuccess } from '../../lib/output.js'🤖 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/cli/src/commands/targets/remove.ts` at line 7, Remove the unused imported symbol printData from the import statement in remove.ts: locate the import line that brings in printData, printDryRun, and printSuccess and delete printData so only printDryRun and printSuccess are imported; ensure no other references to printData remain in the file (e.g., in any helper functions or exports).packages/cli/src/commands/resolve/bridge.ts-32-33 (1)
32-33:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winValidate
--pickas 1-indexed explicitly.
--pick 0is currently treated as “not provided” due the truthy check, which breaks the documented 1-indexed behavior.Suggested fix
- const selected = flags.pick ? candidates[flags.pick - 1] : candidates.length === 1 ? candidates[0] : undefined - if (flags.pick && !selected) throw notFound(`--pick ${flags.pick} is outside the ${candidates.length} matching bridges`, { selector: args.selector, pick: flags.pick, count: candidates.length }) + if (flags.pick !== undefined && flags.pick < 1) { + throw new Error('--pick must be >= 1') + } + const selected = flags.pick !== undefined ? candidates[flags.pick - 1] : candidates.length === 1 ? candidates[0] : undefined + if (flags.pick !== undefined && !selected) throw notFound(`--pick ${flags.pick} is outside the ${candidates.length} matching bridges`, { selector: args.selector, pick: flags.pick, count: candidates.length })🤖 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/cli/src/commands/resolve/bridge.ts` around lines 32 - 33, The code treats flags.pick as truthy, so --pick 0 is ignored; change the logic to explicitly detect when a pick was provided and validate it as a 1-indexed integer within range: use a check like "typeof flags.pick !== 'undefined'" (or Number.isInteger(flags.pick)) to decide selection (set selected = candidates[flags.pick - 1] only when pick is a valid integer between 1 and candidates.length), keep the fallback of selecting the sole candidate when candidates.length === 1, and if a pick was provided but is out of range throw the existing notFound(...) with the same context (selector, pick, count).docs/astro.config.mjs-30-36 (1)
30-36:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRepository links appear to target the wrong GitHub repo.
The docs config points to
beeper/desktop-api-cli, while this project context isbeeper/cli. Social, edit-link, and command-reference links should be aligned to avoid broken/misdirected docs navigation.Proposed fix
- href: 'https://github.com/beeper/desktop-api-cli', + href: 'https://github.com/beeper/cli', @@ - 'https://github.com/beeper/desktop-api-cli/edit/main/docs/', + 'https://github.com/beeper/cli/edit/main/docs/', @@ - link: 'https://github.com/beeper/desktop-api-cli/blob/main/packages/cli/README.md', + link: 'https://github.com/beeper/cli/blob/main/packages/cli/README.md',Also applies to: 87-87
🤖 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 `@docs/astro.config.mjs` around lines 30 - 36, The repo links in the docs config point to the wrong GitHub repository; update all references that use 'beeper/desktop-api-cli' to 'beeper/cli' (including the social href entries, editLink.baseUrl, and any command-reference or similar link fields) so navigation and edit links resolve to the correct repo; search for occurrences of the string 'beeper/desktop-api-cli' in this file and replace them with 'beeper/cli' (ensure editLink.baseUrl remains correctly suffixed with 'docs/' and other link templates keep their existing path structure).packages/cli/src/commands/send/voice.ts-43-45 (1)
43-45:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winLint error: missing blank line before
await printData.This matches the reported ESLint failure and should be fixed to avoid pipeline breakage.
Proposed fix
if (flags['dry-run']) { await printDryRun('send.voice', request, flags.json ? 'json' : 'human') return } + await printData( await sendMessage(client, request), flags.json ? 'json' : 'human',🤖 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/cli/src/commands/send/voice.ts` around lines 43 - 45, The linter flagged a missing blank line before the call to await printData; fix it by inserting a single blank line immediately above the statement that calls await printData(...) (the invocation that awaits sendMessage(client, request) and uses flags.json ? 'json' : 'human'), keeping the surrounding code and indentation intact so the statements referencing sendMessage, client, request, printData, and flags.json remain unchanged.
🧹 Nitpick comments (3)
packages/cli/src/commands/messages/export.ts (1)
26-41: ⚡ Quick winDry-run check happens after API calls.
The
--dry-runcheck at line 29 occurs afterresolveChatID(line 28), which performs an API call to look up the chat. For a true dry-run that avoids side effects, consider checkingflags['dry-run']beforecreateClientandresolveChatID, and printing a dry-run output with the raw selector string instead of the resolvedchatID.If validating the chat selector in dry-run mode is intentional, this is acceptable, but it's inconsistent with other commands in this PR (e.g.,
verify/start.tschecks dry-run before API calls).♻️ Suggested refactor to check dry-run earlier
async run(): Promise<void> { const { flags } = await this.parse(MessagesExport) if (flags['before-cursor'] && flags['after-cursor']) throw new Error('Use only one of --before-cursor or --after-cursor') - if (flags.output !== '-') ensureWritable(flags) - const client = await createClient(flags) - const chatID = await resolveChatID(client, flags.chat, { pick: flags.pick }) if (flags['dry-run']) { await printDryRun('messages.export', { - chatID, + chat: flags.chat, + pick: flags.pick, output: flags.output, beforeCursor: flags['before-cursor'], afterCursor: flags['after-cursor'], after: flags.after, before: flags.before, limit: flags.limit, asc: flags.asc, }, flags.json ? 'json' : 'human') return } + if (flags.output !== '-') ensureWritable(flags) + const client = await createClient(flags) + const chatID = await resolveChatID(client, flags.chat, { pick: flags.pick })🤖 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/cli/src/commands/messages/export.ts` around lines 26 - 41, The dry-run flag is checked after performing API calls (createClient and resolveChatID), causing side effects; move the flags['dry-run'] check to occur before calling createClient and resolveChatID in messages.export, and when in dry-run mode call printDryRun('messages.export', {...}) using the raw selector (flags.chat) and the other CLI flags (output, before-cursor, after-cursor, after, before, limit, asc) rather than the resolved chatID so no API lookup is performed; keep printDryRun usage and the json/human format selection the same.packages/cli/src/commands/contacts/search.ts (1)
35-35: 💤 Low valueConsider simplifying the machine-readable output check.
The expression
!isMachineReadableOutput(flags.json ? 'json' : 'human')is functionally equivalent to!flags.jsonsince those are the only two output formats. The current implementation adds indirection without clear benefit.If additional output formats are planned, the current approach provides future flexibility. Otherwise, consider using
!flags.jsondirectly.🤖 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/cli/src/commands/contacts/search.ts` at line 35, Replace the indirection in constructing useSpinner by directly using the JSON flag: instead of computing useSpinner with isMachineReadableOutput(flags.json ? 'json' : 'human'), set useSpinner based on flags.json (i.e., useSpinner should be the negation of flags.json); if you intend to support more output formats later, keep the isMachineReadableOutput call but document that intent—refer to the useSpinner variable and isMachineReadableOutput(flags.json ? 'json' : 'human') expression and the flags.json flag when making the change.packages/cli/src/commands/presence.ts (1)
20-28: ⚡ Quick winDry-run check happens after API calls.
The
--dry-runcheck at line 25 occurs afterresolveChatID(line 24), which performs an API call. For a true dry-run that avoids side effects, consider checkingflags['dry-run']beforecreateClientandresolveChatID.This is the same pattern as in
messages/export.ts. If you want dry-run to validate selectors, this is acceptable, but it means dry-run mode still makes API calls.🤖 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/cli/src/commands/presence.ts` around lines 20 - 28, The dry-run branch currently runs after createClient and resolveChatID which cause API calls; move the flags['dry-run'] check up so it runs before createClient and resolveChatID to avoid side effects. Specifically, in presence command (around ensureWritable, createClient, resolveChatID, printDryRun) check flags['dry-run'] immediately after ensureWritable (or before any API-creating calls), and if true call printDryRun and return; only call createClient and resolveChatID in the non-dry-run path. If you still need selector validation without API calls, replace resolveChatID with a local validation step for flags.chat/pick before exiting in dry-run.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e1909ded-8979-41e7-934b-9c9261157a6e
⛔ Files ignored due to path filters (3)
docs/bun.lockis excluded by!**/*.lockdocs/public/favicon.svgis excluded by!**/*.svgdocs/src/assets/logo.svgis excluded by!**/*.svg
📒 Files selected for processing (122)
README.mddocs/.gitignoredocs/astro.config.mjsdocs/package.jsondocs/src/content.config.tsdocs/src/content/docs/accounts.mdxdocs/src/content/docs/api.mdxdocs/src/content/docs/auth.mdxdocs/src/content/docs/chats.mdxdocs/src/content/docs/config.mdxdocs/src/content/docs/connect.mdxdocs/src/content/docs/contacts.mdxdocs/src/content/docs/exit-codes.mdxdocs/src/content/docs/export.mdxdocs/src/content/docs/index.mdxdocs/src/content/docs/install.mdxdocs/src/content/docs/media.mdxdocs/src/content/docs/messages.mdxdocs/src/content/docs/plugins.mdxdocs/src/content/docs/presence.mdxdocs/src/content/docs/quickstart.mdxdocs/src/content/docs/rpc.mdxdocs/src/content/docs/scripting.mdxdocs/src/content/docs/send.mdxdocs/src/content/docs/targets.mdxdocs/src/content/docs/update.mdxdocs/src/content/docs/watch.mdxdocs/src/styles/theme.cssdocs/tsconfig.jsonpackages/cli/README.mdpackages/cli/docs/api.mdpackages/cli/docs/config.mdpackages/cli/docs/export.mdpackages/cli/docs/setup.mdpackages/cli/docs/update.mdpackages/cli/docs/watch.mdpackages/cli/package.jsonpackages/cli/scripts/generate-command-map.tspackages/cli/scripts/generate-readme.tspackages/cli/src/commands.generated.tspackages/cli/src/commands/accounts/add.tspackages/cli/src/commands/accounts/remove.tspackages/cli/src/commands/accounts/use.tspackages/cli/src/commands/api/post.tspackages/cli/src/commands/api/request.tspackages/cli/src/commands/auth/email/response.tspackages/cli/src/commands/auth/logout.tspackages/cli/src/commands/chats/archive.tspackages/cli/src/commands/chats/avatar.tspackages/cli/src/commands/chats/description.tspackages/cli/src/commands/chats/disappear.tspackages/cli/src/commands/chats/draft.tspackages/cli/src/commands/chats/focus.tspackages/cli/src/commands/chats/mark-read.tspackages/cli/src/commands/chats/mark-unread.tspackages/cli/src/commands/chats/mute.tspackages/cli/src/commands/chats/notify-anyway.tspackages/cli/src/commands/chats/pin.tspackages/cli/src/commands/chats/priority.tspackages/cli/src/commands/chats/remind.tspackages/cli/src/commands/chats/rename.tspackages/cli/src/commands/chats/start.tspackages/cli/src/commands/chats/unarchive.tspackages/cli/src/commands/chats/unmute.tspackages/cli/src/commands/chats/unpin.tspackages/cli/src/commands/chats/unremind.tspackages/cli/src/commands/config/reset.tspackages/cli/src/commands/config/set.tspackages/cli/src/commands/contacts/search.tspackages/cli/src/commands/export.tspackages/cli/src/commands/install/desktop.tspackages/cli/src/commands/install/server.tspackages/cli/src/commands/man.tspackages/cli/src/commands/media/download.tspackages/cli/src/commands/messages/delete.tspackages/cli/src/commands/messages/edit.tspackages/cli/src/commands/messages/export.tspackages/cli/src/commands/messages/search.tspackages/cli/src/commands/presence.tspackages/cli/src/commands/resolve/account.tspackages/cli/src/commands/resolve/bridge.tspackages/cli/src/commands/resolve/chat.tspackages/cli/src/commands/resolve/contact.tspackages/cli/src/commands/resolve/target.tspackages/cli/src/commands/schema.tspackages/cli/src/commands/send/file.tspackages/cli/src/commands/send/react.tspackages/cli/src/commands/send/sticker.tspackages/cli/src/commands/send/text.tspackages/cli/src/commands/send/unreact.tspackages/cli/src/commands/send/voice.tspackages/cli/src/commands/setup.tspackages/cli/src/commands/targets/add/desktop.tspackages/cli/src/commands/targets/add/remote.tspackages/cli/src/commands/targets/add/server.tspackages/cli/src/commands/targets/disable.tspackages/cli/src/commands/targets/enable.tspackages/cli/src/commands/targets/remove.tspackages/cli/src/commands/targets/restart.tspackages/cli/src/commands/targets/start.tspackages/cli/src/commands/targets/stop.tspackages/cli/src/commands/targets/use.tspackages/cli/src/commands/update.tspackages/cli/src/commands/verify.tspackages/cli/src/commands/verify/approve.tspackages/cli/src/commands/verify/cancel.tspackages/cli/src/commands/verify/qr-confirm.tspackages/cli/src/commands/verify/qr-scan.tspackages/cli/src/commands/verify/recovery-key.tspackages/cli/src/commands/verify/reset-recovery-key.tspackages/cli/src/commands/verify/sas-confirm.tspackages/cli/src/commands/verify/sas.tspackages/cli/src/commands/verify/start.tspackages/cli/src/commands/watch.tspackages/cli/src/lib/command-metadata.tspackages/cli/src/lib/command.tspackages/cli/src/lib/errors.tspackages/cli/src/lib/manifest.tspackages/cli/src/lib/output.tspackages/cli/src/lib/resolve.tspackages/cli/test/cli-smoke.tspackages/cli/test/messages-search-validation.test.ts
💤 Files with no reviewable changes (6)
- packages/cli/docs/api.md
- packages/cli/docs/update.md
- packages/cli/docs/watch.md
- packages/cli/docs/export.md
- packages/cli/docs/setup.md
- packages/cli/docs/config.md
✅ Files skipped from review due to trivial changes (18)
- docs/tsconfig.json
- docs/.gitignore
- docs/src/content.config.ts
- docs/src/content/docs/contacts.mdx
- docs/src/content/docs/media.mdx
- docs/src/content/docs/exit-codes.mdx
- docs/src/content/docs/presence.mdx
- docs/src/content/docs/export.mdx
- docs/src/content/docs/watch.mdx
- docs/src/content/docs/rpc.mdx
- docs/src/content/docs/auth.mdx
- docs/src/content/docs/update.mdx
- docs/src/content/docs/connect.mdx
- docs/src/content/docs/api.mdx
- docs/src/content/docs/plugins.mdx
- README.md
- packages/cli/src/lib/manifest.ts
- packages/cli/README.md
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/cli/README.md (1)
9-9:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winReplace example.com placeholder with actual documentation URL.
The README contains multiple references to
example.comas a placeholder for the documentation site. Before merging, these should be updated to the actual documentation URL.Occurrences:
- Line 9: docs badge
- Line 25: "Read the docs" link
- Lines 205-220: Documentation topic table
Also applies to: 25-25, 205-220
🤖 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/cli/README.md` at line 9, Update the placeholder documentation URL in packages/cli/README.md by replacing all occurrences of "example.com" with the actual docs URL: update the docs badge link (the line containing the badge image), the "Read the docs" anchor, and every link in the documentation topic table (previously lines showing topics) so they point to the real documentation site; double-check for any other "example.com" occurrences in the file and replace them consistently.
🤖 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/cli/src/lib/installations.ts`:
- Line 151: installDesktop() currently throws when request.serverEnv !== 'prod',
which breaks non-prod installs; change the install behavior to stop hard-failing
and instead map the desktop update feed to 'prod' while still passing the
original request.serverEnv to the launched app: in installDesktop(), remove the
throw for request.serverEnv !== 'prod', introduce a local variable (e.g.,
feedEnv) that is set to 'prod' when request.serverEnv !== 'prod' and use feedEnv
for any feed/update URLs, but continue to pass request.serverEnv (or
target.serverEnv) into the app launch/config so the app sees the requested
environment; also add a short warning log when mapping non-prod -> prod so users
know the feed was remapped.
---
Outside diff comments:
In `@packages/cli/README.md`:
- Line 9: Update the placeholder documentation URL in packages/cli/README.md by
replacing all occurrences of "example.com" with the actual docs URL: update the
docs badge link (the line containing the badge image), the "Read the docs"
anchor, and every link in the documentation topic table (previously lines
showing topics) so they point to the real documentation site; double-check for
any other "example.com" occurrences in the file and replace them consistently.
🪄 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: 189263df-a84a-4970-9808-6c723d61c15d
📒 Files selected for processing (11)
docs/src/content/docs/targets.mdxpackages/cli/README.mdpackages/cli/src/commands/install/server.tspackages/cli/src/commands/setup.tspackages/cli/src/commands/targets/add/desktop.tspackages/cli/src/commands/targets/add/server.tspackages/cli/src/lib/installations.tspackages/cli/src/lib/manifest.tspackages/cli/src/lib/server-env.tspackages/cli/src/lib/targets.tspackages/cli/test/cli-smoke.ts
✅ Files skipped from review due to trivial changes (1)
- docs/src/content/docs/targets.mdx
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/cli/src/commands/install/server.ts
- packages/cli/src/commands/targets/add/server.ts
- packages/cli/src/lib/manifest.ts
- packages/cli/src/commands/setup.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: test (ubuntu-latest / bun 1.3.10)
- GitHub Check: test (ubuntu-latest / bun 1.3.10)
- GitHub Check: test (macos-latest / bun 1.3.10)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-05-27T22:40:16.795Z
Learnt from: juulieen
Repo: beeper/cli PR: 13
File: README.md:178-178
Timestamp: 2026-05-27T22:40:16.795Z
Learning: In beeper CLI documentation, the correct recovery-key verification command syntax is:
`beeper verify recovery-key -t <target> --key "<key>"`.
Use the top-level `verify` subcommand and pass the recovery key via the `--key` flag. Do not use the previously incorrect form like `beeper auth verify recovery-key --code KEY`—update any doc snippets to match the supported syntax.
Applied to files:
packages/cli/README.md
🔇 Additional comments (7)
packages/cli/src/lib/targets.ts (1)
6-6: LGTM!Also applies to: 205-205
packages/cli/src/lib/server-env.ts (1)
1-16: LGTM!packages/cli/test/cli-smoke.ts (1)
252-252: LGTM!Also applies to: 284-311, 346-346
packages/cli/src/commands/targets/add/desktop.ts (1)
12-12: LGTM!Also applies to: 19-22
packages/cli/src/lib/installations.ts (1)
92-93: LGTM!Also applies to: 106-106, 120-120
packages/cli/README.md (2)
452-452: LGTM!Also applies to: 501-501, 594-594, 623-623
2578-2578: LGTM!
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
packages/cli/src/lib/command.ts (1)
50-50:⚠️ Potential issue | 🟠 Major | ⚡ Quick winNon-
CLIErrorvalidation failures are being mislabeled as internal bugs.Line 50 now marks every non-
CLIErrorasisBug. Several commands still throw plainErrorfor user validation, so expected usage failures can surface askind: "bug"/internal_errorand show the bug panel.Proposed fix
- const isBug = error instanceof BugError || !(error instanceof CLIError) + const isBug = error instanceof BugError || (!(error instanceof CLIError) && inferredCode === undefined)🤖 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/cli/src/lib/command.ts` at line 50, The current logic sets isBug for any error that isn't a CLIError, causing plain Error-based validation failures to be treated as internal bugs; update the check in command.ts so only explicit BugError instances are treated as bugs (e.g., replace const isBug = error instanceof BugError || !(error instanceof CLIError) with const isBug = error instanceof BugError) and ensure any user-facing validation errors are thrown as CLIError (or converted earlier) so they aren't misclassified.
🤖 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/cli/src/commands/presence.ts`:
- Line 26: Add a blank line before the ensureWritable(flags) statement to
satisfy the `@stylistic/padding-line-between-statements` lint rule; locate the
ensureWritable(flags) call in the presence command (ensureWritable) and insert
one empty line above it so there is padding between the previous statement/block
and the ensureWritable(flags) invocation.
In `@packages/cli/src/commands/schema.ts`:
- Around line 27-29: The code builds requested by filtering this.argv which
contains raw tokens (flags and their values); change it to use the parsed argv
returned from await this.parse(Schema) (e.g., const parsed = await
this.parse(Schema)) and filter parsed.argv instead of this.argv so flag values
like "json" aren’t mistaken for positional paths; update the pathArgs
computation (variable name pathArgs) and the requested assignment to use
parsed.argv.
In `@packages/cli/src/commands/verify/approve.ts`:
- Line 16: Add a blank line before the ensureWritable(flags) call in the
approve.ts file to satisfy the `@stylistic/padding-line-between-statements` rule;
locate the ensureWritable(flags) invocation (and the surrounding write/client
setup code that uses flags) and insert a single empty line immediately above it
so the statement is separated from the preceding block.
In `@packages/cli/src/lib/command.ts`:
- Around line 147-167: The switch in errorCode is triggering stylistic lint
errors because there are no blank lines between case blocks; update the switch
in the errorCode function (the case entries for ExitCodes.Ambiguous,
AuthRequired, CommandNotFound, NotFound, NotReady, Usage, and the default) to
include a blank line between each case block (i.e., a blank line after each
closing brace/return) so `@stylistic/padding-line-between-statements` is
satisfied.
In `@packages/npm/scripts/build.ts`:
- Line 48: The generated launcher() function returns a template literal that
itself contains unescaped/backticked template literals (e.g., the line
containing console.error(`beeper-cli does not ship a binary for
${process.platform}/${process.arch}.`)), which prematurely close the outer
string; fix by replacing inner backticks with regular quotes or string
concatenation (or escape the inner backticks) inside launcher() so the outer
`return \`...\`` remains valid; search for other occurrences inside launcher()
of nested backticks and change them to use single/double quotes or +
concatenation, ensuring any ${...} interpolations intended in the generated code
remain correct.
---
Duplicate comments:
In `@packages/cli/src/lib/command.ts`:
- Line 50: The current logic sets isBug for any error that isn't a CLIError,
causing plain Error-based validation failures to be treated as internal bugs;
update the check in command.ts so only explicit BugError instances are treated
as bugs (e.g., replace const isBug = error instanceof BugError || !(error
instanceof CLIError) with const isBug = error instanceof BugError) and ensure
any user-facing validation errors are thrown as CLIError (or converted earlier)
so they aren't misclassified.
🪄 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: 80a53ca3-850f-4182-af2c-c5f19ff59e28
📒 Files selected for processing (45)
README.mddocs/astro.config.mjspackages/cli/README.mdpackages/cli/scripts/generate-readme.tspackages/cli/src/commands/accounts/add.tspackages/cli/src/commands/auth/logout.tspackages/cli/src/commands/chats/archive.tspackages/cli/src/commands/chats/avatar.tspackages/cli/src/commands/chats/description.tspackages/cli/src/commands/chats/disappear.tspackages/cli/src/commands/chats/draft.tspackages/cli/src/commands/chats/focus.tspackages/cli/src/commands/chats/mark-read.tspackages/cli/src/commands/chats/mark-unread.tspackages/cli/src/commands/chats/notify-anyway.tspackages/cli/src/commands/chats/pin.tspackages/cli/src/commands/chats/remind.tspackages/cli/src/commands/chats/unarchive.tspackages/cli/src/commands/chats/unmute.tspackages/cli/src/commands/chats/unpin.tspackages/cli/src/commands/chats/unremind.tspackages/cli/src/commands/install/server.tspackages/cli/src/commands/messages/export.tspackages/cli/src/commands/presence.tspackages/cli/src/commands/resolve/account.tspackages/cli/src/commands/resolve/bridge.tspackages/cli/src/commands/resolve/contact.tspackages/cli/src/commands/schema.tspackages/cli/src/commands/send/file.tspackages/cli/src/commands/send/text.tspackages/cli/src/commands/send/voice.tspackages/cli/src/commands/setup.tspackages/cli/src/commands/targets/add/desktop.tspackages/cli/src/commands/targets/add/server.tspackages/cli/src/commands/targets/remove.tspackages/cli/src/commands/targets/start.tspackages/cli/src/commands/targets/use.tspackages/cli/src/commands/update.tspackages/cli/src/commands/verify/approve.tspackages/cli/src/lib/command-metadata.tspackages/cli/src/lib/command.tspackages/cli/src/lib/installations.tspackages/cli/src/lib/output.tspackages/cli/src/lib/resolve.tspackages/npm/scripts/build.ts
🚧 Files skipped from review as they are similar to previous changes (22)
- packages/cli/src/commands/chats/mark-unread.ts
- packages/cli/src/commands/chats/unarchive.ts
- packages/cli/src/commands/chats/notify-anyway.ts
- packages/cli/src/commands/chats/avatar.ts
- packages/cli/src/commands/targets/use.ts
- packages/cli/src/commands/chats/draft.ts
- packages/cli/src/commands/chats/unremind.ts
- packages/cli/src/commands/chats/unmute.ts
- packages/cli/src/commands/send/text.ts
- packages/cli/src/commands/targets/start.ts
- packages/cli/src/commands/chats/focus.ts
- packages/cli/src/commands/send/file.ts
- packages/cli/src/commands/targets/add/server.ts
- packages/cli/src/commands/chats/pin.ts
- docs/astro.config.mjs
- packages/cli/src/commands/targets/add/desktop.ts
- packages/cli/src/lib/command-metadata.ts
- packages/cli/src/commands/chats/unpin.ts
- packages/cli/src/commands/targets/remove.ts
- packages/cli/scripts/generate-readme.ts
- packages/cli/src/commands/chats/remind.ts
- packages/cli/src/commands/accounts/add.ts
📜 Review details
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-05-27T22:40:16.795Z
Learnt from: juulieen
Repo: beeper/cli PR: 13
File: README.md:178-178
Timestamp: 2026-05-27T22:40:16.795Z
Learning: In beeper CLI documentation, the correct recovery-key verification command syntax is:
`beeper verify recovery-key -t <target> --key "<key>"`.
Use the top-level `verify` subcommand and pass the recovery key via the `--key` flag. Do not use the previously incorrect form like `beeper auth verify recovery-key --code KEY`—update any doc snippets to match the supported syntax.
Applied to files:
README.mdpackages/cli/README.md
🪛 Biome (2.4.15)
packages/npm/scripts/build.ts
[error] 48-48: Expected a semicolon or an implicit semicolon after a statement, but found none
(parse)
[error] 48-48: Expected a semicolon or an implicit semicolon after a statement, but found none
(parse)
[error] 48-48: Expected a semicolon or an implicit semicolon after a statement, but found none
(parse)
[error] 48-48: Expected a semicolon or an implicit semicolon after a statement, but found none
(parse)
[error] 48-48: Expected a semicolon or an implicit semicolon after a statement, but found none
(parse)
[error] 48-48: Expected a semicolon or an implicit semicolon after a statement, but found none
(parse)
[error] 48-48: Expected a semicolon or an implicit semicolon after a statement, but found none
(parse)
[error] 48-48: expected ( but instead found $
(parse)
[error] 48-48: expected ; but instead found {
(parse)
[error] 48-48: expected , but instead found .
(parse)
[error] 48-48: expected ; but instead found {
(parse)
[error] 48-48: expected , but instead found .
(parse)
[error] 48-48: Expected an identifier but instead found '`'.
(parse)
[error] 58-58: expected ) but instead found beeper
(parse)
[error] 58-58: Expected a semicolon or an implicit semicolon after a statement, but found none
(parse)
[error] 58-58: Expected a semicolon or an implicit semicolon after a statement, but found none
(parse)
[error] 60-60: Expected a semicolon or an implicit semicolon after a statement, but found none
(parse)
[error] 62-62: await is only allowed within async functions and at the top levels of modules.
(parse)
[error] 63-63: await is only allowed within async functions and at the top levels of modules.
(parse)
[error] 64-64: await is only allowed within async functions and at the top levels of modules.
(parse)
[error] 66-66: await is only allowed within async functions and at the top levels of modules.
(parse)
[error] 68-68: await is only allowed within async functions and at the top levels of modules.
(parse)
[error] 73-73: await is only allowed within async functions and at the top levels of modules.
(parse)
[error] 75-75: await is only allowed within async functions and at the top levels of modules.
(parse)
[error] 77-77: await is only allowed within async functions and at the top levels of modules.
(parse)
[error] 78-78: await is only allowed within async functions and at the top levels of modules.
(parse)
[error] 79-79: await is only allowed within async functions and at the top levels of modules.
(parse)
[error] 80-80: await is only allowed within async functions and at the top levels of modules.
(parse)
🪛 ESLint
packages/cli/src/commands/presence.ts
[error] 26-26: Expected blank line before this statement.
(@stylistic/padding-line-between-statements)
packages/cli/src/commands/chats/disappear.ts
[error] 26-26: Expected blank line before this statement.
(@stylistic/padding-line-between-statements)
packages/cli/src/commands/messages/export.ts
[error] 40-40: Expected blank line before this statement.
(@stylistic/padding-line-between-statements)
packages/cli/src/commands/resolve/account.ts
[error] 25-25: Use object destructuring.
(prefer-destructuring)
[error] 26-26: Unexpected negated condition.
(unicorn/no-negated-condition)
packages/cli/src/commands/resolve/contact.ts
[error] 33-33: Expected blank line before this statement.
(@stylistic/padding-line-between-statements)
[error] 33-33: Use .length === 0 when checking length is zero.
(unicorn/explicit-length-check)
[error] 34-34: Use object destructuring.
(prefer-destructuring)
[error] 35-35: Unexpected negated condition.
(unicorn/no-negated-condition)
packages/cli/src/lib/installations.ts
[error] 157-157: Expected blank line before this statement.
(@stylistic/padding-line-between-statements)
packages/cli/src/lib/command.ts
[error] 150-150: Expected blank line before this statement.
(@stylistic/padding-line-between-statements)
[error] 153-153: Expected blank line before this statement.
(@stylistic/padding-line-between-statements)
[error] 156-156: Expected blank line before this statement.
(@stylistic/padding-line-between-statements)
[error] 159-159: Expected blank line before this statement.
(@stylistic/padding-line-between-statements)
[error] 162-162: Expected blank line before this statement.
(@stylistic/padding-line-between-statements)
[error] 165-165: Expected blank line before this statement.
(@stylistic/padding-line-between-statements)
packages/cli/src/commands/auth/logout.ts
[error] 17-17: Expected blank line before this statement.
(@stylistic/padding-line-between-statements)
packages/cli/src/commands/resolve/bridge.ts
[error] 32-32: Use object destructuring.
(prefer-destructuring)
[error] 33-33: Unexpected negated condition.
(unicorn/no-negated-condition)
packages/cli/src/commands/schema.ts
[error] 124-124: Expected blank line before this statement.
(@stylistic/padding-line-between-statements)
[error] 127-127: Expected blank line before this statement.
(@stylistic/padding-line-between-statements)
[error] 130-130: Expected blank line before this statement.
(@stylistic/padding-line-between-statements)
[error] 133-133: Expected blank line before this statement.
(@stylistic/padding-line-between-statements)
packages/cli/src/commands/verify/approve.ts
[error] 16-16: Expected blank line before this statement.
(@stylistic/padding-line-between-statements)
packages/cli/src/lib/output.ts
[error] 127-127: Expected blank line before this statement.
(@stylistic/padding-line-between-statements)
[error] 131-131: Expected blank line before this statement.
(@stylistic/padding-line-between-statements)
🪛 GitHub Actions: CI / 0_test (ubuntu-latest _ bun 1.3.10).txt
packages/npm/scripts/build.ts
[error] 48-48: Typecheck failed at console.error template string. Error(s): Expected ';' but found 'beeper'; Expected ';' but found 'does'; Expected ';' but found 'ship'; Expected ';' but found 'binary'; Expected '(' but found '$'; Expected '}' but found '.'; Expected ';' but found 'platform'; Unexpected '}'.
🪛 GitHub Actions: CI / 1_test (macos-latest _ bun 1.3.10).txt
packages/npm/scripts/build.ts
[error] 48-48: typecheck failed: Syntax error in console.error template string. Error: Expected ";" but found "beeper". (at packages/npm/scripts/build.ts:48:18)
[error] 48-48: typecheck failed: Syntax error. Error: Expected ";" but found "does". (at packages/npm/scripts/build.ts:48:29)
[error] 48-48: typecheck failed: Syntax error. Error: Expected ";" but found "ship". (at packages/npm/scripts/build.ts:48:38)
[error] 48-48: typecheck failed: Syntax error. Error: Expected ";" but found "binary". (at packages/npm/scripts/build.ts:48:45)
[error] 48-48: typecheck failed: Syntax error. Error: Expected "(" but found "$". (at packages/npm/scripts/build.ts:48:56)
[error] 48-48: typecheck failed: Syntax error. Error: Expected "}" but found ".". (at packages/npm/scripts/build.ts:48:65)
[error] 48-48: typecheck failed: Syntax error. Error: Expected ";" but found "platform". (at packages/npm/scripts/build.ts:48:66)
[error] 48-48: typecheck failed: Syntax error. Error: Unexpected "}". (at packages/npm/scripts/build.ts:48:74)
🪛 GitHub Actions: CI / test (macos-latest _ bun 1.3.10)
packages/npm/scripts/build.ts
[error] 48-48: Typecheck failed: Expected ';' but found 'beeper'.
[error] 48-48: Typecheck failed: Expected ';' but found 'does'.
[error] 48-48: Typecheck failed: Expected ';' but found 'ship'.
[error] 48-48: Typecheck failed: Expected ';' but found 'binary'.
[error] 48-48: Typecheck failed: Expected '(' but found '$'.
[error] 48-48: Typecheck failed: Expected '}' but found '.'.
[error] 48-48: Typecheck failed: Expected ';' but found 'platform'.
[error] 48-48: Typecheck failed: Expected ';' but found 'does'. (Also reported: Unexpected '}' at line 48.)
🪛 GitHub Actions: CI / test (ubuntu-latest _ bun 1.3.10)
packages/npm/scripts/build.ts
[error] 48-48: TypeScript typecheck failed: Expected ";" but found "beeper". (at /home/runner/work/cli/cli/packages/npm/scripts/build.ts:48:18)
[error] 48-48: TypeScript typecheck failed: Expected ";" but found "does". (at /home/runner/work/cli/cli/packages/npm/scripts/build.ts:48:29)
[error] 48-48: TypeScript typecheck failed: Expected ";" but found "ship". (at /home/runner/work/cli/cli/packages/npm/scripts/build.ts:48:38)
[error] 48-48: TypeScript typecheck failed: Expected ";" but found "a". (at /home/runner/work/cli/cli/packages/npm/scripts/build.ts:48:45)
[error] 48-48: TypeScript typecheck failed: Expected "(" but found "$". (at /home/runner/work/cli/cli/packages/npm/scripts/build.ts:48:56)
[error] 48-48: TypeScript typecheck failed: Expected "}" but found ".". (at /home/runner/work/cli/cli/packages/npm/scripts/build.ts:48:65)
[error] 48-48: TypeScript typecheck failed: Expected ";" but found "platform". (at /home/runner/work/cli/cli/packages/npm/scripts/build.ts:48:66)
[error] 48-48: TypeScript typecheck failed: Unexpected "}". (at /home/runner/work/cli/cli/packages/npm/scripts/build.ts:48:74)
🔇 Additional comments (19)
packages/cli/src/commands/send/voice.ts (1)
24-26: LGTM!Also applies to: 38-38, 42-44, 46-46
packages/cli/src/commands/chats/description.ts (1)
5-5: LGTM!packages/cli/src/commands/chats/archive.ts (1)
5-5: LGTM!packages/cli/src/commands/update.ts (1)
26-26: LGTM!README.md (1)
8-10: LGTM!Also applies to: 25-25, 205-205, 210-217
packages/cli/src/commands/chats/disappear.ts (1)
23-29: LGTM!packages/cli/src/commands/install/server.ts (1)
6-6: LGTM!Also applies to: 12-12
packages/cli/src/commands/setup.ts (1)
39-39: LGTM!Also applies to: 173-174, 711-722
packages/cli/README.md (1)
9-10: LGTM!Also applies to: 25-25, 205-217, 452-452, 501-501, 594-594, 623-623, 2578-2579
packages/cli/src/commands/auth/logout.ts (1)
10-10: LGTM!Also applies to: 17-19
packages/cli/src/commands/resolve/bridge.ts (1)
32-34: LGTM!packages/cli/src/commands/schema.ts (1)
121-135: LGTM!packages/cli/src/lib/output.ts (1)
102-103: LGTM!Also applies to: 127-134, 289-289
packages/cli/src/commands/messages/export.ts (1)
28-30: LGTM!Also applies to: 40-42
packages/cli/src/commands/resolve/account.ts (1)
25-27: LGTM!packages/cli/src/lib/resolve.ts (1)
39-40: LGTM!packages/cli/src/commands/resolve/contact.ts (1)
28-30: LGTM!Also applies to: 34-36, 58-63
packages/cli/src/commands/chats/mark-read.ts (1)
5-5: LGTM!Also applies to: 17-19
packages/cli/src/lib/installations.ts (1)
151-157: LGTM!
| process.exitCode = process.exitCode ?? code | ||
| const tryMessage = error instanceof CLIError ? error.tryMessage : undefined | ||
| const isBug = !(error instanceof CLIError) || error instanceof BugError | ||
| const isBug = error instanceof BugError || !(error instanceof CLIError) |
There was a problem hiding this comment.
My earlier suggestion overshot — plain throw new Error(...) is now misclassified as kind:'bug'/internal_error: there are 58 sites under packages/cli/src/commands that throw plain Error for user-validation failures (e.g. setup.ts:56,58,60 “Specify at most one of …”, chats/avatar.ts:14 “Provide --file or --clear”, presence.ts:20-21 “--duration must be a positive integer”, every accounts/add.ts validation). With isBug = error instanceof BugError || !(error instanceof CLIError), all of those now print the bug panel and emit internal_error to JSON consumers. Two clean fixes: (A) restore && inferredCode === undefined and tighten the inferExitCode regexes with \b boundaries (drop the bare usage/invalid/required/parse tokens that match library messages like Cannot find required module); or (B) convert every throw new Error(...) in packages/cli/src/commands/** to throw usageError(...)/notFound/etc. from packages/cli/src/lib/errors.ts and add a lint rule banning plain throw new Error(...) in command files.
There was a problem hiding this comment.
There was a problem hiding this comment.
The && inferredCode === undefined guard is restored, and the tightened regex (\binvalid (?:argument|option|flag|value|input)\b, \brequired (?:flag|argument|option|value)\b, \bparse error\b, etc.) correctly stops library messages like Cannot find required module 'foo' from being downgraded to usage errors. Good fix for the library-error regression I called out in 49b0ca88.
Heads up though: my framing of the "58 plain throws" was misleading — I should have flagged this as a pre-existing classification gap rather than a new regression. Re-checking against the actual messages:
setup.ts:56-60"Specify at most one of …"/"Use --local or --oauth with …"chats/avatar.ts:14"Provide --file or --clear",chats/draft.ts:22-23"Provide --text TEXT …"/"--clear cannot be combined …"chats/disappear.ts:21"--seconds must be a positive integer …"presence.ts:20-21"--duration must be a positive integer …"/"--duration only applies when …"messages/export.ts:25,47-48"Use only one of --before-cursor or --after-cursor"/"--after is not a valid ISO timestamp"api/post.ts:26"--body is not valid JSON …"accounts/add.ts"Multiple sign-in methods …","No login flows returned …","… must use name=value form."setup.ts:236"Install requires --install --yes in non-interactive mode."
None of these match either the old loose regex or the new tight one, so they've been classified as kind:"bug" / internal_error the whole time (not something ffcff36 introduced). The proper fix is option (B) from my earlier prompt — swap each of these throw new Error(...) sites for throw usageError(...) (already exported from packages/cli/src/lib/errors.ts:65) so they render as one-line aborts with exit 2 / code:"usage_error". Happy to leave it out of this PR if you want a follow-up, but I'll keep it tracked as a separate functional issue since the current behavior is misleading for JSON consumers.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
packages/cli/src/commands/api/request.ts (1)
28-32: ⚡ Quick winResolve the
@stylistic/padding-line-between-statementslint errors.Static analysis flags missing blank lines before the statements at Lines 28 and 32. Add the expected blank lines to pass the lint check.
🤖 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/cli/src/commands/api/request.ts` around lines 28 - 32, Add the missing blank lines to satisfy the padding-line-between-statements lint rule by inserting a blank line before the conditional branch and before the final await call: ensure there is an empty line before the "if (flags['no-auth']) { … }" block's first statement that calls printData/appRequest and also an empty line before the final "await printData(await appRequest(...), format)" call; locate the usage of flags['no-auth'], appRequest, printData, method and args.path in request.ts and add the blank lines accordingly.packages/cli/src/commands/api/post.ts (1)
28-37: ⚡ Quick winResolve the
@stylistic/padding-line-between-statementslint errors.Static analysis flags missing blank lines before the statements at Lines 28, 32, and 36, which will fail the lint check. Add the expected blank lines before these block statements.
♻️ Proposed fix
} catch { throw new Error(`--body is not valid JSON: ${flags.body}`) } + if (flags['dry-run']) { await printDryRun('api.post', { method: 'POST', path: args.path, body, noAuth: flags['no-auth'], target: flags.target, baseURL: flags['base-url'] }, format) return } + if (flags['no-auth']) { await printData(await appRequest('POST', args.path, { baseURL: flags['base-url'], body, target: flags.target, token: false }), format) return } + const client = await createClient(flags)🤖 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/cli/src/commands/api/post.ts` around lines 28 - 37, The lint rule requires blank lines separating top-level statement blocks; add a single empty line before the "if (flags['dry-run'])" block, before the "if (flags['no-auth'])" block, and before the "const client = await createClient(flags)" statement in this function so there is a blank line between each branch; keep the existing code and callsites (printDryRun, appRequest, printData, createClient, client.post) unchanged and only insert those padding blank lines.packages/cli/src/commands/auth/logout.ts (1)
32-32: ⚡ Quick winResolve the
@stylistic/padding-line-between-statementslint error.Static analysis flags a missing blank line before the
printSuccessstatement at Line 32.♻️ Proposed fix
await clearTargetAuth(target) } + await printSuccess({ message: 'Logged out', detail: token ? 'local token cleared' : 'no token was stored', data: { revoked, hadToken: Boolean(token) } }, format)🤖 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/cli/src/commands/auth/logout.ts` at line 32, Add a blank line before the printSuccess invocation to satisfy the `@stylistic/padding-line-between-statements` rule: inside the logout command in logout.ts, insert a single empty line immediately above the await printSuccess(...) call (the call that uses token, revoked and hadToken) so there is a separating blank line between the preceding statement and the printSuccess await.packages/cli/src/commands/config/set.ts (1)
18-22: ⚡ Quick winResolve the
@stylistic/padding-line-between-statementslint error.Static analysis flags a missing blank line before the
updateConfigstatement at Line 22.♻️ Proposed fix
if (flags['dry-run']) { await printDryRun('config.set', { [args.key]: nextValue }, format) return } + await updateConfig(config => ({ ...config, [args.key]: nextValue }))🤖 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/cli/src/commands/config/set.ts` around lines 18 - 22, The linter wants a blank line separating the conditional dry-run branch from the subsequent side-effect call; inside the config.set logic, add one empty line between the end of the if (flags['dry-run']) { ... return } block and the await updateConfig(...) call so that printDryRun/return is visually separated from updateConfig; locate the flags['dry-run'] check and the await updateConfig(...) invocation and insert a single blank line before updateConfig.
🤖 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/cli/src/commands/api/get.ts`:
- Around line 19-26: Add a blank line before the declaration of const client to
satisfy the padding-line-between-statements rule: after the early-return branch
that calls printData(await appRequest(...)) leave one empty line, then declare
const client = await createClient(flags) and continue with await printData(await
client.get(args.path), format); this touches the block using format, appRequest,
createClient, printData, flags and args.path.
In `@packages/cli/src/commands/config/reset.ts`:
- Around line 12-17: The padding-line lint rule is failing because there is no
blank line between the early return block and the subsequent reset call; add a
single blank line between the if-block that handles flags['dry-run']/printDryRun
and the await resetConfig() call so the sequence (printDryRun -> return) is
visually separated from resetConfig(), then keep the following await
printSuccess({ message: 'Config reset' }, format) as-is; this involves editing
the code around printDryRun, resetConfig, and printSuccess in
packages/cli/src/commands/config/reset.ts.
In `@packages/cli/src/commands/verify/cancel.ts`:
- Around line 16-18: Add a blank line before the declaration of const client in
the verify cancel command to satisfy the
`@stylistic/padding-line-between-statements` ESLint rule: locate the function in
cancel.ts where createClient(flags) is called, and insert an empty line between
the preceding statement/block and the line calling createClient so the sequence
reads with a blank line before const client; keep the rest of the logic using
createClient, client.app.verifications.cancel(flags.id ?? 'active', {}), and
printData unchanged.
---
Nitpick comments:
In `@packages/cli/src/commands/api/post.ts`:
- Around line 28-37: The lint rule requires blank lines separating top-level
statement blocks; add a single empty line before the "if (flags['dry-run'])"
block, before the "if (flags['no-auth'])" block, and before the "const client =
await createClient(flags)" statement in this function so there is a blank line
between each branch; keep the existing code and callsites (printDryRun,
appRequest, printData, createClient, client.post) unchanged and only insert
those padding blank lines.
In `@packages/cli/src/commands/api/request.ts`:
- Around line 28-32: Add the missing blank lines to satisfy the
padding-line-between-statements lint rule by inserting a blank line before the
conditional branch and before the final await call: ensure there is an empty
line before the "if (flags['no-auth']) { … }" block's first statement that calls
printData/appRequest and also an empty line before the final "await
printData(await appRequest(...), format)" call; locate the usage of
flags['no-auth'], appRequest, printData, method and args.path in request.ts and
add the blank lines accordingly.
In `@packages/cli/src/commands/auth/logout.ts`:
- Line 32: Add a blank line before the printSuccess invocation to satisfy the
`@stylistic/padding-line-between-statements` rule: inside the logout command in
logout.ts, insert a single empty line immediately above the await
printSuccess(...) call (the call that uses token, revoked and hadToken) so there
is a separating blank line between the preceding statement and the printSuccess
await.
In `@packages/cli/src/commands/config/set.ts`:
- Around line 18-22: The linter wants a blank line separating the conditional
dry-run branch from the subsequent side-effect call; inside the config.set
logic, add one empty line between the end of the if (flags['dry-run']) { ...
return } block and the await updateConfig(...) call so that printDryRun/return
is visually separated from updateConfig; locate the flags['dry-run'] check and
the await updateConfig(...) invocation and insert a single blank line before
updateConfig.
🪄 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: 888ea263-2436-4542-a190-b1662ae9d9a0
📒 Files selected for processing (53)
.gitignorepackages/cli/src/commands/api/get.tspackages/cli/src/commands/api/post.tspackages/cli/src/commands/api/request.tspackages/cli/src/commands/auth/email/response.tspackages/cli/src/commands/auth/logout.tspackages/cli/src/commands/bridges/show.tspackages/cli/src/commands/chats/archive.tspackages/cli/src/commands/chats/avatar.tspackages/cli/src/commands/chats/description.tspackages/cli/src/commands/chats/focus.tspackages/cli/src/commands/chats/mark-read.tspackages/cli/src/commands/chats/mark-unread.tspackages/cli/src/commands/chats/notify-anyway.tspackages/cli/src/commands/chats/pin.tspackages/cli/src/commands/chats/remind.tspackages/cli/src/commands/chats/unarchive.tspackages/cli/src/commands/chats/unmute.tspackages/cli/src/commands/chats/unpin.tspackages/cli/src/commands/chats/unremind.tspackages/cli/src/commands/config/reset.tspackages/cli/src/commands/config/set.tspackages/cli/src/commands/doctor.tspackages/cli/src/commands/media/download.tspackages/cli/src/commands/messages/search.tspackages/cli/src/commands/plugins/available.tspackages/cli/src/commands/presence.tspackages/cli/src/commands/resolve/chat.tspackages/cli/src/commands/schema.tspackages/cli/src/commands/setup.tspackages/cli/src/commands/targets/disable.tspackages/cli/src/commands/targets/enable.tspackages/cli/src/commands/targets/list.tspackages/cli/src/commands/targets/logs.tspackages/cli/src/commands/targets/remove.tspackages/cli/src/commands/targets/restart.tspackages/cli/src/commands/targets/show.tspackages/cli/src/commands/targets/start.tspackages/cli/src/commands/targets/status.tspackages/cli/src/commands/targets/stop.tspackages/cli/src/commands/targets/use.tspackages/cli/src/commands/verify/approve.tspackages/cli/src/commands/verify/cancel.tspackages/cli/src/lib/command-metadata.tspackages/cli/src/lib/command.tspackages/cli/src/lib/export/index.tspackages/cli/src/lib/ink/render.tsxpackages/cli/src/lib/ink/theme.tspackages/cli/src/lib/installations.tspackages/cli/src/lib/profiles.tspackages/cli/src/lib/resolve.tspackages/cli/src/lib/setup-login.tspackages/npm/scripts/build.ts
💤 Files with no reviewable changes (14)
- packages/cli/src/lib/export/index.ts
- packages/cli/src/commands/chats/unmute.ts
- packages/cli/src/commands/chats/unarchive.ts
- packages/cli/src/commands/chats/unpin.ts
- packages/cli/src/commands/chats/mark-read.ts
- packages/cli/src/commands/chats/mark-unread.ts
- packages/cli/src/commands/chats/archive.ts
- packages/cli/src/commands/chats/pin.ts
- packages/cli/src/commands/chats/focus.ts
- packages/cli/src/commands/chats/remind.ts
- packages/cli/src/commands/chats/notify-anyway.ts
- packages/cli/src/commands/chats/avatar.ts
- packages/cli/src/commands/chats/unremind.ts
- packages/cli/src/commands/chats/description.ts
✅ Files skipped from review due to trivial changes (7)
- .gitignore
- packages/cli/src/commands/plugins/available.ts
- packages/cli/src/commands/targets/list.ts
- packages/cli/src/lib/ink/theme.ts
- packages/cli/src/commands/targets/logs.ts
- packages/cli/src/commands/targets/show.ts
- packages/cli/src/commands/targets/status.ts
🚧 Files skipped from review as they are similar to previous changes (14)
- packages/cli/src/commands/media/download.ts
- packages/cli/src/commands/targets/enable.ts
- packages/cli/src/commands/targets/start.ts
- packages/cli/src/commands/targets/disable.ts
- packages/cli/src/commands/targets/restart.ts
- packages/cli/src/commands/verify/approve.ts
- packages/cli/src/commands/presence.ts
- packages/cli/src/commands/schema.ts
- packages/cli/src/lib/command-metadata.ts
- packages/npm/scripts/build.ts
- packages/cli/src/commands/auth/email/response.ts
- packages/cli/src/commands/targets/remove.ts
- packages/cli/src/lib/installations.ts
- packages/cli/src/lib/command.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: test (macos-latest / bun 1.3.10)
- GitHub Check: test (ubuntu-latest / bun 1.3.10)
- GitHub Check: test (ubuntu-latest / bun 1.3.10)
🧰 Additional context used
🪛 ESLint
packages/cli/src/commands/config/set.ts
[error] 22-22: Expected blank line before this statement.
(@stylistic/padding-line-between-statements)
packages/cli/src/commands/auth/logout.ts
[error] 32-32: Expected blank line before this statement.
(@stylistic/padding-line-between-statements)
packages/cli/src/commands/api/post.ts
[error] 28-28: Expected blank line before this statement.
(@stylistic/padding-line-between-statements)
[error] 32-32: Expected blank line before this statement.
(@stylistic/padding-line-between-statements)
[error] 36-36: Expected blank line before this statement.
(@stylistic/padding-line-between-statements)
packages/cli/src/commands/setup.ts
[error] 140-140: Use switch instead of multiple else-if.
(unicorn/prefer-switch)
packages/cli/src/commands/api/get.ts
[error] 24-24: Expected blank line before this statement.
(@stylistic/padding-line-between-statements)
packages/cli/src/commands/api/request.ts
[error] 28-28: Expected blank line before this statement.
(@stylistic/padding-line-between-statements)
[error] 32-32: Expected blank line before this statement.
(@stylistic/padding-line-between-statements)
packages/cli/src/commands/config/reset.ts
[error] 16-16: Expected blank line before this statement.
(@stylistic/padding-line-between-statements)
packages/cli/src/commands/verify/cancel.ts
[error] 17-17: Expected blank line before this statement.
(@stylistic/padding-line-between-statements)
🔇 Additional comments (13)
packages/cli/src/commands/resolve/chat.ts (1)
19-59: LGTM!packages/cli/src/commands/setup.ts (2)
110-197: LGTM!
39-39: ⚡ Quick winUpdate concern: default/alias regression scope and oclif ordering. This CLI defines
--server-envwithdefault: 'prod'(not'production'), andoptions: [...SERVER_ENVIRONMENTS]plusparse: normalizeServerEnv.normalizeServerEnvmaps'production'→'prod', but whether'production'is accepted depends on whether oclif validates theoptionslist against the raw input before running the customparsefunction; the available evidence doesn’t show that ordering precisely.packages/cli/src/commands/targets/stop.ts (1)
13-22: LGTM!packages/cli/src/commands/targets/use.ts (1)
9-21: LGTM!packages/cli/src/commands/api/request.ts (1)
24-27: 💤 Low valueConfirm intentional GET pass-through under
--dry-run.
--dry-runonly short-circuits for non-GETmethods, soapi request GET … --dry-runstill performs the live request. This is consistent with theensureWritablegating at Line 21 (reads are side-effect-free), so it looks intentional—just confirming it's expected.packages/cli/src/commands/bridges/show.ts (1)
33-39: LGTM!packages/cli/src/commands/doctor.ts (1)
13-19: LGTM!packages/cli/src/commands/messages/search.ts (1)
61-68: LGTM!packages/cli/src/lib/ink/render.tsx (1)
1-1: LGTM!Also applies to: 31-31, 130-143
packages/cli/src/lib/profiles.ts (1)
148-162: LGTM!packages/cli/src/lib/resolve.ts (1)
60-103: LGTM!packages/cli/src/lib/setup-login.ts (1)
31-43: LGTM!
| const format = flags.json ? 'json' : 'human' | ||
| if (flags['no-auth']) { | ||
| await printData(await appRequest('GET', args.path, { baseURL: flags['base-url'], target: flags.target, token: false }), flags.json ? 'json' : 'human') | ||
| await printData(await appRequest('GET', args.path, { baseURL: flags['base-url'], target: flags.target, token: false }), format) | ||
| return | ||
| } | ||
| const client = await createClient(flags) | ||
| await printData(await client.get(args.path), flags.json ? 'json' : 'human') | ||
| await printData(await client.get(args.path), format) | ||
| } |
There was a problem hiding this comment.
Centralizing format is a clean improvement. One lint blocker: ESLint flags a missing blank line before const client on Line 24 (@stylistic/padding-line-between-statements, error severity), which will fail CI.
🔧 Proposed fix
return
}
+
const client = await createClient(flags)
await printData(await client.get(args.path), format)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const format = flags.json ? 'json' : 'human' | |
| if (flags['no-auth']) { | |
| await printData(await appRequest('GET', args.path, { baseURL: flags['base-url'], target: flags.target, token: false }), flags.json ? 'json' : 'human') | |
| await printData(await appRequest('GET', args.path, { baseURL: flags['base-url'], target: flags.target, token: false }), format) | |
| return | |
| } | |
| const client = await createClient(flags) | |
| await printData(await client.get(args.path), flags.json ? 'json' : 'human') | |
| await printData(await client.get(args.path), format) | |
| } | |
| const format = flags.json ? 'json' : 'human' | |
| if (flags['no-auth']) { | |
| await printData(await appRequest('GET', args.path, { baseURL: flags['base-url'], target: flags.target, token: false }), format) | |
| return | |
| } | |
| const client = await createClient(flags) | |
| await printData(await client.get(args.path), format) | |
| } |
🧰 Tools
🪛 ESLint
[error] 24-24: Expected blank line before this statement.
(@stylistic/padding-line-between-statements)
🤖 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/cli/src/commands/api/get.ts` around lines 19 - 26, Add a blank line
before the declaration of const client to satisfy the
padding-line-between-statements rule: after the early-return branch that calls
printData(await appRequest(...)) leave one empty line, then declare const client
= await createClient(flags) and continue with await printData(await
client.get(args.path), format); this touches the block using format, appRequest,
createClient, printData, flags and args.path.
| if (flags['dry-run']) { | ||
| await printDryRun('config.reset', {}, format) | ||
| return | ||
| } | ||
| await resetConfig() | ||
| await printSuccess({ message: 'Config reset' }, flags.json ? 'json' : 'human') | ||
| await printSuccess({ message: 'Config reset' }, format) |
There was a problem hiding this comment.
Add a blank line before resetConfig() to satisfy the padding-line lint rule.
ESLint (@stylistic/padding-line-between-statements) flags Line 16. The dry-run/format logic itself is correct.
🎨 Proposed fix
if (flags['dry-run']) {
await printDryRun('config.reset', {}, format)
return
}
+
await resetConfig()
await printSuccess({ message: 'Config reset' }, format)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (flags['dry-run']) { | |
| await printDryRun('config.reset', {}, format) | |
| return | |
| } | |
| await resetConfig() | |
| await printSuccess({ message: 'Config reset' }, flags.json ? 'json' : 'human') | |
| await printSuccess({ message: 'Config reset' }, format) | |
| if (flags['dry-run']) { | |
| await printDryRun('config.reset', {}, format) | |
| return | |
| } | |
| await resetConfig() | |
| await printSuccess({ message: 'Config reset' }, format) |
🧰 Tools
🪛 ESLint
[error] 16-16: Expected blank line before this statement.
(@stylistic/padding-line-between-statements)
🤖 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/cli/src/commands/config/reset.ts` around lines 12 - 17, The
padding-line lint rule is failing because there is no blank line between the
early return block and the subsequent reset call; add a single blank line
between the if-block that handles flags['dry-run']/printDryRun and the await
resetConfig() call so the sequence (printDryRun -> return) is visually separated
from resetConfig(), then keep the following await printSuccess({ message:
'Config reset' }, format) as-is; this involves editing the code around
printDryRun, resetConfig, and printSuccess in
packages/cli/src/commands/config/reset.ts.
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/cli/src/commands/setup.ts (1)
53-76:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
setup --dry-runstill requires a writable environment.Line 54 runs
ensureWritable(flags)before the new dry-run early return, so the preview fails anywhere the CLI config is read-only. Since this branch only prints derived actions, it should behave like theauth logoutdry-run path and skip the write precondition.🔧 Minimal fix
async run(): Promise<void> { const { flags } = await this.parse(Setup) - ensureWritable(flags) const targetModeCount = [Boolean(flags.remote), flags.server, flags.desktop].filter(Boolean).length if (targetModeCount > 1) throw new Error('Specify at most one of --remote, --server, or --desktop') const authModeCount = [flags.local, flags.oauth, Boolean(flags.email)].filter(Boolean).length if (authModeCount > 1) throw new Error('Specify at most one of --local, --oauth, or --email') if ((flags.local || flags.oauth) && (flags.remote || flags.server || flags.desktop)) { throw new Error('Use --local or --oauth with an existing target, not with --remote, --server, or --desktop.') } + if (!flags['dry-run']) ensureWritable(flags) if (flags['dry-run']) { await printDryRun('setup', { target: flags.target, baseURL: flags['base-url'],🤖 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/cli/src/commands/setup.ts` around lines 53 - 76, The dry-run path currently calls ensureWritable(flags) before deciding to return, causing read-only environments to fail; update setup command logic so the dry-run shortcut is checked and returned before calling ensureWritable(flags) (or make ensureWritable a no-op when flags['dry-run'] is true): move the flags['dry-run'] check (which calls printDryRun) to run prior to invoking ensureWritable(flags) (or add an early return when flags['dry-run'] is set) so printDryRun can run in read-only configs; touch the Setup command entry where ensureWritable and printDryRun are used to implement this change.packages/cli/src/commands/auth/logout.ts (2)
21-32:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd the missing separator before
printSuccess().Line 32 still violates
@stylistic/padding-line-between-statements, which will keep CI red.🎨 Minimal fix
if (token) { const response = await fetch(new URL('/oauth/revoke', target.baseURL), { method: 'POST', headers: { 'content-type': 'application/x-www-form-urlencoded' }, body: new URLSearchParams({ token, token_type_hint: 'access_token' }), signal: AbortSignal.timeout(5000), }).catch(() => undefined) revoked = Boolean(response?.ok) await clearTargetAuth(target) } + await printSuccess({ message: 'Logged out', detail: token ? 'local token cleared' : 'no token was stored', data: { revoked, hadToken: Boolean(token) } }, format)🤖 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/cli/src/commands/auth/logout.ts` around lines 21 - 32, The last statement calling printSuccess(...) needs a blank separator line before it to satisfy stylistic rules; insert a single empty line between the token-handling block (where you call clearTargetAuth(target) and compute revoked) and the final await printSuccess(...) call so there is padding between the if/await block and the printSuccess invocation; target symbols to locate are the revoked/token variables, the fetch + clearTargetAuth(target) sequence, and the await printSuccess(...) call.
14-20:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDry-run hides the
BEEPER_ACCESS_TOKENfailure path.When auth only comes from
BEEPER_ACCESS_TOKEN, the real command throws at Lines 18-20, but--dry-runnow returns a normal payload withhadToken: false. That makes the preview lie about whether logout can actually proceed.🔧 Minimal fix
const format = flags.json ? 'json' : 'human' const target = await resolveTarget({ target: flags.target, baseURL: flags['base-url'] }) const token = target.auth?.accessToken + + if (process.env.BEEPER_ACCESS_TOKEN && !token) { + throw new Error('auth logout cannot clear BEEPER_ACCESS_TOKEN from the environment; unset it in the calling process.') + } + if (flags['dry-run']) { await printDryRun('auth.logout', { target: target.id, baseURL: target.baseURL, hadToken: Boolean(token), revokeToken: Boolean(token) }, format) return } - if (process.env.BEEPER_ACCESS_TOKEN && !target.auth?.accessToken) { - throw new Error('auth logout cannot clear BEEPER_ACCESS_TOKEN from the environment; unset it in the calling process.') - }🤖 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/cli/src/commands/auth/logout.ts` around lines 14 - 20, The dry-run path currently returns a normal payload even when logout would fail due to BEEPER_ACCESS_TOKEN being set and no target.auth?.accessToken; move or duplicate the env check so the same error is thrown during dry-run: before calling printDryRun('auth.logout', ...) check if process.env.BEEPER_ACCESS_TOKEN && !target.auth?.accessToken and throw the same Error('auth logout cannot clear BEEPER_ACCESS_TOKEN from the environment; unset it in the calling process.'), so printDryRun only runs when the real command would proceed; reference flags['dry-run'], printDryRun, process.env.BEEPER_ACCESS_TOKEN, and target.auth?.accessToken.
♻️ Duplicate comments (1)
packages/cli/src/commands/api/get.ts (1)
24-24:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRestore the blank line before
const client.CI still reports
@stylistic/padding-line-between-statementshere after the early return block.🤖 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/cli/src/commands/api/get.ts` at line 24, Add a blank line before the statement "const client = await createClient(flags)" so there is a padding line separating it from the preceding early-return block; locate the code in the get command where createClient(flags) is awaited and ensure a single empty line exists above that const declaration to satisfy `@stylistic/padding-line-between-statements`.
🤖 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/cli/src/commands/api/post.ts`:
- Around line 28-37: Add missing blank lines between the conditional branches in
the api.post command: insert a blank line before the if (flags['dry-run'])
branch, another blank line before the if (flags['no-auth']) branch, and a blank
line before the const client = await createClient(flags) statement so the
sequence of checks involving flags['dry-run'], printDryRun, flags['no-auth'],
appRequest, createClient and client.post complies with
`@stylistic/padding-line-between-statements`.
In `@packages/cli/src/commands/api/request.ts`:
- Around line 24-32: Add the required blank line padding between the conditional
blocks in the api.request flow: insert a blank line before the "if
(flags['no-auth'])" branch and another blank line before the final "await
printData(await appRequest(...))" call so that the sequence (dry-run -> no-auth
-> default) has separating empty lines; adjust the block around printDryRun,
appRequest, and printData to satisfy `@stylistic/padding-line-between-statements`
without changing logic.
In `@packages/cli/src/commands/config/reset.ts`:
- Around line 9-14: The ensureWritable check is applied even for dry-run; change
the control flow so that when flags['dry-run'] is true the code calls
printDryRun('config.reset', {}, format) and returns before calling
ensureWritable. Concretely, either move the ensureWritable(flags) call below the
dry-run branch or wrap it in if (!flags['dry-run']) so ensureWritable is only
enforced for real mutations; keep references to ensureWritable, flags,
printDryRun and ConfigReset to locate the change.
In `@packages/cli/src/commands/config/set.ts`:
- Around line 18-22: The code lacks the required blank separator before the
subsequent statement: after the conditional that checks flags['dry-run'] and
calls printDryRun (and returns), insert a blank line before the call to
updateConfig to satisfy `@stylistic/padding-line-between-statements`;
specifically, ensure there is an empty line between the return from the
printDryRun branch and the updateConfig(() => ...) invocation that updates
[args.key] to nextValue.
- Around line 17-20: The dry-run hides a "clear" intent because nextValue is
undefined when args.value === '' so printDryRun('config.set', { [args.key]:
nextValue }, ...) omits the key; change the value used for the dry-run payload
so an explicit marker is shown (e.g., use null) — compute a separate
payloadValue (payloadValue = args.value === '' ? null : args.value) and pass
that to printDryRun while leaving nextValue/actual set logic unchanged; update
references in the config.set flow (args.value, nextValue, printDryRun) so JSON
dry-run shows the clear intent.
In `@packages/npm/scripts/build.ts`:
- Line 33: The download() helper lacks request/socket timeouts and doesn't clean
up partial files on stall/errors; update download() to use an AbortController
(or set a socket timeout) to abort the request after a configurable timeout, and
on abort/error ensure the created write stream (createWriteStream(destination))
is closed and the partial destination file (and tempDir if applicable) is
removed (use rm with force) before rejecting; also attach cleanup logic to
response 'error', file 'error', and the outer request 'error' handlers so
artifact.file/destination is always removed on failure and redirects continue to
propagate the same cleanup behavior.
---
Outside diff comments:
In `@packages/cli/src/commands/auth/logout.ts`:
- Around line 21-32: The last statement calling printSuccess(...) needs a blank
separator line before it to satisfy stylistic rules; insert a single empty line
between the token-handling block (where you call clearTargetAuth(target) and
compute revoked) and the final await printSuccess(...) call so there is padding
between the if/await block and the printSuccess invocation; target symbols to
locate are the revoked/token variables, the fetch + clearTargetAuth(target)
sequence, and the await printSuccess(...) call.
- Around line 14-20: The dry-run path currently returns a normal payload even
when logout would fail due to BEEPER_ACCESS_TOKEN being set and no
target.auth?.accessToken; move or duplicate the env check so the same error is
thrown during dry-run: before calling printDryRun('auth.logout', ...) check if
process.env.BEEPER_ACCESS_TOKEN && !target.auth?.accessToken and throw the same
Error('auth logout cannot clear BEEPER_ACCESS_TOKEN from the environment; unset
it in the calling process.'), so printDryRun only runs when the real command
would proceed; reference flags['dry-run'], printDryRun,
process.env.BEEPER_ACCESS_TOKEN, and target.auth?.accessToken.
In `@packages/cli/src/commands/setup.ts`:
- Around line 53-76: The dry-run path currently calls ensureWritable(flags)
before deciding to return, causing read-only environments to fail; update setup
command logic so the dry-run shortcut is checked and returned before calling
ensureWritable(flags) (or make ensureWritable a no-op when flags['dry-run'] is
true): move the flags['dry-run'] check (which calls printDryRun) to run prior to
invoking ensureWritable(flags) (or add an early return when flags['dry-run'] is
set) so printDryRun can run in read-only configs; touch the Setup command entry
where ensureWritable and printDryRun are used to implement this change.
---
Duplicate comments:
In `@packages/cli/src/commands/api/get.ts`:
- Line 24: Add a blank line before the statement "const client = await
createClient(flags)" so there is a padding line separating it from the preceding
early-return block; locate the code in the get command where createClient(flags)
is awaited and ensure a single empty line exists above that const declaration to
satisfy `@stylistic/padding-line-between-statements`.
🪄 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: 1f36d472-5015-49fd-bfa0-e4c40634eba9
📒 Files selected for processing (53)
.gitignorepackages/cli/src/commands/api/get.tspackages/cli/src/commands/api/post.tspackages/cli/src/commands/api/request.tspackages/cli/src/commands/auth/email/response.tspackages/cli/src/commands/auth/logout.tspackages/cli/src/commands/bridges/show.tspackages/cli/src/commands/chats/archive.tspackages/cli/src/commands/chats/avatar.tspackages/cli/src/commands/chats/description.tspackages/cli/src/commands/chats/focus.tspackages/cli/src/commands/chats/mark-read.tspackages/cli/src/commands/chats/mark-unread.tspackages/cli/src/commands/chats/notify-anyway.tspackages/cli/src/commands/chats/pin.tspackages/cli/src/commands/chats/remind.tspackages/cli/src/commands/chats/unarchive.tspackages/cli/src/commands/chats/unmute.tspackages/cli/src/commands/chats/unpin.tspackages/cli/src/commands/chats/unremind.tspackages/cli/src/commands/config/reset.tspackages/cli/src/commands/config/set.tspackages/cli/src/commands/doctor.tspackages/cli/src/commands/media/download.tspackages/cli/src/commands/messages/search.tspackages/cli/src/commands/plugins/available.tspackages/cli/src/commands/presence.tspackages/cli/src/commands/resolve/chat.tspackages/cli/src/commands/schema.tspackages/cli/src/commands/setup.tspackages/cli/src/commands/targets/disable.tspackages/cli/src/commands/targets/enable.tspackages/cli/src/commands/targets/list.tspackages/cli/src/commands/targets/logs.tspackages/cli/src/commands/targets/remove.tspackages/cli/src/commands/targets/restart.tspackages/cli/src/commands/targets/show.tspackages/cli/src/commands/targets/start.tspackages/cli/src/commands/targets/status.tspackages/cli/src/commands/targets/stop.tspackages/cli/src/commands/targets/use.tspackages/cli/src/commands/verify/approve.tspackages/cli/src/commands/verify/cancel.tspackages/cli/src/lib/command-metadata.tspackages/cli/src/lib/command.tspackages/cli/src/lib/export/index.tspackages/cli/src/lib/ink/render.tsxpackages/cli/src/lib/ink/theme.tspackages/cli/src/lib/installations.tspackages/cli/src/lib/profiles.tspackages/cli/src/lib/resolve.tspackages/cli/src/lib/setup-login.tspackages/npm/scripts/build.ts
💤 Files with no reviewable changes (14)
- packages/cli/src/commands/chats/notify-anyway.ts
- packages/cli/src/commands/chats/mark-read.ts
- packages/cli/src/commands/chats/archive.ts
- packages/cli/src/lib/export/index.ts
- packages/cli/src/commands/chats/unremind.ts
- packages/cli/src/commands/chats/avatar.ts
- packages/cli/src/commands/chats/unpin.ts
- packages/cli/src/commands/chats/unmute.ts
- packages/cli/src/commands/chats/pin.ts
- packages/cli/src/commands/chats/unarchive.ts
- packages/cli/src/commands/chats/remind.ts
- packages/cli/src/commands/chats/mark-unread.ts
- packages/cli/src/commands/chats/focus.ts
- packages/cli/src/commands/chats/description.ts
✅ Files skipped from review due to trivial changes (7)
- .gitignore
- packages/cli/src/commands/plugins/available.ts
- packages/cli/src/commands/targets/list.ts
- packages/cli/src/commands/bridges/show.ts
- packages/cli/src/commands/targets/show.ts
- packages/cli/src/lib/ink/theme.ts
- packages/cli/src/commands/targets/status.ts
🚧 Files skipped from review as they are similar to previous changes (22)
- packages/cli/src/commands/targets/use.ts
- packages/cli/src/commands/targets/logs.ts
- packages/cli/src/commands/targets/start.ts
- packages/cli/src/commands/targets/stop.ts
- packages/cli/src/commands/targets/restart.ts
- packages/cli/src/commands/auth/email/response.ts
- packages/cli/src/commands/media/download.ts
- packages/cli/src/lib/setup-login.ts
- packages/cli/src/commands/targets/disable.ts
- packages/cli/src/commands/targets/enable.ts
- packages/cli/src/commands/targets/remove.ts
- packages/cli/src/commands/verify/approve.ts
- packages/cli/src/lib/ink/render.tsx
- packages/cli/src/commands/messages/search.ts
- packages/cli/src/commands/presence.ts
- packages/cli/src/lib/command-metadata.ts
- packages/cli/src/lib/resolve.ts
- packages/cli/src/commands/resolve/chat.ts
- packages/cli/src/lib/installations.ts
- packages/cli/src/lib/profiles.ts
- packages/cli/src/lib/command.ts
- packages/cli/src/commands/schema.ts
📜 Review details
🧰 Additional context used
🪛 ESLint
packages/cli/src/commands/verify/cancel.ts
[error] 17-17: Expected blank line before this statement.
(@stylistic/padding-line-between-statements)
packages/cli/src/commands/api/get.ts
[error] 24-24: Expected blank line before this statement.
(@stylistic/padding-line-between-statements)
packages/cli/src/commands/api/post.ts
[error] 28-28: Expected blank line before this statement.
(@stylistic/padding-line-between-statements)
[error] 32-32: Expected blank line before this statement.
(@stylistic/padding-line-between-statements)
[error] 36-36: Expected blank line before this statement.
(@stylistic/padding-line-between-statements)
packages/cli/src/commands/api/request.ts
[error] 28-28: Expected blank line before this statement.
(@stylistic/padding-line-between-statements)
[error] 32-32: Expected blank line before this statement.
(@stylistic/padding-line-between-statements)
packages/cli/src/commands/auth/logout.ts
[error] 32-32: Expected blank line before this statement.
(@stylistic/padding-line-between-statements)
packages/cli/src/commands/config/reset.ts
[error] 16-16: Expected blank line before this statement.
(@stylistic/padding-line-between-statements)
packages/cli/src/commands/config/set.ts
[error] 22-22: Expected blank line before this statement.
(@stylistic/padding-line-between-statements)
packages/cli/src/commands/setup.ts
[error] 140-140: Use switch instead of multiple else-if.
(unicorn/prefer-switch)
🔇 Additional comments (2)
packages/cli/src/commands/verify/cancel.ts (1)
13-17: Missing blank line beforeconst clientstill fails lint.This is the same
@stylistic/padding-line-between-statementsviolation already called out earlier, and it still appears at Line 17.packages/cli/src/commands/doctor.ts (1)
13-17: LGTM!
| if (flags['dry-run']) { | ||
| await printDryRun('api.post', { method: 'POST', path: args.path, body, noAuth: flags['no-auth'], target: flags.target, baseURL: flags['base-url'] }, format) | ||
| return | ||
| } | ||
| if (flags['no-auth']) { | ||
| await printData(await appRequest('POST', args.path, { baseURL: flags['base-url'], body, target: flags.target, token: false }), flags.json ? 'json' : 'human') | ||
| await printData(await appRequest('POST', args.path, { baseURL: flags['base-url'], body, target: flags.target, token: false }), format) | ||
| return | ||
| } | ||
| const client = await createClient(flags) | ||
| await printData(await client.post(args.path, { body }), flags.json ? 'json' : 'human') | ||
| await printData(await client.post(args.path, { body }), format) |
There was a problem hiding this comment.
Add the missing blank lines between these branches.
This block still violates @stylistic/padding-line-between-statements before the dry-run branch, before the no-auth branch, and before const client, so lint will keep failing.
🧰 Tools
🪛 ESLint
[error] 28-28: Expected blank line before this statement.
(@stylistic/padding-line-between-statements)
[error] 32-32: Expected blank line before this statement.
(@stylistic/padding-line-between-statements)
[error] 36-36: Expected blank line before this statement.
(@stylistic/padding-line-between-statements)
🤖 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/cli/src/commands/api/post.ts` around lines 28 - 37, Add missing
blank lines between the conditional branches in the api.post command: insert a
blank line before the if (flags['dry-run']) branch, another blank line before
the if (flags['no-auth']) branch, and a blank line before the const client =
await createClient(flags) statement so the sequence of checks involving
flags['dry-run'], printDryRun, flags['no-auth'], appRequest, createClient and
client.post complies with `@stylistic/padding-line-between-statements`.
| if (flags['dry-run'] && method !== 'GET') { | ||
| await printDryRun('api.request', { method, path: args.path, body, noAuth: flags['no-auth'], target: flags.target, baseURL: flags['base-url'] }, format) | ||
| return | ||
| } | ||
| if (flags['no-auth']) { | ||
| await printData(await appRequest(method, args.path, { baseURL: flags['base-url'], body, target: flags.target, token: false }), flags.json ? 'json' : 'human') | ||
| await printData(await appRequest(method, args.path, { baseURL: flags['base-url'], body, target: flags.target, token: false }), format) | ||
| return | ||
| } | ||
| await printData(await appRequest(method, args.path, { baseURL: flags['base-url'], body, target: flags.target }), flags.json ? 'json' : 'human') | ||
| await printData(await appRequest(method, args.path, { baseURL: flags['base-url'], body, target: flags.target }), format) |
There was a problem hiding this comment.
Insert the required padding between these statements.
@stylistic/padding-line-between-statements still fails in this block, so please add the missing blank lines before the no-auth branch and before the final printData(...) call.
🧰 Tools
🪛 ESLint
[error] 28-28: Expected blank line before this statement.
(@stylistic/padding-line-between-statements)
[error] 32-32: Expected blank line before this statement.
(@stylistic/padding-line-between-statements)
🤖 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/cli/src/commands/api/request.ts` around lines 24 - 32, Add the
required blank line padding between the conditional blocks in the api.request
flow: insert a blank line before the "if (flags['no-auth'])" branch and another
blank line before the final "await printData(await appRequest(...))" call so
that the sequence (dry-run -> no-auth -> default) has separating empty lines;
adjust the block around printDryRun, appRequest, and printData to satisfy
`@stylistic/padding-line-between-statements` without changing logic.
| const { flags } = await this.parse(ConfigReset) | ||
| ensureWritable(flags) | ||
| const format = flags.json ? 'json' : 'human' | ||
| if (flags['dry-run']) { | ||
| await printDryRun('config.reset', {}, format) | ||
| return |
There was a problem hiding this comment.
Skip ensureWritable() for --dry-run.
The new dry-run path returns before any config mutation, but Line 10 still enforces the writable precondition first. In a read-only environment, beeper config reset --dry-run will fail even though it only prints the preview.
🔧 Minimal fix
async run(): Promise<void> {
const { flags } = await this.parse(ConfigReset)
- ensureWritable(flags)
const format = flags.json ? 'json' : 'human'
+
+ if (!flags['dry-run']) ensureWritable(flags)
+
if (flags['dry-run']) {
await printDryRun('config.reset', {}, format)
return
}🤖 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/cli/src/commands/config/reset.ts` around lines 9 - 14, The
ensureWritable check is applied even for dry-run; change the control flow so
that when flags['dry-run'] is true the code calls printDryRun('config.reset',
{}, format) and returns before calling ensureWritable. Concretely, either move
the ensureWritable(flags) call below the dry-run branch or wrap it in if
(!flags['dry-run']) so ensureWritable is only enforced for real mutations; keep
references to ensureWritable, flags, printDryRun and ConfigReset to locate the
change.
| const nextValue = args.value === '' ? undefined : args.value | ||
| if (flags['dry-run']) { | ||
| await printDryRun('config.set', { [args.key]: nextValue }, format) | ||
| return |
There was a problem hiding this comment.
Preserve clear intent in JSON dry-run output.
When args.value === '', nextValue becomes undefined, so the dry-run payload at Line 19 is likely serialized without that key at all. --dry-run --json for a clear then looks like an empty/no-op change instead of “clear this setting”.
🔧 One way to make the preview explicit
const format = flags.json ? 'json' : 'human'
const nextValue = args.value === '' ? undefined : args.value
if (flags['dry-run']) {
- await printDryRun('config.set', { [args.key]: nextValue }, format)
+ await printDryRun(
+ 'config.set',
+ nextValue === undefined
+ ? { key: args.key, clear: true }
+ : { key: args.key, value: nextValue },
+ format,
+ )
return
}🤖 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/cli/src/commands/config/set.ts` around lines 17 - 20, The dry-run
hides a "clear" intent because nextValue is undefined when args.value === '' so
printDryRun('config.set', { [args.key]: nextValue }, ...) omits the key; change
the value used for the dry-run payload so an explicit marker is shown (e.g., use
null) — compute a separate payloadValue (payloadValue = args.value === '' ? null
: args.value) and pass that to printDryRun while leaving nextValue/actual set
logic unchanged; update references in the config.set flow (args.value,
nextValue, printDryRun) so JSON dry-run shows the clear intent.
| if (flags['dry-run']) { | ||
| await printDryRun('config.set', { [args.key]: nextValue }, format) | ||
| return | ||
| } | ||
| await updateConfig(config => ({ ...config, [args.key]: nextValue })) |
There was a problem hiding this comment.
Add the missing separator before updateConfig().
Line 22 still trips @stylistic/padding-line-between-statements, so this will fail lint as-is.
🎨 Minimal fix
if (flags['dry-run']) {
await printDryRun('config.set', { [args.key]: nextValue }, format)
return
}
+
await updateConfig(config => ({ ...config, [args.key]: nextValue }))📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (flags['dry-run']) { | |
| await printDryRun('config.set', { [args.key]: nextValue }, format) | |
| return | |
| } | |
| await updateConfig(config => ({ ...config, [args.key]: nextValue })) | |
| if (flags['dry-run']) { | |
| await printDryRun('config.set', { [args.key]: nextValue }, format) | |
| return | |
| } | |
| await updateConfig(config => ({ ...config, [args.key]: nextValue })) |
🧰 Tools
🪛 ESLint
[error] 22-22: Expected blank line before this statement.
(@stylistic/padding-line-between-statements)
🤖 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/cli/src/commands/config/set.ts` around lines 18 - 22, The code lacks
the required blank separator before the subsequent statement: after the
conditional that checks flags['dry-run'] and calls printDryRun (and returns),
insert a blank line before the call to updateConfig to satisfy
`@stylistic/padding-line-between-statements`; specifically, ensure there is an
empty line between the return from the printDryRun branch and the
updateConfig(() => ...) invocation that updates [args.key] to nextValue.
| file.on('error', reject) | ||
| }).on('error', reject) | ||
| }) | ||
| return "#!/usr/bin/env node\nimport { createHash } from 'node:crypto'\nimport { createWriteStream, existsSync } from 'node:fs'\nimport { chmod, mkdir, readFile, rename, rm } from 'node:fs/promises'\nimport { get } from 'node:https'\nimport { homedir, platform as osPlatform, arch as osArch, tmpdir } from 'node:os'\nimport { dirname, join } from 'node:path'\nimport { fileURLToPath } from 'node:url'\nimport { spawn } from 'node:child_process'\n\nconst packageRoot = join(dirname(fileURLToPath(import.meta.url)), '..')\nconst manifest = JSON.parse(await readFile(join(packageRoot, 'binaries.json'), 'utf8'))\nconst platform = targetPlatform()\nconst artifact = manifest.artifacts.find(item => item.platform === platform)\n\nif (!artifact) {\n console.error(`beeper-cli does not ship a binary for ${process.platform}/${process.arch}.`)\n process.exit(1)\n}\n\nconst cacheDir = process.env.BEEPER_CLI_BINARY_CACHE_DIR || join(homedir(), '.cache', 'beeper-cli', manifest.version)\nconst binPath = join(cacheDir, 'bin', manifest.command || 'beeper')\n\nconst expectedBinarySha256 = artifact.binarySha256 || artifact.sha256\n\nif (!existsSync(binPath) || await sha256(binPath).catch(() => '') !== expectedBinarySha256) {\n const tempDir = join(tmpdir(), `beeper-cli-${manifest.version}-${process.pid}`)\n const archivePath = join(tempDir, artifact.file)\n const downloadURL = `https://github.com/beeper/cli/releases/download/v${manifest.version}/${artifact.file}`\n logStep(`installing beeper-cli ${manifest.version} for ${platform}`)\n await rm(tempDir, { recursive: true, force: true })\n await mkdir(tempDir, { recursive: true })\n await download(downloadURL, archivePath)\n logStep('verifying download')\n const actual = await sha256(archivePath)\n if (actual !== artifact.sha256) {\n await rm(tempDir, { recursive: true, force: true })\n console.error(`beeper-cli binary checksum mismatch for ${artifact.file}.`)\n process.exit(1)\n }\n logStep('extracting binary')\n await extract(archivePath, tempDir)\n const extractedBin = join(tempDir, 'bin', manifest.command || 'beeper')\n await chmod(extractedBin, 0o755)\n logStep(`caching binary in ${cacheDir}`)\n await rm(cacheDir, { recursive: true, force: true })\n await mkdir(dirname(binPath), { recursive: true })\n await rename(extractedBin, binPath)\n await rm(tempDir, { recursive: true, force: true })\n logStep('ready')\n}\n\nif (process.env.BEEPER_CLI_LAUNCHER_DEBUG === '1') logStep(`starting ${binPath}`)\nconst child = spawn(binPath, process.argv.slice(2), { stdio: 'inherit', env: process.env })\nchild.on('exit', (code, signal) => {\n if (signal) process.kill(process.pid, signal)\n process.exit(code ?? 1)\n})\n\nfunction logStep(message) {\n console.error(`beeper-cli: ${message}`)\n}\n\nfunction targetPlatform() {\n const os = osPlatform()\n const cpu = osArch()\n const normalizedOS = os === 'darwin' || os === 'linux' ? os : os === 'win32' ? 'windows' : os\n const normalizedArch = cpu === 'x64' || cpu === 'arm64' ? cpu : cpu\n return `${normalizedOS}-${normalizedArch}`\n}\n\nasync function sha256(path) {\n const hash = createHash('sha256')\n hash.update(await readFile(path))\n return hash.digest('hex')\n}\n\nasync function download(url, destination, redirects = 0) {\n if (redirects > 10) throw new Error(`Too many redirects while downloading ${artifact.file}`)\n\n logStep(`downloading ${artifact.file}`)\n await new Promise((resolve, reject) => {\n get(url, response => {\n if ([301, 302, 303, 307, 308].includes(response.statusCode ?? 0) && response.headers.location) {\n response.resume()\n const nextURL = new URL(response.headers.location, url).toString()\n logStep(`redirecting to ${new URL(nextURL).host}`)\n download(nextURL, destination, redirects + 1).then(resolve, reject)\n return\n }\n if (response.statusCode !== 200) {\n response.resume()\n reject(new Error(`Download failed with HTTP ${response.statusCode}: ${url}`))\n return\n }\n const total = Number(response.headers['content-length'] ?? 0)\n let downloaded = 0\n let nextLoggedPercent = 25\n const file = createWriteStream(destination, { mode: 0o755 })\n response.on('data', chunk => {\n downloaded += chunk.length\n if (!total) return\n const percent = Math.floor(downloaded / total * 100)\n while (percent >= nextLoggedPercent && nextLoggedPercent <= 100) {\n logStep(`downloaded ${nextLoggedPercent}%`)\n nextLoggedPercent += 25\n }\n })\n response.pipe(file)\n file.on('finish', () => file.close(resolve))\n file.on('error', reject)\n }).on('error', reject)\n })\n}\n\nasync function extract(archivePath, destination) {\n if (artifact.file.endsWith('.zip')) {\n await run('/usr/bin/ditto', ['-x', '-k', archivePath, destination])\n return\n }\n if (artifact.file.endsWith('.tar.gz')) {\n await run('tar', ['-xzf', archivePath, '-C', destination])\n return\n }\n throw new Error(`Unsupported beeper-cli archive: ${artifact.file}`)\n}\n\nasync function run(command, args) {\n await new Promise((resolve, reject) => {\n const child = spawn(command, args, { stdio: 'ignore' })\n child.on('error', reject)\n child.on('exit', code => {\n if (code === 0) resolve()\n else reject(new Error(`${command} ${args.join(' ')} exited with ${code}`))\n })\n })\n}" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate file
ls -la packages/npm/scripts/build.ts
# Show relevant sections around download() and where line 33 is in file
# Use sed to print with line numbers range
python3 - <<'PY'
import itertools
path="packages/npm/scripts/build.ts"
with open(path,'r',encoding='utf-8') as f:
lines=f.readlines()
for i in range(1, len(lines)+1):
if "async function download" in lines[i-1]:
start=max(1,i-30)
end=min(len(lines), i+120)
print(f"--- {path} lines {start}-{end} around async function download (line {i}) ---")
for j in range(start,end+1):
print(f"{j:5d}: {lines[j-1].rstrip()}")
break
else:
print("Did not find async function download")
PY
# Search within file for timeout/abort-related usage
rg -n "timeout|setTimeout|AbortController|abort\(|signal:" packages/npm/scripts/build.ts || true
# Also check for agent usage
rg -n "agent|http(s)?\\.Agent" packages/npm/scripts/build.ts || trueRepository: beeper/cli
Length of output: 7412
Add timeouts (and cleanup) to launcher downloads
The generated download() helper uses node:https get() with no request/socket timeout (no AbortController), so a stalled connection can hang during npm install; add a timeout and close the write stream + remove the partially downloaded file (and/or tempDir) before rejecting.
🤖 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/npm/scripts/build.ts` at line 33, The download() helper lacks
request/socket timeouts and doesn't clean up partial files on stall/errors;
update download() to use an AbortController (or set a socket timeout) to abort
the request after a configurable timeout, and on abort/error ensure the created
write stream (createWriteStream(destination)) is closed and the partial
destination file (and tempDir if applicable) is removed (use rm with force)
before rejecting; also attach cleanup logic to response 'error', file 'error',
and the outer request 'error' handlers so artifact.file/destination is always
removed on failure and redirects continue to propagate the same cleanup
behavior.
1 participant
Summary
docs/, refresh the top-level README, and regenerate the CLI README/docs with links to the published docs.beeper schema, command aliases likebeeper send,beeper search, andbeeper ls, command metadata, broader--dry-runcoverage for mutating commands, and machine-readable output controls for formats, field selection, results-only output, text, and ids.ok/error, improve resolver and schema validation errors, and make spinners respect machine-readable output modes.local,dev,staging, andprod.Testing
messages searchvalidation test updated for the new structured error payload.