fix: replies fail when memory flush is exhausted#100618
Conversation
Co-authored-by: 忻役 <[email protected]> Co-authored-by: Jerry-Xin <[email protected]>
openclaw-barnacle
Bot
added
docs
![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: b62c414076
ℹ️ 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".
| fallbackNoticeSelectedModel: undefined, | ||
| fallbackNoticeActiveModel: undefined, | ||
| fallbackNoticeReason: undefined, | ||
| compactionCount: 0, |
There was a problem hiding this comment.
When this reset mints a fresh session after memory-flush exhaustion or a role-ordering conflict, it resets compactionCount to 0 but the spread above still preserves authProfileOverrideCompactionCount. resolveSessionAuthProfileOverride rotates auto-selected profiles only when compactionCount > authProfileOverrideCompactionCount (src/agents/auth-profiles/session-override.ts:251-275), so a session reset from compaction 5 can keep the old auto profile pinned until the new session compacts six more times. Clear or rebase the auth-profile compaction marker along with the other reset-only state.
Useful? React with 👍 / 👎.
|
Codex review: needs changes before merge. Reviewed July 6, 2026, 1:38 AM ET / 05:38 UTC. Summary PR surface: Source +17, Tests +245, Docs 0. Total +262 across 16 files. Reproducibility: yes. Current main source still returns visible memory-flush errors as a terminal reply before the normal run, and the PR body includes after-fix live Gateway output showing repeated flush 500s while all 12 messages complete. Review metrics: 2 noteworthy metrics.
Stored data model Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Maintainer decision needed
Security Review findings
Review detailsBest possible solution: Fix the reset auth-profile marker, then land this or an equivalent fail-open memoryFlush recovery only after maintainers accept the session-rotation and notice semantics. Do we have a high-confidence way to reproduce the issue? Yes. Current main source still returns visible memory-flush errors as a terminal reply before the normal run, and the PR body includes after-fix live Gateway output showing repeated flush 500s while all 12 messages complete. Is this the best way to solve the issue? No as-is. The fail-open recovery is a plausible narrow fix for the bug, but the reset helper must clear or rebase auth-profile compaction state and maintainers still need to accept the recovery and notice semantics. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 89f911f322c9. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +17, Tests +245, Docs 0. Total +262 across 16 files. View PR surface stats
Acceptance criteria:
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
clawsweeper
Bot
added
proof: sufficient
|
Land-ready maintainer proof:
Known proof gap: no live third-party provider outage was induced. The contributor's deterministic Gateway proof used repeated mock-provider HTTP 500 responses and showed all 12 normal replies completing; maintainer proof exercised the same recovery state machine locally and in isolated AWS. |
|
Merged via squash.
|
* fix(reply): recover from memory flush exhaustion Co-authored-by: 忻役 <[email protected]> Co-authored-by: Jerry-Xin <[email protected]> * test(reply): remove unused memory flush result --------- Co-authored-by: Jerry-Xin <[email protected]>
* fix(reply): recover from memory flush exhaustion Co-authored-by: 忻役 <[email protected]> Co-authored-by: Jerry-Xin <[email protected]> * test(reply): remove unused memory flush result --------- Co-authored-by: Jerry-Xin <[email protected]>
1 participant
Closes #85645
Replacement for #88968. The contributor branch was takeover-ready, but GitHub's verified fork editor could not preserve workspace symlink modes while replaying current
main. This maintainer branch carries the reviewed tree as one commit and preserves contributor credit.What Problem This Solves
Fixes an issue where users could stop receiving normal agent replies when pre-compaction memory maintenance repeatedly failed. The maintenance error replaced the real reply, and the oversized active session remained vulnerable to the same failure on later messages.
Why This Change Was Made
Memory-flush failures are now non-terminal to the user turn. The helper returns an explicit maintenance outcome; after three failures the runner rotates to a fresh active session while preserving the old transcript, clears stale compaction/flush state, and optionally emits a degraded maintenance notice through the existing
agents.defaults.compaction.notifyUserpath.Resolved embedded runs that contain visible error payloads now enter the same failure counter instead of being stamped successful. If that run already completed a transcript rotation, the new session binding is preserved before the failure is recorded.
User Impact
Normal replies continue when memory maintenance is temporarily unavailable. Persistently failing, oversized sessions recover automatically after the existing retry budget; operators retain the previous transcript for recovery or forensics. Default behavior remains silent unless compaction notices are enabled.
Evidence
main: visible memory-flush errors terminate the turn before the normal agent run.aws, leasecbx_882f0c2cb4bb, runrun_c7e91a20d30f: 7 focused files, 330 tests passed; hydration disabled, public networking, scrubbed launch environment, pinned checksum-verified Node 24 bootstrap. Lease auto-stopped.node scripts/run-vitest.mjs src/auto-reply/reply/agent-runner-memory.test.ts— 47 tests passed.git diff --check— clean.Co-authored with @Jerry-Xin; the squashed commit retains both contributor commit identities.