feat(site): surface chat lifecycle hook outcomes in the chats UI - #27430
Conversation
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".
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".
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".
d835b06 to
a3855c0
Compare
38d0e66 to
5cad5e0
Compare
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.
…obbering newer server state
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.
|
Rebased onto current Revalidated after the rebase: One note on the red
|
|
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,
|
…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.
|
Root cause: 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:
The fix is one commit and is separable if you would prefer it on its own branch.
|
|
@codex review |


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_updateinstead 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_filerenders 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_useas failed instead ofRan <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. ItsfillFormhelper wrapped an already-retryingfindByLabelTextinwaitFor, so the two 1s budgets raced and a slow first render failedtest-jswith "Timed out in waitFor". This is separable from the rest of the PR if you would rather it land on its own.