Repository navigation
fix: clear Side chat when its parent conversation is deleted - #139974
Merged
Merged
Conversation
Contributor
|
🦞👀 Pull request received. I will update this pull request when review starts. ClawSweeper review completeClawSweeper finished reviewing this revision. The review result is being finalized. |
Contributor
|
Codex review: needs maintainer review before merge. Reviewed September 6, 2026, 6:32 AM ET / 10:32 UTC. ClawSweeper reviewWhat this changesClear a conversation’s in-memory Side chat after committed deletion, cancel its active question, and preserve other agents’ threads and newer conversation generations. Merge readiness✅ Ready for maintainer review This remains a useful, focused fix absent from current main and v2026.9.2. No actionable introduced defects were found. Priority: P2 Review scores
Verification
How this fits togetherThe Gateway holds Side chat separately from the main conversation history. Committed session lifecycle events identify deleted conversations so the Side chat service can discard matching dialogue and cancel pending answers. flowchart TD
A[Conversation deletion] --> B[Session store commits deletion]
B --> C[Event with agent and previous session identity]
C --> D{Matching Side chat generation?}
D -->|Yes| E[Clear thread and cancel question]
D -->|No| F[Preserve existing thread]
E --> G[Recreated conversation opens empty]
Before mergeNone. Agent review detailsSecurityNone. PR surfaceSource +46, Tests +228, Docs 0. Total +274 across 25 files. View PR surface stats
Review metrics
Root-cause clusterRelationship: Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Technical reviewBest possible solution: Keep Side chat cleanup attached to committed lifecycle events, with agent and generation matching and the existing cancellation owner. Do we have a high-confidence way to reproduce the issue? Yes: populate Side chat, delete its parent, recreate the same key, and read Side chat before another question. Current-main source establishes the stale-cache path, and the paired report records that flow; this review did not execute it. Is this the best way to solve the issue? Yes: consuming the existing committed deletion event repairs the lifecycle owner without adding another store, configuration option, or public API. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against e74543854408. LabelsLabel justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (1 earlier review cycle)
|
Carry the resolved operation agent through internal session-identity events so bare keys retain their owner. Match companion cleanup to that scoped identity and the cached previous generation when deletion commits. Preserve reset timing, stored representations, schema, and public protocol.
Keep the newly integrated observer fixture aligned with the required operation scope on committed session identity events.
steipete
force-pushed
the
codex/qa-companion-delete-20260906
branch
from
September 6, 2026 10:24
bd94ee5 to
be0a581
Compare
Contributor
Author
|
Merged via squash.
|
vincentkoc
added a commit
to ylcn91/openclaw
that referenced
this pull request
Sep 6, 2026
* origin/main: (47 commits) ci: record CPU time for Node test jobs (openclaw#140054) test(crabbox): consolidate wrapper boundary coverage (openclaw#140058) fix(test): keep native filesystem access checks portable (openclaw#140047) fix: preserve models when OAuth preparation fails (openclaw#139951) feat(update): recover failed updates with bounded repair (openclaw#139495) refactor(plugins): share managed npm package traversal (openclaw#140040) chore(ui): refresh control ui locales (openclaw#140059) refactor(auth): share profile failure state reset (openclaw#140049) fix(cron): surface skipped on-exit payloads when jobs are busy (openclaw#139987) fix(codex): preserve exec-server WebSocket backpressure under Bun (openclaw#140048) fix(agents): ground exec approvals in tool results (openclaw#140043) refactor(policy): reuse authored policy value helpers (openclaw#140019) docs: keep ClawSweeper status inspection read-only (openclaw#139958) fix: reduce task maintenance stalls with large histories (openclaw#140038) fix(code-mode): identify unawaited promise output (openclaw#140028) fix(test): release memory search fixtures after failed phases (openclaw#140013) test(gateway): isolate Serve hello lifecycle fixtures (openclaw#140044) fix(microsoft-foundry): bound Entra token retention across accounts (openclaw#127216) fix(process): preserve the cause of cleanup observation failures (openclaw#140015) fix: clear Side chat when its parent conversation is deleted (openclaw#139974) ...
github-actions Bot
pushed a commit
to Desicool/openclaw
that referenced
this pull request
Sep 7, 2026
…w#139974) * fix(gateway): clear side chat after its parent session is deleted Carry the resolved operation agent through internal session-identity events so bare keys retain their owner. Match companion cleanup to that scoped identity and the cached previous generation when deletion commits. Preserve reset timing, stored representations, schema, and public protocol. * test(gateway): scope observer reset fixture to its agent Keep the newly integrated observer fixture aligned with the required operation scope on committed session identity events.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #139971.
What Problem This Solves
Fixes an issue where users deleting and recreating a conversation would see Side chat exchanges from the deleted conversation. The Gateway retained the old in-memory Side chat under the reused key.
Why This Change Was Made
Side chat now consumes the existing committed identity-deletion event and clears only a thread whose scoped key and previous backing session ID match. Cleanup reuses the existing reset/cancellation owner and unsubscribes when the service is disposed.
The internal identity event now carries the already-resolved operation agent from its canonical producers. Bare keys need that scope, while qualified keys retain their own embedded agent. Inferring the agent from a database owner or matching session ID alone would clear the wrong thread when agents share a bare key or ID.
Production is net +46 lines, tests net +228, and docs net 0 across 25 files. The production growth carries the missing operation scope through existing event producers and adds one scoped deletion listener; it adds no parallel store, fallback, schema, migration, public protocol, SDK surface, or configuration option. Database transactions, stored representations, event grouping and publication timing stay with their existing owners.
User Impact
Recreated conversations begin with empty Side chat. Deletion cancels an active Side chat ask for the removed generation and preserves other agents and newer backing generations. The Control UI reference now includes session deletion in the Side chat cleanup lifecycle.
Evidence
Exact candidate
be0a581878ddfe51b1d63c9bbbc3c1b85bc01af1, based on9c0cf2b6a56f0da6c5c23928eb63f8223f980449.95f963e342ab476912c23c0e7fb9660c69bb9980retained the old Side chat after committed parent deletion and recreation of the same key with a new backing ID. The public state response and visible UI both showed the old exchange before any new question. Its affected owner bodies were matched to the unpatched candidate lineage.be0a5818with current locked dependencies passed with zero failed attempts. Reopened Side chat was empty before a fresh ask; the second agent retained its original exchange; the recreated first conversation received one new exchange without old dialogue. There were three synthetic model requests, nine successful tracked browser responses, six successful public admin RPCs, and no mainchat.send. All owned processes, ports and temporary runtime state were cleaned up.bd94ee5candidate passed 261 focused tests with current locked dependencies, covering committed deletion, stale expected-ID preservation, reset/unrelated-state behavior, same-ID cross-agent bare-global isolation, old-event/new-generation safety, and active-ask cancellation. After current-main integration, 90 cases across four complete observer/companion/lifecycle/event suites, owning gateway test types, targeted lint and format passed on the final candidate. Its uncachedciArtifactsruntime/UI build passed in 124.76 seconds, with exact source, lock, build stamps and full artifact inventories verified.The live proof uses qualified agent/session keys and a synthetic loopback Responses provider, not a hosted model. Same-ID collisions are regression-test evidence because public session creation cannot choose backing IDs.
A separate existing limitation remains: Side chat asks against a bare
globalpolicy key lose their independent policy owner before provider dispatch, raisingAGENT_SELECTION_REQUIRED. That failed attempt performed no deletion and predates this repair. This PR does not repair or bypass that policy boundary, claim successful global end-to-end proof, or change security/persistence contracts to hide it. The global owner-propagation follow-up requires the relevant security owner.Initial CI 34023960500 caught a new upstream observer reset fixture that omitted the event’s required agent scope. The repair patch rebased identically, and one fixture line now supplies its existing
mainagent. Production behavior was not changed to accommodate the test. The focused checks, full-candidate review, uncached build and qualified live flow above were refreshed before this push; earlier proof remains separately identified.attachment-2004965384.webm
Final CI 34027358506 passed on the exact published candidate. Current-main composition was checked at
b72a36a77d1b9fe5b7acb4e94042fddf827d7e40: the core cleanup and event producers are unchanged; disjoint documentation, repository-fork metadata, and added lifecycle-test changes combine cleanly. The recent ClawSweeper review has no actionable findings or before-merge items.