feat: add session thread management#98510
Conversation
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: 56ef85cb06
ℹ️ 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".
| if (showArchived) { | ||
| params.archived = true; |
There was a problem hiding this comment.
When the operator has toggled the Sessions page to the archived-only view, createChatSessionsLoadOverrides(state) used by Chat controls still carries sessionsShowArchived: true into loadSessions via refreshSessionOptions. With this new params.archived = true branch, changing the chat model/thinking/fast mode refreshes the global session list with only archived rows, dropping the active chat session metadata from state.sessionsResult and breaking current-session controls/sidebar state until another active reload happens. Force chat refreshes to request active sessions instead of inheriting the Sessions-page archived filter.
Useful? React with 👍 / 👎.
| @click=${(event: MouseEvent) => { | ||
| event.stopPropagation(); | ||
| props.onPatch(row.key, { archived: row.archived !== true }); |
There was a problem hiding this comment.
Archiving from the Sessions table now just calls onPatch, and the app-level handler delegates directly to patchSession without the fallback logic used by the chat session picker. If this row is the current chat session, the row disappears from active listings but state.sessionKey still points at the archived thread, so returning to Chat can continue a hidden archived session instead of switching back to the agent main session as the new docs promise.
Useful? React with 👍 / 👎.
|
Codex review: needs changes before merge. Reviewed July 4, 2026, 2:29 PM ET / 18:29 UTC. Summary Reproducibility: yes. by source inspection: call sessions.patch with archived or pinned for a missing key; applySessionEntryPatchProjection supplies no existingEntry, projectSessionsPatchEntry synthesizes a SessionEntry, and the management branch writes it. I did not run tests because review is read-only. Review metrics: 1 noteworthy metric.
Stored data model Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Mantis proof suggestion Risk before merge
Maintainer options:
Copy recommended automerge instructionNext step before merge
Security Review findings
Review detailsBest possible solution: Reject missing-row archive/restore and pin/unpin mutations while preserving existing non-management patch adoption, then have maintainers explicitly accept the broad Gateway protocol, generated-client, UI, and Codex lifecycle surface. Do we have a high-confidence way to reproduce the issue? Yes by source inspection: call sessions.patch with archived or pinned for a missing key; applySessionEntryPatchProjection supplies no existingEntry, projectSessionsPatchEntry synthesizes a SessionEntry, and the management branch writes it. I did not run tests because review is read-only. Is this the best way to solve the issue? No. The feature direction is plausible and mostly in the right owner boundary, but the current patch is not the best merge shape until missing row-management mutations fail closed. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 6da5db21a633. Label changesLabel justifications:
Evidence reviewedAcceptance 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
Review history (5 earlier review cycles)
|
clawsweeper
Bot
added
rating: 🧂 unranked krab
clawsweeper
Bot
added
proof: sufficient
56ef85c to
a73046d
Compare
openclaw-barnacle
Bot
added
channel: voice-call
github-actions
Bot
added
the
dependencies-changed
Dependency graph guard clearedThis PR no longer has blocked dependency graph changes. A future dependency graph change requires a fresh
|
40d0b0f to
c53ebb7
Compare
openclaw-barnacle
Bot
removed
channel: discord
Squash of codex/thread-management (025aefc3ad1) onto origin/main: pin/archive/rename sessions via sessions.patch, archived-aware sessions.list, lifecycle fencing, read-only archived chat, SDK + Swift protocol support, Control UI session management.
Chat picker and sidebar recents share session-row primitives: single-line rows, relative timestamps, rename/archive/pin revealed on hover or focus, accent pin badge for pinned rows, and an active-run spinner in the trail slot. Sidebar floats pinned sessions above recency via the shared comparator and gains archive/pin actions through the unified sessions-view patch fallback. Archive eligibility is one shared policy (canArchiveSessionRow); the sidebar/picker active-run tooltip now uses the real sessionsView.activeRun locale key.
Integration fixes after rebasing onto current main: sessions_list mailbox test expectations learn the archived/pinned row fields and archived:false list param; gateway agent admission treats a session as deleted only when both the requested and canonical alias sets miss it (legacy bare-main stores and exec-approval followups read under different spellings); cron persist tests keep a consistent store across claim-guarded persist calls; the ACP abort hook test asserts abort propagation instead of signal identity; drop dead lifecycle writes flagged by no-useless-assignment and fix the promise-executor return in the codex compact test.
Sidebar session rows are wrapper divs with an inner link now: update the navigation browser tests and chat-flow Playwright selectors. Seed a real per-test session store for the auto-fallback admission guard instead of depending on leftover host files at /tmp/sessions.json. Teach the test-projects routing fixture about the suites that newly import the shared temp-dir helper. Document the Codex thread-format contract for archivedAt/pinnedAt (flag derived from server-stamped timestamp, epoch ms here vs Codex epoch seconds) at the type and in the session docs.
The auto-fallback suite now imports the shared temp-dir helper for its seeded session store, so the top-level helper routing fixture must list it in the auto-reply plan.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3da41b0eb4
ℹ️ 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".
| if (!result.compacted) { | ||
| const details = result.result?.details; | ||
| if (details?.pending === true || details?.signal === "thread/compact/start") { | ||
| if (details?.pending === true) { |
There was a problem hiding this comment.
When a newer CLI talks to an older Gateway/Codex runtime that still returns the previous accepted-async shape (ok:true, compacted:false, result.details.signal:"thread/compact/start" without pending), this now falls through to the no-op message. That makes openclaw sessions compact report “No compaction needed” even though native Codex compaction was started; keep recognizing the old signal marker alongside the new terminal response shape.
Useful? React with 👍 / 👎.
1 participant
Related: #54397
Closes #88568
AI-assisted: yes. Implementation and redesign were reviewed with the repository's structured autoreview workflow.
What Problem This Solves
OpenClaw sessions are durable conversation threads, but their management controls were fragmented: frequent sessions disappear into recency ordering, stale sessions stay in every active list unless deleted, and renaming was disconnected from the Chat surfaces. The first iteration of this PR added the controls but rendered three always-visible bordered buttons on every picker row, which crushed session labels and made the picker read as a toolbar instead of a list.
Why This Change Was Made
Pin, archive/restore, and rename land on the existing session model instead of a second topic store:
sessions.listseparates active and archived sessions,sessions.patchowns management mutations, and livesessions.changedevents keep every subscribed UI coherent. Archived chat sessions become read-only, lifecycle fencing keeps concurrent runs/compaction from resurrecting archived state, and the SDK plus Swift protocol expose the same contracts.The management UI is redesigned to be minimal, following the pattern of modern thread lists (e.g. the Codex app):
:focus-withinkeeps them keyboard-reachable); the pin action doubles as the accent pinned badge, so it stays visible for pinned rows.canArchiveSessionRow) protects agent main sessions, live runs, and global/unknown scopes across the sidebar, picker, and Sessions table.User Impact
Users can now:
sessions_list.Agent main sessions and sessions with active runs cannot be archived. Archiving clears the pin, archived chat sessions are read-only, and archiving the selected session from any surface returns the UI to that agent's main session.
Before / After
Chat session picker — before (always-visible bordered buttons, truncated labels) vs after (minimal rows, hover-revealed actions, pinned badge, run spinner):
Sidebar recents — pinned badge on Release planning, run spinner on Data migration, hover-revealed archive/pin on Research notes:
Evidence
OPENCLAW_CAPTURE_UI_PROOF=1 node scripts/run-vitest.mjs run --config test/vitest/vitest.ui-e2e.config.ts --configLoader runner ui/src/ui/e2e/session-management.e2e.test.ts— real Chromium GUI flow covering pinned-first sidebar ordering, run spinner in both surfaces, hover/focus reveal (computed-opacity assertions), pinned badge without hover, main-session archive protection, and rename/pin/archive patch round-trips. Screenshots above are captured by this run.node scripts/run-vitest.mjs run ui/src/ui/views/chat.test.ts ui/src/ui/views/sessions.test.ts ui/src/ui/controllers/sessions.test.ts ui/src/ui/app-render.helpers.node.test.ts ui/src/ui/app-chat.test.ts— 440 tests passed.pnpm test src/gateway/sessions-patch.test.ts src/gateway/session-utils.test.ts src/gateway/server.sessions.store-rpc.test.ts src/gateway/server.sessions.delete-lifecycle.test.ts src/gateway/server.sessions.list-changed.test.ts src/agents/tools/sessions-list-tool.test.ts.pnpm tsgo:coreandpnpm tsgo:test:ui— passed.pnpm build— passed, including the Control UI production bundle; no[INEFFECTIVE_DYNAMIC_IMPORT]warnings.sessions.sessionDetails.activeRun); both surfaces now usesessionsView.activeRun.main: gateway agent admission now treats a session as deleted only when both the requested and canonical alias sets miss it — a canonical-only re-read misreported legacy bare-mainstores as deleted, while a requested-only read broke exec-approval followups (src/gateway/server-methods/agent.ts);sessions_listmailbox test expectations learned thearchived/pinnedrow fields; cron persist tests keep one consistent store across claim-guarded persist calls; dead lifecycle writes flagged byno-useless-assignmentremoved.