fix(agents): preserve media side merges during prompt release#100490
Conversation
openclaw-barnacle
Bot
added
agents
d9fb2ce to
a1f920d
Compare
|
Codex review: needs changes before merge. Reviewed July 5, 2026, 8:18 PM ET / 00:18 UTC. Summary PR surface: Source +62, Tests +256, Docs +1. Total +319 across 5 files. Reproducibility: yes. source-reproducible: current main accepts delivery mirror message rows but does not classify ordinary side leaf controls in the normal prompt-release path. I did not rerun the live Weixin flow, but the PR and source PR include after-fix logs showing 11 media deliveries completed without the takeover error. Review metrics: 1 noteworthy metric.
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: Merge the narrow session-state fix after removing the direct CHANGELOG entry, exact-head checks finish, and maintainers accept the guarded side-leaf boundary. Do we have a high-confidence way to reproduce the issue? Yes, source-reproducible: current main accepts delivery mirror message rows but does not classify ordinary side leaf controls in the normal prompt-release path. I did not rerun the live Weixin flow, but the PR and source PR include after-fix logs showing 11 media deliveries completed without the takeover error. Is this the best way to solve the issue? Mostly yes: the classifier plus SessionManager pre-mutation guard is the narrow owner-boundary fix for the observed false takeover. The direct CHANGELOG edit should be removed because release notes already belong in the PR body or squash metadata. Full review comments:
Overall correctness: patch is correct AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against c9349d1fb4db. Label changesLabel justifications:
Evidence reviewedPR surface: Source +62, Tests +256, Docs +1. Total +319 across 5 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 at exact head The final implementation admits prompt-released delivery side leaves only when the active leaf and current side cursor remain unchanged. SessionManager validates the full batch before mutation; active-branch changes, explicit cursor resets, malformed records, non-side controls, and duplicate ids continue through takeover recovery. Proof:
This is the current-main replacement for #100033; @scotthuang's co-authorship is preserved. The release-note credit is deferred to the grouped maintainer changelog after code landing to avoid repeated merge starvation from unrelated concurrent changelog updates. |
a1f920d to
8c54821
Compare
![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: 8c54821602
ℹ️ 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".
| ### Fixes | ||
|
|
||
| - **Remote browser reliability:** bound persistent Playwright tab enumeration by the existing remote CDP timeout budget and retire timed-out connection attempts so late completions cannot restore a stuck connection. (#80147, #58968) Thanks @HemantSudarshan and @KeaneYan. | ||
| - **Prompt-release media delivery:** accept active-leaf-preserving side-append controls while an embedded run temporarily releases its session lock, so successive message-tool media replies merge without a false session-takeover failure. (#100033) Thanks @scotthuang. |
There was a problem hiding this comment.
This normal fix edits CHANGELOG.md, but the root AGENTS.md says CHANGELOG.md is release-only and normal fixes should keep release-note context in the PR body, squash message, or direct commit instead. Leaving this in Unreleased can make the release generator duplicate or stale the note, so this entry should be removed from the commit.
Useful? React with 👍 / 👎.
4afa7b6 to
766166b
Compare
Co-authored-by: scotthuang <[email protected]>
Co-authored-by: scotthuang <[email protected]>
766166b to
8b466cb
Compare
|
Merged via squash.
|
…aw#100490) * fix(agents): preserve active leaf during side merges Co-authored-by: scotthuang <[email protected]> * fix(agents): validate explicit side-branch cursors Co-authored-by: scotthuang <[email protected]> * chore: defer prompt-release media changelog --------- Co-authored-by: scotthuang <[email protected]>
…aw#100490) * fix(agents): preserve active leaf during side merges Co-authored-by: scotthuang <[email protected]> * fix(agents): validate explicit side-branch cursors Co-authored-by: scotthuang <[email protected]> * chore: defer prompt-release media changelog --------- Co-authored-by: scotthuang <[email protected]>
1 participant
What Problem This Solves
Users sending multiple visible media replies while an embedded prompt lock is released could receive the media and then see the run fail with
session file changed while embedded prompt lock was released.This replaces #100033 with a current-main, maintainer-owned branch because the contributor fork could not be updated without retaining stale ancestry.
Why This Change Was Made
Delivery mirrors append a side branch containing messages plus
leafcontrols. Prompt-release recovery previously classified the messages but treated every leaf control as an ownership takeover.The new path accepts only side-mode leaf controls whose target remains the active leaf and whose append cursor matches the current side branch. Validation happens before mutation; active-branch changes, cursor resets, malformed records, and non-side controls still trigger takeover recovery.
User Impact
Batched media replies can finish without a false post-delivery failure. The session ownership fence remains fail-closed for prompt-affecting changes.
Evidence
attempt.session-lock.test.tsandsession-manager.test.ts.git diff --checkpassed.Co-authored with @scotthuang; contributor commits and changelog attribution are preserved.