CLI v2#7
Open
batuhan wants to merge 15 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
| import {readFile} from 'node:fs/promises'; | ||
| import {join, resolve} from 'node:path'; | ||
|
|
||
| const root = resolve(new URL('..', import.meta.url).pathname); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Node.js ESM, why is using new URL(...).pathnamefor filesystem paths problematic (especially on Windows or paths with spaces), and isfileURLToPath(new URL(...)) the recommended approach?
💡 Result:
Using new URL(...).pathname for filesystem paths in Node.js ESM is problematic because URL.pathname returns a URL-encoded path that does not match the platform-specific filesystem path format expected by Node.js APIs like fs functions.[1][2] On Windows, it produces paths starting with a forward slash (e.g., /C:/path/ instead of C:\path\), which are invalid for filesystem operations.[2][1] It also fails to use backslashes and may not handle drive letters correctly.[3] For paths with spaces or special characters, pathname percent-encodes them (e.g., space becomes %20), but filesystem paths require literal spaces or proper escaping without URL encoding.[1] Examples include file:///hello world becoming /hello%20world, which fs rejects.[1] Similar issues occur with non-ASCII characters, UNC paths, and certain control characters.[1][4] The official Node.js documentation explicitly warns against this and recommends fileURLToPath(new URL(...)) from the url module, which correctly decodes percent-encoded characters and produces a fully-resolved, platform-specific absolute path.[1] Examples from docs: - new URL('file:///C:/path/').pathname → /C:/path/ (incorrect on Windows)[1] - fileURLToPath('file:///C:/path/') → C:\path\ (correct)[1] - new URL('file:///hello world').pathname → /hello%20world (incorrect)[1] - fileURLToPath('file:///hello world') → /hello world (correct on POSIX)[1] This is the standard pattern for emulating dirname/filename: import { fileURLToPath } from 'node:url'; import { dirname } from 'node:path'; const filename = fileURLToPath(import.meta.url); const dirname = dirname(filename); [2][5][6]
Citations:
- 1: https://nodejs.org/dist/latest-v16.x/docs/api/url.html
- 2: new URL('.', import.meta.url).pathname produces erroneous path on Windows 10 nodejs/node#37845
- 3: Absolute Windows paths are handled as invalid URL path in ESM import() nodejs/node#34765
- 4: url: Cannot load ESM module with newline in file name nodejs/node#23696
- 5: https://egghead.io/lessons/node-js-handle-relative-paths-in-a-node-js-esm-project-with-import-meta-url
- 6: https://stackoverflow.com/questions/75004188/what-does-fileurltopathimport-meta-url-do
🏁 Script executed:
# Check if the file exists and read its contents
cat -n scripts/check-api-copy.mjsRepository: beeper/desktop-api-cli
Length of output: 3855
Replace .pathname with fileURLToPath for correct cross-platform path resolution.
Line 5 uses .pathname which produces invalid filesystem paths on Windows (e.g., /C:/path/ instead of C:\path\) and fails to decode percent-encoded characters (spaces, special chars become %20, etc.). The fileURLToPath() utility from node:url is the official Node.js recommended approach for converting import.meta.url-derived file URLs to filesystem paths.
Proposed fix
import {readFile} from 'node:fs/promises';
-import {join, resolve} from 'node:path';
+import {join} from 'node:path';
+import {fileURLToPath} from 'node:url';
-const root = resolve(new URL('..', import.meta.url).pathname);
+const root = fileURLToPath(new URL('..', import.meta.url));
const {apiCopy} = await import('../dist/lib/copy.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 `@scripts/check-api-copy.mjs` at 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).
Comment on lines
+10
to
+17
| if (token) { | ||
| await fetch(new URL('/oauth/revoke', config.baseURL), { | ||
| method: 'POST', | ||
| headers: { 'content-type': 'application/x-www-form-urlencoded' }, | ||
| body: new URLSearchParams({ token, token_type_hint: 'access_token' }), | ||
| }).catch(() => undefined) | ||
| } | ||
| await updateConfig(current => ({ ...current, auth: undefined })) |
There was a problem hiding this comment.
Prevent logout from hanging on revoke and always clear local auth.
Line 11 awaits an unbounded network call; if it stalls, execution may never reach Line 17, leaving local auth intact.
Suggested fix
async run(): Promise<void> {
const config = await readConfig()
const token = config.auth?.accessToken
- if (token) {
- await fetch(new URL('/oauth/revoke', config.baseURL), {
- method: 'POST',
- headers: { 'content-type': 'application/x-www-form-urlencoded' },
- body: new URLSearchParams({ token, token_type_hint: 'access_token' }),
- }).catch(() => undefined)
- }
- await updateConfig(current => ({ ...current, auth: undefined }))
+ try {
+ if (token) {
+ const controller = new AbortController()
+ const timeout = setTimeout(() => controller.abort(), 5000)
+ try {
+ await fetch(new URL('/oauth/revoke', config.baseURL), {
+ method: 'POST',
+ headers: { 'content-type': 'application/x-www-form-urlencoded' },
+ body: new URLSearchParams({ token, token_type_hint: 'access_token' }),
+ signal: controller.signal,
+ }).catch(() => undefined)
+ } finally {
+ clearTimeout(timeout)
+ }
+ }
+ } finally {
+ await updateConfig(current => ({ ...current, auth: undefined }))
+ }
this.log('Logged out')
}🤖 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/auth/logout.ts` around lines 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.
| debug: Flags.boolean({ default: false }), | ||
| ids: Flags.boolean({ default: false, description: 'Print only chat IDs' }), | ||
| inbox: Flags.string({ options: ['primary', 'low-priority', 'archive'] }), | ||
| 'include-muted': Flags.boolean({ allowNo: true, description: 'Include muted chats. Use --no-include-muted for a tighter search.' }), |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In @oclif/core, what value does Flags.boolean return by default when defaultis omitted andallowNo: true is set?
💡 Result:
In @oclif/core, Flags.boolean returns false by default when default is omitted, even if allowNo: true is set; i.e., the flag value is false unless the user passes --<flag> (true) or --no-<flag> (false). [1][2] Explanation (grounded in code/docs): Flags.boolean defaults to false unless default is explicitly set to true. [1] The allowNo option only enables the --no- form; it does not change the default boolean value. [1][2]
Citations:
🏁 Script executed:
# First, let's check if the file exists and examine the relevant section
head -30 src/commands/chats/search.tsRepository: beeper/desktop-api-cli
Length of output: 1866
🏁 Script executed:
# Search for the include-muted flag in the file to see exact context
rg "include-muted" src/commands/chats/search.ts -B 2 -A 2Repository: beeper/desktop-api-cli
Length of output: 812
🏁 Script executed:
# Search for usage of this flag throughout the codebase
rg "include-muted\|includeeMuted" --type ts --type jsRepository: beeper/desktop-api-cli
Length of output: 48
--include-muted default is inverted.
Line 18 sets allowNo: true without an explicit default. Per @oclif/core behavior, this defaults to false, meaning muted chats are excluded by default. However, the description "Include muted chats. Use --no-include-muted for a tighter search." implies muted chats should be included by default. The flag logic is backwards.
Suggested fix
- '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.' }),📝 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.
Suggested change
| '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/chats/search.ts` at 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.
Comment on lines
+16
to
+18
| const config = await readConfig() | ||
| const value = args.key ? config[args.key as 'baseURL' | 'auth'] : config | ||
| printData(value, flags.json ? 'json' : 'human') |
There was a problem hiding this comment.
Redact auth secrets in config get output by default.
Line 18 can print raw auth data (including access tokens), which is a credential-leak risk in logs, shell history, and shared terminals.
Suggested fix
async run(): Promise<void> {
const { args, flags } = await this.parse(ConfigGet)
const config = await readConfig()
- const value = args.key ? config[args.key as 'baseURL' | 'auth'] : config
+ const safeConfig = {
+ ...config,
+ auth: config.auth ? { ...config.auth, accessToken: '[redacted]' } : config.auth,
+ }
+ const value = args.key ? safeConfig[args.key as 'baseURL' | 'auth'] : safeConfig
printData(value, flags.json ? 'json' : 'human')
}📝 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.
Suggested change
| const config = await readConfig() | |
| const value = args.key ? config[args.key as 'baseURL' | 'auth'] : config | |
| printData(value, flags.json ? 'json' : 'human') | |
| const config = await readConfig() | |
| const safeConfig = { | |
| ...config, | |
| auth: config.auth ? { ...config.auth, accessToken: '[redacted]' } : config.auth, | |
| } | |
| const value = args.key ? safeConfig[args.key as 'baseURL' | 'auth'] : safeConfig | |
| printData(value, 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 `@src/commands/config/get.ts` around lines 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).
Comment on lines
+24
to
+27
| if (!quote && /\s/.test(char)) { | ||
| if (current) { | ||
| tokens.push(current) | ||
| current = '' |
There was a problem hiding this comment.
Preserve empty quoted arguments instead of dropping them.
At Line 25 and Line 37, token emission is gated by if (current), so ""/'' are lost. Example: a "" b becomes ["a","b"] instead of ["a","","b"].
Proposed fix
export function splitCommandLine(input: string): string[] {
const tokens: string[] = []
let current = ''
+ let tokenStarted = false
let quote: '"' | "'" | undefined
let escaped = false
for (const char of input) {
if (escaped) {
current += char
+ tokenStarted = true
escaped = false
continue
}
if (char === '\\' && quote !== "'") {
escaped = true
continue
}
if ((char === '"' || char === "'") && (!quote || quote === char)) {
+ if (!quote) tokenStarted = true
quote = quote ? undefined : char
continue
}
if (!quote && /\s/.test(char)) {
- if (current) {
+ if (tokenStarted) {
tokens.push(current)
current = ''
+ tokenStarted = false
}
continue
}
current += char
+ tokenStarted = true
}
if (escaped) current += '\\'
if (quote) throw new Error(`Unclosed ${quote} quote`)
- if (current) tokens.push(current)
+ if (tokenStarted) tokens.push(current)
return tokens
}Also applies to: 37-37
🤖 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/argv.ts` around lines 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.
Comment on lines
+24
to
+33
| try { | ||
| const raw = await readFile(configPath(), 'utf8') | ||
| const parsed = JSON.parse(raw) as Partial<Config> | ||
| return { | ||
| baseURL: parsed.baseURL || process.env.BEEPER_DESKTOP_BASE_URL || process.env.BEEPER_BASE_URL || defaultBaseURL, | ||
| auth: parsed.auth, | ||
| } | ||
| } catch { | ||
| return { baseURL: process.env.BEEPER_DESKTOP_BASE_URL || process.env.BEEPER_BASE_URL || defaultBaseURL } | ||
| } |
There was a problem hiding this comment.
Avoid silent fallback on config parse/read errors.
The broad catch in readConfig hides malformed config and access errors, which makes auth/baseURL failures hard to diagnose.
💡 Suggested fix
export async function readConfig(): Promise<Config> {
try {
const raw = await readFile(configPath(), 'utf8')
const parsed = JSON.parse(raw) as Partial<Config>
return {
baseURL: parsed.baseURL || process.env.BEEPER_DESKTOP_BASE_URL || process.env.BEEPER_BASE_URL || defaultBaseURL,
auth: parsed.auth,
}
- } catch {
- return { baseURL: process.env.BEEPER_DESKTOP_BASE_URL || process.env.BEEPER_BASE_URL || defaultBaseURL }
+ } catch (error) {
+ if ((error as NodeJS.ErrnoException)?.code === 'ENOENT') {
+ return { baseURL: process.env.BEEPER_DESKTOP_BASE_URL || process.env.BEEPER_BASE_URL || defaultBaseURL }
+ }
+ throw error
}
}📝 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.
Suggested change
| try { | |
| const raw = await readFile(configPath(), 'utf8') | |
| const parsed = JSON.parse(raw) as Partial<Config> | |
| return { | |
| baseURL: parsed.baseURL || process.env.BEEPER_DESKTOP_BASE_URL || process.env.BEEPER_BASE_URL || defaultBaseURL, | |
| auth: parsed.auth, | |
| } | |
| } catch { | |
| return { baseURL: process.env.BEEPER_DESKTOP_BASE_URL || process.env.BEEPER_BASE_URL || defaultBaseURL } | |
| } | |
| try { | |
| const raw = await readFile(configPath(), 'utf8') | |
| const parsed = JSON.parse(raw) as Partial<Config> | |
| return { | |
| baseURL: parsed.baseURL || process.env.BEEPER_DESKTOP_BASE_URL || process.env.BEEPER_BASE_URL || defaultBaseURL, | |
| auth: parsed.auth, | |
| } | |
| } catch (error) { | |
| if ((error as NodeJS.ErrnoException)?.code === 'ENOENT') { | |
| return { baseURL: process.env.BEEPER_DESKTOP_BASE_URL || process.env.BEEPER_BASE_URL || defaultBaseURL } | |
| } | |
| throw 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/lib/config.ts` around lines 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.
Comment on lines
+94
to
+108
| async function exchangeToken(tokenEndpoint: string, clientID: string, code: string, codeVerifier: string): Promise<TokenResponse> { | ||
| const body = new URLSearchParams({ | ||
| grant_type: 'authorization_code', | ||
| client_id: clientID, | ||
| code, | ||
| code_verifier: codeVerifier, | ||
| }) | ||
| const response = await fetch(tokenEndpoint, { | ||
| method: 'POST', | ||
| headers: { 'content-type': 'application/x-www-form-urlencoded' }, | ||
| body, | ||
| }) | ||
| if (!response.ok) throw new Error(`OAuth token exchange failed: ${response.status} ${await response.text()}`) | ||
| return response.json() as Promise<TokenResponse> | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if redirect_uri is set during authorization but not in token exchange
rg -n -C3 'redirect_uri' src/lib/oauth.tsRepository: beeper/desktop-api-cli
Length of output: 754
🌐 Web query:
Does OAuth 2.0 RFC 6749 and PKCE RFC 7636 require redirect_uri in the token exchange request when it was included in the authorization request?
💡 Result:
Yes, OAuth 2.0 RFC 6749 requires the redirect_uri in the token request (authorization code grant) if it was included in the authorization request, and the values must be identical [1][2]. The authorization server must ensure the redirect_uri is present in this case and matches [1][2]. RFC 7636 (PKCE) builds on this by adding code_verifier to the standard authorization code token request defined in RFC 6749 section 4.1.3, without altering the redirect_uri requirement [3][4]. Note that "token exchange" typically refers to RFC 8693, which uses a different grant_type and does not require redirect_uri [5], but the query context matches the authorization code flow in RFC 6749+PKCE.
Citations:
- 1: https://rfc-editor.org/rfc/rfc6749
- 2: https://datatracker.ietf.org/doc/html/rfc6749
- 3: https://tools.ietf.org/html/rfc7636
- 4: https://datatracker.ietf.org/doc/html/rfc7636
- 5: https://rfc-editor.org/rfc/rfc8693
Missing redirect_uri in token exchange request violates RFC 6749.
OAuth 2.0 RFC 6749 requires the redirect_uri parameter in the token exchange request when it was included in the authorization request. Line 37 sets redirect_uri in the authorization URL, but the token exchange request (lines 95–100) omits it from the body. The function also lacks the redirectURI parameter needed to include it.
Pass redirectURI to the exchangeToken function and add it to the URLSearchParams body:
async function exchangeToken(tokenEndpoint: string, clientID: string, redirectURI: string, code: string, codeVerifier: string): Promise<TokenResponse> {
const body = new URLSearchParams({
grant_type: 'authorization_code',
client_id: clientID,
redirect_uri: redirectURI,
code,
code_verifier: codeVerifier,
})
// ... rest of function
}🤖 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 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.
Comment on lines
+241
to
+246
| export async function collectPage<T>(iterable: AsyncIterable<T>, limit?: number): Promise<T[]> { | ||
| const items: T[] = [] | ||
| for await (const item of iterable) { | ||
| items.push(item) | ||
| if (limit && items.length >= limit) break | ||
| } |
There was a problem hiding this comment.
Honor explicit limit = 0 in collectPage.
Line 245 treats 0 as “no limit”, so collectPage(..., 0) drains the full stream instead of returning an empty page.
💡 Suggested fix
export async function collectPage<T>(iterable: AsyncIterable<T>, limit?: number): Promise<T[]> {
+ if (limit !== undefined && limit <= 0) return []
const items: T[] = []
for await (const item of iterable) {
items.push(item)
- if (limit && items.length >= limit) break
+ if (limit !== undefined && items.length >= limit) break
}
return items
}📝 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.
Suggested change
| export async function collectPage<T>(iterable: AsyncIterable<T>, limit?: number): Promise<T[]> { | |
| const items: T[] = [] | |
| for await (const item of iterable) { | |
| items.push(item) | |
| if (limit && items.length >= limit) break | |
| } | |
| export async function collectPage<T>(iterable: AsyncIterable<T>, limit?: number): Promise<T[]> { | |
| if (limit !== undefined && limit <= 0) return [] | |
| const items: T[] = [] | |
| for await (const item of iterable) { | |
| items.push(item) | |
| if (limit !== undefined && items.length >= limit) break | |
| } | |
| return items | |
| } |
🤖 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/output.ts` around lines 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.
Comment on lines
+98
to
+103
| async function retrieveChat(client: any, input: string): Promise<AnyRecord | undefined> { | ||
| try { | ||
| return await client.chats.retrieve(input, { maxParticipants: 0 }) | ||
| } catch { | ||
| return undefined | ||
| } |
There was a problem hiding this comment.
Do not swallow all retrieve failures.
Lines 101-102 hide every error type, including auth/network/server failures. That can misroute resolution and produce misleading downstream errors.
💡 Suggested fix
async function retrieveChat(client: any, input: string): Promise<AnyRecord | undefined> {
try {
return await client.chats.retrieve(input, { maxParticipants: 0 })
- } catch {
- return undefined
+ } catch (error) {
+ const message = error instanceof Error ? error.message : String(error)
+ if (/not\s*found|404/i.test(message)) return undefined
+ throw error
}
}📝 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.
Suggested change
| async function retrieveChat(client: any, input: string): Promise<AnyRecord | undefined> { | |
| try { | |
| return await client.chats.retrieve(input, { maxParticipants: 0 }) | |
| } catch { | |
| return undefined | |
| } | |
| async function retrieveChat(client: any, input: string): Promise<AnyRecord | undefined> { | |
| try { | |
| return await client.chats.retrieve(input, { maxParticipants: 0 }) | |
| } catch (error) { | |
| const message = error instanceof Error ? error.message : String(error) | |
| if (/not\s*found|404/i.test(message)) return undefined | |
| throw 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/lib/resolve.ts` around lines 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.
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
| async run(): Promise<void> { | ||
| const { args, flags } = await this.parse(BridgeLoginSubmitUserInput) | ||
| const client = await createClient(flags) | ||
| const body = JSON.parse(flags.body) as Record<string, string> |
There was a problem hiding this comment.
Handle invalid --body JSON explicitly before API submission.
Line 23 can throw on malformed input and currently exits with an uncaught parse error. Add guarded parsing plus minimal object/string-value validation so CLI failures are actionable.
Proposed fix
- const body = JSON.parse(flags.body) as Record<string, string>
+ let body: Record<string, string>
+ try {
+ const parsed: unknown = JSON.parse(flags.body)
+ if (typeof parsed !== 'object' || parsed === null || Array.isArray(parsed)) {
+ this.error('--body must be a JSON object with string values')
+ }
+ if (Object.values(parsed as Record<string, unknown>).some((v) => typeof v !== 'string')) {
+ this.error('--body must be a JSON object with string values')
+ }
+ body = parsed as Record<string, string>
+ } catch (error) {
+ this.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/bridge/login/submit-user-input.ts` at 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.
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
| cookie: Flags.string({ description: 'Cookie value for non-interactive login, in name=value form. Repeat for multiple cookies.', multiple: true }), | ||
| debug: Flags.boolean({ default: false }), | ||
| field: Flags.string({ description: 'Field value for non-interactive login, in id=value form. Repeat for multiple fields.', multiple: true }), | ||
| flow: Flags.string({ description: 'Login flow ID. If omitted, Desktop chooses the default flow.' }), |
There was a problem hiding this comment.
Don't silently fall back to the first login flow.
Line 56/58 turns an omitted --flow into loginFlows[0] whenever the chooser is skipped (--json or --guided=false). That makes the selected auth method depend on backend ordering and contradicts the flag help that says the default flow will be chosen. In these cases, require --flow explicitly instead of guessing.
Suggested fix
let flowID = flags.flow
if (!flowID) {
const flows = await client.matrix.bridges.auth.listFlows(accountType.bridgeID)
const loginFlows = flows.flows ?? []
- if (loginFlows.length > 1 && flags['non-interactive']) throw new Error(`Multiple sign-in methods are available for ${accountType.displayName}. Pass --flow.`)
- flowID = loginFlows.length > 1 && flags.guided && !flags.json
- ? await chooseLoginFlow(loginFlows)
- : loginFlows[0]?.id
+ if (loginFlows.length > 1) {
+ if (flags.guided && !flags.json && !flags['non-interactive']) {
+ flowID = await chooseLoginFlow(loginFlows)
+ } else {
+ throw new Error(`Multiple sign-in methods are available for ${accountType.displayName}. Pass --flow.`)
+ }
+ } else {
+ flowID = loginFlows[0]?.id
+ }
if (!flowID) throw new Error(`No login flows returned for ${accountType.displayName}.`)
if (!flags.json && loginFlows.length > 1) this.log(`Using flow ${flowID}`)
}Also applies to: 51-60
🤖 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/accounts/add.ts` at 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.
Comment on lines
+30
to
+58
| export function printAccountLoginStep(step: AccountLoginStep): void { | ||
| const common = step as CommonStep | ||
| process.stdout.write(`step: ${common.type}\n`) | ||
| if (common.instructions) process.stdout.write(`${common.instructions}\n`) | ||
| if (common.login_id) process.stdout.write(`login_id: ${common.login_id}\n`) | ||
| if (common.step_id) process.stdout.write(`step_id: ${common.step_id}\n`) | ||
|
|
||
| if ('display_and_wait' in step) { | ||
| const display = step.display_and_wait | ||
| process.stdout.write(`display: ${display.type}\n`) | ||
| if (display.data) process.stdout.write(`${display.data}\n`) | ||
| if (display.image_url) process.stdout.write(`image: ${display.image_url}\n`) | ||
| } else if ('user_input' in step) { | ||
| for (const field of step.user_input.fields) { | ||
| const details = [field.type, field.description, field.options?.length ? `options: ${field.options.join(', ')}` : undefined] | ||
| .filter(Boolean) | ||
| .join(' | ') | ||
| process.stdout.write(`field ${field.id}: ${field.name}${details ? ` (${details})` : ''}\n`) | ||
| } | ||
| } else if ('cookies' in step) { | ||
| process.stdout.write(`url: ${step.cookies.url}\n`) | ||
| if (step.cookies.user_agent) process.stdout.write(`user_agent: ${step.cookies.user_agent}\n`) | ||
| if (step.cookies.wait_for_url_pattern) process.stdout.write(`wait_for_url_pattern: ${step.cookies.wait_for_url_pattern}\n`) | ||
| for (const field of step.cookies.fields) process.stdout.write(`cookie field ${field.name}: ${field.type}\n`) | ||
| if (step.cookies.extract_js) process.stdout.write(`extract_js:\n${step.cookies.extract_js}\n`) | ||
| } else if ('complete' in step) { | ||
| process.stdout.write(`complete: ${step.complete.user_login_id ?? 'yes'}\n`) | ||
| } | ||
| } |
There was a problem hiding this comment.
Reserve stdout for the final payload.
This helper writes guided step output and prompts to stdout unconditionally. Any caller that wants machine-readable stdout — including the new accounts add --json path — will get mixed human text and invalid JSON. Send interactive/progress output to stderr or make the output stream configurable.
Also applies to: 60-64, 120-123
🤖 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/account-login.ts` around lines 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.
Comment on lines
+101
to
+110
| if ('cookies' in step) { | ||
| const body: Record<string, string> = {} | ||
| for (const field of step.cookies.fields) { | ||
| if (options.cookies?.[field.name] !== undefined) { | ||
| body[field.name] = options.cookies[field.name]! | ||
| continue | ||
| } | ||
|
|
||
| if (options.nonInteractive) throw new Error(`Missing required cookie ${field.name}. Pass --cookie ${field.name}=... or run without --non-interactive.`) | ||
| body[field.name] = await promptText(`${field.name}: `) |
There was a problem hiding this comment.
Don't echo cookie values in the terminal.
The cookies branch collects authentication secrets through promptText(), which echoes the value back to the screen. That exposes session material in terminal scrollback and shared screens. Use a masked/no-echo prompt path for cookie input.
Also applies to: 120-123
🤖 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/account-login.ts` around lines 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.
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!
| static override summary = 'Create a new recovery key' | ||
| static override flags = { | ||
| 'base-url': Flags.string({ description: 'Beeper Desktop API base URL' }), | ||
| 'recovery-code': Flags.string({ description: 'Existing recovery key, if available' }), |
There was a problem hiding this comment.
Consider the security implications of passing recovery codes via CLI flags.
Recovery codes are sensitive E2EE credentials. CLI flags are visible in shell history, process listings (ps), and logs, which can expose the recovery code to unauthorized access.
If the Desktop API supports it, consider prompting for the recovery code interactively (with hidden input) or reading from stdin/file when the flag is not provided. If the flag must remain for automation scenarios, document the security risk and recommend using environment variables or secure input methods.
🤖 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/app/e2ee/recovery-code/reset/begin.ts` at 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.
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
Comment on lines
3
to
+5
| permissions: | ||
| contents: write | ||
| id-token: write |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether this repo actually uses OIDC/provenance flows in release automation.
rg -n --type=yaml -C2 'id-token:\s*write|ACTIONS_ID_TOKEN_REQUEST_URL|provenance|sigstore|cosign|aws-actions/configure-aws-credentials|azure/login|google-github-actions/auth' .github/workflows
rg -n -C2 'ACTIONS_ID_TOKEN_REQUEST_URL|--provenance|sigstore|cosign' package.json README.md scriptsRepository: beeper/desktop-api-cli
Length of output: 329
🏁 Script executed:
cat -n .github/workflows/publish-release.ymlRepository: beeper/desktop-api-cli
Length of output: 1532
Remove unused OIDC permission.
id-token: write is unused by all workflow steps and unnecessarily broadens privileges. The workflow only requires contents: write for GitHub release operations via the gh CLI.
Suggested fix
permissions:
contents: write
- id-token: write📝 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.
Suggested change
| permissions: | |
| contents: write | |
| id-token: write | |
| permissions: | |
| contents: write |
🤖 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 @.github/workflows/publish-release.yml around lines 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.
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.