feat(web): simplify message delivery controls - #1314
Conversation
Why: users need one understandable delivery choice, a truthful whole-conversation Stop, and an explicit destructive Steer confirmation instead of overlapping cancellation semantics.
Why: a failed durable stop request must not leave the conversation appearing stopped without feedback.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f81b884278
ℹ️ 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".
| ? dispositionCarrierSupport === 'exact' | ||
| ? (messageDisposition.oneShot ?? undefined) | ||
| : 'next_work' | ||
| ? (messageDisposition.oneShot ?? messageDisposition.effective) |
There was a problem hiding this comment.
When active work first appears and the preference GET is still pending or has failed, messageDisposition.effective remains the local next_work default. Sending that value explicitly gives it precedence in resolveMessageDispositionForAdmission, so a persisted global continue_current preference is silently ignored and the message is queued. Send only the one-shot override explicitly, or wait until preference hydration succeeds before sending the effective value.
Useful? React with 👍 / 👎.
| // #1307: a conversation-level Stop is always a full-thread stop. Per-cat | ||
| // cancellation remains an internal control-plane primitive and Steer uses | ||
| // its dedicated, explicitly destructive path. | ||
| cancelFn(threadId, undefined); |
There was a problem hiding this comment.
When the new force-reset POST rejects or returns a non-2xx response, cancelInvocation only displays an error toast, but handleStop immediately proceeds to clear callbacks, active invocation slots, and streaming state. The server invocation therefore continues while the UI removes its liveness indicators and Stop control, potentially also dropping subsequent output until a refresh or rehydration. Return and await the request result, clearing local state only after success or restoring it on failure.
Useful? React with 👍 / 👎.
Why: delivery preference belongs with append custody; this PR now contains only the destructive Stop and Steer interaction.
Why: a rejected whole-thread Stop must leave the visible invocation state intact so users can retry or inspect the failure.
|
@codex review the latest head; prior inline findings were addressed in subsequent commits. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: de0db64c4b
ℹ️ 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".
| // its dedicated, explicitly destructive path. | ||
| const clearStoppedThread = () => { | ||
| clearPendingCallbacksForThread(threadId); | ||
| const isActiveThreadStop = threadId === store.currentThreadId; |
There was a problem hiding this comment.
When a user stops foreground thread A and navigates to an already-running thread B before the force-reset request resolves, store is the Zustand snapshot captured before the request, so this comparison still treats A as active. The success callback then calls active-thread helpers such as clearAllActiveInvocations(), which operate on the store's current thread and clear B's running/streaming UI instead of only A. Read the current state after the await (or always perform thread-scoped cleanup) before selecting the cleanup branch.
Useful? React with 👍 / 👎.
Why: a force-reset can finish after navigation; re-reading the store prevents the prior thread's Stop from clearing the newly active thread's running state.
|
@codex review the latest head; this addresses the newly reported cross-thread async Stop P1. |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. 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". |
1 participant
What
Closes #1307.
Verification
Known non-blocking