feat(site): surface chat lifecycle hook outcomes in the chats UI by ibetitsmike · Pull Request #27430 · coder/coder · GitHub
Skip to content

feat(site): surface chat lifecycle hook outcomes in the chats UI - #27430

Merged
ibetitsmike merged 35 commits into
mainfrom
mike/chat-hooks/api
Jul 29, 2026
Merged

feat(site): surface chat lifecycle hook outcomes in the chats UI#27430
ibetitsmike merged 35 commits into
mainfrom
mike/chat-hooks/api

Conversation

@ibetitsmike

@ibetitsmike ibetitsmike commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator

Surfaces chat lifecycle hook outcomes in the chats UI. Final PR of the lifecycle hooks stack (#27401, #27428, #27429), all now merged.

  • Show hook notices attached to their user message as timeline notes (role="note" so historical notices stay out of the screen reader's assertive live region), and show an info tooltip for notices on queued messages.

  • Cache the full inserted message batch from send and edit responses so hook-inserted messages survive stream reconnects and queue promotion.

  • Reconcile the promoted queue head after sending to an errored chat so a missed or delayed queue update neither duplicates nor hides messages, and clear the stale error status so the Thinking indicator appears before the websocket status event.

  • Ignore an authoritative queue snapshot that still contains a just-promoted message: queued messages are delete-only, so such a snapshot predates the promotion and would both re-show the promoted message and drop messages queued since. Fresh snapshots apply in full and clear the suppression.

  • Cache the store's reconciled queue on queue_update instead of the raw event, so a stale update cannot re-show a promoted message after REST re-hydration.

  • Refresh chat details when a send or edit fails, because a failed hook dispatch can move the chat to the error state.

  • Surface tool result error text in the tool rows: the execute failure tooltip shows the actual error instead of a hardcoded "Command failed", and a failed write_file renders an error label with the result error text instead of "Wrote " with an args-derived diff of content that was never written. This makes hook tool denials legible in the timeline, and benefits every failed execute or write.

  • Label a tool call blocked by pre_tool_use as failed instead of Ran <command>, matching what the write and edit tools already do. The wording derives from the tool-result error flag, so a command that ran and exited non-zero is unaffected.

  • Render a hook notice below the message it annotates rather than above it, which reads correctly for a "your prompt was rewritten" card.

  • Give both hook outcomes their own treatment on the create path, where they previously fell through to the generic error alert and an expected policy decision appeared with a stack trace, response data, and a workspaces action. Classification keys on the structured response body rather than the status code, so ordinary permission errors keep their existing rendering.

  • Unrelated to the hooks work, de-flake SchedulePage.test.tsx. Its fillForm helper wrapped an already-retrying findByLabelText in waitFor, so the two 1s budgets raced and a slow first render failed test-js with "Timed out in waitFor". This is separable from the rest of the PR if you would rather it land on its own.

This PR was written by Mux, an AI coding agent, on Mike's behalf.

@github-actions

github-actions Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

ibetitsmike commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator Author

@ibetitsmike

Copy link
Copy Markdown
Collaborator Author

@codex review

@ibetitsmike
ibetitsmike force-pushed the mike/chat-hooks/chatd branch from bbc1484 to d0f7244 Compare July 22, 2026 20:12
@ibetitsmike
ibetitsmike force-pushed the mike/chat-hooks/api branch from 6743ac5 to fdcf4b8 Compare July 22, 2026 20:12
@ibetitsmike

Copy link
Copy Markdown
Collaborator Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Can't wait for the next one!

Reviewed commit: fdcf4b843f

ℹ️ 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".

@ibetitsmike
ibetitsmike force-pushed the mike/chat-hooks/chatd branch from d0f7244 to 5250a86 Compare July 22, 2026 20:24
@ibetitsmike
ibetitsmike force-pushed the mike/chat-hooks/api branch from fdcf4b8 to 44a8e4a Compare July 22, 2026 20:25
@ibetitsmike

Copy link
Copy Markdown
Collaborator Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread coderd/exp_chats.go Outdated
@ibetitsmike
ibetitsmike force-pushed the mike/chat-hooks/chatd branch from 5250a86 to e62369a Compare July 22, 2026 20:37
@ibetitsmike
ibetitsmike force-pushed the mike/chat-hooks/api branch from 44a8e4a to 7b0db9d Compare July 22, 2026 20:37
@ibetitsmike

Copy link
Copy Markdown
Collaborator Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread coderd/exp_chats.go Outdated
Comment thread site/src/pages/AgentsPage/components/ChatConversation/ConversationTimeline.tsx Outdated
@ibetitsmike
ibetitsmike force-pushed the mike/chat-hooks/chatd branch from e62369a to d835b06 Compare July 22, 2026 20:52
@ibetitsmike
ibetitsmike force-pushed the mike/chat-hooks/api branch from 7b0db9d to 38d0e66 Compare July 22, 2026 20:52
@ibetitsmike

Copy link
Copy Markdown
Collaborator Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread site/src/api/queries/chatMessageEdits.ts
Comment thread coderd/exp_chats.go Outdated
@ibetitsmike
ibetitsmike force-pushed the mike/chat-hooks/chatd branch from d835b06 to a3855c0 Compare July 22, 2026 21:27
@ibetitsmike
ibetitsmike force-pushed the mike/chat-hooks/api branch from 38d0e66 to 5cad5e0 Compare July 22, 2026 21:27
Thread the extracted execute result error into the failure tooltip
instead of the hardcoded 'Command failed', and make write_file errors
render honestly: an error label instead of 'Wrote <file>', the result
error text in the expanded view, and no args-derived synthetic diff for
content that was never written. Covers lifecycle hook denials, which
previously looked like successful writes or opaque command failures.
…t field

Label failed edit_files calls 'Failed to edit' with the result error
text in the expanded view, matching the write_file error rendering, and
include the new errorText field in the execute render data unit test.
acceptServerChatStatus armed a resync that applied the currently cached
chatRecord.status immediately, so a failed send or edit could replace a
live websocket "running" with a stale REST "waiting" and make
shouldApplyMessagePart drop assistant parts.

The resync now waits for the chat query's dataUpdatedAt to advance past
the value captured when it was armed. Object identity does not work here
because TanStack Query structural sharing preserves the chatRecord
reference when a refetch returns value-equal data, which would leave the
resync armed forever and never apply an unchanged status.

Also covers the cross-chat send guard with an interaction story and drops
a store test that restated the guard instead of exercising it.
A status or error event arriving after acceptServerChatStatus armed the
resync but before the invalidated chat query resolved was overwritten by
the older REST status, which could strand the turn and make
shouldApplyMessagePart drop the retry's assistant deltas.

The resync now captures the server status version when armed and skips
the overwrite when the websocket advanced it in the meantime.
The send and edit failure paths armed a resync from the render that
started the request, so a rejection arriving after the user navigated
away captured the previous chat's dataUpdatedAt. The newly active
chat's higher cached timestamp then satisfied the freshness check at
once, overwriting a websocket-delivered status and clearing the
websocket-authoritative guard.

acceptServerChatStatus now ignores calls whose chat is no longer the
active one, which covers both failure paths at their single shared
entry point.
A send that promotes the queue head suppresses that ID locally, but a
concurrent queue mutation in another tab can leave the row queued
server-side, so the transcript and the queue both showed it until the
next authoritative snapshot.

The page now issues one uncursored messages request after the promotion
through a dedicated react-query key and hands the result to the store,
which clears the promotion markers and applies the snapshot in a single
transition. The store returns the queue it applied, so the caller caches
the filtered result rather than the raw response.

Stale applies are fenced on two axes: the convergence fence advances on
every accepted snapshot and whenever the active chat changes, and the
store drops a response whose originating chat is no longer displayed.
A send whose response lands after the user switched chats reconciles
from the queue currently cached for that chat, falling back to the
pre-send snapshot only when nothing is cached, so a queue update
received mid-send is not overwritten.
Three UAT defects in how the chat UI presents lifecycle hook outcomes.

A tool call blocked by pre_tool_use rendered as 'Ran <command>', as if it had
executed. Derive the failed wording from the tool-result error flag, matching
the write and edit tools. A command that ran and exited non-zero is not a
protocol error, so it keeps its existing wording.

A hook notice rendered above the message it annotates, which reads backwards
for a 'your prompt was rewritten' card.

Both hook outcomes on the create path fell through to the generic error alert,
so an expected policy decision appeared with a stack trace and a workspaces
action. Give each its own branch, keyed on the structured response body rather
than the status code so ordinary permission errors keep their treatment.
@ibetitsmike

Copy link
Copy Markdown
Collaborator Author

Rebased onto current main now that #27401, #27428, #27429, and the message-order pair (#27619, #27620) are all merged. Used --onto against the old parent tip so the squash merges did not inflate the diff: git range-diff reports all 33 commits identical, and the PR is still 26 files, +2225/-106.

Revalidated after the rebase: tsc -p . clean, biome clean on all 26 changed files, and 443 unit tests pass across 17 files.

One note on the red Storybook check: it is the @coder/pixel-storybook visual-snapshot upload failing with HTTP 401 You are not currently signed in against pixel.coder.com, aborting after 11 report failures on unrelated pages-taskpage--* stories. The same failure hit #27545 and #27339, which both merged. It is not in the required job's needs list and no CI job runs the Storybook interaction tests.

Mux is working on Mike's behalf.

@ibetitsmike

Copy link
Copy Markdown
Collaborator Author

Ran a strict branch-wide comment audit after the rebase. Audited 61 added or modified comment blocks: deleted 24 that restated code or tests, shortened 10, corrected 5 unproven or imprecise claims, and kept 22 unchanged because they preserve concurrency, ordering, accessibility, or external-contract context.

The cleanup changed only comments and blank lines. Revalidated with biome, tsc -p ., git diff --check, the full pre-commit hook, and a post-cleanup inventory of all 37 remaining in-scope comment blocks.

Mux is working on Mike's behalf.

…le test

fillForm wrapped an already-retrying findByLabelText in waitFor, so the
inner and outer 1s budgets raced and a slow first render failed with
'Timed out in waitFor'. Await the query directly with a timeout that fits
inside the enclosing test budget.

Verified by delaying the quiet-hours response 1500ms: the previous code
fails at SchedulePage.test.tsx:20:8 with the exact CI signature, and the
new code does not.
@ibetitsmike

Copy link
Copy Markdown
Collaborator Author

test-js failed on the previous head in SchedulePage.test.tsx, which this PR does not touch (the file is byte-identical to main). The head before it, containing the same code minus comment-only edits, passed test-js. So this was a pre-existing flake, not a regression here.

Root cause: fillForm wrapped an already-retrying findByLabelText in waitFor. Both default to a 1s budget, so they raced and the effective wait was ~1s no matter the test's 15s timeout. A slow first render then failed at SchedulePage.test.tsx:20:8 with Timed out in waitFor.

Verified by red-green rather than by rerunning until green. Delaying the quiet-hours response by 1500ms reproduces the exact CI signature on the old code and not on the new code:

Variant Result
Original code + 1500ms delay fails at :20:8, Timed out in waitFor (CI signature)
Fixed code + 1500ms delay that failure is gone
Fixed code, no delay 3/3 pass

The fix is one commit and is separable if you would prefer it on its own branch.

Storybook is red here for an unrelated reason: the @coder/pixel-storybook snapshot upload gets HTTP 401 from pixel.coder.com and aborts. It hit #27545 and #27339 too, both of which merged, and it is not in the required job's needs list.

Mux is working on Mike's behalf.

@ibetitsmike

Copy link
Copy Markdown
Collaborator Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants