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:
WalkthroughAdds a Worktrunk-based workspace workflow: project lifecycle hooks, Zellij layout and session management, a start script that derives branch-specific ports and env vars, helpers to open the web UI, per-user config, environment-driven backend/frontend port wiring, and setup/install helpers. Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer/CLI
participant Start as .config/wt/start.sh
participant Z as Zellij
participant Backend as Uvicorn Backend
participant WebUI as Vite Dev Server
participant Browser as Browser
Dev->>Start: run start.sh (optional --restart)
Start->>Start: detect BRANCH, SESSION_NAME, compute ports, export KILN_*/VITE_API_PORT
Start->>Z: check for existing session
alt session exists and not --restart
Z-->>Start: session present
Start->>Z: attach to session
else create or restart
Start->>Z: create session with layout.kdl
Z-->>Start: session started
Start->>Backend: start uvicorn with KILN_PORT
Start->>WebUI: start npm dev with VITE_API_PORT / KILN_FRONTEND_PORT
end
WebUI->>Browser: developer or `.config/wt/bin/web` opens KILN_WEB_URL
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a robust workspace management system designed to significantly improve the developer experience for parallel development. By integrating Worktrunk for Git worktrees and Zellij for interactive terminal sessions, developers can now easily create, manage, and switch between isolated development environments. Each workspace is configured with dynamic port allocation for backend and frontend services, a dedicated coding agent, and a convenient 'web' command to launch the application in a browser, streamlining the setup and operation of multiple feature branches. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive set of utilities for managing parallel development workspaces using git worktrees and zellij. The new workflow, scripts, and configurations enable developers to work on multiple features concurrently in isolated environments with dedicated ports. The implementation is well-thought-out. I have a couple of suggestions to improve portability and fix a potential issue in one of the scripts.
📊 Coverage ReportOverall Coverage: 91% Diff: origin/main...HEAD
Summary
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.config/wt/bin/web (1)
2-2: Add non-macOS opener fallback forweb.Line 2 hardcodes
open, which is macOS-only and fails on Linux and Windows. The project supports all three platforms. A fallback chain (open/xdg-open/wslview) aligns with existing cross-platform patterns in the codebase and makes this command portable.🛠️ Suggested implementation
#!/usr/bin/env bash -open "${KILN_WEB_URL:-http://localhost:8758}" +URL="${KILN_WEB_URL:-http://localhost:8758}" +if command -v open >/dev/null 2>&1; then + exec open "$URL" +elif command -v xdg-open >/dev/null 2>&1; then + exec xdg-open "$URL" +elif command -v wslview >/dev/null 2>&1; then + exec wslview "$URL" +else + echo "No supported opener found (open, xdg-open, or wslview)." >&2 + exit 1 +fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.config/wt/bin/web at line 2, The script currently calls the macOS-only command "open" to launch KILN_WEB_URL, causing failures on Linux/Windows; update the "web" script to try a cross-platform fallback chain: attempt "open" first, then "xdg-open", then "wslview" (or fail with a clear message), using the existing KILN_WEB_URL variable (KILN_WEB_URL:-http://localhost:8758) and exiting non-zero on total failure; implement this by replacing the single "open" invocation with a conditional/fallback invocation that tries "open || xdg-open || wslview" (or equivalent shell if/command checks) so the script works on macOS, Linux, and WSL/Windows.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.config/wt/README.md:
- Around line 11-14: The README step that writes the global Worktrunk config
uses a destructive redirect (the shown echo to ~/.config/worktrunk/config.toml)
which can overwrite an existing file; change the instructions to avoid
clobbering user configs by either appending only if the file or key is missing
(e.g., use a non-destructive append or test for file/key existence first) or
instruct the user to manually add the line to their existing
~/.config/worktrunk/config.toml. Update the snippet referencing
~/.config/worktrunk/config.toml and the echo/redirect command so it is
non-destructive and only writes the worktree-path setting when it will not
overwrite existing settings.
In @.config/wt/start.sh:
- Line 30: The script currently runs cd "$REPO_ROOT" without checking for
failure; update start.sh to check the exit status of cd (the command at cd
"$REPO_ROOT") and exit with a non-zero status or log a clear error if it fails,
so the script does not continue with the wrong working directory; ensure the
check is performed immediately after the cd command and refer to the cd
"$REPO_ROOT" invocation and any usage of LAYOUT that relies on the working
directory.
---
Nitpick comments:
In @.config/wt/bin/web:
- Line 2: The script currently calls the macOS-only command "open" to launch
KILN_WEB_URL, causing failures on Linux/Windows; update the "web" script to try
a cross-platform fallback chain: attempt "open" first, then "xdg-open", then
"wslview" (or fail with a clear message), using the existing KILN_WEB_URL
variable (KILN_WEB_URL:-http://localhost:8758) and exiting non-zero on total
failure; implement this by replacing the single "open" invocation with a
conditional/fallback invocation that tries "open || xdg-open || wslview" (or
equivalent shell if/command checks) so the script works on macOS, Linux, and
WSL/Windows.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 01d556e8-457b-406c-b5ce-c1514e84d399
📒 Files selected for processing (12)
.config/wt.toml.config/wt/README.md.config/wt/bin/web.config/wt/config.toml.config/wt/layout.kdl.config/wt/start.sh.config/wt/user_settings.sh.example.gitignoreapp/desktop/dev_server.pyapp/web_ui/src/lib/api_client.tslibs/server/kiln_server/server.pyutils/setup_env.sh
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.config/wt/layout.kdl (1)
16-19: Fail fast when required env vars are missing.
agentandwebuiassume env bootstrap succeeded. Adding explicit guards gives immediate, actionable failures instead of silent/opaque tab exits.Suggested hardening
tab name="agent" { pane command="bash" start_suspended=false { - args "-c" "$KILN_CODER_CMD" + args "-c" ": \"${KILN_CODER_CMD:?KILN_CODER_CMD is not set}\"; $KILN_CODER_CMD" } } @@ tab name="webui" cwd="app/web_ui" { pane command="bash" start_suspended=false { - args "-c" "npm run dev -- --host --port $KILN_FRONTEND_PORT" + args "-c" ": \"${KILN_FRONTEND_PORT:?KILN_FRONTEND_PORT is not set}\"; npm run dev -- --host --port \"$KILN_FRONTEND_PORT\"" } }Also applies to: 27-29
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.config/wt/layout.kdl around lines 16 - 19, Add explicit environment-variable guards so tabs fail fast when required vars are missing: in the "agent" tab (tab name="agent") and similarly in the "webui" tab (lines handling pane command="bash" and args "-c" "$KILN_CODER_CMD"), check for the required env vars (e.g., KILN_CODER_CMD and any webui-specific vars) at startup and emit a clear error and non-zero exit if absent; update the pane command invocation to run a small shell snippet that validates the vars before running "$KILN_CODER_CMD" to ensure immediate, actionable failures instead of silent tab exits.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.config/wt/layout.kdl:
- Around line 16-19: Add explicit environment-variable guards so tabs fail fast
when required vars are missing: in the "agent" tab (tab name="agent") and
similarly in the "webui" tab (lines handling pane command="bash" and args "-c"
"$KILN_CODER_CMD"), check for the required env vars (e.g., KILN_CODER_CMD and
any webui-specific vars) at startup and emit a clear error and non-zero exit if
absent; update the pane command invocation to run a small shell snippet that
validates the vars before running "$KILN_CODER_CMD" to ensure immediate,
actionable failures instead of silent tab exits.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 812783dd-2fe0-4160-ab9f-56c0dfc512a7
📒 Files selected for processing (3)
.config/wt/bin/web.config/wt/layout.kdl.config/wt/start.sh
🚧 Files skipped from review as they are similar to previous changes (2)
- .config/wt/bin/web
- .config/wt/start.sh
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
utils/setup_env.sh (1)
16-30: Consider checking for Homebrew availability before usingbrew.The script uses
brew installfor both worktrunk and zellij, which assumes Homebrew is available. On Linux systems or macOS without Homebrew, this will fail with abrew: command not founderror and exit abruptly due toset -e.Consider adding a check with a helpful message for users without Homebrew:
💡 Suggested improvement
if [[ "$install_workspaces" =~ ^[Yy]$ ]]; then + if ! command -v brew &>/dev/null; then + echo "Error: Homebrew (brew) is required to install workspaces dependencies." + echo "Install Homebrew from https://brew.sh or manually install 'worktrunk' and 'zellij'." + exit 1 + fi + if ! command -v wt &>/dev/null; then🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@utils/setup_env.sh` around lines 16 - 30, The script assumes Homebrew is present when calling "brew install" for wt and zellij; add a preliminary check using "command -v brew" and, if brew is missing, print a clear, actionable message (e.g., instruct to install Homebrew or provide an alternative package manager) and skip the brew install steps instead of attempting them (so the rest of setup doesn't fail under set -e). Update the block around the wt and zellij installs to only run "brew install" and related commands when brew is available, and otherwise echo guidance to the user on how to install Homebrew or manually install wt/zellij.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.config/wt/README.md:
- Around line 148-155: The code block showing the Zellij config (keybinds
clear-defaults=true { ... shared_except "locked" { bind "Alt b" ... } }) needs a
language identifier for syntax highlighting; update the opening triple-backtick
to ```kdl so the block is fenced as KDL. Locate the block containing the
keybinds/shared_except/bind entries and change the fence only (add "kdl" after
the backticks) without altering the content.
---
Nitpick comments:
In `@utils/setup_env.sh`:
- Around line 16-30: The script assumes Homebrew is present when calling "brew
install" for wt and zellij; add a preliminary check using "command -v brew" and,
if brew is missing, print a clear, actionable message (e.g., instruct to install
Homebrew or provide an alternative package manager) and skip the brew install
steps instead of attempting them (so the rest of setup doesn't fail under set
-e). Update the block around the wt and zellij installs to only run "brew
install" and related commands when brew is available, and otherwise echo
guidance to the user on how to install Homebrew or manually install wt/zellij.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 0ceb3a73-0f30-4250-bf76-7a763f844902
📒 Files selected for processing (3)
.config/wt/README.md.config/wt/layout.kdlutils/setup_env.sh
🚧 Files skipped from review as they are similar to previous changes (1)
- .config/wt/layout.kdl
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
.config/wt/README.md (2)
153-160:⚠️ Potential issue | 🟡 MinorAdd a language identifier to the Zellij config code fence.
Line 153 should use
```kdlfor proper syntax highlighting and to satisfy MD040.📝 Minimal fix
-``` +```kdl keybinds clear-defaults=true { shared_except "locked" { bind "Alt b" { GoToPreviousTab; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.config/wt/README.md around lines 153 - 160, The code fence for the Zellij config block is missing a language identifier; update the opening fence from ``` to ```kdl so the block beginning with "keybinds clear-defaults=true {" (and its nested "shared_except \"locked\"" and "bind" lines) uses KDL syntax highlighting to satisfy MD040.
16-19:⚠️ Potential issue | 🟡 MinorMake the Worktrunk config step non-destructive.
Line 18 uses
>and can overwrite an existing global~/.config/worktrunk/config.toml. Please switch to append-if-missing (or instruct manual edit) to avoid clobbering unrelated user settings.🛠️ Suggested doc update
mkdir -p ~/.config/worktrunk -echo 'worktree-path = ".worktrees/{{ branch | sanitize }}"' > ~/.config/worktrunk/config.toml +CONFIG="${HOME}/.config/worktrunk/config.toml" +touch "$CONFIG" +if ! grep -q '^worktree-path\s*=' "$CONFIG"; then + printf 'worktree-path = ".worktrees/{{ branch | sanitize }}"\n' >> "$CONFIG" +fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.config/wt/README.md around lines 16 - 19, The current echo command ("echo 'worktree-path = \"...\"' > ~/.config/worktrunk/config.toml") is destructive and will overwrite an existing config; change it to a non-destructive conditional append such as: create the directory with mkdir -p as before, then check for an existing worktree-path entry (e.g., using grep -q '^worktree-path' ~/.config/worktrunk/config.toml) and only append the line with >> or tee -a if the key is missing; alternatively update the docs to instruct users to manually add the worktree-path line to ~/.config/worktrunk/config.toml to avoid clobbering existing settings.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@utils/setup_env.sh`:
- Around line 16-28: The script assumes Homebrew exists before calling brew for
package installs (wt, zellij); add a prerequisite check at the top of
setup_env.sh that runs command -v brew and if missing prints a clear error like
"Homebrew is required for this script; please install it: https://brew.sh" and
exits non-zero, so subsequent uses of brew (e.g., the wt and zellij install
blocks) won't produce a generic command-not-found error; alternatively, guard
each brew install with that same command and a friendly message, referencing the
wt and zellij install sections to locate where to apply the check.
---
Duplicate comments:
In @.config/wt/README.md:
- Around line 153-160: The code fence for the Zellij config block is missing a
language identifier; update the opening fence from ``` to ```kdl so the block
beginning with "keybinds clear-defaults=true {" (and its nested "shared_except
\"locked\"" and "bind" lines) uses KDL syntax highlighting to satisfy MD040.
- Around line 16-19: The current echo command ("echo 'worktree-path = \"...\"' >
~/.config/worktrunk/config.toml") is destructive and will overwrite an existing
config; change it to a non-destructive conditional append such as: create the
directory with mkdir -p as before, then check for an existing worktree-path
entry (e.g., using grep -q '^worktree-path' ~/.config/worktrunk/config.toml) and
only append the line with >> or tee -a if the key is missing; alternatively
update the docs to instruct users to manually add the worktree-path line to
~/.config/worktrunk/config.toml to avoid clobbering existing settings.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 344a40a8-e8c1-4db2-a98f-06ebda5115ea
📒 Files selected for processing (3)
.config/wk.yml.config/wt/README.mdutils/setup_env.sh
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
utils/setup_env.sh (1)
36-41: Consider pinning worktree_tui to a specific version or commit.Installing from a git URL without a version tag means different developers may get different versions over time. For reproducibility, consider pinning to a release tag or commit hash:
uv tool install "git+https://github.com/scosman/worktree_tui@v1.0.0" # or @<commit-sha>This is a minor concern for a dev tool and can be deferred.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@utils/setup_env.sh` around lines 36 - 41, The install step for the worktree TUI uses an unpinned git URL which can yield different versions over time; update the uv tool install invocation that currently references "git+https://github.com/scosman/worktree_tui" (seen near the wk check) to pin to a stable tag or commit (for example append `@vX.Y.Z` or @<commit-sha>) so all developers get the same version; ensure the conditional that checks command -v wk and the echo lines remain unchanged while only replacing the install URL string in the uv tool install call.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@utils/setup_env.sh`:
- Around line 46-52: The current setup_env.sh block creates a symlink from
WT_CONFIG to WT_PROJECT_CONFIG which can leave the user's global config
(~/.config/worktrunk/config.toml) broken if the project is moved or deleted;
change this to copy the file content instead of symlinking (replace the ln -sf
step with a safe copy from WT_PROJECT_CONFIG to WT_CONFIG, ensuring
WT_CONFIG_DIR exists and preserving permissions), or if you must keep the
symlink add a clear echo after linking that warns the user the global config
points into the project and will break if the project is moved/removed; update
the code paths referencing WT_CONFIG, WT_PROJECT_CONFIG and WT_CONFIG_DIR
accordingly.
---
Nitpick comments:
In `@utils/setup_env.sh`:
- Around line 36-41: The install step for the worktree TUI uses an unpinned git
URL which can yield different versions over time; update the uv tool install
invocation that currently references
"git+https://github.com/scosman/worktree_tui" (seen near the wk check) to pin to
a stable tag or commit (for example append `@vX.Y.Z` or @<commit-sha>) so all
developers get the same version; ensure the conditional that checks command -v
wk and the echo lines remain unchanged while only replacing the install URL
string in the uv tool install call.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: be9841a8-88b1-4e3d-90a0-307fd7908b39
📒 Files selected for processing (8)
.agents/card_style.md.agents/frontend_controls.md.agents/frontend_design_guide.md.agents/python_test_guide.md.agents/tables_style.mdhooks_mcp.yamlpyproject.tomlutils/setup_env.sh
| if [ ! -e "$WT_CONFIG" ] && [ -f "$WT_PROJECT_CONFIG" ]; then | ||
| mkdir -p "$WT_CONFIG_DIR" | ||
| ln -sf "$WT_PROJECT_CONFIG" "$WT_CONFIG" | ||
| echo " Linked worktrunk config: $WT_CONFIG -> $WT_PROJECT_CONFIG" | ||
| else | ||
| echo " Worktrunk config already present." | ||
| fi |
There was a problem hiding this comment.
Symlink ties global config to this project checkout.
The symlink approach means if this project directory is ever moved or deleted, the user's global ~/.config/worktrunk/config.toml becomes a broken symlink, causing wt commands to fail silently or with confusing errors.
The README's manual setup (context snippet 3) copies the content directly instead:
echo 'worktree-path = ".worktrees/{{ branch | sanitize }}"' > ~/.config/worktrunk/config.tomlConsider copying the content rather than symlinking, or at minimum document this dependency in the output message so users understand the relationship.
📋 Alternative: Copy content instead of symlink
if [ ! -e "$WT_CONFIG" ] && [ -f "$WT_PROJECT_CONFIG" ]; then
mkdir -p "$WT_CONFIG_DIR"
- ln -sf "$WT_PROJECT_CONFIG" "$WT_CONFIG"
- echo " Linked worktrunk config: $WT_CONFIG -> $WT_PROJECT_CONFIG"
+ cp "$WT_PROJECT_CONFIG" "$WT_CONFIG"
+ echo " Created worktrunk config: $WT_CONFIG"
else🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@utils/setup_env.sh` around lines 46 - 52, The current setup_env.sh block
creates a symlink from WT_CONFIG to WT_PROJECT_CONFIG which can leave the user's
global config (~/.config/worktrunk/config.toml) broken if the project is moved
or deleted; change this to copy the file content instead of symlinking (replace
the ln -sf step with a safe copy from WT_PROJECT_CONFIG to WT_CONFIG, ensuring
WT_CONFIG_DIR exists and preserving permissions), or if you must keep the
symlink add a clear echo after linking that warns the user the global config
points into the project and will break if the project is moved/removed; update
the code paths referencing WT_CONFIG, WT_PROJECT_CONFIG and WT_CONFIG_DIR
accordingly.
Utilities to have many parallel workspaces for development
webcommand: run this from workspace to launch web app homepageSee the new README for details. Next step is TUI to make commands nice. Then docker sandbox support.
workspaces.mp4
Summary by CodeRabbit
New Features
Documentation
Chores