"Remove command suggestions feature"

[humanagent]

Remove inline command suggestions and switch conversation command to /c <n> across CLI components, updating help and colors in src/cli.tsx

The CLI removes the command suggestions feature and updates command syntax and UI colors. Major changes focus on input handling, help text, and conversation switching, with supporting layout and styling updates.

• Drop inline command suggestions and related state in App and Input, and remove CommandSuggestions component in src/components/CommandSuggestions.tsx
• Change conversation switch command from /chat <n> to /c <n> and update parsing and errors in App in src/cli.tsx
• Update help text to document /b and /c <n> and remove /list from help in src/cli.tsx
• Apply grey tone constants DIM_GREY and LIGHT_GREY across UI components including src/components/CommandPalette.tsx, src/components/Layout.tsx, and src/components/Sidebar.tsx
• Reuse a global agent and improve initialization/cleanup in useXMTP in src/hooks/useXMTP.ts
• Summarize non-string message content in formatMessage in src/utils/formatters.ts

📍Where to Start

Start with command handling and input changes in the App component and help text updates in src/cli.tsx, then review removal of CommandSuggestions in src/components/CommandSuggestions.tsx.

---

📊 Macroscope summarized c4c543c. 10 files reviewed, 27 issues evaluated, 23 issues filtered, 0 comments posted

🗂️ Filtered Issues

src/cli.tsx — 0 comments posted, 3 evaluated, 3 filtered

• line 135: The error auto-clear timeout created in setErrorWithTimeout is not cleaned up on component unmount. This can leave a pending timer and attempt to set state on an unmounted component, violating single paired cleanup and potentially causing a memory leak or warning. Add a useEffect with a cleanup to clear errorTimeout on unmount, or store timers in a ref and clear them reliably in all exit paths. [ Out of scope ]
• line 260: In the /list command handling, the code reads store.showSidebar from a previously captured snapshot immediately after calling store.toggleSidebar(), which toggles the sidebar state asynchronously. Because store is a snapshot returned by useStore.getState() prior to the toggle, store.showSidebar will still reflect the old value, causing the status message to be inverted or stale. To ensure correctness, re-fetch the state after toggling (e.g., const newState = useStore.getState(); setStatusMessage(newState.showSidebar ? "Sidebar shown" : "Sidebar hidden");). [ Out of scope ]
• line 307: In the /refresh command, after await refreshConversations(), the status message uses conversations.length from the hook's state (useXMTP). This value is likely stale at that moment due to asynchronous state updates; the refreshed list is set later via setConversations. As a result, the message can display an incorrect conversation count. Use the freshly retrieved list length (e.g., return the new list length from refreshConversations or read from useStore.getState().conversations.length after refresh) to ensure accuracy. [ Out of scope ]

src/components/ChatView.tsx — 0 comments posted, 3 evaluated, 2 filtered

• line 41: Indentation for multi-line messages is computed using prefix.length, which measures code units rather than terminal display width. If msg.sender or msg.timestamp contain wide Unicode characters (e.g., emojis, CJK), combining marks, or zero-width characters, the computed indent will not align with the visible prefix, causing misaligned continuation lines. The component does not normalize by display width (e.g., wcwidth) or strip non-visible characters when computing indent. [ Out of scope ]
• line 80: List item keys use msg.id (<Box key={msg.id}>). If messages includes duplicate id values (not prevented by the types), React will reuse elements incorrectly, causing stale or mismatched rows to render and making updates unreliable. There is no guard or normalization to enforce uniqueness of msg.id. [ Out of scope ]

src/components/CommandPalette.tsx — 0 comments posted, 4 evaluated, 2 filtered

