feat: surface chat hook outcomes in the chats API and UI#27430
feat: surface chat hook outcomes in the chats API and UI#27430ibetitsmike wants to merge 1 commit into
Conversation
Docs previewCheck off each page once it's been reviewed. If a page changes in a later push, its checkbox clears automatically so it gets a fresh look. Pages not yet wired into the docs navigation aren't listed here. |
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
953ead2 to
bbc1484
Compare
d7f1dba to
6743ac5
Compare
|
@codex review |
bbc1484 to
d0f7244
Compare
6743ac5 to
fdcf4b8
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
d0f7244 to
5250a86
Compare
fdcf4b8 to
44a8e4a
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 44a8e4a966
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| httpapi.Write(ctx, rw, http.StatusBadGateway, codersdk.Response{ | ||
| Message: "Chat lifecycle hook dispatch failed.", | ||
| Detail: fmt.Sprintf("Lifecycle hook dispatch %s failed (%s).", hookErr.DispatchID, hookErr.Class), | ||
| }) |
There was a problem hiding this comment.
Preserve hook dispatch failure kind
When a lifecycle hook dispatch fails on create/send/edit/tool-result submission, this helper now returns a plain codersdk.Response, so the 502 JSON body only has message and detail and drops the hook_dispatch_failed classification. In the Agents page, that immediate API error is cached as a generic chat error before the follow-up refetch, and cached non-usage errors take precedence over chat.last_error, so users keep seeing a generic request failure instead of the hook-specific failure. Return a chat error payload, or another response type, that includes kind: "hook_dispatch_failed" and preserve it in the client.
Useful? React with 👍 / 👎.
5250a86 to
e62369a
Compare
44a8e4a to
7b0db9d
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7b0db9d844
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // Return the full inserted batch because hook notices can have lower IDs. | ||
| // A client that caches only the user message could skip them when | ||
| // reconnecting with after_id. | ||
| for _, inserted := range sendResult.InsertedMessages { |
There was a problem hiding this comment.
Include promoted batches in queued send responses
When an errored chat already has a queued prompt, sendMessageE1 queues the new prompt and also promotes the previous queue head into history. Because this new messages population only runs in the non-queued branch, that user-visible promoted batch is omitted from the response; if the WebSocket delivery is missed, the client has no cache update for the inserted prompt until some later transcript refetch. Serialize InsertedMessages regardless of whether the current prompt was queued, while still keeping message reserved for non-queued sends.
Useful? React with 👍 / 👎.
| // Keep links in dimmed notices out of accessibility navigation. | ||
| inert={isAfterEditingMessage ? true : undefined} | ||
| > | ||
| <Alert className="my-1"> |
There was a problem hiding this comment.
Avoid live alerts for historical hook notices
When a chat with persisted lifecycle-hook notices is opened or refetched, each historical notice is rendered with the shared Alert component, which hard-codes role="alert". That makes old transcript content an assertive live region for screen-reader users instead of ordinary message history, so loading a conversation with several notices can announce them as urgent new alerts. Use a non-live role/container here, or pass an overridable role to Alert for timeline content.
Useful? React with 👍 / 👎.
Maps user_prompt_submit denials and dispatch failures to HTTP responses, returns hook-inserted message batches and ended state from send and edit endpoints, links files after prompt overrides, adds the hook_dispatch_failed chat error kind, and renders hook notices and ended sends in the chat UI.
e62369a to
d835b06
Compare
7b0db9d to
38d0e66
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 38d0e669fd
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const preservedMessages = page.messages.filter( | ||
| (message) => | ||
| message.id !== optimisticMessageId && message.id !== responseMessage.id, | ||
| message.id !== optimisticMessageId && !responseIDs.has(message.id), | ||
| ); |
There was a problem hiding this comment.
Remove stale hook notices when reconciling edits
When editing a user message that already had lifecycle-hook notices, the backend soft-deletes those same-turn prefix rows before inserting the replacement, but this reconciliation only filters the optimistic/replacement IDs and then adds the new response batch. If the edit WebSocket history_reset is missed or the client reconnects with after_id at the replacement, the first-page cache keeps the old system notice and displays it beside the new notice; the response needs to carry deleted IDs or the cache needs to remove prior same-turn hook prefixes as part of edit reconciliation.
Useful? React with 👍 / 👎.
| httpapi.Write(ctx, rw, http.StatusBadGateway, codersdk.Response{ | ||
| Message: "Chat lifecycle hook dispatch failed.", | ||
| Detail: fmt.Sprintf("Lifecycle hook dispatch %s failed (%s).", hookErr.DispatchID, hookErr.Class), | ||
| }) |
There was a problem hiding this comment.
Return the hook failure kind in API errors
For hook dispatch failures during create/send/edit/tool-result submission, this writes a plain codersdk.Response, which has only message/detail/validations; there is no kind field for clients to distinguish hook_dispatch_failed from an ordinary 502. In the create-chat path no chat row is persisted for a later last_error refresh, and in the send/edit UI catch path the synchronous API error is cached as kind generic, so users and API clients lose the specific lifecycle-hook failure classification.
Useful? React with 👍 / 👎.
Stack Context
Chat lifecycle hooks stack: #27427 (DB schema) -> #27401 (dispatch backend) -> #27428 (chatstate transitions) -> #27429 (chatd wiring) -> #27430 (this, top).
API and UI ship together because the send/edit response shapes change (full
messagesbatches), so the site does not compile against the regenerated types without its client updates.What
API (
coderd/exp_chats.go,codersdk/chats.go)user_prompt_submitdenials map to 403 with the hook reason; dispatch failures map to 502 with thehook_dispatch_failederror kind.messages(full inserted batch, hooks may prepend notices); successful edits always return the replacementmessage.UI (
site/src)messagesbatch on send and reconcile edit batches.Why
Top of the stack: surfaces everything below to clients. The frontend delta is small and inseparable from the response shape change.