Add worktree-backed workspace lifecycle scripts - #24
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds workspace rootWorkingDirectory throughout models, persistence, workspace creation, and AppleScript snapshots; introduces an AppleScript "delete workspace" command and controller delete(workspace:) path; and adds new shell scripts and common utilities for worktree creation, bootstrap, and deletion workflows. Changes
Sequence Diagram(s)sequenceDiagram
actor AppleScriptCaller
participant DeleteCmd as "DeleteWorkspaceScriptCommand"
participant Controller as "ShellraiserScriptingController"
participant Manager as "WorkspaceManager"
participant Persistence as "Workspace Persistence"
AppleScriptCaller->>DeleteCmd: performDefaultImplementation()
DeleteCmd->>DeleteCmd: resolvedWorkspace()
alt workspace resolved
DeleteCmd->>Controller: delete(workspace:)
Controller->>Manager: deleteScriptWorkspace(id:)
Manager->>Persistence: remove workspace by id
Persistence-->>Manager: success/failure
Manager-->>Controller: Bool
Controller-->>DeleteCmd: Bool
alt deletion succeeded
DeleteCmd-->>AppleScriptCaller: return nil (success)
else deletion failed
DeleteCmd->>AppleScriptCaller: set scriptErrorNumber & scriptErrorString
end
else workspace not resolved
DeleteCmd-->>AppleScriptCaller: return workspace argument error
end
sequenceDiagram
actor User
participant DeleteScript as "workspace-delete.sh"
participant Common as "workspace-shellraiser-common.sh"
participant Git as "Git"
participant AppleScript as "Shellraiser AppleScript"
User->>DeleteScript: invoke delete workflow
DeleteScript->>Common: resolve repo, workspace, worktree info
Common->>Git: query worktree & branch status
Git-->>Common: worktree_root, branch_name, status
DeleteScript->>Common: confirm_with_user? (if required)
DeleteScript->>Common: delete_shellraiser_workspace_for_root(worktree_root)
Common->>AppleScript: invoke DeleteWorkspaceScriptCommand for matching workspace
AppleScript-->>Common: deletion result
DeleteScript->>Git: remove worktree / delete branch (maybe force)
DeleteScript-->>User: print summary of Shellraiser, worktree, branch statuses
Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/workspace-claude`+lazygit.sh:
- Around line 13-16: prepare_workspace_worktree() currently hides whether the
worktree was just created so bootstrap_ghostty_for_worktree() is never called
for new worktrees; modify the launcher to use the status-bearing variant of
prepare_workspace_worktree (or otherwise capture its creation status) and, after
unpacking its return values (worktree_root and branch_name), call
bootstrap_ghostty_for_worktree "$worktree_root" only when the helper indicates
the worktree was newly created, then proceed to
run_shellraiser_workspace_with_commands "$worktree_root" "$workspace_name"
"claude" "lazygit".
In `@scripts/workspace-codex`+lazygit.sh:
- Around line 13-16: prepare_workspace_worktree no longer surfaces whether a
worktree was created, so bootstrap_ghostty_for_worktree never runs for
first-time worktrees; update the call to capture the creation status and invoke
the bootstrap when appropriate. Change the read to IFS=$'\t' read -r
worktree_root branch_name created_status < <(prepare_workspace_worktree
"$repo_root" "$workspace_name"), then after the null-checks add: if [[
"$created_status" == "created" ]]; then bootstrap_ghostty_for_worktree
"$worktree_root"; fi, keeping the subsequent call to
run_shellraiser_workspace_with_commands unchanged (references:
prepare_workspace_worktree, bootstrap_ghostty_for_worktree,
run_shellraiser_workspace_with_commands).
In `@scripts/workspace-delete.sh`:
- Around line 34-35: The guard currently only prevents deleting the primary repo
worktree; add an explicit check that the script is not being run from the
worktree being deleted by comparing the current working directory (PWD or
$(pwd)) to expected_worktree_path and invocation_repo_root and refuse early
using fail_with_message if they match; update the conditional around
expected_worktree_path/main_repo_root/invocation_repo_root (or add a separate
if) so the script exits before attempting to remove the worktree when the
invocation worktree is the target.
In `@scripts/workspace-shellraiser-common.sh`:
- Around line 447-459: The launcher currently always runs the AppleScript 'new
workspace named workspaceName with configuration config', which creates
duplicate Shellraiser workspaces and breaks
delete_shellraiser_workspace_for_root(); change the logic to first check for an
existing workspace whose initial working directory equals workspaceRoot (the
same attribute delete_shellraiser_workspace_for_root() uses) and if found
reuse/select that workspace instead of creating a new one, otherwise proceed to
create; alternatively, if reuse isn't possible implement an early failure with a
clear error when a matching workspace already exists so
delete_shellraiser_workspace_for_root() remains able to target the session.
In `@scripts/worktree-create.sh`:
- Around line 11-26: Replace the fragile use of $0 in the zsh branch that sets
shellraiser_script_path: detect zsh via ZSH_VERSION and ZSH_EVAL_CONTEXT as
already done, and when in zsh set shellraiser_script_path using the zsh-specific
current-file expansion (${(%):-%x}) so that shellraiser_script_sourced is
derived from a reliable sourced-file path; keep the existing bash behavior and
the rest of the script unchanged (symbols: shellraiser_script_path, ZSH_VERSION,
ZSH_EVAL_CONTEXT, shellraiser_script_sourced).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4943a40c-039a-4898-80cf-95d006034288
📒 Files selected for processing (16)
Sources/Shellraiser/Infrastructure/AppleScript/ScriptableTerminal.swiftSources/Shellraiser/Infrastructure/AppleScript/ShellraiserScriptCommands.swiftSources/Shellraiser/Infrastructure/AppleScript/ShellraiserScripting.xmlSources/Shellraiser/Infrastructure/AppleScript/ShellraiserScriptingController.swiftSources/Shellraiser/Infrastructure/AppleScript/WorkspaceManager+AppleScript.swiftSources/Shellraiser/Models/WorkspaceModel.swiftSources/Shellraiser/Services/Workspaces/WorkspaceCatalogManager.swiftSources/Shellraiser/Services/Workspaces/WorkspaceManager+WorkspaceLifecycle.swiftTests/ShellraiserTests/AgentTypeAndWorkspaceModelTests.swiftTests/ShellraiserTests/WorkspaceManagerAppleScriptTests.swiftTests/ShellraiserTests/WorkspaceTestSupport.swiftscripts/workspace-claude+lazygit.shscripts/workspace-codex+lazygit.shscripts/workspace-delete.shscripts/workspace-shellraiser-common.shscripts/worktree-create.sh
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripts/workspace-shellraiser-common.sh (1)
60-61:⚠️ Potential issue | 🟠 MajorReplace bash-only
read -pusage in the shared prompt helpers.The
scripts/worktree-create.shscript detects and supports both bash and zsh (lines 3-22), then sourcesscripts/workspace-shellraiser-common.shunconditionally. However,workspace-shellraiser-common.shusesread -r -pfor prompts, which is bash-specific. In zsh,read -preads from the coprocess instead of displaying a prompt, causing interactive flows to fail.Use portable prompt form instead:
💡 Portable prompt form
- if ! read -r -p "Workspace name: " workspace_name; then + printf '%s' "Workspace name: " >&2 + if ! read -r workspace_name; then fail_workspace_name_required fi ... - if ! read -r -p "Worktree name: " worktree_name; then + printf '%s' "Worktree name: " >&2 + if ! read -r worktree_name; then fail_worktree_name_required fi ... - if ! read -r -p "$prompt_message [y/N] " response; then + printf '%s' "$prompt_message [y/N] " >&2 + if ! read -r response; then return 1 fiAlso applies to: lines 79-80, 312-313
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/workspace-shellraiser-common.sh` around lines 60 - 61, The prompt uses bash-only read -r -p syntax (e.g., if ! read -r -p "Workspace name: " workspace_name; then fail_workspace_name_required) which breaks in zsh; replace each occurrence with the portable form: print the prompt with printf (e.g., printf '%s' "Workspace name: ") then call read -r workspace_name and keep the same error handling (call fail_workspace_name_required on empty/failure). Update the other similar occurrences (the prompts that set variables referenced in the same file) the same way so all uses of read -r -p are replaced with printf + read -r while preserving the existing variable names and failure functions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/workspace-shellraiser-common.sh`:
- Around line 221-225: The code derives worktree_path from the caller-provided
repo_root which makes paths dependent on linked worktrees; instead canonicalize
to the main repo root before computing the worktree path by calling
resolve_main_repo_root (or equivalent) and using that result when invoking
resolve_workspace_worktree_path and any subsequent logic that expects the main
repo root; update the local variable usage so branch_name is still from
slugify_workspace_name but worktree_path is computed with the canonical main
repo root returned by resolve_main_repo_root rather than the original repo_root.
In `@scripts/worktree-create.sh`:
- Around line 11-12: Add a narrow ShellCheck suppression for SC2296 around the
zsh-only expansion so the bash-shebang lint doesn't complain: locate the branch
that sets shellraiser_script_path using the zsh expansion ("${(%):-%x}") in the
if [[ -n "${ZSH_VERSION:-}" ]]; then block and add a single-line or immediately
preceding directive to disable SC2296 (e.g. a comment "# shellcheck
disable=SC2296") only for that line, keeping the suppression minimal and scoped
to that zsh-specific assignment.
---
Outside diff comments:
In `@scripts/workspace-shellraiser-common.sh`:
- Around line 60-61: The prompt uses bash-only read -r -p syntax (e.g., if !
read -r -p "Workspace name: " workspace_name; then fail_workspace_name_required)
which breaks in zsh; replace each occurrence with the portable form: print the
prompt with printf (e.g., printf '%s' "Workspace name: ") then call read -r
workspace_name and keep the same error handling (call
fail_workspace_name_required on empty/failure). Update the other similar
occurrences (the prompts that set variables referenced in the same file) the
same way so all uses of read -r -p are replaced with printf + read -r while
preserving the existing variable names and failure functions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1f1b8cb4-4ceb-4deb-a715-460d9e9def02
📒 Files selected for processing (5)
scripts/workspace-claude+lazygit.shscripts/workspace-codex+lazygit.shscripts/workspace-delete.shscripts/workspace-shellraiser-common.shscripts/worktree-create.sh
🚧 Files skipped from review as they are similar to previous changes (1)
- scripts/workspace-claude+lazygit.sh
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/workspace-shellraiser-common.sh`:
- Around line 35-56: In both resolve_main_repo_root and
prepare_workspace_worktree_with_status, add an immediate "return 1" after every
call to fail_with_message so the function exits on error instead of continuing;
specifically update the fail_with_message occurrences inside
resolve_main_repo_root (the blocks that fail to resolve common_git_dir or
canonical repo root) and the five fail_with_message calls inside
prepare_workspace_worktree_with_status so each is followed by return 1 to ensure
the function halts and propagates failure.
- Around line 101-117: slugify_workspace_name currently returns slugs that can
be rejected by Git (e.g., containing ".." or ending with ".lock"); after
producing the initial $slug in slugify_workspace_name, validate it with git
check-ref-format --branch "$slug" and if it fails, sanitize further by
collapsing consecutive dots (replace '\.\.+' with '.'), remove any trailing
".lock" suffix and trim leading/trailing dots or dashes, then re-run git
check-ref-format --branch; if it still fails, call fail_with_message with a
clear error that the workspace name cannot be converted to a Git-safe slug.
Ensure you operate on the same local variables (slug, workspace_name) and
preserve the existing printf '%s\n' "$slug" return behavior when validation
passes.
In `@scripts/worktree-create.sh`:
- Around line 58-64: Summary: command substitutions for resolve_repo_root and
resolve_worktree_name (and the process substitution invoking
prepare_workspace_worktree_with_status) can swallow failures when script is
sourced; add explicit failure propagation. Edit the lines invoking
resolve_repo_root() and resolve_worktree_name() to append "|| return 1" after
each command substitution (the assignments to repo_root and worktree_name) and
also append "|| return 1" after the process substitution that reads from
prepare_workspace_worktree_with_status so that failures from resolve_repo_root,
resolve_worktree_name, or prepare_workspace_worktree_with_status immediately
return (rather than producing empty variables); keep references to the same
variable names repo_root, worktree_name, worktree_root, branch_name,
worktree_status and the existing fail_with_message call.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 79cfe564-b7fe-4fe8-bd1b-c39ab9aed581
📒 Files selected for processing (2)
scripts/workspace-shellraiser-common.shscripts/worktree-create.sh
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/workspace-shellraiser-common.sh`:
- Around line 316-341: The function currently calls fail_with_message (which
only returns a non-zero status when the script is sourced) but then continues
execution; update the error paths to stop further work by adding an explicit
return 1 immediately after each fail_with_message invocation: specifically after
the submodule init failure (the block that runs git submodule update --init
--recursive against $worktree_root), after the check that [[ ! -d
"$ghostty_root" ]], after the command -v zig check, and after the zig build
failure block (the zig build invocation against $ghostty_root) so the function
exits early on each detected failure.
- Around line 254-259: The helper results are not checked so failures produce
empty variables and allow continuation; after assigning main_repo_root,
branch_name, and worktree_path (from resolve_main_repo_root,
slugify_workspace_name, resolve_workspace_worktree_path) verify each command
succeeded and abort (log error and exit/return non‑zero) if any is empty or the
helper returned non‑zero, and likewise check the return value of
ensure_local_branch_exists and abort on failure; update the logic around
repo_root/main_repo_root/branch_name/worktree_path and
ensure_local_branch_exists to propagate errors instead of silently continuing.
- Around line 396-407: The safety and error-return logic in the worktree removal
block is incorrect: when worktree_path == main_repo_root you must exit the
function immediately (add a return 1 after the fail_with_message) to prevent
removing the primary repo, and when invoking git -C "$repo_root" worktree remove
"$worktree_path" ensure the function only returns success if the command
actually succeeded (do not unconditionally return 0 after calling
fail_with_message on failure); change the flow around the git worktree remove
call so that on failure you call fail_with_message and return a non-zero status,
and only return 0 when the git command succeeds (referencing the existing
symbols fail_with_message, registered_worktree_exists, worktree_is_clean and the
git -C "$repo_root" worktree remove invocation).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7c4d17d8-b377-4559-bfef-688a27225ae6
📒 Files selected for processing (2)
scripts/workspace-shellraiser-common.shscripts/worktree-create.sh
🚧 Files skipped from review as they are similar to previous changes (1)
- scripts/worktree-create.sh
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripts/workspace-shellraiser-common.sh (1)
65-82:⚠️ Potential issue | 🟠 MajorWrite prompts to stderr and return error status explicitly.
Lines 71 and 91 write prompts to stdout, which are captured alongside the input value when these helpers are called with
$(...)command substitution inscripts/worktree-create.shand the launcher scripts. Additionally, both helpers fall through to their finalprintfeven after the error handlers, converting a failed validation into exit status 0.Add
>&2to prompt output and explicitreturn 1after each error path:Suggested fix
- printf '%s' "Workspace name: " + printf '%s' "Workspace name: " >&2 if ! read -r workspace_name; then fail_workspace_name_required + return 1 fi @@ if [[ -z "${workspace_name//[[:space:]]/}" ]]; then fail_workspace_name_required + return 1 fi @@ - printf '%s' "Worktree name: " + printf '%s' "Worktree name: " >&2 if ! read -r worktree_name; then fail_worktree_name_required + return 1 fi @@ if [[ -z "${worktree_name//[[:space:]]/}" ]]; then fail_worktree_name_required + return 1 fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/workspace-shellraiser-common.sh` around lines 65 - 82, In resolve_workspace_name(), change the prompt printf to write to stderr (add >&2) so the "Workspace name: " prompt isn't captured by command substitution, and after each error path (the read failure branch that calls fail_workspace_name_required and the empty-name validation branch) ensure the function returns a non-zero status (e.g., return 1) so it doesn't fall through to the final printf; keep the final printf only for the success path. Reference: resolve_workspace_name, the prompt printf and the two error branches that currently call fail_workspace_name_required.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/workspace-shellraiser-common.sh`:
- Around line 315-321: The guard calling fail_with_message when IFS read from
prepare_workspace_worktree_with_status yields empty values doesn't stop
execution, so add an explicit non-zero exit after the failure: after the call to
fail_with_message in the branch that checks [[ -n "$worktree_path" && -n
"$branch_name" && -n "$worktree_status" ]], follow it with a return 1 (or exit 1
if this is a top-level script) so the function that uses
prepare_workspace_worktree_with_status, the read into
worktree_path/branch_name/worktree_status, and the subsequent printf won't
execute on error; ensure the change is made where those symbols appear
(prepare_workspace_worktree_with_status, fail_with_message, printf).
- Around line 492-496: The function currently calls fail_with_message
"Shellraiser workspace deletion failed for '$workspace_root'." but continues
execution and prints an unset $result; update the error branch to immediately
exit the function with a non-zero status by adding a return 1 (or equivalent)
right after the fail_with_message call so the subsequent printf '%s\n' "$result"
is not executed on failure; locate the block that contains fail_with_message and
the printf and insert the return 1 directly after the fail_with_message
invocation.
---
Outside diff comments:
In `@scripts/workspace-shellraiser-common.sh`:
- Around line 65-82: In resolve_workspace_name(), change the prompt printf to
write to stderr (add >&2) so the "Workspace name: " prompt isn't captured by
command substitution, and after each error path (the read failure branch that
calls fail_workspace_name_required and the empty-name validation branch) ensure
the function returns a non-zero status (e.g., return 1) so it doesn't fall
through to the final printf; keep the final printf only for the success path.
Reference: resolve_workspace_name, the prompt printf and the two error branches
that currently call fail_workspace_name_required.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9b90dcb0-5203-4ec8-94e4-2a8fc8792da0
📒 Files selected for processing (1)
scripts/workspace-shellraiser-common.sh
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.
Create and reuse per-workspace Git worktrees before launching Shellraiser
sessions, including Ghostty bootstrap for new worktrees. Add a matching
workspace-delete script that removes the Shellraiser workspace, linked
worktree, and branch with safety checks for dirty worktrees and unmerged
branches.
Extend AppleScript support with stable workspace root paths and direct
workspace deletion so scripts can target the correct workspace reliably.
Persist the workspace root directory in the model and add tests covering
backfill, stable root behavior, and scripted deletion.
Summary by CodeRabbit
New Features
CLI / Scripts
Tests