feat: add resume command for cross-slot session picker#106
Open
Andres-Briones wants to merge 5 commits into
Open
feat: add resume command for cross-slot session picker#106Andres-Briones wants to merge 5 commits into
Andres-Briones wants to merge 5 commits into
Conversation
Interactive fzf-based session picker that scans all slots, detects active sessions via PID checks, and resumes in the correct slot. Handles busy slots by copying the session to an idle slot.
Reviewer's GuideImplements a new host-only Flow diagram for _cmd_resume logic and status handlingflowchart TD
A_start[Start_claudebox_resume] --> B_parse
B_parse[Parse_flags_-n_-a_-A_-h] --> C_deps
C_deps[Check_dependencies_fzf_jq_docker] -->|missing| Z_err_no_deps[Print_error_and_exit]
C_deps -->|ok| D_scan_root
D_scan_root[Locate_projects_dir_~/.claudebox/projects] -->|not_found| Z_no_projects[Print_no_projects_and_exit]
D_scan_root -->|found| E_validate_project
E_validate_project[Validate_current_project_unless_-A] -->|invalid| Z_not_in_project[Print_use_-A_and_exit]
E_validate_project -->|ok| F_build_desc
F_build_desc[Scan_history_jsonl_for_descriptions] --> G_scan_sessions
G_scan_sessions[Enumerate_projects_and_slots_via_crc32_chain] --> H_scan_slot_ws
H_scan_slot_ws[Scan_workspace_jsonl_files] --> I_filter_and_collect
I_filter_and_collect[Filter_small_files_and_subagents_collect_metadata] --> J_status
J_status[Determine_running_and_active_status] --> K_build_lists
K_build_lists[Write_sessions_titles_descriptions_TSV] --> L_limit
L_limit[Sort_by_mtime_dedup_sessionIds_apply_limit_or_all] -->|no_sessions| Z_no_sessions[Print_no_sessions_and_exit]
L_limit -->|has_sessions| M_build_fzf_input
M_build_fzf_input[Format_fzf_rows_with_colors_and_status] -->|empty| Z_no_sessions
M_build_fzf_input --> N_run_fzf
N_run_fzf[Invoke_fzf_picker] -->|esc_or_none| Z_exit_ok[Exit_success_no_selection]
N_run_fzf -->|selection| O_extract
O_extract[Extract_sessionId_slot_hash_slot_idx_project_path_status] --> P_status_branch
P_status_branch -->|ACTIVE| Q_active
P_status_branch -->|RUNNING_busy| R_busy
P_status_branch -->|IDLE| S_idle
Q_active[Print_active_warning_do_not_resume] --> Z_active_exit[Exit_with_error]
R_busy[Find_idle_slot_copy_session_jsonl] -->|no_idle| Z_no_idle[Print_no_idle_slots_and_exit]
R_busy -->|idle_found| T_prepare_resume_busy[Update_slot_idx_to_idle]
S_idle[Prepare_resume_in_selected_slot] --> T_prepare_resume_idle
T_prepare_resume_busy --> U_env
T_prepare_resume_idle --> U_env
U_env[Set_PROJECT_env_and_container_names] --> V_run_container
V_run_container[run_claudebox_container_interactive_--resume_sessionId] --> W_attach[User_attached_to_container]
W_attach --> X_end[End]
Z_err_no_deps --> X_end
Z_no_projects --> X_end
Z_not_in_project --> X_end
Z_no_sessions --> X_end
Z_exit_ok --> X_end
Z_active_exit --> X_end
Z_no_idle --> X_end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- When
-Ais not used,_cmd_resumeassumesPROJECT_DIRis set and valid forgenerate_parent_folder_name, but there’s no explicit guard for being outside a project directory; consider failing fast with a clear error ifPROJECT_DIRis unset or invalid instead of relying on upstream state. - The CLI marks
resumeas having"none"requirements inget_command_requirements, but_cmd_resumedepends on a working Docker daemon (fordocker psanddocker exec); aligning the requirement classification with this behavior will keep the CLI’s expectations consistent.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- When `-A` is not used, `_cmd_resume` assumes `PROJECT_DIR` is set and valid for `generate_parent_folder_name`, but there’s no explicit guard for being outside a project directory; consider failing fast with a clear error if `PROJECT_DIR` is unset or invalid instead of relying on upstream state.
- The CLI marks `resume` as having `"none"` requirements in `get_command_requirements`, but `_cmd_resume` depends on a working Docker daemon (for `docker ps` and `docker exec`); aligning the requirement classification with this behavior will keep the CLI’s expectations consistent.
## Individual Comments
### Comment 1
<location path="lib/commands.resume.sh" line_range="97" />
<code_context>
+
+ while [ $# -gt 0 ]; do
+ case "$1" in
+ -n) limit="$2"; shift 2 ;;
+ -a) show_all=true; shift ;;
+ -A|--all-projects) all_projects=true; shift ;;
</code_context>
<issue_to_address>
**issue:** Validate the numeric argument to -n before using it as a line limit.
If `-n` is passed without a value or with a non-numeric value, `limit` may be empty/invalid and `head -n "$limit"` will error or behave unexpectedly. Please validate that `$2` exists and is numeric (e.g. `^[0-9]+$`) before assigning, and exit with a clear error message otherwise.
</issue_to_address>
### Comment 2
<location path="lib/commands.resume.sh" line_range="386-387" />
<code_context>
+ local date_str size_h title desc display status status_display
+ date_str=$(_resume_format_date "$mtime")
+ size_h=$(_resume_human_size "$size")
+ title=$(grep "^${sid} " "$titles_file" 2>/dev/null | head -1 | cut -f2- || true)
+ desc=$(grep "^${sid} " "$desc_file" 2>/dev/null | head -1 | cut -f2- || true)
+
+ if [ -n "$title" ]; then
</code_context>
<issue_to_address>
**issue (bug_risk):** Quote or otherwise sanitize `sid` when using it in grep patterns.
Here `sid` is used as a regex. If it contains metacharacters (e.g. `.`, `[]`), the match may be wrong. To avoid this, either use `grep -F` for fixed-string matching or escape regex metacharacters in `sid` before building the pattern.
</issue_to_address>
### Comment 3
<location path="lib/commands.resume.sh" line_range="460-469" />
<code_context>
+ local resume_parent_dir="$projects_dir/$(generate_parent_folder_name "$project_path")"
</code_context>
<issue_to_address>
**issue (bug_risk):** Check that the source session file exists before copying in the RUNNING case.
In the RUNNING branch you construct `source_jsonl` and then always `cp` it into the idle slot. If that file was deleted or never created (e.g., manual cleanup, partial write), `cp` will fail and the resume will continue with an incomplete session. Add a check that `$source_jsonl` exists (and ideally is non-empty) before copying, and abort the resume with a clear error if it is missing or invalid.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Claudebox passes control flags (e.g. --dangerously-skip-permissions) to all command handlers. The resume parser now silently skips them instead of erroring.
- Validate -n argument is numeric before using as limit - Guard PROJECT_DIR when not using -A (clear error if outside project) - Use grep -F for session ID lookups (avoid regex metachar issues) - Check source .jsonl exists before copying in busy-slot case
Author
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Consider breaking
_cmd_resumeinto smaller helper functions (e.g., argument parsing, project/slot enumeration, session collection, fzf rendering, resume execution) to improve readability and maintainability of this ~500-line function. - You’re manually calling
statin multiple places to get file sizes and mtimes; it may be more robust and consistent to centralize the cross-platformstatlogic into a shared helper (similar to_resume_get_mtime) and reuse it for size as well. - Since
uname -sis called repeatedly in_resume_get_mtimeand_resume_format_date, you could cache the OS type once in a variable to avoid redundant process invocations on every file processed.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider breaking `_cmd_resume` into smaller helper functions (e.g., argument parsing, project/slot enumeration, session collection, fzf rendering, resume execution) to improve readability and maintainability of this ~500-line function.
- You’re manually calling `stat` in multiple places to get file sizes and mtimes; it may be more robust and consistent to centralize the cross-platform `stat` logic into a shared helper (similar to `_resume_get_mtime`) and reuse it for size as well.
- Since `uname -s` is called repeatedly in `_resume_get_mtime` and `_resume_format_date`, you could cache the OS type once in a variable to avoid redundant process invocations on every file processed.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
- Cache uname -s once in _RESUME_PLATFORM module variable - Add _resume_get_size helper for cross-platform file size - Extract _resume_build_descriptions for history.jsonl parsing - Extract _resume_discover_sessions and _resume_scan_workspace - Extract _resume_build_fzf_input for display line construction - Extract _resume_execute for resume logic (active/busy/idle) - _cmd_resume is now a thin orchestrator calling the phases No behavioral changes — pure refactoring for readability.
Header at top, most recent sessions first, cursor starts on newest.
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
Adds
claudebox resume— an interactive session picker (viafzf) that lets users resume any session from any slot without copy-pasting session IDs.The problem
Claudebox isolates
~/.claude/per slot. When a slot switches,/resumeonly sees sessions from the current slot — making sessions from other slots invisible. Users have to manually find and copy.jsonlfiles between slots.How it works
The command runs entirely on the host:
~/.claudebox/projects/using the CRC32 chain to enumerate slot directoriescustom-titlefrom.jsonl) and descriptions (first user message fromhistory.jsonl)docker ps) and verifying active PIDs (docker exec kill -0)fzfpicker with date, size, status, project, slot number, and descriptionrun_claudebox_containerin the correct slotSession statuses
.jsonlto an idle slot, resumes there--resumeUsage
Requirements
fzf— fuzzy finder for interactive selectionjq— JSON processing for session metadataChanges
lib/commands.resume.sh—_cmd_resume()implementationlib/cli.sh— addedresumetoSCRIPT_COMMANDSand"none"requirementslib/commands.sh— source, dispatch, and help textDesign decisions
"none"(pure host command) — it only needs Docker at the end when launching the containerrun_claudebox_containerdirectly (same pattern as_cmd_slot)_resume_to avoid namespace collisionsif/then(not&&) per the project'sset -econventionsstat/datedifferencesTest plan
claudebox resumefrom a project directory with multiple slots-Aflag shows sessions from multiple projectsSummary by Sourcery
Add a host-only interactive resume command that lets users pick and resume sessions across slots and projects via an fzf-based picker.
New Features:
Enhancements: