fix: session resume — scan all workspaces, skip current empty session#3216
Open
TheArchitectit wants to merge 2 commits into
Open
fix: session resume — scan all workspaces, skip current empty session#3216TheArchitectit wants to merge 2 commits into
TheArchitectit wants to merge 2 commits into
Conversation
…ce loading Three improvements to the /resume command: 1. /resume latest now skips the current empty session When a new session is created on startup (with 0 messages), /resume latest previously returned that empty session. Now it skips sessions with message_count == 0 and excludes the current session ID via the new exclude_id parameter, so it finds the previous session with actual conversation history. 2. Unified load_session_excluding() replaces load_session_loose() The previous load_session_loose() only handled cross-workspace resume for aliases. The new load_session_excluding() combines the loose workspace validation logic with the exclude_id parameter, simplifying the call chain and ensuring all resume paths skip the current empty session when appropriate. 3. All existing session scanning paths (global root + project-local .claw/sessions/) are already in place from prior commits, and now the exclude_id filter is applied consistently across both local and global session scans. Changes: - session_control.rs: Add resolve_reference_excluding() that delegates from resolve_reference(), adding optional exclude_id filtering for alias references. - session_control.rs: Add latest_session_excluding() that delegates from latest_session(), filtering out excluded session IDs and sessions with 0 messages in both local and global scan paths. - session_control.rs: Add load_session_excluding() that replaces load_session_loose(), combining cross-workspace alias handling with the exclude_id parameter. - main.rs: Add load_session_reference_excluding() that delegates from load_session_reference(), using the new store method. - main.rs: Wire LiveCli::resume_session() to pass the current session ID as the exclude_id so /resume latest skips the current empty session. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
The unified session resolution with load_session_excluding() is a clean approach. I notice CI has 2 failing checks (build + cargo test) — worth investigating before merge. One edge case question: what happens when all sessions have 0 messages (fresh install / all deleted)? A clear error message would be better than silently returning nothing. |
- Fix latest_session_alias_resolves_most_recent_managed_session test:
the test created sessions with 0 messages, which are now filtered out
by the message_count > 0 check in latest_session_excluding(). Updated
the test to call push_user_text() before saving so sessions have
at least one message and are findable by /resume latest.
- Add distinct error message when all sessions are empty (0 messages).
Previously, the same "no managed sessions found" message was returned
whether there were zero sessions or all sessions had 0 messages. Now:
- No sessions at all → "no managed sessions found in {path}. Start
claw to create a session..."
- Sessions exist but all empty → "all sessions are empty (0 messages)
in {path}. This usually means a fresh claw session is running but
no messages have been sent yet. Wait for a response in your other
session, then try --resume latest again."
- Add test for the all-sessions-empty error path.
Addresses reviewer feedback on ultraworkers#3216.
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
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.
Problem
When a user runs
/resume latestfrom an active REPL session, it returns the most recently created session — which is always the empty session just created on startup. The user expects to resume the previous session with actual conversation history, but gets a blank session instead.Additionally, the session loading path had two separate code paths:
load_session_loose()for cross-workspace alias resume andload_session()for explicit references. This split meant the exclude-id logic (needed to skip the current empty session) would need to be duplicated across both paths.Solution
Unified the session loading path into a single
load_session_excluding()method that:exclude_idparameter to skip the current session/resume latestload_session_loose()) with the exclude logic in one methodThe
/resumehandler in the CLI now passes the current session ID as the exclude parameter, so/resume latestalways skips the current empty session and returns the previous session with actual conversation history.Changes
rust/crates/runtime/src/session_control.rsresolve_reference_excluding(reference, exclude_id)— new method; delegates fromresolve_reference(), adds optional exclude_id filtering for alias referenceslatest_session_excluding(exclude_id)— new method; delegates fromlatest_session(), filters out excluded session IDs and sessions with 0 messages in both local and global scan pathsload_session_excluding(reference, exclude_id)— new method; replacesload_session_loose(), combining cross-workspace alias handling with the exclude_id parameterrust/crates/rusty-claude-cli/src/main.rsload_session_reference_excluding(reference, exclude_id)— new function; delegates fromload_session_reference(), uses the new store methodLiveCli::resume_session()— now passesSome(&self.session.id)as the exclude_id so/resume latestskips the current empty sessionDiff verification
No upstream reverts — all changes are additive with refactored delegations.
Testing
clawin a directory with existing sessions/resume latest— should resume the previous session (with messages), not the current empty one/resume latestfrom a different workspace directory — should still find sessions across all workspacescargo test -p runtime session_control— all 16 existing session_control tests pass