fix(sessions): retire stale direct DM rows after dmScope changes#77880
Merged
fix(sessions): retire stale direct DM rows after dmScope changes#77880
Conversation
Contributor
|
Codex review: needs changes before merge. Summary Reproducibility: yes. as a source-level reproduction for the PR regression: current main can route a binding-owned DM to Real behavior proof Next step before merge Security Review findings
Review detailsBest possible solution: Keep the explicit dry-runnable cleanup flow, but make stale direct-row detection binding-aware so it only retires rows that current routing configuration cannot still own. Do we have a high-confidence way to reproduce the issue? Yes, as a source-level reproduction for the PR regression: current main can route a binding-owned DM to Is this the best way to solve the issue? No, not yet. The explicit cleanup/archive approach is a good direction, but the proposed stale-key test must honor route-binding Full review comments:
Overall correctness: patch is incorrect Acceptance criteria:
What I checked:
Likely related people:
Remaining risk / open question:
Codex review notes: model gpt-5.5, reasoning high; reviewed against 58c706451e1d. |
8f22e8c to
9583e28
Compare
BunsDev
added a commit
that referenced
this pull request
May 6, 2026
Fixes #76957. Restores the Control UI /new hook lifecycle through an explicit sessions.create emitCommandHooks opt-in, preserving hook-free defaults for programmatic parent-session creates. Validation: - pnpm protocol:check - pnpm test src/gateway/server.sessions.reset-hooks.test.ts ui/src/ui/app-render.helpers.node.test.ts - pnpm exec oxlint on touched TS files - pnpm exec oxfmt --check --threads=1 on touched files - git diff --check - OPENCLAW_LOCAL_CHECK=1 OPENCLAW_LOCAL_CHECK_MODE=throttled env NODE_OPTIONS=--max-old-space-size=4096 pnpm check:changed - GitHub PR checks green on 3a446ec - ClawSweeper re-review completed with no blocking findings and security cleared Duplicate triage: - #77376, #77004, and #76967 were superseded closed attempts for #76957 - #77562 is a closed duplicate issue - #77880 mentions #76957 but is not a duplicate of this hook fix
steipete
pushed a commit
that referenced
this pull request
May 6, 2026
Fixes #76957. Restores the Control UI /new hook lifecycle through an explicit sessions.create emitCommandHooks opt-in, preserving hook-free defaults for programmatic parent-session creates. Validation: - pnpm protocol:check - pnpm test src/gateway/server.sessions.reset-hooks.test.ts ui/src/ui/app-render.helpers.node.test.ts - pnpm exec oxlint on touched TS files - pnpm exec oxfmt --check --threads=1 on touched files - git diff --check - OPENCLAW_LOCAL_CHECK=1 OPENCLAW_LOCAL_CHECK_MODE=throttled env NODE_OPTIONS=--max-old-space-size=4096 pnpm check:changed - GitHub PR checks green on 3a446ec - ClawSweeper re-review completed with no blocking findings and security cleared Duplicate triage: - #77376, #77004, and #76967 were superseded closed attempts for #76957 - #77562 is a closed duplicate issue - #77880 mentions #76957 but is not a duplicate of this hook fix (cherry picked from commit 49c4a13)
9583e28 to
dc26825
Compare
github-actions Bot
pushed a commit
to Desicool/openclaw
that referenced
this pull request
May 9, 2026
Fixes openclaw#76957. Restores the Control UI /new hook lifecycle through an explicit sessions.create emitCommandHooks opt-in, preserving hook-free defaults for programmatic parent-session creates. Validation: - pnpm protocol:check - pnpm test src/gateway/server.sessions.reset-hooks.test.ts ui/src/ui/app-render.helpers.node.test.ts - pnpm exec oxlint on touched TS files - pnpm exec oxfmt --check --threads=1 on touched files - git diff --check - OPENCLAW_LOCAL_CHECK=1 OPENCLAW_LOCAL_CHECK_MODE=throttled env NODE_OPTIONS=--max-old-space-size=4096 pnpm check:changed - GitHub PR checks green on 3a446ec - ClawSweeper re-review completed with no blocking findings and security cleared Duplicate triage: - openclaw#77376, openclaw#77004, and openclaw#76967 were superseded closed attempts for openclaw#76957 - openclaw#77562 is a closed duplicate issue - openclaw#77880 mentions openclaw#76957 but is not a duplicate of this hook fix
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.
Summary
openclaw sessions cleanup --fix-dm-scopeto detect stale direct-DM session rows aftersession.dmScopeis returned tomain.retire-dm-scope, removes them only when the operator applies cleanup, and archives their transcript files as.deleted.*instead of silently dropping history.sessions.cleanupRPC, protocol schema, generated Swift mirrors, docs, and changelog.Fixes #47561.
Fixes #45554.
Behavior
When
session.dmScopeismain, the cleanup repair targets old direct-message session key shapes such as:agent:<agent>:direct:<peer>agent:<agent>:<channel>:direct:<peer>agent:<agent>:<channel>:<account>:direct:<peer>It skips the active key and does nothing unless
--fix-dm-scopeis explicitly set. Dry-run mode reports the planned rows without mutating the store.Triage Notes
/new.request failed (502)response, so this PR records the duplicate relationship in the body and closes both issues through the fix.Verification
pnpm test src/config/sessions/store.pruning.integration.test.ts src/commands/sessions-cleanup.test.ts src/cli/program/register.status-health-sessions.test.ts src/gateway/server.sessions.store-rpc.test.tspnpm exec oxlint src/config/sessions/cleanup-service.ts src/commands/sessions-cleanup.ts src/cli/program/register.status-health-sessions.ts src/gateway/protocol/schema/sessions.ts src/gateway/server-methods/sessions.ts src/config/sessions/store.pruning.integration.test.ts src/commands/sessions-cleanup.test.ts src/cli/program/register.status-health-sessions.test.tspnpm exec oxfmt --check --threads=1 CHANGELOG.md docs/cli/sessions.md docs/concepts/session.md src/config/sessions/cleanup-service.ts src/commands/sessions-cleanup.ts src/cli/program/register.status-health-sessions.ts src/gateway/protocol/schema/sessions.ts src/gateway/server-methods/sessions.ts src/config/sessions/store.pruning.integration.test.ts src/commands/sessions-cleanup.test.ts src/cli/program/register.status-health-sessions.test.tsgit diff --checkpnpm protocol:checkpnpm changed:lanes --jsonSecurity
No new network access, credential handling, process execution, dependency, or permission surface. The repair is explicit operator-invoked cleanup of local session-store rows and preserves transcript history by archiving removed row transcripts.