CLI v2#7
Open
batuhan wants to merge 46 commits into
Open
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis pull request introduces a new TypeScript-based Beeper CLI implemented with Oclif featuring numerous commands for managing accounts, chats, messages, assets, E2EE flows, and more. It adds OpenTUI skill documentation in MDX for React and Solid bindings, core concepts, components, keymap APIs, and plugin slots. Also included are CLI configuration management, OAuth PKCE helpers, command output formatting, argument parsing, interactive TUI application code, and release packaging & publishing scripts. Legacy Go code, tests, and shell completion scripts were removed. ChangesBeeper CLI Commands and Client
Node.js Release & Packaging Scripts
Interactive Terminal App and UI Helpers
CLI Configuration Module
OAuth PKCE Authentication Helpers
CLI Output and Pagination Utilities
CLI Entrypoints and Helpers
Command Line Argument Parsing
Account Login Helpers and CLI Command
ID Resolution and CLI Runner Helpers
Async wait helpers
OpenTUI Skill Documentation
✨ Finishing Touches🧪 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: 18
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (14)
.agents/skills/opentui/docs/components/ascii-font.mdx-81-84 (1)
81-84:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winTypo: "Pallet" should be "Palette".
The font style name appears to be "Palette" (a range of colors/styles) rather than "Pallet" (a wooden shipping platform).
📝 Proposed fix
-// Pallet style font +// Palette style font { - font: "pallet" + font: "palette" }🤖 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 @.agents/skills/opentui/docs/components/ascii-font.mdx around lines 81 - 84, The font style name is misspelled as "pallet" in the font config object; update the value for the font key (the object containing font: "pallet") to "palette" so the style name reads font: "palette" (ensure you update any identical occurrences of "pallet" in the same component to maintain consistency)..agents/skills/opentui/docs/keymap/hosts.mdx-55-55 (1)
55-55:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse correct "macOS" branding.
Apple's operating system should be capitalized as "macOS" per official branding guidelines.
📝 Proposed fix
-| `platform` | `macos`, `windows`, `linux` or `unknown` | +| `platform` | `macOS`, `windows`, `linux` or `unknown` |🤖 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 @.agents/skills/opentui/docs/keymap/hosts.mdx at line 55, Update the platform table entry so Apple’s OS uses the correct branding: replace the literal `macos` string with `macOS` in the table row that documents the `platform` field (the cell currently showing `macos`, `windows`, `linux` or `unknown`), preserving the rest of the format and punctuation..agents/skills/opentui/docs/components/tab-select.mdx-158-158 (1)
158-158:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse the exported event constant instead of a string literal.
Line 158 uses the string literal
"itemSelected"while the rest of the documentation consistently demonstratesTabSelectRenderableEvents.ITEM_SELECTED. Use the constant to maintain consistency with the earlier examples and prevent silent drift if event names change.Proposed doc fix
-tabs.on("itemSelected", (index, option) => { +tabs.on(TabSelectRenderableEvents.ITEM_SELECTED, (index, option) => {🤖 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 @.agents/skills/opentui/docs/components/tab-select.mdx at line 158, Replace the hard-coded event string in the event listener with the exported enum/constant to keep docs consistent: change the tabs.on("itemSelected", (index, option) => { ...) usage to use TabSelectRenderableEvents.ITEM_SELECTED so the listener references the exported event constant (TabSelectRenderableEvents.ITEM_SELECTED) instead of the string literal..agents/skills/opentui/docs/components/select.mdx-79-82 (1)
79-82:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd
SelectOptiontype import to event handler snippets.The code examples at lines 81 and 91 use
SelectOptionas a type annotation in function parameters but don't import it. Copy-pasting these snippets into a TypeScript file will fail to compile with "SelectOption is not defined."Fix
-import { SelectRenderableEvents } from "@opentui/core" +import { SelectRenderableEvents, type SelectOption } from "@opentui/core"Also applies to line 91.
🤖 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 @.agents/skills/opentui/docs/components/select.mdx around lines 79 - 82, The examples register a handler using menu.on(SelectRenderableEvents.ITEM_SELECTED, ...) and annotate the callback parameter with SelectOption but never import that type, causing TypeScript compile errors; fix by adding an import for SelectOption from "@opentui/core" alongside the existing SelectRenderableEvents import so the two snippets (the handler at the menu.on call shown and the similar one on line 91) reference a defined type and compile correctly..agents/skills/opentui/docs/keymap/react.mdx-115-117 (1)
115-117:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd React and OpenTUI imports to the
reactiveMatcherFromStore()code example.The snippet on lines 115–127 uses
useMemo,reactiveMatcherFromStore, anduseBindingswithout showing their imports. While the "Basic usage" example earlier includes complete imports, this pattern example omits them, making it fail if copied into a standalone file.+import { useMemo } from "react" +import { reactiveMatcherFromStore, useBindings } from "@opentui/keymap/react" + const matcher = useMemo( () => reactiveMatcherFromStore(store.subscribe, store.getSnapshot, (mode) => mode === "normal"), [store], ) useBindings(🤖 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 @.agents/skills/opentui/docs/keymap/react.mdx around lines 115 - 117, The example is missing imports for React hooks and OpenTUI helpers; add imports for useMemo (from 'react' or 'preact' depending on the project), and import reactiveMatcherFromStore and useBindings from the OpenTUI package so the snippet using useMemo, reactiveMatcherFromStore, and useBindings is complete and copy-pasteable; update the top of the example to include these imports and ensure the named symbols (useMemo, reactiveMatcherFromStore, useBindings) match the existing exported names used elsewhere in the docs.src/commands/unarchive.ts-7-7 (1)
7-7:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse unarchive-specific help summary.
Line 7 points to
apiCopy.chats.archive, so help text for this command is likely misleading. Use an unarchive-specific summary key/string.Suggested change
- static override summary = apiCopy.chats.archive + static override summary = apiCopy.chats.unarchive🤖 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 `@src/commands/unarchive.ts` at line 7, The command's help summary wrongly reuses apiCopy.chats.archive; update the static override summary in the unarchive command (the static override summary field) to reference the unarchive-specific copy key/string (e.g., apiCopy.chats.unarchive or the appropriate unarchive summary key) so the CLI help shows an accurate description for the unarchive command.src/commands/message-expiry.ts-24-24 (1)
24-24:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAlign validation error text with actual rule.
Line 24 allows
0but the message says “positive integer”. Use “non-negative integer” (or change the check to<= 0).Proposed fix
- if (expiry !== null && (!Number.isInteger(expiry) || expiry < 0)) throw new Error('SECONDS must be a positive integer or "off"') + if (expiry !== null && (!Number.isInteger(expiry) || expiry < 0)) throw new Error('SECONDS must be a non-negative integer or "off"')🤖 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 `@src/commands/message-expiry.ts` at line 24, The validation message says "positive integer" but the code permits 0; update the check and message to be consistent: either change the condition in the message/throw in the message to say "non-negative integer" (so keep the current condition using expiry !== null && (!Number.isInteger(expiry) || expiry < 0)) or tighten the validation to forbid 0 by using expiry <= 0 and keep "positive integer" in the Error; locate the validation around the expiry variable in the message-expiry command (the if that throws Error('SECONDS must be a positive integer or "off"')) and make the corresponding change so the thrown error text matches the numeric check.scripts/check-api-copy.mjs-21-22 (1)
21-22:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd drift coverage for
contacts.list.
apiCopy.contacts.listis defined but not included inchecks, so this script won’t detect drift for that entry.Proposed fix
['chats.start', 'resources/chats/chats.d.ts', 'start'], + ['contacts.list', 'resources/accounts/contacts.d.ts', 'list'], ['contacts.search', 'resources/accounts/contacts.d.ts', 'search'],🤖 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 `@scripts/check-api-copy.mjs` around lines 21 - 22, The checks array in scripts/check-api-copy.mjs is missing coverage for apiCopy.contacts.list, so add an entry for it (e.g., ['contacts.list', 'resources/contacts.d.ts', 'list']) to the list of checks so the script will detect drift for contacts.list; update the checks array near the existing ['contacts.search', ...] and ['messages.delete', ...] entries to include that tuple referencing the apiCopy key (contacts.list), the d.ts resource (resources/contacts.d.ts), and the method name ('list').src/commands/config/set.ts-8-14 (1)
8-14:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winValidate
baseURLbefore writing config.Line 13 persists raw input, so malformed URLs can poison config and cause later command failures.
Suggested fix
async run(): Promise<void> { const { args } = await this.parse(ConfigSet) + if (args.key === 'baseURL') { + try { + new URL(args.value) + } catch { + this.error(`Invalid baseURL: ${args.value}`) + } + } await updateConfig(config => ({ ...config, [args.key]: args.value })) this.log(`${args.key}=${args.value}`) }🤖 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 `@src/commands/config/set.ts` around lines 8 - 14, When handling the ConfigSet.run() flow, validate args.value when args.key === 'baseURL' before calling updateConfig to avoid persisting malformed URLs; in run(), check if args.key === 'baseURL' and attempt to parse args.value with the URL constructor (or otherwise validate scheme/host), and if parsing fails call this.error(...) or throw a descriptive error instead of writing to updateConfig, otherwise proceed with updateConfig(config => ({ ...config, [args.key]: args.value })) and this.log.src/commands/send-file.ts-51-55 (1)
51-55:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd an explicit guard for missing
pendingMessageIDwhen--waitis enabled.The wait path currently assumes
pendingMessageIDexists; if it does not, this fails deeper inwaitForMessagewith a less clear error.Suggested fix
if (flags.wait) { + if (!result.pendingMessageID) { + this.error('Cannot wait for resolution: send did not return a pendingMessageID.') + } const resolved = await waitForMessage(client, chatID, result.pendingMessageID, { intervalMs: flags['wait-interval'], timeoutMs: flags['wait-timeout'], })🤖 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 `@src/commands/send-file.ts` around lines 51 - 55, When flags.wait is true, the code calls waitForMessage(client, chatID, result.pendingMessageID, ...) without verifying result.pendingMessageID; add an explicit guard that checks if result.pendingMessageID is present before calling waitForMessage and if missing throw or return a clear error (e.g., log and exit or reject) indicating that pendingMessageID is required for the --wait path. Update the block around flags.wait to validate result.pendingMessageID and only call waitForMessage when it exists, otherwise provide a descriptive error mentioning pendingMessageID and the --wait flag.src/commands/create-chat.ts-24-30 (1)
24-30:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winEnforce single-chat participant cardinality before sending the request.
--type singlecurrently allows multiple--participantvalues, which can produce an ambiguous/invalid create payload and fail downstream.Suggested fix
async run(): Promise<void> { const { flags } = await this.parse(CreateChat) const client = await createClient(flags) const accountID = await resolveAccountID(client, flags.account) + if (flags.type === 'single' && flags.participant.length !== 1) { + this.error('`--type single` requires exactly one `--participant`.') + } + const result = await client.chats.create({ accountID, messageText: flags.message, participantIDs: flags.participant, title: flags.title, type: flags.type as 'single' | 'group', })🤖 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 `@src/commands/create-chat.ts` around lines 24 - 30, Before calling client.chats.create, validate the participant cardinality when flags.type === 'single': check flags.participant (or the parsed participant array) and if type is 'single' ensure exactly one participant is provided; if not, throw or print a clear error and exit (or return) to avoid sending an invalid payload to client.chats.create. Locate the create call (client.chats.create) and the CLI flags (flags.participant, flags.type, flags.message, etc.) and add this guard so only a single participant is allowed for 'single' chats.src/commands/api/post.ts-20-21 (1)
20-21:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winHandle invalid
--bodyJSON with a CLI-friendly error.Line 20 parses untrusted CLI input directly; malformed JSON will throw an unhandled exception. Wrap in try-catch and rethrow with a clear message (other commands like
watch.tsandrpc.tsalready follow this pattern).Suggested patch
- const body = JSON.parse(flags.body) as Record<string, unknown> + let body: Record<string, unknown> + try { + const parsed = JSON.parse(flags.body) + if (parsed === null || Array.isArray(parsed) || typeof parsed !== 'object') { + throw new Error('Body must be a JSON object') + } + body = parsed as Record<string, unknown> + } catch (error) { + throw new Error(`Invalid --body JSON: ${(error as Error).message}`) + }🤖 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 `@src/commands/api/post.ts` around lines 20 - 21, The code parses flags.body directly which will throw on malformed JSON; wrap the JSON.parse(flags.body) call in a try/catch (same pattern used in watch.ts and rpc.ts) and when JSON.parse fails, throw a CLI-friendly error (e.g., new Error("Invalid JSON for --body: " + err.message)) so the user sees a clear message; ensure you still call client.post(args.path, { body }) and printData(await client.post(...), flags.json ? 'json' : 'human') after successful parsing, referencing flags.body, client.post, and printData.src/commands/messages/search.ts-20-23 (1)
20-23:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd explicit defaults to negatable boolean flags to match help text.
Lines 20 and 22 declare negatable flags without explicit defaults. The help text describes specific intended behaviors ("Exclude low-priority chats" and "Include muted chats"), but when these flags are omitted, they resolve to
undefinedbefore being passed to the API (lines 42–43). This creates a mismatch: the help text implies these flags have defaults, but the implementation passesundefinedinstead.Align the flag definitions with their documented behavior:
Proposed fix
- 'exclude-low-priority': Flags.boolean({ allowNo: true, description: 'Exclude low-priority chats. Use --no-exclude-low-priority to include all.' }), + 'exclude-low-priority': Flags.boolean({ default: true, allowNo: true, description: 'Exclude low-priority chats. Use --no-exclude-low-priority to include all.' }), @@ - 'include-muted': Flags.boolean({ allowNo: true, description: 'Include muted chats. Use --no-include-muted for a tighter search.' }), + 'include-muted': Flags.boolean({ default: true, allowNo: true, description: 'Include muted chats. Use --no-include-muted for a tighter search.' }),🤖 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 `@src/commands/messages/search.ts` around lines 20 - 23, The negatable flags 'exclude-low-priority' and 'include-muted' are missing explicit defaults so they can be undefined when passed to the API; update their Flags.boolean definitions (the declarations for 'exclude-low-priority' and 'include-muted') to include explicit default: true (to match the help text semantics: exclude low-priority by default and include muted by default) so the values read in run()/where you build the search params and pass them to the API are always booleans rather than undefined.src/lib/manifest.ts-66-66 (1)
66-66:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix
unarchivedescription to avoid archive/unarchive ambiguity.Line 66 currently reuses
apiCopy.chats.archive, sounarchiveis surfaced with archive wording in help/manifest output.Suggested patch
- { command: 'unarchive', description: apiCopy.chats.archive }, + { command: 'unarchive', description: 'Unarchive a chat' },🤖 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 `@src/lib/manifest.ts` at line 66, The manifest entry for the 'unarchive' command is using the wrong description (it references apiCopy.chats.archive), causing help text to show archive wording for unarchive; update the manifest object for the 'unarchive' command (the entry with command: 'unarchive') to use the correct description key (e.g., apiCopy.chats.unarchive) or add a new unarchive string to apiCopy.chats and reference that so the help/manifest displays the proper unarchive wording.
🧹 Nitpick comments (3)
src/commands/contacts/search.ts (1)
28-30: ⚡ Quick winSurface per-account lookup failures when debugging.
The catch block currently suppresses all errors silently, which can hide partial failures. Emit a warning in
--debugmode so skipped accounts are visible.Suggested fix
- } catch { + } catch (error) { // Some networks reject exact lookups for some identifiers; keep trying the rest. + if (flags.debug) { + this.warn( + `contacts.search failed for account ${accountID}: ${error instanceof Error ? error.message : String(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 `@src/commands/contacts/search.ts` around lines 28 - 30, The empty catch is hiding per-account lookup failures; change it to catch (err) and, when the CLI debug flag is enabled (e.g., options.debug / flags.debug / program.opts().debug or the module's debug variable used in this command), emit a warning including the account identifier being queried and the error (err.message or err.stack) so skipped accounts are visible during debugging; keep the existing behavior of continuing to try other accounts after logging. Reference the catch block in the contacts search flow (the per-account lookup inside the search command) and the surrounding variables for the account id and debug flag to implement this.src/lib/oauth.ts (1)
110-115: ⚖️ Poor tradeoffConsider using safer URL opening without shell interpretation.
While the URL is constructed via the URL API (which encodes special characters), spawning a shell command with the URL string could theoretically be vulnerable to command injection in edge cases, especially on Windows. Consider using a library like
open(npm package) that handles cross-platform URL opening more safely.🤖 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 `@src/lib/oauth.ts` around lines 110 - 115, The openExternal function currently builds a platform-specific shell command and calls spawn (spawn(...) in openExternal), which can allow shell interpretation risks; replace this implementation to use a cross-platform opener library (e.g., the `open` npm package) instead of manually spawning shell commands. Import and call the library (e.g., open(url)) from openExternal and remove the platform branching and spawn/unref logic so URLs are opened safely across macOS/Windows/Linux without constructing cmd/open/xdg-open arguments.src/commands/watch.ts (1)
24-24: ⚡ Quick winType assertion bypasses WebSocket API type safety.
The cast
as unknown as string[]at line 24 forces an options object with headers into a parameter position that doesn't support it. TypeScript's default DOM WebSocket type (no explicit lib configuration in tsconfig) expects the second parameter to bestring | string[]for subprotocols, not an options object. While this works at runtime with Node.js's built-in WebSocket (v21+), the type assertion hides a real type mismatch.Either configure
tsconfig.jsonto include the correct library types for your Node.js environment, or avoid the type cast by using proper WebSocket initialization patterns.🤖 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 `@src/commands/watch.ts` at line 24, The code is bypassing TypeScript types by casting the options object to string[] for the WebSocket constructor; remove the unsafe cast and use the Node.js/ws types so headers are correctly typed: import the WebSocket implementation/type from 'ws' (or adjust tsconfig to provide Node WebSocket libs), then replace the instantiation with new WebSocket(url, { headers: { Authorization: `Bearer ${token}` } }) using the imported WebSocket type (referencing WebSocket, url, token) so the headers option is accepted by the constructor without type assertions.
🤖 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 `@scripts/check-api-copy.mjs`:
- Line 5: The code uses new URL('..', import.meta.url).pathname to build root
which yields incorrect Windows paths and doesn't decode percent-encodings;
replace the .pathname usage with Node's fileURLToPath to convert the
import.meta.url-derived URL to a proper filesystem path (import fileURLToPath
from 'node:url'), then call fileURLToPath(new URL('..', import.meta.url)) and
pass that into resolve to set root (refer to the root variable, resolve call,
and new URL(import.meta.url) in the file).
In `@src/commands/auth/login.ts`:
- Around line 38-48: The OAuth branch returns after loginWithPKCE without
persisting the token; update the code in the oauth branch (the block using
loginWithPKCE and the local token variable) to call the same persistence routine
used for other flows—e.g., invoke finishLogin(token) or call
updateConfig/saveAuth with the token fields (clientID, access token, expires_in,
refresh token, scope, etc.) before logging and returning—so the OAuth
credentials are written to the CLI config/state for subsequent commands.
In `@src/commands/auth/logout.ts`:
- Around line 10-17: The logout flow awaits the remote revoke call (fetch to
URL('/oauth/revoke', config.baseURL)) which can hang and prevent clearing local
auth; change the logic in logout.ts so the revoke request is fire-and-forget or
bounded by a timeout and does not block updateConfig. Specifically, call the
revoke endpoint without awaiting it (or wrap the fetch in a Promise.race with a
short timeout) and ensure updateConfig(current => ({ ...current, auth: undefined
})) always runs (use try/finally or simply trigger revoke asynchronously),
referencing the existing token variable, the fetch(...) call, and updateConfig
to locate and modify the code.
In `@src/commands/chats/search.ts`:
- Line 18: The 'include-muted' flag is inverted because allowNo: true without an
explicit default yields false; update the Flags.boolean call for 'include-muted'
to include default: true (e.g., Flags.boolean({ allowNo: true, default: true,
description: 'Include muted chats. Use --no-include-muted for a tighter search.'
})) so muted chats are included by default while still supporting the
--no-include-muted override; ensure the change is applied to the 'include-muted'
flag definition.
In `@src/commands/config/get.ts`:
- Around line 16-18: The current code prints raw auth data which can leak
credentials; before calling printData, redact sensitive fields in the
config.auth object (e.g., accessToken, refreshToken, secret) by replacing them
with a placeholder like "[REDACTED]" when printing the whole config or the auth
key by default. Locate the usage of readConfig, args.key and printData in this
file and, if args.key is undefined or args.key === 'auth', create a shallow copy
of config (or config.auth) and replace known secret properties with "[REDACTED]"
then pass that sanitized value to printData (leave JSON output behavior intact
or respect flags.json if you want to allow machine-readable full dumps).
In `@src/commands/contacts/list.ts`:
- Around line 32-34: The --ids branch calls printIDs(items) but printIDs expects
id/chatID/messageID while contact rows usually carry userID; update the --ids
handling in the list command to pass the correct identifier by mapping items to
userID when present (e.g. items.map(i => i.userID ?? i.id)) before calling
printIDs, or modify printIDs to prefer userID for contact objects; ensure you
reference the flags.ids check and the printIDs call so returned IDs are contact
userIDs, falling back to existing id/chatID/messageID values if userID is
missing.
- Around line 27-31: The loop currently requests up to flags.limit per account,
allowing the final items array to exceed the global limit; fix by computing
remaining = flags.limit - items.length before each fetch, break early if
remaining <= 0, and pass remaining into collectPage instead of flags.limit when
calling client.accounts.contacts.list(accountID, { query: flags.query }); keep
pushing accountID into each item and break the outer loop once items.length >=
flags.limit.
In `@src/commands/description.ts`:
- Around line 24-27: The command handler currently accepts both args.description
and the --clear flag and silently treats description as cleared; change the
validation to reject this conflicting input by checking if flags.clear and
args.description are both truthy and throwing a clear error (e.g., throw new
Error('Provide either DESCRIPTION or --clear, not both')) before calling
createClient/resolveChatID/client.chats.update/printData so the conflict is
detected early and accidental data loss is prevented.
In `@src/commands/doctor.ts`:
- Line 21: The fetch call that probes the server (the request using new
URL('/v1/info', baseURL)) lacks a timeout and can hang; wrap it with an
AbortController, pass controller.signal into fetch, start a timer (e.g., const
timeoutId = setTimeout(() => controller.abort(), <timeoutMs>)) before calling
fetch, and clear the timer (clearTimeout(timeoutId)) after the response or on
error so the request is aborted on timeout and resources are cleaned up;
reference the fetch call, baseURL, and the AbortController usage when making the
change.
In `@src/commands/messages/index.ts`:
- Around line 27-28: The code computes cursor and direction with inconsistent
precedence (cursor = flags.before ?? flags.after but direction = flags.after ?
'after' : flags.before ? 'before' : undefined), which can pair a before cursor
with an after direction; update the pagination handling in the command (use the
flags object) to enforce mutual exclusion or a unified precedence: either (A)
validate and throw/print an error if both flags.before and flags.after are
provided, or (B) pick a single precedence rule and apply it consistently to both
cursor and direction (e.g., if flags.after takes precedence, set cursor =
flags.after and direction = 'after' when both present). Ensure changes touch the
variables cursor and direction so the built pagination params are always
consistent.
In `@src/commands/rpc.ts`:
- Around line 38-42: The error response currently hardcodes id: null in the
process.stdout.write JSON, breaking request correlation; update the error path
that writes the response (the process.stdout.write({...}) block) to preserve the
incoming request id instead of null—e.g. use the existing request id variable in
scope (such as id or request.id, or req.id) with a fallback to null (id: id ??
(request?.id ?? null)) so error responses carry the original request identifier.
In `@src/commands/start-chat.ts`:
- Around line 30-40: The command currently constructs an empty user object and
calls tryStartChat even when no recipient selector is provided; add a pre-check
before calling tryStartChat (around the construction of user) to validate that
either args.query is set or at least one identity flag (flags.email, flags.name,
flags.id, flags.phone, flags.username) is present, and if none are provided fail
fast by returning an error / printing a clear message and exiting (or throwing)
instead of calling tryStartChat; update the code paths around
userQueryFromInput, the local variable user, and the tryStartChat invocation to
enforce this validation.
In `@src/commands/status.ts`:
- Around line 17-19: The fetch in the status probe (the call creating response
with fetch(new URL('/v1/info', baseURL))) has no timeout and can hang; update it
to use AbortSignal.timeout(timeoutMs) (Node v16.14+), pass the resulting signal
to fetch (e.g., fetch(url, { signal })), and handle the abort by catching the
thrown DOMException/Error and converting it into a clear timeout error before
proceeding with the existing response.ok check; keep references to the same URL
'/v1/info' and baseURL and choose a sensible timeout value (e.g., 5s) as
timeoutMs.
In `@src/lib/argv.ts`:
- Around line 24-27: The tokenizer drops empty quoted arguments because token
emission is gated by truthiness (if (current)) — change emission logic in the
loop that handles whitespace and quote boundaries so that you push current even
when it's an empty string if that empty string was produced by closing a quote.
Concretely: introduce a small flag (e.g., closedQuote) that you set when you
close a quote (when quote is toggled off), and then in both emission sites (the
whitespace handler and the end-of-input flush) replace the truthy check with a
condition that pushes current when current !== '' OR closedQuote is true; reset
closedQuote after pushing. This preserves ""/'' as empty tokens while still
preventing spurious empty tokens from normal consecutive spaces.
In `@src/lib/config.ts`:
- Around line 24-33: The readConfig function currently swallows all errors in
its try/catch and silently falls back to default baseURL which hides malformed
JSON or permission errors; update readConfig to distinguish errors: attempt
readFile/configPath and JSON.parse as before, but if readFile fails with a "file
not found" (ENOENT) return the safe fallback, whereas if JSON.parse throws or
any other I/O error occurs, rethrow or surface the error (or log it and then
throw) so callers can detect malformed config; keep the same returned shape
(baseURL and auth when parse succeeds) and reference the existing symbols
readConfig, configPath(), defaultBaseURL, parsed.auth when making the changes.
In `@src/lib/oauth.ts`:
- Around line 94-108: The token exchange omits redirect_uri and the
exchangeToken signature lacks a redirectURI parameter, which violates RFC 6749;
update the exchangeToken function signature (exchangeToken) to accept a
redirectURI argument and include redirect_uri: redirectURI in the
URLSearchParams body used for the POST so the token request matches the original
authorization request.
In `@src/lib/output.ts`:
- Around line 241-246: In collectPage, the current logic treats limit=0 as "no
limit" and pushes one item before breaking; change the loop to check the
explicit presence of a limit before pushing by using if (limit !== undefined &&
items.length >= limit) break at the top of the for-await loop (use strict !==
undefined rather than truthiness) so collectPage(..., 0) returns an empty array
and any explicit numeric limit is honored; update the function collectPage and
its local items/limit handling accordingly.
In `@src/lib/resolve.ts`:
- Around line 98-103: The retrieveChat function currently swallows all errors
from client.chats.retrieve which hides auth/network/server issues; change its
error handling to only return undefined for a genuine “not found” case and
rethrow (or log and rethrow) other errors: inspect the caught error inside
retrieveChat (from client.chats.retrieve) for a not-found indicator (e.g.,
status/code/message) and return undefined only then, otherwise throw the error
(or call processLogger.error before throwing) so upstream can handle
auth/network/server failures appropriately.
---
Minor comments:
In @.agents/skills/opentui/docs/components/ascii-font.mdx:
- Around line 81-84: The font style name is misspelled as "pallet" in the font
config object; update the value for the font key (the object containing font:
"pallet") to "palette" so the style name reads font: "palette" (ensure you
update any identical occurrences of "pallet" in the same component to maintain
consistency).
In @.agents/skills/opentui/docs/components/select.mdx:
- Around line 79-82: The examples register a handler using
menu.on(SelectRenderableEvents.ITEM_SELECTED, ...) and annotate the callback
parameter with SelectOption but never import that type, causing TypeScript
compile errors; fix by adding an import for SelectOption from "@opentui/core"
alongside the existing SelectRenderableEvents import so the two snippets (the
handler at the menu.on call shown and the similar one on line 91) reference a
defined type and compile correctly.
In @.agents/skills/opentui/docs/components/tab-select.mdx:
- Line 158: Replace the hard-coded event string in the event listener with the
exported enum/constant to keep docs consistent: change the
tabs.on("itemSelected", (index, option) => { ...) usage to use
TabSelectRenderableEvents.ITEM_SELECTED so the listener references the exported
event constant (TabSelectRenderableEvents.ITEM_SELECTED) instead of the string
literal.
In @.agents/skills/opentui/docs/keymap/hosts.mdx:
- Line 55: Update the platform table entry so Apple’s OS uses the correct
branding: replace the literal `macos` string with `macOS` in the table row that
documents the `platform` field (the cell currently showing `macos`, `windows`,
`linux` or `unknown`), preserving the rest of the format and punctuation.
In @.agents/skills/opentui/docs/keymap/react.mdx:
- Around line 115-117: The example is missing imports for React hooks and
OpenTUI helpers; add imports for useMemo (from 'react' or 'preact' depending on
the project), and import reactiveMatcherFromStore and useBindings from the
OpenTUI package so the snippet using useMemo, reactiveMatcherFromStore, and
useBindings is complete and copy-pasteable; update the top of the example to
include these imports and ensure the named symbols (useMemo,
reactiveMatcherFromStore, useBindings) match the existing exported names used
elsewhere in the docs.
In `@scripts/check-api-copy.mjs`:
- Around line 21-22: The checks array in scripts/check-api-copy.mjs is missing
coverage for apiCopy.contacts.list, so add an entry for it (e.g.,
['contacts.list', 'resources/contacts.d.ts', 'list']) to the list of checks so
the script will detect drift for contacts.list; update the checks array near the
existing ['contacts.search', ...] and ['messages.delete', ...] entries to
include that tuple referencing the apiCopy key (contacts.list), the d.ts
resource (resources/contacts.d.ts), and the method name ('list').
In `@src/commands/api/post.ts`:
- Around line 20-21: The code parses flags.body directly which will throw on
malformed JSON; wrap the JSON.parse(flags.body) call in a try/catch (same
pattern used in watch.ts and rpc.ts) and when JSON.parse fails, throw a
CLI-friendly error (e.g., new Error("Invalid JSON for --body: " + err.message))
so the user sees a clear message; ensure you still call client.post(args.path, {
body }) and printData(await client.post(...), flags.json ? 'json' : 'human')
after successful parsing, referencing flags.body, client.post, and printData.
In `@src/commands/config/set.ts`:
- Around line 8-14: When handling the ConfigSet.run() flow, validate args.value
when args.key === 'baseURL' before calling updateConfig to avoid persisting
malformed URLs; in run(), check if args.key === 'baseURL' and attempt to parse
args.value with the URL constructor (or otherwise validate scheme/host), and if
parsing fails call this.error(...) or throw a descriptive error instead of
writing to updateConfig, otherwise proceed with updateConfig(config => ({
...config, [args.key]: args.value })) and this.log.
In `@src/commands/create-chat.ts`:
- Around line 24-30: Before calling client.chats.create, validate the
participant cardinality when flags.type === 'single': check flags.participant
(or the parsed participant array) and if type is 'single' ensure exactly one
participant is provided; if not, throw or print a clear error and exit (or
return) to avoid sending an invalid payload to client.chats.create. Locate the
create call (client.chats.create) and the CLI flags (flags.participant,
flags.type, flags.message, etc.) and add this guard so only a single participant
is allowed for 'single' chats.
In `@src/commands/message-expiry.ts`:
- Line 24: The validation message says "positive integer" but the code permits
0; update the check and message to be consistent: either change the condition in
the message/throw in the message to say "non-negative integer" (so keep the
current condition using expiry !== null && (!Number.isInteger(expiry) || expiry
< 0)) or tighten the validation to forbid 0 by using expiry <= 0 and keep
"positive integer" in the Error; locate the validation around the expiry
variable in the message-expiry command (the if that throws Error('SECONDS must
be a positive integer or "off"')) and make the corresponding change so the
thrown error text matches the numeric check.
In `@src/commands/messages/search.ts`:
- Around line 20-23: The negatable flags 'exclude-low-priority' and
'include-muted' are missing explicit defaults so they can be undefined when
passed to the API; update their Flags.boolean definitions (the declarations for
'exclude-low-priority' and 'include-muted') to include explicit default: true
(to match the help text semantics: exclude low-priority by default and include
muted by default) so the values read in run()/where you build the search params
and pass them to the API are always booleans rather than undefined.
In `@src/commands/send-file.ts`:
- Around line 51-55: When flags.wait is true, the code calls
waitForMessage(client, chatID, result.pendingMessageID, ...) without verifying
result.pendingMessageID; add an explicit guard that checks if
result.pendingMessageID is present before calling waitForMessage and if missing
throw or return a clear error (e.g., log and exit or reject) indicating that
pendingMessageID is required for the --wait path. Update the block around
flags.wait to validate result.pendingMessageID and only call waitForMessage when
it exists, otherwise provide a descriptive error mentioning pendingMessageID and
the --wait flag.
In `@src/commands/unarchive.ts`:
- Line 7: The command's help summary wrongly reuses apiCopy.chats.archive;
update the static override summary in the unarchive command (the static override
summary field) to reference the unarchive-specific copy key/string (e.g.,
apiCopy.chats.unarchive or the appropriate unarchive summary key) so the CLI
help shows an accurate description for the unarchive command.
In `@src/lib/manifest.ts`:
- Line 66: The manifest entry for the 'unarchive' command is using the wrong
description (it references apiCopy.chats.archive), causing help text to show
archive wording for unarchive; update the manifest object for the 'unarchive'
command (the entry with command: 'unarchive') to use the correct description key
(e.g., apiCopy.chats.unarchive) or add a new unarchive string to apiCopy.chats
and reference that so the help/manifest displays the proper unarchive wording.
---
Nitpick comments:
In `@src/commands/contacts/search.ts`:
- Around line 28-30: The empty catch is hiding per-account lookup failures;
change it to catch (err) and, when the CLI debug flag is enabled (e.g.,
options.debug / flags.debug / program.opts().debug or the module's debug
variable used in this command), emit a warning including the account identifier
being queried and the error (err.message or err.stack) so skipped accounts are
visible during debugging; keep the existing behavior of continuing to try other
accounts after logging. Reference the catch block in the contacts search flow
(the per-account lookup inside the search command) and the surrounding variables
for the account id and debug flag to implement this.
In `@src/commands/watch.ts`:
- Line 24: The code is bypassing TypeScript types by casting the options object
to string[] for the WebSocket constructor; remove the unsafe cast and use the
Node.js/ws types so headers are correctly typed: import the WebSocket
implementation/type from 'ws' (or adjust tsconfig to provide Node WebSocket
libs), then replace the instantiation with new WebSocket(url, { headers: {
Authorization: `Bearer ${token}` } }) using the imported WebSocket type
(referencing WebSocket, url, token) so the headers option is accepted by the
constructor without type assertions.
In `@src/lib/oauth.ts`:
- Around line 110-115: The openExternal function currently builds a
platform-specific shell command and calls spawn (spawn(...) in openExternal),
which can allow shell interpretation risks; replace this implementation to use a
cross-platform opener library (e.g., the `open` npm package) instead of manually
spawning shell commands. Import and call the library (e.g., open(url)) from
openExternal and remove the platform branching and spawn/unref logic so URLs are
opened safely across macOS/Windows/Linux without constructing cmd/open/xdg-open
arguments.
🪄 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: 7cda4489-7b12-4994-9f64-9eecfa712ee6
⛔ Files ignored due to path filters (2)
bun.lockis excluded by!**/*.lockpackage-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (151)
.agents/skills/opentui/SKILL.md.agents/skills/opentui/config.ts.agents/skills/opentui/docs/bindings/react.mdx.agents/skills/opentui/docs/bindings/solid.mdx.agents/skills/opentui/docs/components/ascii-font.mdx.agents/skills/opentui/docs/components/box.mdx.agents/skills/opentui/docs/components/code.mdx.agents/skills/opentui/docs/components/diff.mdx.agents/skills/opentui/docs/components/frame-buffer.mdx.agents/skills/opentui/docs/components/input.mdx.agents/skills/opentui/docs/components/line-number.mdx.agents/skills/opentui/docs/components/markdown.mdx.agents/skills/opentui/docs/components/scrollbar.mdx.agents/skills/opentui/docs/components/scrollbox.mdx.agents/skills/opentui/docs/components/select.mdx.agents/skills/opentui/docs/components/slider.mdx.agents/skills/opentui/docs/components/tab-select.mdx.agents/skills/opentui/docs/components/text.mdx.agents/skills/opentui/docs/components/textarea.mdx.agents/skills/opentui/docs/core-concepts/audio.mdx.agents/skills/opentui/docs/core-concepts/colors.mdx.agents/skills/opentui/docs/core-concepts/console.mdx.agents/skills/opentui/docs/core-concepts/constructs.mdx.agents/skills/opentui/docs/core-concepts/keyboard.mdx.agents/skills/opentui/docs/core-concepts/layout.mdx.agents/skills/opentui/docs/core-concepts/lifecycle.mdx.agents/skills/opentui/docs/core-concepts/notifications.mdx.agents/skills/opentui/docs/core-concepts/renderables-vs-constructs.mdx.agents/skills/opentui/docs/core-concepts/renderables.mdx.agents/skills/opentui/docs/core-concepts/renderer.mdx.agents/skills/opentui/docs/getting-started.mdx.agents/skills/opentui/docs/keymap/addons.mdx.agents/skills/opentui/docs/keymap/core.mdx.agents/skills/opentui/docs/keymap/custom-addons.mdx.agents/skills/opentui/docs/keymap/hosts.mdx.agents/skills/opentui/docs/keymap/overview.mdx.agents/skills/opentui/docs/keymap/react.mdx.agents/skills/opentui/docs/keymap/solid.mdx.agents/skills/opentui/docs/plugins/core.mdx.agents/skills/opentui/docs/plugins/react.mdx.agents/skills/opentui/docs/plugins/slots.mdx.agents/skills/opentui/docs/plugins/solid.mdx.agents/skills/opentui/docs/reference/color-matrix.mdx.agents/skills/opentui/docs/reference/env-vars.mdx.agents/skills/opentui/docs/reference/tree-sitter.mdx.github/workflows/ci.yml.github/workflows/publish-release.yml.gitignore.goreleaser.ymlREADME.mdTYPESCRIPT_CLI.mdbin/check-release-environmentbin/dev.jsbin/run.jspackage.jsonrelease-please-config.jsonscripts/build-homebrew-archive.mjsscripts/check-api-copy.mjsscripts/publish-homebrew-formula.mjsskills-lock.jsonsrc/commands/accounts.tssrc/commands/api/get.tssrc/commands/api/post.tssrc/commands/app/e2ee/recovery-code/mark-backed-up.tssrc/commands/app/e2ee/recovery-code/reset/begin.tssrc/commands/app/e2ee/recovery-code/reset/confirm.tssrc/commands/app/e2ee/recovery-code/verify.tssrc/commands/app/status.tssrc/commands/archive.tssrc/commands/assets/download.tssrc/commands/assets/upload.tssrc/commands/auth/login.tssrc/commands/auth/logout.tssrc/commands/auth/status.tssrc/commands/avatar.tssrc/commands/chat.tssrc/commands/chat/open.tssrc/commands/chats/index.tssrc/commands/chats/search.tssrc/commands/clear-draft.tssrc/commands/commands.tssrc/commands/config/get.tssrc/commands/config/path.tssrc/commands/config/reset.tssrc/commands/config/set.tssrc/commands/contacts/list.tssrc/commands/contacts/search.tssrc/commands/create-chat.tssrc/commands/current-user.tssrc/commands/delete-message.tssrc/commands/description.tssrc/commands/doctor.tssrc/commands/draft.tssrc/commands/edit.tssrc/commands/export.tssrc/commands/focus.tssrc/commands/inbox.tssrc/commands/interactive.tssrc/commands/llm.tssrc/commands/login.tssrc/commands/logout.tssrc/commands/low-priority.tssrc/commands/mark-read.tssrc/commands/mark-unread.tssrc/commands/message-expiry.tssrc/commands/message.tssrc/commands/messages/index.tssrc/commands/messages/search.tssrc/commands/mute.tssrc/commands/notify-anyway.tssrc/commands/pin.tssrc/commands/react.tssrc/commands/read.tssrc/commands/remind.tssrc/commands/reply-file.tssrc/commands/reply.tssrc/commands/rpc.tssrc/commands/search.tssrc/commands/send-file.tssrc/commands/send.tssrc/commands/shell.tssrc/commands/start-chat.tssrc/commands/status.tssrc/commands/tail.tssrc/commands/thread.tssrc/commands/threads.tssrc/commands/title.tssrc/commands/unarchive.tssrc/commands/unmute.tssrc/commands/unpin.tssrc/commands/unreact.tssrc/commands/unread.tssrc/commands/unremind.tssrc/commands/watch.tssrc/commands/whoami.tssrc/lib/app-api.tssrc/lib/argv.tssrc/lib/client.tssrc/lib/config.tssrc/lib/copy.tssrc/lib/export/index.tssrc/lib/interactive.tssrc/lib/manifest.tssrc/lib/oauth.tssrc/lib/output.tssrc/lib/pkce.tssrc/lib/resolve.tssrc/lib/runner.tssrc/lib/wait.tstest/cli-smoke.mjstsconfig.json
💤 Files with no reviewable changes (1)
- .goreleaser.yml
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/commands/bridge/login.ts (1)
36-37: ⚡ Quick winAlways emit final human-readable output in guided mode.
In non-JSON mode, guided execution currently skips
printBridgeLoginStep(...)entirely in this command. Consider printing the finalresultfor consistent CLI output across modes.Proposed change
- if (flags.json) printData(result, 'json') - else if (!flags.guided) printBridgeLoginStep(result) + if (flags.json) printData(result, 'json') + else printBridgeLoginStep(result)🤖 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 `@src/commands/bridge/login.ts` around lines 36 - 37, The CLI currently skips emitting the final human-readable result when running in guided mode because the conditional only calls printBridgeLoginStep(result) when !flags.guided; change the branching so that if flags.json you call printData(result, 'json'), otherwise always call printBridgeLoginStep(result) (i.e., remove the !flags.guided guard), ensuring printBridgeLoginStep(result) runs for both guided and non-guided non-JSON flows.
🤖 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 `@src/commands/bridge/login/submit-cookies.ts`:
- Line 23: Wrap the JSON.parse(flags.body) call in a try/catch and validate the
resulting value before casting: catch JSON parse errors and emit a clear CLI
error referencing the --body flag (rather than letting it throw), then ensure
the parsed value is an object and every property value is a string (i.e. the
intended Record<string,string> shape); if validation fails emit a descriptive
CLI error and exit/return. Use the existing flags.body variable and the const
body assignment location in submit-cookies to implement this guard and
validation.
In `@src/commands/bridge/login/submit-user-input.ts`:
- Line 23: Wrap the JSON.parse of flags.body in a guarded parse (try/catch) and
return a clear CLI error on parse failure; after parsing, validate that the
result is an object (not null/array) and that each property value is a string
(or coerce/validate accordingly) before proceeding to API submission — update
the parsing around const body = JSON.parse(flags.body) so malformed JSON or
invalid shapes produce actionable error messages (e.g., via the same CLI error
path) instead of throwing.
---
Nitpick comments:
In `@src/commands/bridge/login.ts`:
- Around line 36-37: The CLI currently skips emitting the final human-readable
result when running in guided mode because the conditional only calls
printBridgeLoginStep(result) when !flags.guided; change the branching so that if
flags.json you call printData(result, 'json'), otherwise always call
printBridgeLoginStep(result) (i.e., remove the !flags.guided guard), ensuring
printBridgeLoginStep(result) runs for both guided and non-guided non-JSON flows.
🪄 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: 0b7ad9c4-fe51-481c-8f53-5cab32253393
📒 Files selected for processing (19)
package.jsonsrc/commands/app/e2ee/verification/accept.tssrc/commands/app/e2ee/verification/cancel.tssrc/commands/app/e2ee/verification/qr/confirm-scanned.tssrc/commands/app/e2ee/verification/qr/scan.tssrc/commands/app/e2ee/verification/sas/confirm.tssrc/commands/app/e2ee/verification/sas/start.tssrc/commands/app/e2ee/verification/start.tssrc/commands/bridge/flows.tssrc/commands/bridge/login.tssrc/commands/bridge/login/start.tssrc/commands/bridge/login/submit-cookies.tssrc/commands/bridge/login/submit-user-input.tssrc/commands/bridge/login/wait.tssrc/commands/bridge/logins.tssrc/commands/bridge/logout.tssrc/commands/bridge/whoami.tssrc/lib/bridge-login.tssrc/lib/manifest.ts
✅ Files skipped from review due to trivial changes (3)
- src/commands/app/e2ee/verification/cancel.ts
- src/lib/bridge-login.ts
- src/commands/app/e2ee/verification/sas/confirm.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/lib/manifest.ts
- package.json
📜 Review details
🔇 Additional comments (12)
src/commands/app/e2ee/verification/start.ts (1)
1-23: LGTM!src/commands/app/e2ee/verification/qr/confirm-scanned.ts (1)
1-24: LGTM!src/commands/app/e2ee/verification/sas/start.ts (1)
1-24: LGTM!src/commands/app/e2ee/verification/accept.ts (1)
1-24: LGTM!src/commands/app/e2ee/verification/qr/scan.ts (1)
1-24: LGTM!src/commands/bridge/logins.ts (1)
1-21: LGTM!src/commands/bridge/logout.ts (1)
1-22: LGTM!src/commands/bridge/whoami.ts (1)
1-21: LGTM!src/commands/bridge/flows.ts (1)
1-21: LGTM!src/commands/bridge/login/start.ts (1)
1-29: LGTM!src/commands/bridge/login/wait.ts (1)
1-29: LGTM!src/commands/bridge/login.ts (1)
1-35: LGTM!Also applies to: 38-39
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@src/commands/accounts/add.ts`:
- Line 22: The code currently falls back to loginFlows[0] when the --flow flag
is omitted and the chooser is skipped (e.g., flags.json or flags.guided ===
false); change this so we do not silently pick the first backend flow: when
flags.flow is undefined and the chooser is skipped (check the same
chooserSkipped/flags.json || flags.guided === false condition used near the flow
selection), throw a user-facing error asking the caller to provide --flow rather
than assigning loginFlows[0]. Update the selection logic that references
loginFlows and the flow flag (the flow: Flags.string flag parsing and the code
that sets selectedFlow = flags.flow ?? loginFlows[0]) to validate and require
flags.flow in chooser-skipped paths and only use a default when the interactive
chooser runs.
In `@src/lib/account-login.ts`:
- Around line 101-110: The cookie prompt currently uses promptText and therefore
echoes sensitive cookie values to the terminal; update the 'cookies' branch in
the handler (the block gated by if ('cookies' in step)) to use the
masked/no-echo prompt function instead of promptText (e.g., promptSecret /
promptPassword / promptNoEcho) when collecting body[field.name], preserving the
existing checks that use options.cookies and options.nonInteractive and the same
error message on non-interactive; apply the same replacement for the second
cookie collection occurrence referenced around lines 120-123 so all cookie
inputs are captured without terminal echo.
- Around line 30-58: printAccountLoginStep currently writes human-readable
progress to process.stdout which corrupts machine-readable output; change it to
write to process.stderr (or accept an optional writable stream parameter
defaulting to process.stderr) and use that stream for all writes in
printAccountLoginStep; apply the same change to the other helper functions in
this file that emit interactive output so callers wanting JSON can still use
stdout unmodified.
🪄 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: 78f2f52a-be95-4973-8447-befb9ad53f55
📒 Files selected for processing (4)
package.jsonsrc/commands/accounts/add.tssrc/lib/account-login.tssrc/lib/manifest.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- package.json
- src/lib/manifest.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
test/e2e-staging.mjs (2)
13-14: ⚡ Quick winHardcoded developer-specific path reduces test portability.
The default path
/Users/batuhan/.codex/worktrees/43bc/beeper/desktopis specific to a particular developer's machine and won't work for other developers or CI environments.Consider either requiring
BEEPER_DESKTOP_ROOTto be set (throwing an error if missing) or using a more generic default like a relative path from the repo root. This makes the test more portable and prevents confusing failures when other developers run it.🤖 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 `@test/e2e-staging.mjs` around lines 13 - 14, defaultDesktopRoot is set to a developer-specific absolute path and makes desktopRoot unreliable for other environments; change the logic around defaultDesktopRoot/desktopRoot so that either (a) if process.env.BEEPER_DESKTOP_ROOT is not set the script throws a clear error asking the caller to set BEEPER_DESKTOP_ROOT, or (b) compute a portable default (e.g., a repo-relative path using __dirname or process.cwd()) and fall back to that; update the code that reads defaultDesktopRoot/desktopRoot to implement one of these approaches and ensure error messaging or the relative-path computation mentions BEEPER_DESKTOP_ROOT, defaultDesktopRoot, and desktopRoot for clarity.
982-982: 💤 Low valueInconsistent SIGKILL timeout.
Line 982 waits 1000ms between SIGTERM and SIGKILL, while line 858 in
runProcesswaits 2000ms for the same pattern. Consider using a consistent timeout duration for graceful shutdown attempts.🤖 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 `@test/e2e-staging.mjs` at line 982, The sleep call waiting 1000ms between SIGTERM and SIGKILL is inconsistent with the 2000ms used in runProcess; replace the magic number with a shared constant (e.g., GRACEFUL_SHUTDOWN_TIMEOUT) or update the sleep(1000) to sleep(2000) so both the standalone wait and the wait inside runProcess use the same duration; locate the sleep call and the runProcess implementation and use the same identifier (GRACEFUL_SHUTDOWN_TIMEOUT) or matching literal to ensure consistent graceful shutdown timing.
🤖 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 `@src/commands/app/e2ee/recovery-code/reset/begin.ts`:
- Line 10: The current CLI defines a sensitive 'recovery-code' flag using
Flags.string which can expose E2EE recovery codes in shell history and process
listings; change the implementation in begin.ts so that if the 'recovery-code'
flag is omitted the command prompts interactively with hidden input (or reads
from stdin/file) instead of relying on a visible flag, add a note in the command
help/documentation recommending environment variables or secure input methods
for automation, and if you must keep the flag for automation, clearly document
the security risks and consider marking the flag as deprecated or providing an
alternative secure input flow.
---
Nitpick comments:
In `@test/e2e-staging.mjs`:
- Around line 13-14: defaultDesktopRoot is set to a developer-specific absolute
path and makes desktopRoot unreliable for other environments; change the logic
around defaultDesktopRoot/desktopRoot so that either (a) if
process.env.BEEPER_DESKTOP_ROOT is not set the script throws a clear error
asking the caller to set BEEPER_DESKTOP_ROOT, or (b) compute a portable default
(e.g., a repo-relative path using __dirname or process.cwd()) and fall back to
that; update the code that reads defaultDesktopRoot/desktopRoot to implement one
of these approaches and ensure error messaging or the relative-path computation
mentions BEEPER_DESKTOP_ROOT, defaultDesktopRoot, and desktopRoot for clarity.
- Line 982: The sleep call waiting 1000ms between SIGTERM and SIGKILL is
inconsistent with the 2000ms used in runProcess; replace the magic number with a
shared constant (e.g., GRACEFUL_SHUTDOWN_TIMEOUT) or update the sleep(1000) to
sleep(2000) so both the standalone wait and the wait inside runProcess use the
same duration; locate the sleep call and the runProcess implementation and use
the same identifier (GRACEFUL_SHUTDOWN_TIMEOUT) or matching literal to ensure
consistent graceful shutdown timing.
🪄 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: b91035be-31dd-4c91-897b-d7d9626707a2
📒 Files selected for processing (8)
README.mdTYPESCRIPT_CLI.mdpackage.jsonsrc/commands/app/e2ee/recovery-code/reset/begin.tssrc/commands/app/e2ee/verification/start.tssrc/commands/auth/login.tssrc/lib/oauth.tstest/e2e-staging.mjs
✅ Files skipped from review due to trivial changes (2)
- package.json
- TYPESCRIPT_CLI.md
🚧 Files skipped from review as they are similar to previous changes (3)
- src/commands/app/e2ee/verification/start.ts
- src/lib/oauth.ts
- src/commands/auth/login.ts
📜 Review details
🧰 Additional context used
🪛 LanguageTool
README.md
[uncategorized] ~251-~251: The official name of this software platform is spelled with a capital “H”.
Context: ...pm and Homebrew. Push a v* tag to run .github/workflows/publish-release.yml. The re...
(GITHUB)
🔇 Additional comments (2)
src/commands/app/e2ee/recovery-code/reset/begin.ts (1)
14-21: LGTM!test/e2e-staging.mjs (1)
753-785: LGTM!
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 @.github/workflows/publish-release.yml:
- Around line 3-5: Remove the unused OIDC permission by deleting the id-token:
write entry from the workflow permissions block so only contents: write remains;
update the permissions mapping (the permissions key and its children) to no
longer include the id-token field, leaving the workflow with just contents:
write for GitHub release operations.
In `@README.md`:
- Line 129: Update the example command line that currently reads beeper remind
"Family" 2026-05-13T12:00:00Z to use a future or placeholder ISO-8601 timestamp
(e.g., 2026-12-01T12:00:00Z or <FUTURE_TIMESTAMP>) so copy/paste users won't hit
an already-passed time; change the literal in the README example accordingly
wherever the remind example appears.
- Around line 210-211: The README's examples show `commands --json` and `llm` as
standalone binaries; update the documentation to show them as subcommands by
prefixing each with the CLI root `beeper` (e.g., `beeper commands --json` and
`beeper llm`) so copy/paste yields correct invocation; locate the lines
referencing `commands --json` and `llm` and replace them with the prefixed forms
and update any surrounding explanatory text to reflect they are subcommands of
`beeper`.
🪄 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: 170a2bb2-2c9d-4c5c-a5fb-8d8753b5e903
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (68)
.github/actions/setup-go/action.yml.github/workflows/publish-release.ymlREADME.mdcmd/beeper-desktop-cli/main.gogo.modinternal/apiform/encoder.gointernal/apiform/form.gointernal/apiform/form_test.gointernal/apiquery/encoder.gointernal/apiquery/query.gointernal/apiquery/query_test.gointernal/autocomplete/autocomplete.gointernal/autocomplete/autocomplete_test.gointernal/autocomplete/shellscripts/bash_autocomplete.bashinternal/autocomplete/shellscripts/fish_autocomplete.fishinternal/autocomplete/shellscripts/pwsh_autocomplete.ps1internal/autocomplete/shellscripts/zsh_autocomplete.zshinternal/binaryparam/binary_param.gointernal/binaryparam/binary_param_test.gointernal/debugmiddleware/debug_middleware.gointernal/debugmiddleware/debug_middleware_test.gointernal/jsonview/explorer.gointernal/jsonview/staticdisplay.gointernal/mocktest/mocktest.gointernal/requestflag/innerflag.gointernal/requestflag/innerflag_test.gointernal/requestflag/requestflag.gointernal/requestflag/requestflag_test.gopackage.jsonpkg/cmd/account.gopkg/cmd/account_test.gopkg/cmd/accountcontact.gopkg/cmd/accountcontact_test.gopkg/cmd/asset.gopkg/cmd/asset_test.gopkg/cmd/beeperdesktopapi.gopkg/cmd/beeperdesktopapi_test.gopkg/cmd/chat.gopkg/cmd/chat_test.gopkg/cmd/chatmessagereaction.gopkg/cmd/chatmessagereaction_test.gopkg/cmd/chatreminder.gopkg/cmd/chatreminder_test.gopkg/cmd/cmd.gopkg/cmd/cmdutil.gopkg/cmd/cmdutil_test.gopkg/cmd/cmdutil_unix.gopkg/cmd/cmdutil_windows.gopkg/cmd/flagoptions.gopkg/cmd/flagoptions_test.gopkg/cmd/info.gopkg/cmd/info_test.gopkg/cmd/message.gopkg/cmd/message_test.gopkg/cmd/suggest.gopkg/cmd/version.gorelease-please-config.jsonscripts/bootstrapscripts/buildscripts/build-homebrew-archive.mjsscripts/linkscripts/lintscripts/publish-homebrew-formula.mjsscripts/runscripts/testscripts/unlinkscripts/utils/upload-artifact.shsrc/commands/llm.ts
💤 Files with no reviewable changes (41)
- internal/autocomplete/shellscripts/bash_autocomplete.bash
- pkg/cmd/chatmessagereaction.go
- internal/apiform/encoder.go
- .github/actions/setup-go/action.yml
- pkg/cmd/asset_test.go
- pkg/cmd/account_test.go
- pkg/cmd/chat_test.go
- pkg/cmd/beeperdesktopapi.go
- pkg/cmd/chatreminder_test.go
- internal/autocomplete/shellscripts/zsh_autocomplete.zsh
- go.mod
- pkg/cmd/accountcontact_test.go
- pkg/cmd/chat.go
- pkg/cmd/chatmessagereaction_test.go
- pkg/cmd/accountcontact.go
- internal/jsonview/staticdisplay.go
- pkg/cmd/beeperdesktopapi_test.go
- internal/autocomplete/shellscripts/fish_autocomplete.fish
- pkg/cmd/account.go
- internal/debugmiddleware/debug_middleware_test.go
- internal/apiform/form.go
- internal/autocomplete/autocomplete.go
- internal/apiquery/query.go
- internal/binaryparam/binary_param_test.go
- internal/requestflag/innerflag.go
- pkg/cmd/cmd.go
- internal/debugmiddleware/debug_middleware.go
- internal/binaryparam/binary_param.go
- pkg/cmd/chatreminder.go
- internal/apiquery/encoder.go
- internal/jsonview/explorer.go
- pkg/cmd/asset.go
- internal/requestflag/innerflag_test.go
- internal/autocomplete/shellscripts/pwsh_autocomplete.ps1
- cmd/beeper-desktop-cli/main.go
- internal/requestflag/requestflag_test.go
- internal/requestflag/requestflag.go
- internal/apiquery/query_test.go
- internal/autocomplete/autocomplete_test.go
- internal/mocktest/mocktest.go
- internal/apiform/form_test.go
📜 Review details
🧰 Additional context used
🪛 LanguageTool
README.md
[uncategorized] ~248-~248: The official name of this software platform is spelled with a capital “H”.
Context: ...GitHub Releases. Push a v* tag to run .github/workflows/publish-release.yml. The re...
(GITHUB)
🔇 Additional comments (3)
package.json (1)
1-95: LGTM!.github/workflows/publish-release.yml (1)
15-47: LGTM!README.md (1)
1-128: LGTM!Also applies to: 130-209, 212-260
Multiple refactors and improvements across the CLI: - commands/current-user: use appRequest helper instead of manual fetch and token handling. - lib/client: prefer BEEPER_ACCESS_TOKEN env or config auth token before falling back to desktop auth; requireToken reads env/config first. - lib/desktop-auth: prefer probing the explicit baseURL first and concurrently probe remaining candidates via Promise.any for faster discovery. - lib/export: major rewrite to stream message and attachment data to disk using file handles, append to partial files, emit an attachments manifest as JSONL, and avoid accumulating large in-memory arrays; optimize chat collection for provided chat IDs with Promise.all; change downloadMessageAttachments to return counts and accept a write-entry callback; remove appendJSONL helper and add a MIME extension map helper. - lib/output: lazy-load Ink renderers to reduce startup cost, centralize JSON/JSONL writing, and make startStream async; rendering functions now dynamically import from ink/render. - lib/ink/render and lib/ui: small cleanup (removed comment and kept exports). Purpose: reduce memory pressure for large exports, speed up desktop discovery, support environment-provided tokens, and defer heavy Ink module loads to improve CLI startup performance.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
README.md (1)
100-101:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winPrefix CLI subcommands with
beeperfor copy/paste correctness.Line 100 and Line 101 currently read like standalone binaries; they should be documented as
beepersubcommands.Suggested fix
-`commands --json` prints a compact command manifest for tools and agents. -`llm` prints a concise human-readable command guide. +`beeper commands --json` prints a compact command manifest for tools and agents. +`beeper llm` prints a concise human-readable command guide.🤖 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 `@README.md` around lines 100 - 101, Update the README lines that describe the CLI so the two commands are shown as beeper subcommands; change the text referencing `commands --json` to `beeper commands --json` and the text referencing `llm` to `beeper llm` so examples are copy/paste accurate and clearly indicate these are subcommands of the `beeper` binary.
🤖 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 `@README.md`:
- Around line 674-680: Several table cells contain unescaped pipe characters
that break Markdown table parsing (MD056); update each option cell value to
escape the internal '|' characters (e.g., in the entries for
`--inbox=<primary|low-priority|archive>`, `--scope=<titles|participants>`, and
`--type=<single|group|any>`) so the pipes are rendered literally (use `\|`) and
ensure the same change is applied to the other occurrences referenced in the
comment (the cells at the other listed locations).
---
Duplicate comments:
In `@README.md`:
- Around line 100-101: Update the README lines that describe the CLI so the two
commands are shown as beeper subcommands; change the text referencing `commands
--json` to `beeper commands --json` and the text referencing `llm` to `beeper
llm` so examples are copy/paste accurate and clearly indicate these are
subcommands of the `beeper` binary.
🪄 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: a5cb5fdc-7740-4e22-9396-b6fd19069ad0
⛔ Files ignored due to path filters (2)
go.sumis excluded by!**/*.sumpackage-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (153)
.github/actions/setup-go/action.yml.github/workflows/publish-release.ymlREADME.mdcmd/beeper-desktop-cli/main.gogo.modinternal/apiform/encoder.gointernal/apiform/form.gointernal/apiform/form_test.gointernal/apiquery/encoder.gointernal/apiquery/query.gointernal/apiquery/query_test.gointernal/autocomplete/autocomplete.gointernal/autocomplete/autocomplete_test.gointernal/autocomplete/shellscripts/bash_autocomplete.bashinternal/autocomplete/shellscripts/fish_autocomplete.fishinternal/autocomplete/shellscripts/pwsh_autocomplete.ps1internal/autocomplete/shellscripts/zsh_autocomplete.zshinternal/binaryparam/binary_param.gointernal/binaryparam/binary_param_test.gointernal/debugmiddleware/debug_middleware.gointernal/debugmiddleware/debug_middleware_test.gointernal/jsonview/explorer.gointernal/jsonview/staticdisplay.gointernal/mocktest/mocktest.gointernal/requestflag/innerflag.gointernal/requestflag/innerflag_test.gointernal/requestflag/requestflag.gointernal/requestflag/requestflag_test.gopackage.jsonpkg/cmd/account.gopkg/cmd/account_test.gopkg/cmd/accountcontact.gopkg/cmd/accountcontact_test.gopkg/cmd/asset.gopkg/cmd/asset_test.gopkg/cmd/beeperdesktopapi.gopkg/cmd/beeperdesktopapi_test.gopkg/cmd/chat.gopkg/cmd/chat_test.gopkg/cmd/chatmessagereaction.gopkg/cmd/chatmessagereaction_test.gopkg/cmd/chatreminder.gopkg/cmd/chatreminder_test.gopkg/cmd/cmd.gopkg/cmd/cmdutil.gopkg/cmd/cmdutil_test.gopkg/cmd/cmdutil_unix.gopkg/cmd/cmdutil_windows.gopkg/cmd/flagoptions.gopkg/cmd/flagoptions_test.gopkg/cmd/info.gopkg/cmd/info_test.gopkg/cmd/message.gopkg/cmd/message_test.gopkg/cmd/suggest.gopkg/cmd/version.gorelease-please-config.jsonscripts/bootstrapscripts/buildscripts/build-homebrew-archive.mjsscripts/generate-readme.mjsscripts/linkscripts/lintscripts/publish-homebrew-formula.mjsscripts/runscripts/testscripts/unlinkscripts/utils/upload-artifact.shsrc/commands/accounts.tssrc/commands/accounts/add.tssrc/commands/api/get.tssrc/commands/api/post.tssrc/commands/app/e2ee/recovery-code/mark-backed-up.tssrc/commands/app/e2ee/recovery-code/reset/begin.tssrc/commands/app/e2ee/recovery-code/reset/confirm.tssrc/commands/app/e2ee/recovery-code/verify.tssrc/commands/app/e2ee/verification/accept.tssrc/commands/app/e2ee/verification/cancel.tssrc/commands/app/e2ee/verification/qr/confirm-scanned.tssrc/commands/app/e2ee/verification/qr/scan.tssrc/commands/app/e2ee/verification/sas/confirm.tssrc/commands/app/e2ee/verification/sas/start.tssrc/commands/app/e2ee/verification/start.tssrc/commands/app/status.tssrc/commands/archive.tssrc/commands/assets/download.tssrc/commands/assets/upload.tssrc/commands/auth/login.tsxsrc/commands/auth/logout.tssrc/commands/auth/status.tssrc/commands/avatar.tssrc/commands/chat.tssrc/commands/chats/index.tssrc/commands/chats/search.tssrc/commands/clear-draft.tssrc/commands/commands.tssrc/commands/config/get.tssrc/commands/config/path.tssrc/commands/config/reset.tssrc/commands/config/set.tssrc/commands/contacts/list.tssrc/commands/contacts/search.tssrc/commands/create-chat.tssrc/commands/current-user.tssrc/commands/delete-message.tssrc/commands/description.tssrc/commands/doctor.tssrc/commands/draft.tssrc/commands/edit.tssrc/commands/focus.tssrc/commands/inbox.tssrc/commands/llm.tssrc/commands/low-priority.tssrc/commands/message-expiry.tssrc/commands/message.tssrc/commands/messages/index.tssrc/commands/messages/search.tssrc/commands/mute.tssrc/commands/notify-anyway.tssrc/commands/pin.tssrc/commands/react.tssrc/commands/read.tssrc/commands/remind.tssrc/commands/reply-file.tssrc/commands/reply.tssrc/commands/search.tssrc/commands/send-file.tssrc/commands/send.tssrc/commands/start-chat.tssrc/commands/status.tssrc/commands/title.tssrc/commands/unarchive.tssrc/commands/unmute.tssrc/commands/unpin.tssrc/commands/unreact.tssrc/commands/unread.tssrc/commands/unremind.tssrc/commands/watch.tssrc/lib/client.tssrc/lib/config.tssrc/lib/desktop-auth.tssrc/lib/export/index.tssrc/lib/ink/components.tsxsrc/lib/ink/format.tssrc/lib/ink/render.tsxsrc/lib/ink/spinner.tsxsrc/lib/ink/theme.tssrc/lib/manifest.tssrc/lib/output.tssrc/lib/ui.tstest/cli-smoke.mjstest/e2e-staging.mjstsconfig.json
💤 Files with no reviewable changes (28)
- internal/binaryparam/binary_param.go
- internal/apiform/form_test.go
- internal/apiquery/query.go
- internal/autocomplete/autocomplete_test.go
- internal/autocomplete/shellscripts/bash_autocomplete.bash
- pkg/cmd/account_test.go
- internal/mocktest/mocktest.go
- internal/requestflag/innerflag.go
- go.mod
- internal/debugmiddleware/debug_middleware_test.go
- internal/autocomplete/shellscripts/pwsh_autocomplete.ps1
- cmd/beeper-desktop-cli/main.go
- internal/requestflag/innerflag_test.go
- internal/autocomplete/shellscripts/fish_autocomplete.fish
- internal/autocomplete/shellscripts/zsh_autocomplete.zsh
- internal/jsonview/staticdisplay.go
- .github/actions/setup-go/action.yml
- internal/apiquery/query_test.go
- internal/requestflag/requestflag.go
- internal/requestflag/requestflag_test.go
- internal/jsonview/explorer.go
- pkg/cmd/account.go
- internal/apiform/form.go
- internal/apiform/encoder.go
- internal/autocomplete/autocomplete.go
- internal/apiquery/encoder.go
- internal/binaryparam/binary_param_test.go
- internal/debugmiddleware/debug_middleware.go
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/publish-release.yml
📜 Review details
🧰 Additional context used
🪛 LanguageTool
README.md
[uncategorized] ~1781-~1781: The official name of this software platform is spelled with a capital “H”.
Context: ...GitHub Releases. Push a v* tag to run .github/workflows/publish-release.yml. The re...
(GITHUB)
🪛 markdownlint-cli2 (0.22.1)
README.md
[warning] 674-674: Table column count
Expected: 3; Actual: 5; Too many cells, extra data will be missing
(MD056, table-column-count)
[warning] 679-679: Table column count
Expected: 3; Actual: 4; Too many cells, extra data will be missing
(MD056, table-column-count)
[warning] 680-680: Table column count
Expected: 3; Actual: 5; Too many cells, extra data will be missing
(MD056, table-column-count)
[warning] 823-823: Table column count
Expected: 3; Actual: 4; Too many cells, extra data will be missing
(MD056, table-column-count)
[warning] 1223-1223: Table column count
Expected: 3; Actual: 4; Too many cells, extra data will be missing
(MD056, table-column-count)
[warning] 1230-1230: Table column count
Expected: 3; Actual: 7; Too many cells, extra data will be missing
(MD056, table-column-count)
🔇 Additional comments (1)
package.json (1)
1-100: LGTM!
Promote login/logout and send subcommands to canonical commands, remove compatibility aliases, and add shared base command flags for JSON, events, and read-only handling. Regenerate README and update smoke/e2e coverage for the new command names.
Member
Author
|
@indent review |
|
Member
Author
|
@coderabbitai Review-body items handled:
|
|
Tip For best results, initiate chat on the files or code changes.
Would you like me to resolve the corresponding open review comments for any of these items? |
Delete the vendored OpenTUI skill and documentation from .agents/skills so this repository no longer carries that agent-specific reference material.
|
Review posted to the banner above. Headline findings: Critical \u2014 release/CI pipeline is non-functional as-shipped:
Functional: Latent: Nits called out inline (unused import in Tag me with |
![indent[bot]](https://avatars.githubusercontent.com/in/1426746?s=60&v=4){kind=small}
| node-version: 24 | ||
| cache: npm | ||
|
|
||
| - name: Install dependencies |
There was a problem hiding this comment.
CI cannot install dependencies. The repo only contains pnpm-lock.yaml (root package.json declares packageManager: pnpm@10.25.0 and there is no package-lock.json anywhere — verified with find . -maxdepth 3 -name package-lock.json -o -name pnpm-lock.yaml). actions/setup-node@v6 with cache: npm fails because it can't locate a package-lock.json/npm-shrinkwrap.json to hash, and npm ci exits with The 'npm ci' command can only install with an existing package-lock.json…. Switch to pnpm install (e.g. add pnpm/action-setup@v4 before setup-node, set cache: pnpm, and run pnpm install --frozen-lockfile). The same fix is needed in publish-release.yml.
|
|
||
| - name: Bootstrap | ||
| run: ./scripts/bootstrap | ||
| - name: Test |
There was a problem hiding this comment.
pnpm is not on PATH. Root package.json scripts call pnpm -r typecheck / pnpm -r test / pnpm -r build, but the workflow only installs Node via setup-node — there's no pnpm/action-setup, corepack enable, or npm i -g pnpm step. npm run typecheck would shell out to pnpm and fail with command not found. Same issue in publish-release.yml. Add a pnpm install step and prefer calling pnpm typecheck / pnpm test directly.
| - name: Run tests | ||
| run: ./scripts/test | ||
| - name: Package Homebrew archive | ||
| run: npm run pack:homebrew |
There was a problem hiding this comment.
pack:homebrew doesn't exist at the workspace root. grep pack:homebrew package.json returns nothing; the script is only declared in packages/cli/package.json. npm run pack:homebrew from the repo root exits with npm error Missing script: pack:homebrew. Either delegate via pnpm (pnpm --filter beeper-cli run pack:homebrew) or set working-directory: packages/cli for this step. Same fix needed in publish-release.yml.
| tag="${GITHUB_REF_NAME}" | ||
| if ! gh release view "${tag}" >/dev/null 2>&1; then | ||
| gh release create "${tag}" --title "${tag}" --generate-notes --verify-tag | ||
| fi |
There was a problem hiding this comment.
dist/release/*.tar.gz resolves to the wrong directory. scripts/build-homebrew-archive.mjs computes root = resolve('..', import.meta.url) (which is packages/cli/) and writes archives to packages/cli/dist/release/. The workflow's default working-directory is the repo root, so dist/release/*.tar.gz matches nothing and gh release upload fails with no such file or directory. Fix by uploading packages/cli/dist/release/*.tar.gz packages/cli/dist/release/homebrew.json (or set working-directory: packages/cli for this step). If publish-homebrew-formula.mjs is then run with the same metadata path, the Homebrew formula will also point at a real asset.
| import {spawn} from 'node:child_process'; | ||
|
|
||
| const root = resolve(new URL('..', import.meta.url).pathname); | ||
| const packageJsonPath = join(root, 'package.json'); |
There was a problem hiding this comment.
package-lock.json doesn't exist. The repo is a pnpm workspace; find packages/cli -maxdepth 2 -name package-lock.json returns nothing. Lines 10/35–36 unconditionally cp(packageLockPath, ...) and line 53 then runs npm ci. Both fail. Switch the production install to pnpm (stage pnpm-lock.yaml next to package.json and run pnpm install --prod --frozen-lockfile in libexec/), or commit a real package-lock.json for the package.
| const ws = new WebSocket(url, { headers: { Authorization: `Bearer ${token}` } } as unknown as string[]) | ||
|
|
||
| if (flags.json) { | ||
| await this.runJSON(ws, subscribed, flags.events) |
There was a problem hiding this comment.
Authorization header is silently dropped. Node's global WebSocket (which this code uses — there's no ws dependency in packages/cli/package.json) implements the WHATWG spec new WebSocket(url, protocols?). The second argument is treated as a list of subprotocols, not headers; the as unknown as string[] cast only hides the type error. As a result the bearer token is never sent and the Desktop API rejects the upgrade. Add the ws package (new (await import('ws')).default(url, { headers: { Authorization: ... } })) or move the token onto a query string the server accepts. Drop the cast once the real call type-checks.
| printIDs(items) | ||
| return | ||
| } | ||
| printList(items, flags.json ? 'json' : 'human', { |
There was a problem hiding this comment.
printList is not awaited. printList returns a Promise that, in human mode, only resolves after Ink's waitUntilExit() fires inside renderOnce (see lib/ink/render.tsx). Without await, run() resolves immediately and oclif tears down the process before the renderer flushes, producing truncated output. commands/accounts.ts, contacts/list.ts, and contacts/search.ts already await printList(...); do the same here and in chats/search.ts:52, messages/index.ts:38, and messages/search.ts:58. JSON mode happens to write synchronously which is why the bug isn't caught by smoke tests.
|
|
||
| async function retrieveChat(client: any, input: string): Promise<AnyRecord | undefined> { | ||
| try { | ||
| return await client.chats.retrieve(input, { maxParticipants: 0 }) |
There was a problem hiding this comment.
Wrong SDK option name. Every other call site uses maxParticipantCount (commands/chat.ts:23, lib/export/index.ts:97, lib/export/index.ts:161). maxParticipants is silently ignored by the SDK, so this fast-path retrieval always returns the full participant list — wasteful for large group chats and a Functional bug the moment the SDK starts rejecting unknown params. Change to { maxParticipantCount: 0 }.
Suggested change
| return await client.chats.retrieve(input, { maxParticipants: 0 }) | |
| return await client.chats.retrieve(input, { maxParticipantCount: 0 }) |
| progress(options, `Export directory: ${options.outDir}`) | ||
| const accounts = await client.accounts.list() | ||
| await writeJSONAtomic(join(options.outDir, 'accounts.json'), accounts) | ||
| progress(options, `Accounts: ${accounts.length}`) |
There was a problem hiding this comment.
Account list shape is handled inconsistently. commands/accounts.ts:7-10 defines accountItems() to handle both array and {items} shapes — a strong signal the SDK is not always returning a raw array. Here we treat the result as an array directly (accounts.length, JSON-stringified into accounts.json), and lib/resolve.ts:19,40 does the same cast. If the wrapped shape ever shows up, resolution silently matches nothing and accounts.json becomes {"items":[…]} with the count line printing Accounts: undefined. Extract accountItems into the shared lib and use it everywhere.
| if (options.downloadAttachments) { | ||
| const downloaded = await downloadMessageAttachments(client, chatDir, message, writeAttachmentEntry) | ||
| attachmentCount += downloaded | ||
| } |
There was a problem hiding this comment.
Resume can silently drop attachments. The flow appends the message to messages.partial.jsonl (line 210) before downloading attachments (line 216), and chatState.attachmentCount is only saved at page boundaries (lines 224-228). If the export is interrupted mid-message, the dedupe seen set (built from the partial JSONL at line 184) marks the message as already-handled on resume, so downloadMessageAttachments is never called for it again. The resumed run reports the smaller attachmentCount while orphaned, partially-downloaded files may sit in attachments/. Either download attachments before writing the message, or track per-message attachment completion in checkpoint state.
| import { BeeperCommand } from '../lib/command.js' | ||
| import { createClient } from '../lib/client.js' | ||
| import { apiCopy, cliCopy } from '../lib/copy.js' | ||
| import { printData, printList } from '../lib/output.js' |
There was a problem hiding this comment.
Unused: cliCopy is imported but never referenced in this file. Remove it from the import list to keep things tidy.
Suggested change
| import { printData, printList } from '../lib/output.js' | |
| import { apiCopy } from '../lib/copy.js' |
| const { args, flags } = await this.parse(ContactsList) | ||
| const client = await createClient(flags) | ||
| const accountIDs = await resolveAccountIDs(client, [args.account], { allowMultiplePerInput: true }) ?? [args.account] | ||
| const useSpinner = !flags.json && !flags.ids |
There was a problem hiding this comment.
Dead branch: resolveAccountIDs returns undefined only when its inputs is empty/missing, and otherwise either returns a non-empty array or throws. Since args.account is required (line 14), ?? [args.account] is never taken. Drop it so a real “no account matches…” error from resolveAccountIDs surfaces to the user instead of silently falling back to a literal that would later fail upstream.
Suggested change
| const useSpinner = !flags.json && !flags.ids | |
| const accountIDs = await resolveAccountIDs(client, [args.account], { allowMultiplePerInput: true })! |
Introduce a stable plugin SDK entrypoint for third-party oclif plugins by adding src/plugin-sdk.ts and exporting it from package.json. Document plugin usage and author guidance in the CLI README, and enable plugin management by adding @oclif/plugin-plugins to the oclif plugins list and dependencies.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.