fix: handle thinking/redacted_thinking block immutability on context pruning#49714
fix: handle thinking/redacted_thinking block immutability on context pruning#49714imaddde867 wants to merge 2 commits intoopenclaw:mainfrom
Conversation
Greptile SummaryThis PR fixes three related issues that surfaced when Anthropic extended-thinking sessions hit context pruning: (1) Key changes:
Confidence Score: 4/5
Prompt To Fix All With AIThis is a comment left during a code review.
Path: src/agents/pi-embedded-helpers/errors.ts
Line: 737-739
Comment:
**Thinking-block check placed after missing-tool-call check**
The new `isThinkingBlockImmutabilityError` guard is positioned _after_ `isMissingToolCallInputError` but _before_ the generic `"type":"invalid_request_error"` regex. That ordering is intentional and correct — the Anthropic thinking-block rejection is itself an `invalid_request_error`, so the specific check must precede the catch-all.
One minor note: if a future error message somehow starts with a tool-call-input pattern _and_ contains the thinking-block phrase (highly unlikely), the tool-call handler would win. The current order appears fine given real-world messages, but it's worth documenting the dependency in a brief comment so the ordering is not accidentally swapped later.
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: src/auto-reply/reply/agent-runner-execution.ts
Line: 565-575
Comment:
**`resetSessionAfterRoleOrderingConflict` reused for a semantically different error**
The function name implies it handles role-ordering conflicts specifically, but here it is called for a thinking-block immutability error. If `resetSessionAfterRoleOrderingConflict` ever gains role-ordering-specific logic (e.g., a guard that checks whether the error actually looks like a role conflict before resetting), it could inadvertently skip the reset for thinking-block errors.
Consider either:
- renaming the function to something more generic like `resetSessionAfterHistoryConflict`, or
- introducing a thin wrapper / a dedicated `resetSessionAfterThinkingConflict` that delegates to the same underlying reset.
As-is this works, but the semantic mismatch is a latent maintenance hazard.
How can I resolve this? If you propose a fix, please make it concise.Last reviewed commit: "fix: handle thinking..." |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![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: cddf119cba
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1d08eb03c0
ℹ️ 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".
…ility on context pruning Fixes leaked 'thinking or redacted_thinking blocks cannot be modified' error message reaching chat channels during context pruning. Also drops redacted_thinking from context estimator to prevent over-pruning. Source: openclaw#49714
imaddde867
force-pushed
the
fix/thinking-block-pruning-error
branch
from
42cf7c8 to
0867445
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0867445e76
ℹ️ 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".
…pruning
Context pruning with Anthropic extended thinking triggers API rejection
("thinking or redacted_thinking blocks cannot be modified") and the raw
error leaks to end-user chat channels.
- Strip redacted_thinking blocks in dropThinkingBlocks (same immutability
constraint as thinking blocks)
- Exclude thinking block chars from pruner context estimate (they are
stripped before reaching the API, so counting them causes over-pruning)
- Classify the thinking-block immutability error with a friendly message
and auto-recover via session reset
- Add tests for all three changes
Closes openclaw#49573
imaddde867
force-pushed
the
fix/thinking-block-pruning-error
branch
from
0867445 to
037cb79
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 037cb79808
ℹ️ 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".
Address review feedback: document why isThinkingBlockImmutabilityError must precede the generic invalid_request_error regex.
imaddde867
force-pushed
the
fix/thinking-block-pruning-error
branch
from
dcdc022 to
c79d589
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dcdc022335
ℹ️ 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".
1 participant
Summary
dropThinkingBlocksnow also stripsredacted_thinkingblocks, (2) pruner char estimate excludes thinking blocks (they're stripped before the API, so counting them caused over-pruning), (3) new error classifier catches the specific Anthropic rejection and returns a friendly message, (4) auto-recovery resets the session (matching the existing role-ordering recovery pattern).Change Type (select all)
Scope (select all touched areas)
Linked Issue/PR
User-visible / Behavior Changes
Security Impact (required)
Repro + Verification
Environment
contextPruning.mode: "cache-ttl"with extended thinking sessionsSteps
Expected
Actual (before fix)
Evidence
All 3 test suites pass (37 tests total):
Human Verification (required)
Review Conversations
Compatibility / Migration
Failure Recovery (if this breaks)
Risks and Mitigations
thinking.*blocks?.*cannot.*modifiedcould match unrelated errorsinvalidRequesthandler so it only intercepts the exact caseAI-assisted: Yes (lightly tested via unit tests; I understand what the code does)
Testing degree: Fully tested with 4 new unit tests across 3 test files