Skip to content

fix(chat): prevent aborted runs from reactivating conversation state#916

Merged
su8su merged 2 commits intomainfrom
fix_stop_conversation
Apr 25, 2026
Merged

fix(chat): prevent aborted runs from reactivating conversation state#916
su8su merged 2 commits intomainfrom
fix_stop_conversation

Conversation

@su8su
Copy link
Copy Markdown
Contributor

@su8su su8su commented Apr 25, 2026

No description provided.

Copy link
Copy Markdown
Contributor

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

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: 27f06c56ca

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

Comment thread src/pages/Chat/index.tsx
// gate it on activeRunId being present.
const isLatestOpenRun = nextUserIndex === -1
&& (sending || pendingFinal || hasAnyStreamContent || runStillExecutingTools);
&& (sending || pendingFinal || hasAnyStreamContent || (runStillExecutingTools && !!activeRunId));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 Badge Mirror activeRunId abort handling into the live chat store

This new isLatestOpenRun gate relies on activeRunId being cleared on abort, but the app’s actual useChatStore implementation is still src/stores/chat.ts (exported there) where abortRun does not clear activeRunId and later events can re-arm sending for that same run. That means this condition can stay true after Stop for tool-active segments, so aborted runs can still look/open as active. Apply the same abort/run-id filtering changes to src/stores/chat.ts (or wire the modular store files into production) before depending on this guard.

Useful? React with 👍 / 👎.

@su8su su8su merged commit 9a360eb into main Apr 25, 2026
5 checks passed
@su8su su8su deleted the fix_stop_conversation branch April 25, 2026 07:09
Copy link
Copy Markdown
Contributor

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

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: c687706378

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

Comment thread src/pages/Chat/index.tsx
Comment on lines 288 to +289
const isLatestOpenRun = nextUserIndex === -1
&& (sending || pendingFinal || hasAnyStreamContent || runStillExecutingTools);
&& (sending || pendingFinal || hasAnyStreamContent || (runStillExecutingTools && !!activeRunId));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 Badge Keep tool-gap run open when phase events clear activeRunId

This gate now requires activeRunId for runStillExecutingTools, but the gateway phase-end path clears both sending and activeRunId during intermediate tool rounds (src/stores/gateway.ts phase end handling). In tool-heavy runs, that creates a window where the run is still active (exactly the case runStillExecutingTools was meant to cover) but isLatestOpenRun becomes false, so the execution card/"thinking" state collapses until another stream event arrives.

Useful? React with 👍 / 👎.

Comment on lines +33 to +37
if (eventState !== 'aborted') {
if (lastAbortedRunId === '*' && !activeRunId && get().sending) {
queueBlockedRunEvent(runId, event);
}
return;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 Badge Do not blanket-drop non-aborted events under '*' abort state

When _lastAbortedRunId is '*' (set if Stop is pressed before a runId is known), this branch returns for every non-aborted event with any runId, and only queues in one narrow case; if the subsequent send path never yields returnedRunId (it is typed optional), the wildcard is never narrowed/cleared and future delta/final events are effectively discarded. That can leave the chat stuck without streaming updates after an early abort.

Useful? React with 👍 / 👎.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant