-
Notifications
You must be signed in to change notification settings - Fork 0
v1 #1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
v1 #1
Conversation
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
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.
Rework XMTP CLI to v2 with centralized
useXMTPstate, command palette, keyboard navigation, and updated entrypoint insrc/cli.tsxThis PR replaces the old in-component XMTP logic with a hook-driven architecture and a new layout, adds a command palette and keyboard navigation, and updates the CLI entrypoint and help to v2 usage and commands.
useXMTPfor agent, conversations, streaming, and messaging in cli.tsxuseStoreand add keyboard bindings throughuseKeyboardwith UI composed in Layout.tsx📍Where to Start
Start with the new CLI flow in
Appwithin src/cli.tsx, then reviewuseXMTPin src/hooks/useXMTP.ts to understand agent initialization, conversation loading, and streaming, followed by the composed UI in src/components/Layout.tsx.📊 Macroscope summarized 4b7fd02. 16 files reviewed, 84 issues evaluated, 76 issues filtered, 2 comments posted
🗂️ Filtered Issues
src/cli-old.tsx — 0 comments posted, 28 evaluated, 25 filtered
/c <number>while the app handles/chat <number>. This mismatch causes user-entered/cto be unrecognized and/chatto be undocumented, creating uncertainty about the correct command and a visible runtime UX failure. [ Low confidence ]XMTP_CLIENT_WALLET_KEYandXMTP_CLIENT_DB_ENCRYPTION_KEYare required, but the implementation falls back to generating a private key and random DB encryption key when they are missing. This contradiction creates uncertainty about required configuration and expected behavior. [ Low confidence ]isGroupusesconversation.constructor.name === "Group"for type/instance detection. Relying on constructor names is brittle and can fail under minification, differing realms, or subclassing, leading to misclassification. If misclassified, the code may callgetEthereumAddresson a group or skip DM-specific rendering incorrectly, causing wrong UI and potential errors. [ Low confidence ]isGrouptype guard relies onconversation.constructor.name === "Group", which is a fragile runtime check. Constructor names can be altered by minification, bundlers, cross-realm objects, or SDK changes. If it misclassifies aGroupas not a group, downstream code assumes a DM and accesses DM-only fields, leading to runtime errors. This misclassification is reachable and causes incorrect behavior inHeaderwhen it casts toDmand usespeerInboxId. [ Low confidence ]isDmrelies onconversation.constructor.name === "Dm"to discriminate theConversationsubtype. Usingconstructor.nameis a fragile runtime check: it can fail under minification/obfuscation, cross-realm objects, subclassing, or SDK implementation changes where the class name differs (e.g., renamed, mangled). A false negative will cause valid DM conversations to be treated as non-DM, altering behavior (e.g., skipping DM-specific logic when scanning existing conversations) without an explicit error. Prefer a stable discriminant provided by the SDK (e.g., akindproperty or an official type guard) orinstanceof Dmif the Dm constructor is imported from the same realm. [ Low confidence ]isEthAddressvalidates only by checkingstartsWith("0x")andlength === 42. It does not verify that the 40 characters after0xare valid hexadecimal digits. This will treat any 42-character string starting with0x(including invalid characters like 'Z' or Unicode letters) as an Ethereum address. In the example usage, this can causenewGroupWithIdentifiersornewDmWithIdentifierto be called with invalid Ethereum identifiers, likely resulting in downstream API errors or hard-to-diagnose failures. Additionally, this function rejects uppercase0Xprefixes, which are sometimes encountered, causing valid addresses to be misclassified and routed down the non-Ethereum path. [ Low confidence ]erroris typed asunknownand is cast toError(const err = error as Error;) and thenerr.messageis used to build the error string. Iferroris not actually anError(e.g., a string, number, plain object without amessagefield, ornull/undefined),err.messagewill beundefined, producing an unhelpful message like"<context>: undefined". This silently drops the original error value and yields misleading output rather than a graceful fallback. [ Low confidence ]Auto-clear error after specified time (default 5 seconds)but the implementation does not provide a default.clearAfteris optional and only if it is truthy does the timeout run. WhenclearAfterisundefined, the error will not auto-clear, contradicting the stated default behavior. [ Already posted ]if (clearAfter)treats0as falsy and will not schedule the timeout whenclearAfteris0. If0is provided to mean immediate clearing (a valid number), this leads to unexpected behavior. The check should explicitly test forclearAfter !== undefinedor>= 0depending on intended contract. [ Low confidence ]handleErrorwill schedule overlapping timers, allowing an earlier timer to clear a later error, causing race conditions and inconsistent UI/CLI state. Additionally, if the owning component or process is torn down before the timer fires,setError("")may attempt to update state after unmount, causing warnings or errors in React-like environments. A single paired cleanup is missing: store the timeout ID, cancel prior pending clears on new errors, and clear on unmount. [ Low confidence ]Header, the DM branch accesses(conversation as Dm).peerInboxId.slice(0, 16)based solely onisGroup(conversation)being false. IfisGroupmisclassifies aGroup(or any non-DM conversation) as not a group,conversationis not aDm, makingpeerInboxIdundefined and causing a runtime error on.slice. This path is reachable due to the fragileisGroupguard and lacks a safe narrowing check forDm. [ Low confidence ]getEthereumAddressassumesconversation.members()returns an iterable array. Ifmembers()returnsnull,undefined, or a non-iterable due to transient errors or SDK behavior, thefor...ofloop will throw, causing the effect to reject and the UI to fail without recovery. There is no defensive check for empty/missing members. [ Already posted ]getEthereumAddressmay return the current user’s own Ethereum address instead of the peer’s address in a DM. It iterates allmembers()and returns the first Ethereum identifier found, without excluding self. For DMs, you typically want the other participant’s identifier; returning self results in incorrect labeling of the DM in the UI. [ Already posted ]loadAddressesdoes not handle errors fromgetEthereumAddressorconversation.members(). If either rejects, the promise returned byloadAddresseswill reject, which in React can surface as an unhandled error and the component will remain stuck withloadingset totrue. There is notry/catchnor a fallback to setloadingtofalseon failure, so the UI can show "Loading conversations..." indefinitely and the error is not contained. [ Already posted ]loadAddressesdoes not guard against race conditions whenconversationschanges while a previous load is in flight. Multiple invocations can overlap; a slower, older invocation may complete last and overwriteaddressMapandloadingwith stale data. There is no cancellation or staleness check, leading to potential display of wrong addresses and incorrectloadingstate transitions. [ Low confidence ]mapcallback, whenpeerShortis falsy (no address found), the function returnsundefinedand renders nothing for that conversation, yielding incomplete UI with no fallback label (e.g., a placeholder orconv.name). This violates the visible contract of listing conversations and leaves users with no way to select or view such DMs. [ Low confidence ]addressMap[conv.id]is undefined, returning nothing from themap()callback. This results in inconsistent or missing list items, while selection uses indices from the fullconversationsarray. Users may see fewer items than actually exist, and indices will not match the visible list. [ Low confidence ]setErrorWithTimeoutis not cleaned up on component unmount, potentially causing setState calls after unmount. There is nouseEffectteardown to clearerrorTimeoutwhen the component exits or when a new timeout is set in quick succession beyond the explicit local clear. This can lead to memory leak warnings and violates single paired cleanup after introducing a timer effect. [ Low confidence ]walletKeyanddbEncryptionKeyif either is missing, discarding any provided value. The conditionif (!walletKey || !dbEncryptionKey)regenerates both values, meaning a caller who sets onlyXMTP_CLIENT_DB_ENCRYPTION_KEYbut notXMTP_CLIENT_WALLET_KEYwill have their encryption key silently replaced. Each missing value should be generated independently to preserve any provided key. [ Low confidence ]finalInboxState[0]is accessed without verifying the array has at least one element. IfClient.inboxStateFromInboxIdsreturns an empty array (e.g., transient network error or unknown inbox ID),finalInboxState[0]will beundefinedand accessing.installations.lengthwill throw. [ Low confidence ]formatMessagelabels messages not sent by the current user with a shortened version of the current user's own address, rather than the actual sender. This misidentifies message senders in the UI. It should resolve and display the peer's identifier/address for incoming messages. [ Already posted ]client.conversations.streamAllMessages()is started, but there is no paired cleanup call or cancellation token, and nouseEffectteardown. This violates at-most-once and single paired cleanup guarantees after introducing an external resource. [ Low confidence ]envvalue propagated asXmtpEnvwithout runtime validation.parseArgs()assignsconst env = (process.env.XMTP_ENV as XmtpEnv) || "production";which preserves any non-empty, possibly invalid string asenv. This value is returned typed asXmtpEnvand passed to<App env={env} />, likely intoAgent.create. IfXMTP_ENVis set to an unsupported value, downstream code may fail at runtime. A defensive validation (e.g., check against allowed set) is needed. [ Low confidence ]privateKeyToAddresswith potentially empty or invalid private key inparseArgs()dev auto-detect path. When no--agentis provided andenv === "dev", the code readsprocess.env.XMTP_WALLET_KEY || ""intoautoAgentAddressKeyand immediately callsprivateKeyToAddress(autoAgentAddressKey as \0x${string}`). If the env var is not set (empty string) or is not a valid 0x-prefixed 32-byte key,viem/accounts` will throw at runtime. There is no try/catch around this call, so the CLI will crash instead of handling the error or falling back. [ Already posted ]privateKeyToAddresswith potentially invalid key inmain()when no agents are specified. The code readswalletKey = process.env.XMTP_WALLET_KEY || "", checks only that it is truthy, and then callsprivateKeyToAddress(walletKey as \0x${string}`). IfXMTP_WALLET_KEYis set but malformed (e.g., missing0xprefix, wrong length),privateKeyToAddress` will throw at runtime and crash the CLI. [ Already posted ]src/cli.tsx — 0 comments posted, 10 evaluated, 10 filtered
XMTP_CLIENT_WALLET_KEYandXMTP_CLIENT_DB_ENCRYPTION_KEY, butparseArgs()usesprocess.env.XMTP_WALLET_KEYto auto-detect the agent address. This contradiction means users following the help will setXMTP_CLIENT_WALLET_KEY, and auto-detection will silently fail because the code readsXMTP_WALLET_KEY. This creates uncertainty about which variable is correct and yields incorrect runtime behavior (no auto-detected agent). [ Low confidence ]useKeyboard, the up/down arrow handlers callonSwitchConversation?.("prev"|"next")wheninMainMenuis false, but theAppcomponent does not supplyonSwitchConversation. As a result, arrow navigation within a conversation produces no state change and no visible effect. [ Already posted ]key.tabto complete a suggestion. Ink'suseInputkey object does not document atabproperty in all versions; relying onkey.tabmay result in non-functional tab completion. If Ink does not setkey.tab, this branch will never execute. Consider handlinginput === "\t"or verifying Ink's API for tab support. [ Low confidence ]useInputhandler AND also trigger theonInputSubmit(text input submit) path inhandleInputSubmit, leading to double-application or contradictory outcomes. For example, selecting/toggle-sidebarvia suggestions runscommand.action(), buthandleInputSubmitdoesn't recognize/toggle-sidebar(only/back,/list,/chat,/new,/refresh,/exit), which then emits an "Unknown command" error. This violates at-most-once execution and contract parity for commands in the suggestions path. [ Low confidence ]setErrorWithTimeoutis never cleaned up on component unmount, which can cause setState calls (setError,setErrorTimeout) after the component has unmounted. This is a runtime bug leading to memory leaks and React warnings. Add a cleanup effect to clear the timeout on unmount. Offending logic is insetErrorWithTimeoutwhereconst timeout = setTimeout(...)is stored inerrorTimeoutstate without a corresponding teardown. [ Low confidence ]refreshConversations()completes in the/refreshcommand, the code reportsFound ${conversations.length} conversationsusing the hook'sconversationsstate captured before the refresh. SincerefreshConversationsupdatesconversationsasynchronously, the count can be stale or incorrect. The message should derive from the returned list length or from the updated state (e.g., read after the state update) to maintain contract parity. [ Already posted ]envis derived as(process.env.XMTP_ENV as XmtpEnv) || "production"without validating the actual value against the allowedXmtpEnvdomain. IfXMTP_ENVis set to an invalid string (e.g.,"prod"), it will be treated as a truthy value and passed through as theenvprop to<App>, potentially causing downstream misconfiguration or errors. A type assertion does not enforce runtime validity; a whitelist check is needed. [ Out of scope ]env(devvsproduction). Previously it was gated onenv === "dev". This can cause unintended agent selection in production ifXMTP_WALLET_KEYis set, altering externally visible behavior without a clear opt-in. Combined with the env var name mismatch in help, users may not realize production auto-detection is active or may fail to activate it if they set the documented variable. [ Low confidence ]parseArgs()callsprivateKeyToAddress(walletKey as0x${string})withwalletKeyderived fromprocess.env.XMTP_WALLET_KEYwithout validating the format. IfXMTP_WALLET_KEYis set but malformed (e.g., missing0xprefix or wrong length),privateKeyToAddresswill throw, causing the CLI to crash during startup. Previously this auto-detection was restricted toenv === "dev"; now it runs unconditionally, making the crash reachable in production environments whenagentsis empty. [ Already posted ]parseArgsusesXMTP_WALLET_KEYto auto-detect and populateagents, which are passed asagentIdentifiers, while the XMTP client initialization (inuseXMTP) expectsXMTP_CLIENT_WALLET_KEYand will generate a random private key if it’s not set. If onlyXMTP_WALLET_KEYis provided, the app may start with a randomly generated agent that doesn’t match theagentIdentifiers, creating conversations from a different identity than intended. This is a contract inconsistency between CLI argument parsing and client initialization. Align the env var names or cascade the same key to both places so the agent identity matches the provided identifiers. [ Low confidence ]src/components/ChatView.tsx — 0 comments posted, 2 evaluated, 2 filtered
ChatView,heightis used to computevisibleMessages = messages.slice(-height)and passed tominHeight={height}. Ifheightis 0 or negative,slice(-height)will behave unintuitively (e.g.,height=0returns all messages;height=-5returns from index 5), andminHeightmay be invalid for the layout, causing incorrect rendering. There is no validation ensuringheightis a positive integer. [ Already posted ]conversationis null,ChatViewrendersConversationSelectorwithconversations={allConversations}defaulting to[]andselectedIndex={selectedConversationIndex}defaulting to0. If theConversationSelectorimplementation assumes the selected index points to a valid element, passing0with an empty array can cause an out-of-bounds access and a runtime error. There is no guard adjusting the index when the list is empty or ensuring it is within bounds. [ Already posted ]src/components/CommandPalette.tsx — 0 comments posted, 6 evaluated, 5 filtered
Command.actionfunction defined by theCommandinterface. Instead, it forwards the selectedCommandtoonExecute. This creates a contract mismatch: callers supplyingcommandswithactionset may reasonably expect the palette to invokecommand.action(). Unless explicitly documented, this can lead to commands never executing their intended actions. [ Low confidence ]selectedIndexis only reset onquerychange (useEffect([query])). If thecommandsprop changes (e.g., new data is loaded, items removed),selectedIndexmay point to a now-invalid position. There is no guard to clamp or resetselectedIndexoncommandschange, enabling out-of-bounds selection and undefined execution targets. [ Already posted ]onExecuteis invoked beforeonClosewithout error containment. IfonExecutethrows (e.g., user command fails),onCloseis not called, leaving the palette open and violating the lifecycle expectation documented in the UI footer. Wrap execution to ensureonCloseis always invoked exactly once. [ Low confidence ]selectedIndexcan become out-of-bounds relative tofilteredCommands, leading toonExecutereceivingundefinedat runtime. Two reachable cases: (1) whenfilteredCommands.length === 0, the up-arrow handler setsselectedIndexto-1(filteredCommands.length - 1), and ifcommandslater becomes non-empty without aquerychange, pressing Enter passesfilteredCommands[-1](undefined) toonExecute; (2) whencommandschanges (shrinks) without aquerychange,selectedIndexmay remain larger than the newfilteredCommands.length - 1, causingfilteredCommands[selectedIndex]to beundefined. The only guard isfilteredCommands.length > 0, which does not prevent out-of-bounds or negative index access. [ Already posted ]filteredCommands.slice(0, 10)), but selection navigation wraps across the fullfilteredCommands.length. This allowsselectedIndexto point at items beyond the first 10, which are not rendered/highlighted. Pressing Enter will execute a non-visible item, violating the visible selection invariant and causing confusing behavior. [ Already posted ]src/components/CommandSuggestions.tsx — 0 comments posted, 3 evaluated, 3 filtered
CommandSuggestionshighlights based onselectedIndexcompared against indices ofsuggestions.slice(0, 6). IfselectedIndexis negative,NaN, or refers to an item outside the sliced range (>= 6 or >=suggestions.length), no item is highlighted, causing inconsistent selection UI. There’s no clamping or normalization to ensureselectedIndexmaps within the rendered subset, so selection can silently disappear when the list is long. [ Already posted ]suggestion.idis used as the Reactkey(<Box key={suggestion.id}>). Ifidvalues are not unique across the rendered slice, React reconciliation can behave incorrectly (items reordering, incorrect reuse), causing subtle UI bugs. There’s no explicit guarantee or validation of uniqueness at runtime. [ Low confidence ]Textreceivescolor={RED}whereREDis a hex string ("#fc4c34"). Ink’sTextcolorprop is typically limited to named ANSI colors; passing a hex string may be ignored or fall back, resulting in missing highlight color at runtime. This causes the selected item arrow and name to potentially render without the intended color. [ Low confidence ]src/components/ConversationSelector.tsx — 0 comments posted, 5 evaluated, 5 filtered
ConversationSelectorPropsdeclares a requiredonSelectcallback prop, but theConversationSelectorcomponent never destructures or invokesonSelect. This breaks contract parity: callers may passonSelectexpecting selection behavior, yet the component provides no path to use it. It also creates ambiguity about how selection is handled and can lead to inconsistent UX where "Enter" does nothing within this component. [ Low confidence ]colorprop (e.g.,color={RED}whereREDis"#fc4c34") andbackgroundColorwith"#2a2a2a". Depending on Ink version,Text/Boxmay only accept named colors, not hex strings, leading to missing colors or inconsistent rendering. Consider using supported named colors or props that explicitly accept hex (or verify Ink supports hex values). [ Low confidence ]selectedIndexis less than -1 or greater than the last conversation index, no item is highlighted and the UI becomes ambiguous. There is no fallback state or error indicator whenselectedIndexis out-of-range, making it harder to recover from navigation state bugs. [ Code style ]conv.idwithout validating uniqueness. Ifconversationscontains duplicateids, React reconciliation can produce stale or incorrect row updates, violating the rule-validated sequence constraint (uniqueness) and causing rendering artifacts. [ Code style ]formatTime(conv.lastMessageAt)is called whenconv.lastMessageAtis truthy but without validating it is aDateinstance. IflastMessageAtis a non-Date value (e.g., a string due to deserialization),formatTimewill attemptgetTime()and may yieldNaN-based calculations or throw when callingtoLocaleDateString, causing runtime errors or malformed output. Guard withinstanceof Date(and!isNaN(date.getTime())) before invokingformatTime. [ Low confidence ]src/components/Sidebar.tsx — 0 comments posted, 5 evaluated, 4 filtered
conversationsis always a non-null array and immediately readsconversations.length. If a caller passesnullorundefined(e.g., due to an async fetch not yet resolved or an integration without TS checks), this will throw a runtime error. Consider a defensive default (e.g.,conversations ?? []) or a prop-level guard. [ Low confidence ]conv.idas the Reactkeywithout ensuring uniqueness can cause incorrect list diffing/rendering when two conversations share the sameid. React relies on unique keys to preserve element identity; duplicates can lead to items being merged, reordered incorrectly, or failing to update. Consider enforcing unique IDs or fallback to a composite key (e.g., index + id) with a clear uniqueness guarantee. [ Low confidence ]RED = "#fc4c34",DIM_RED = "#cc3f2a") via theText colorprop (e.g.,color={RED}). Depending on Ink version,Textmay only accept named colors rather than hex strings. If hex is unsupported, colors may be ignored or could throw at runtime. Use supported color names or Ink’s mechanisms (e.g., Chalk hex via custom rendering) to ensure consistent behavior. [ Low confidence ]width. Forconv.name.length > 20, the component renders a fixed"${first6}...${last6}"(15 characters) regardless of the container width. Ifwidthis less than the space required by the line elements, this can overflow/clobber layout. Conversely, ifwidthis much larger, the name is unnecessarily truncated. The truncation strategy should considerwidthto avoid overflow and use available space. [ Low confidence ]src/hooks/useXMTP.ts — 0 comments posted, 5 evaluated, 5 filtered
getEthereumAddress, the code assumesmember.accountIdentifiersis a defined array and calls.find(...)on it. IfaccountIdentifiersisundefinedornullfor any member, this will throw a runtime error. There is no guard or fallback to handle missing identifiers. [ Already posted ]toConversationInfo, theelsebranch castsconversationtoDm(const dm = conversation as Dm;) and then usesdm.peerInboxId.slice(0, 16). IfisGroupmisclassifies aGroupor ifconversationis not aDmfor any reason,peerInboxIdmay beundefined, causing a runtimeTypeErrorwhen calling.sliceonundefined. This path lacks any guard to verifypeerInboxIdis present. [ Low confidence ]streamRef.currentis assigned and the for-await loop keeps running and callingsetMessageseven after unmount, which can cause memory leaks and setState-after-unmount warnings. The effect cleanup only clears the periodic refresh interval, not the message stream. [ Low confidence ]useXMTPdoes not handle conversation switching.startMessageStreamstarts a single global stream and filters messages by theconv.idpassed on first start. Later calls (e.g., fromsetCurrentConversationById) won't start a new stream becauseisStreamingRef.currentis already true, and the filtering continues to target the original conversation only. This breaks live updates for subsequent selected conversations. [ Already posted ]useXMTP.findOrCreateConversation. After creating a conversation, the code usessetConversations([...conversations, convInfo]). Becauseconversationsis captured from the hook's closure, concurrent updates or rapid successive additions can be lost (classic React state staleness). Use the functional updatersetConversations(prev => [...prev, convInfo])to avoid dropping entries. [ Low confidence ]src/store/state.ts — 0 comments posted, 6 evaluated, 6 filtered
env: (process.env.XMTP_ENV as XmtpEnv) || "production"silently trusts and coerces the environment string. IfXMTP_ENVis set to an invalid value (not a member ofXmtpEnv), the cast suppresses type safety and stores an invalidenvat runtime. Additionally, an empty string inXMTP_ENVwill be treated as falsy and coerced to"production", potentially masking configuration errors. Downstream consumers relying on validXmtpEnvsemantics can misbehave or select the wrong environment. [ Low confidence ]process.env.XMTP_ENVis read directly in a frontend store initializer (useStore), which can crash at runtime in browser environments whereprocessis undefined. Many bundlers exposeimport.meta.envor inlineprocess.env.*during build; if that substitution is not present, accessingprocess.envthrows aReferenceErrorat store creation time. This occurs atenv: (process.env.XMTP_ENV as XmtpEnv) || "production"and would take down the app during initialization. [ Low confidence ]setAgentsetsconnectionStatusto"connected"regardless of the validity of the providedagent(including whenagentisnull). This can yield an inconsistent state:agentisnull,addressandinboxIdare empty strings, yetconnectionStatussays connected. That violates the implied invariant that a connected state requires a non-null agent. [ Low confidence ]setCurrentConversation(null)setsselectedConversationIndexto0, which contradicts the store’s design where-1represents “New Chat / no selection”. This can select the first conversation even whencurrentConversationis explicitly cleared. In the case whereconversationsis empty, it also sets an out-of-range index0. Example usage shows the need to immediately callselectConversation(-1)to counteract this. The setter should setselectedConversationIndexto-1whenconversationisnull. [ Already posted ]setCurrentConversation(conversation)setsselectedConversationIndexto the result offindIndex, which is-1if the provided conversation isn’t present inconversations. This yields a mismatch:currentConversationis set to a non-null value, butselectedConversationIndexbecomes-1, which elsewhere represents “New Chat”. The UI and navigation logic can become inconsistent. The setter should handle-1(not found) explicitly instead of silently assigning-1. [ Already posted ]selectConversation(index)updates onlyselectedConversationIndexand does not updatecurrentConversation. This creates possible divergence between the selected index and thecurrentConversationobject used elsewhere. For example, navigation and selection will change the index, butcurrentConversationcan remain pointing to a stale/previous conversation ornull. This breaks interface parity between actions that change conversation selection vs. conversation object and leads to inconsistent UI/state. [ Already posted ]src/utils/formatters.ts — 0 comments posted, 8 evaluated, 8 filtered
formatMessagecallsmessage.sentAt.toLocaleTimeString(...)without validating thatmessage.sentAtis a validDate. IfsentAtis missing, null, not aDateinstance, or an invalidDate, this can throw a runtimeRangeErroror cause an unexpected failure. There is no guard or fallback. [ Low confidence ]formatMessagedisplays the wrongsenderfor non-self messages: it usesselfAddress(the viewer's address) instead of any field from themessageitself. As written, every incoming message from others will be labeled with the viewer's truncated address rather than the actual sender, which is a correctness bug and misleading to users. The fallback tomessage.senderInboxIdonly occurs whenselfAddressis falsy, not when it is present but unrelated to the sender. [ Already posted ]formatMessageusesmessage.senderInboxId.slice(...)without guarding formessage.senderInboxIdbeing missing or non-string. If it isundefinedor null, this will throw a runtime error. The path is reachable whenselfAddressis falsy for a non-self message. [ Already posted ]formatMessageattempts to stringify non-stringmessage.contentusingJSON.stringifyinside atryblock (pretty-print), then falls back inside thecatchto a secondJSON.stringifywithout a surroundingtry/catch. If the object contains circular references or unsupported types (e.g.,BigIntvalues),JSON.stringifywill throw again and bubble out, causing a runtime error. There is no final safe fallback. [ Low confidence ]formatMessagecan returncontentasundefinedwhenmessage.contentisundefined:JSON.stringify(undefined)returnsundefined. This violates the declaredFormattedMessagecontract (content: string) and can cause downstream consumers expecting a string to fail at runtime (e.g., calling string methods). There is no check or default for this case. [ Low confidence ]formatAddresscan compute a negativeendwhenlength < 3(end = length - start - 3). Passing a negative value intoaddress.slice(-end)alters semantics (becomes a positive index), which can produce output longer than the requestedlengthor otherwise malformed. No guard ensureslengthis large enough, so smalllengthvalues lead to unexpected results. [ Already posted ]formatTimetreats future dates as "now": ifdateis in the future,diffMsis negative anddiffMins < 1is true, so it returns "now". This is misleading for future timestamps and breaks time semantics. No guard differentiates past vs future. [ Already posted ]formatTimelacks validation fordatebeing a validDate. Ifdate.getTime()returnsNaN(invalid date), all comparisons will be false, leading to the finaltoLocaleDateStringcall, which can throw aRangeErrorfor an invalid time value. There is no guard or fallback for invalid dates. [ Already posted ]src/utils/helpers.ts — 0 comments posted, 3 evaluated, 3 filtered
isGrouprelies onconversation.constructor.name === "Group", which is brittle at runtime. If the SDK classes are minified, proxied, or instantiated across realms,constructor.namemay not equal "Group" even for aGroup. This can causetoConversationInfoto misclassify aGroupas aDm, leading to unsafe casts and downstream property access errors. [ Low confidence ]isDmusesconversation.constructor.name === "Dm", which is brittle for the same reasons asisGroup. If used elsewhere (it is imported), it can misclassify and lead to unsafe casts and property access. Even if not exercised in the shown code, it represents a reachable runtime risk when called. [ Low confidence ]handleErrorcastsunknowntoErrorand useserr.message. Iferroris a non-Error value (e.g., a string or a plain object),err.messagewill beundefinedand the function returns a message like"context: undefined", losing the original error content. While it avoids crashes, it silently drops data and produces unhelpful output. [ Code style ]