Conversation
…oser status components
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8ac38bb891
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const guard = await rendererExtensionRegistry.runChatBeforeSend({ | ||
| text: textToSend, | ||
| attachments: attachmentsToSend, | ||
| targetAgentId, | ||
| }); |
There was a problem hiding this comment.
Block repeat submits during beforeSend hook await
handleSend now awaits runChatBeforeSend before the composer is cleared or sending is set, so the send controls remain active for the full hook duration. When a beforeSend hook is slow, users can press Enter/click send multiple times and trigger duplicate onSend calls with the same payload; they can also type new text that is later erased by the delayed setInput('') from the first send path. Add an immediate local in-flight guard (or disable the composer) before awaiting extension hooks.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This PR extends the renderer extension system to support chat-specific hooks and UI, enabling extensions to gate outgoing messages (before-send hooks) and render additional status indicators in the chat composer.
Changes:
- Add
chatextension surface area (beforeSendhooks + composer status components) to extension types and public exports. - Implement
runChatBeforeSendandgetChatComposerStatusComponentsin the extension registry. - Wire the new registry APIs into
ChatInput(guard send + render status components + toast on failure).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/pages/Chat/ChatInput.tsx | Calls runChatBeforeSend before sending and renders extension-provided composer status components. |
| src/extensions/types.ts | Introduces chat extension types (ChatExtension, ChatBeforeSend*, ChatComposerStatusProps) and adds chat to RendererExtension. |
| src/extensions/registry.ts | Adds registry APIs for chat status components and the before-send hook runner. |
| src/extensions/index.ts | Re-exports the newly added chat extension types. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const handleSend = useCallback(async () => { | ||
| if (!canSend) return; | ||
| const readyAttachments = attachments.filter(a => a.status === 'ready'); | ||
| // Capture values before clearing — clear input immediately for snappy UX, | ||
| // but keep attachments available for the async send | ||
| const textToSend = input.trim(); | ||
| const attachmentsToSend = readyAttachments.length > 0 ? readyAttachments : undefined; | ||
|
|
||
| const guard = await rendererExtensionRegistry.runChatBeforeSend({ | ||
| text: textToSend, | ||
| attachments: attachmentsToSend, | ||
| targetAgentId, | ||
| }); | ||
| if (!guard.ok) { | ||
| if (guard.message) { | ||
| toast.error(guard.message); | ||
| } | ||
| return; |
There was a problem hiding this comment.
handleSend now awaits runChatBeforeSend before clearing the input/attachments. While that async hook is running, the UI still allows repeated Enter/click sends (since canSend stays true), which can trigger duplicate submissions. Consider disabling sending while hooks run (local isGuardRunning state/ref) or clearing optimistically and restoring on guard failure to keep the previous “snappy UX” behavior without allowing double-sends.
| const guard = await rendererExtensionRegistry.runChatBeforeSend({ | ||
| text: textToSend, | ||
| attachments: attachmentsToSend, | ||
| targetAgentId, | ||
| }); | ||
| if (!guard.ok) { | ||
| if (guard.message) { | ||
| toast.error(guard.message); | ||
| } |
There was a problem hiding this comment.
New behavior depends on rendererExtensionRegistry.runChatBeforeSend gating sends, but there’s no unit test coverage ensuring: (1) a failing hook prevents onSend from being called and (2) the error message is surfaced (toast). Adding tests alongside existing tests/unit/chat-input.test.tsx would help prevent regressions.
| async runChatBeforeSend(context: ChatBeforeSendContext): Promise<ChatBeforeSendResult> { | ||
| for (const ext of this.extensions) { | ||
| for (const hook of ext.chat?.beforeSend ?? []) { | ||
| try { | ||
| const result = await hook(context); | ||
| if (!result.ok) { | ||
| return result; | ||
| } | ||
| } catch (err) { | ||
| return { | ||
| ok: false, | ||
| message: err instanceof Error ? err.message : String(err), | ||
| }; | ||
| } |
There was a problem hiding this comment.
runChatBeforeSend swallows hook exceptions by converting them into { ok: false, message } without logging which extension/hook failed. This makes diagnosing broken extensions difficult and loses stack traces. Consider logging (at least console.warn/error) with ext.id and hook index/name before returning the failure result.
Summary
Related Issue(s)
Type of Change
Validation
Checklist