• line 38: selectedIndex is only reset when query changes (useEffect with [query]), but not when commands or the computed filteredCommands.length change. If the number of filtered commands shrinks or grows due to commands prop updates, selectedIndex may become out-of-bounds (e.g., remain at a higher index than the new list length). Pressing Enter will then execute onExecute(filteredCommands[selectedIndex]) with undefined, or highlight mismatches. Add an effect to clamp or reset selectedIndex whenever filteredCommands.length changes, or include commands in the dependency array and recompute safely. [ Low confidence ]
• line 58: Pressing Enter may trigger both the global useInput handler (key.return) and TextInput's onSubmit, causing onExecute and onClose to be called twice. Ink's useInput receives input for the app and ink-text-input also emits onSubmit on Enter; without coordination, double-application can happen. Fix by handling Enter in one place only (e.g., remove key.return handling when the TextInput is focused, or suppress onSubmit and rely solely on useInput). [ Low confidence ]

src/components/ConversationSelector.tsx — 0 comments posted, 2 evaluated, 2 filtered

• line 49: conversations is accessed directly (conversations.length) without any defensive check. If the parent passes undefined or null at runtime (TypeScript types are erased), conversations.length will throw a TypeError, crashing rendering. This happens at multiple points: the divider visibility check and the main conditional rendering of the list. Add a guard like Array.isArray(conversations) or a default value to ensure all inputs reach a defined terminal UI state. [ Out of scope ]
• line 77: Using key={conv.id} for list items without enforcing uniqueness can cause React/Ink reconciliation issues if id values are duplicated. This can lead to incorrect item updates, selections highlighting the wrong row, or stale UI. The code does not validate uniqueness and provides no fallback strategy. Ensure keys are unique (e.g., combine id with index or enforce unique IDs upstream) to preserve correct runtime behavior. [ Out of scope ]

src/components/Input.tsx — 0 comments posted, 2 evaluated, 2 filtered

• line 26: borderColor on Box is set to the hex string RED = "#fc4c34" when commandMode is true (line 26). Ink’s Box component expects a color name string supported by Chalk; hex strings may not be recognized depending on Ink/Chalk integration. At runtime this can result in an uncolored border or a default color, causing inconsistent UI rendering without an error. To ensure correct behavior, use a supported color name (e.g., "red") or verify that the specific Ink version supports hex via Chalk for borderColor. [ Low confidence ]
• line 43: Contract parity change: the hint text 💡 Press Enter to execute command is now rendered whenever commandMode is true (line 43). Previously, the hint was gated by !showSuggestions ({commandMode && !showSuggestions && (...)}) and command suggestions UI was present. With suggestions removed, this change may cause the hint to appear in contexts where it was previously suppressed, altering the external UI contract and potentially causing overlapping guidance if any downstream UI reintroduces suggestions or relies on the old gating. Verify that this unconditional hint under commandMode preserves intended UX and doesn’t conflict with any remaining or future suggestions UI. [ Low confidence ]

src/components/Sidebar.tsx — 0 comments posted, 1 evaluated, 1 filtered

• line 75: Conversation name truncation uses conv.name.slice(0, width - 12). When width is small (e.g., less than 12), the end index becomes negative, causing string slicing from 0 to len + (width - 12). This can result in unexpected truncation, including rendering an empty name even when there is some available space, leading to confusing UI. There is no clamp to ensure the slice end is non-negative or to fall back to a minimal visible label. [ Out of scope ]

src/hooks/useXMTP.ts — 0 comments posted, 9 evaluated, 8 filtered

• line 21: globalAgent is a module-level singleton with no teardown or scoping. It persists across component unmounts and can outlive the owning instance, potentially causing memory/resource retention and cross-instance leakage (e.g., subscriptions held by globalAgent.client). There’s also no invalidation when inputs like env change. Consider scoping by env, adding an explicit teardown path that clears globalAgent when the last consumer unmounts, or using a provider-level singleton with lifecycle management. [ Low confidence ]
• line 121: Multiple component instances using the hook will share globalAgent. The second instance’s early-return path sets agent, address, inboxId, and url, and flips isLoading to false, but does not perform conversation sync, initial conversation resolution based on agentIdentifiers, or schedule the refresh interval. This leads to asymmetry: the first instance has conversations and background refresh; subsequent instances appear initialized but miss these effects. Fix by factoring initialization steps so they run for instances adopting globalAgent or by scoping agent state per instance. [ Low confidence ]
• line 121: Early-return path sets isLoading to false when adopting globalAgent but does not perform conversation sync, agentIdentifiers resolution, or start the message stream/refresh interval. If agentIdentifiers changes after the first initialization, no attempt is made to find/create the new conversation because the effect returns early. This produces a visible mismatch: isLoading indicates ready, but conversations may not reflect the new identifiers. Move the conversation resolution/startup steps to a separate effect keyed on agentIdentifiers and agent, or perform them even when reusing globalAgent. [ Low confidence ]
• line 122: Early-return path reuses globalAgent regardless of env changes. If env prop changes, the hook will adopt the previously initialized globalAgent (bound to the old env) and set isLoading to false without reinitializing for the new environment. This produces mismatched environment behavior and stale configuration. Fix by scoping globalAgent per env or invalidating it when env changes. [ Low confidence ]
• line 169: finalInboxState[0] is accessed without verifying the array is non-empty. If Client.inboxStateFromInboxIds([newAgent.client.inboxId], env) returns an empty array (e.g., transient backend issue), finalInboxState[0] will be undefined and finalInboxState[0].installations.length will throw at runtime. Add an explicit check that finalInboxState.length > 0 and handle the error or fallback gracefully. [ Low confidence ]
• line 219: isInitializingRef can block initialization permanently after a failure is recovered via adopting globalAgent in a subsequent render. In the catch block, isInitializingRef.current is set to false, but if an error occurs after globalAgent has been set, future runs will early-return due to globalAgent but keep isInitializingRef.current potentially true (on success path), which suppresses any re-init logic when inputs change. The primary fix is to reset isInitializingRef.current to false in the success path too (e.g., at the end of initAgent or in a finally). [ Low confidence ]
• line 231: Stream cleanup sets isStreamingRef.current = false and streamRef.current = null, but does not actually cancel or break the underlying AsyncIterable<DecodedMessage>. If the stream source continues to produce values, it may leak resources or continue background work. Ensure the streaming API supports cancellation and invoke it (e.g., abortController.abort() or a provided stop()), or wrap the iterator loop with a cancellation check that terminates the generator. [ Low confidence ]
• line 237: Removing onError and onStatusChange from the effect dependency array means the effect captures stale callback references. If these callbacks change after the first render, subsequent initialization or status updates will continue invoking the old callbacks, violating contract parity for callback updates. Restore onError and onStatusChange to the dependency array or memoize them as stable callbacks. [ Low confidence ]

src/utils/formatters.ts — 0 comments posted, 3 evaluated, 3 filtered

• line 9: Potential runtime TypeError when deriving timestamp: message.sentAt.toLocaleTimeString(...) assumes message.sentAt is a valid Date. If message.sentAt is undefined, null, or a non-Date (e.g., string), calling toLocaleTimeString will throw. There is no guard to validate message.sentAt. [ Out of scope ]
• line 15: Incorrect sender formatting for non-self messages: when isFromSelf is false, the code uses selfAddress (the current user's address) to label the sender instead of the actual message sender. This yields a misleading sender value for all incoming messages whenever selfAddress is provided. The logic on lines 15–19 results in sender being set to a truncated selfAddress for non-self messages, rather than using a field from message (e.g., message.senderInboxId or a sender address). This is a runtime behavior bug producing incorrect UI output: the counterparty’s messages appear labeled as the current user. [ Out of scope ]
• line 19: Potential runtime TypeError when truncating senderInboxId: message.senderInboxId.slice(0, 8) assumes message.senderInboxId is a string. If message.senderInboxId is undefined, null, or otherwise non-string, calling slice will throw. There is no guard before using it in the non-self branch. [ Out of scope ]