fix(agent-core): drop thinking-only assistant messages from projected requests - #2689
fix(agent-core): drop thinking-only assistant messages from projected requests#2689xiahuaaaa wants to merge 1 commit into
Conversation
![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: 115114adbc
ℹ️ 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".
| // Unencrypted thinking blocks carry nothing the provider wire can | ||
| // represent as message content: the OpenAI bases drop them out of | ||
| // `content` (`convertContentPart` → `null`, moved to `reasoning_content`) | ||
| // and the Anthropic base moves them to `thinking` blocks. A message left | ||
| // with only unencrypted thinking would serialize with neither content nor |
There was a problem hiding this comment.
The newly added implementation narration here, together with the helper-level block above wireSendableContent, violates the package's header-only comment convention. Consolidate the externally relevant responsibility into the top-of-file header and remove the comments beside statements/functions so this v2 module remains consistent with its documented structure.
AGENTS.md reference: packages/agent-core-v2/AGENTS.md:L15-L19
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed — moved the rationale into the top-of-file header and removed the inline block in emit plus the helper-level JSDoc, so the v2 module keeps its header-only comment convention.
… requests Interrupting a response mid-thinking (ESC) seals a partial assistant message holding only an unencrypted thinking fragment. Every protocol base moves thinking out of `content` (OpenAI `convertContentPart` returns null, Anthropic maps it to `thinking` blocks), so the next request serialized that message with neither content nor tool_calls and was rejected by the provider (400 "Invalid assistant message: content or tool_calls must be set"), permanently wedging the session. Treat unencrypted thinking as not wire-sendable in the projector's vacuous-message drop check: such messages are now dropped from outgoing requests while the context keeps the reasoning for display and resume. Signed thinking (`encrypted`) is preserved for reasoning providers that require the echo back verbatim.
115114a to
db67e3a
Compare
🦋 Changeset detectedLatest commit: db67e3a The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
1 participant
Related Issue
Resolve #2691 — reproducible ESC-interrupt crash (bug report).
Problem
Reproducible session crash: pressing ESC to interrupt an ongoing response wedges the session permanently. When the stream is cancelled mid-thinking (the model's first output chunk is usually a thinking part), the interrupted step leaves a partial assistant message in the conversation history that holds only an unencrypted thinking fragment — no text, no tool calls.
On the next user prompt, that message is sealed and projected into the request. Every protocol base moves thinking out of
content:convertContentPartreturnsnullforthinkparts; the reasoning is moved toreasoning_content.thinkparts becomethinkingblocks.So the sealed message serializes with neither
contentnortool_calls, and the provider rejects it:Every subsequent request fails the same way (the message stays in history), so the session cannot recover — the TUI shows the error and each retry re-fails. Reproduced with a third-party OpenAI-compatible provider (deepseek) on the v2 engine; the v1 engine has the identical drop/keep logic and the same failure mode.
What changed
The projector already drops "wholly-vacuous" assistant messages (nothing the provider wire can represent) — e.g. a message whose only part is an empty thinking block. The gap was that a non-empty unencrypted thinking block was treated as wire-sendable content, when it serializes to nothing on every protocol.
Both engines now define what is actually sendable on the provider wire — everything except unencrypted thinking blocks — and drop assistant messages whose only parts are unencrypted thinking:
packages/agent-core/src/agent/context/projector.ts(v1 engine)packages/agent-core-v2/src/agent/contextProjector/contextProjectorService.ts(v2 engine)The context history still keeps the interrupted reasoning (visible in the transcript, preserved on resume); only the outgoing request drops it, and a
vacuous_message_droppedprojection repair is reported as before. Signed thinking (encrypted) is deliberately preserved — reasoning providers require it echoed back verbatim.Checklist
gen-changesetsskill, or this PR needs no changeset.gen-docsskill, or this PR needs no doc update.