Route worktree and graveyard CLI through daemon#308
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 25 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThis PR adds worktree and graveyard command support end-to-end: new text-route contracts and renderers, daemon routing with loopback/CLI access control proxying to a project service, installed shim CLI dispatch, and corresponding tests/documentation updates reclassifying these commands from LEGACY to CUT. ChangesWorktree/Graveyard Text Route Feature
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant CLI as aimux CLI (shim)
participant Daemon
participant ProjectService
CLI->>Daemon: curl GET/POST /core/worktree/*-text or /core/graveyard/*-text
Daemon->>Daemon: check LOCAL_CLI_TEXT_ROUTES (loopback/cli-only 403)
alt authorized
Daemon->>ProjectService: getProjectServiceJson / postProjectServiceJson
ProjectService-->>Daemon: JSON payload
Daemon->>Daemon: validate required array/fields
Daemon->>Daemon: renderCoreWorktree*/renderCoreGraveyard* lines
Daemon-->>CLI: text or JSON response
else unauthorized
Daemon-->>CLI: 403 loopback-only / cli-only
end
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (4)
scripts/installed-aimux-shim.sh (2)
609-643: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate arg-parsing logic instead of reusing
aimux_parse_project_json_args.The
cleanupbranch reimplements the--project/--project=/--jsonparsing loop already extracted intoaimux_parse_project_json_args(Lines 500-527), just adding--dry-run. This duplication risks silent drift (e.g., if the shared helper's validation changes, this copy won't get the fix).♻️ Suggested consolidation
- cleanup) - shift - project_root="$(pwd -P 2>/dev/null)" || return 1 - json=0 - dry_run=0 - while [ "$#" -gt 0 ]; do - case "$1" in - --project) - shift - [ "$#" -gt 0 ] || return 1 - case "$1" in -*) return 1 ;; esac - project_root="$1" - ;; - --project=*) - project_root="${1#--project=}" - [ -n "$project_root" ] || return 1 - ;; - --json) - json=1 - ;; - --dry-run) - dry_run=1 - ;; - *) - return 1 - ;; - esac - shift - done - project_root="$(aimux_resolve_project_arg "$project_root")" || return 1 - path="/core/graveyard/cleanup-text" - [ "$json" -eq 1 ] && path="/core/graveyard/cleanup-text?json=1" - aimux_post_query_text_route "$path" 120 \ - --data-urlencode "project=$project_root" --data-urlencode "dryRun=$dry_run" - ;; + cleanup) + shift + dry_run=0 + remaining_args="" + while [ "$#" -gt 0 ]; do + case "$1" in + --dry-run) dry_run=1 ;; + *) remaining_args="$remaining_args $1" ;; + esac + shift + done + aimux_parse_project_json_args $remaining_args || return 1 + path="/core/graveyard/cleanup-text" + [ "$AIMUX_PARSED_JSON" -eq 1 ] && path="/core/graveyard/cleanup-text?json=1" + aimux_post_query_text_route "$path" 120 \ + --data-urlencode "project=$AIMUX_PARSED_PROJECT" --data-urlencode "dryRun=$dry_run" + ;;Note: unquoted
$remaining_argsword-splitting is acceptable here since values are simple flags/paths, but verify no arguments containing spaces are expected.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/installed-aimux-shim.sh` around lines 609 - 643, The cleanup command in the command-dispatch block is duplicating the `--project`/`--project=`/`--json` parsing already handled by `aimux_parse_project_json_args`, so update `cleanup` to reuse that helper and only handle the extra `--dry-run` flag locally. Keep `project_root` resolution via `aimux_resolve_project_arg`, and make sure the cleanup branch still passes both the parsed project value and `dryRun` to `aimux_post_query_text_route` without reimplementing the shared parsing loop.
538-542: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate GET-request construction for worktree/graveyard list.
Both blocks reimplement the same GET pattern already factored into
aimux_curl_project_text_route(Lines 229-235), differing only in that they need a caller-resolved project instead ofpwd. Consider generalizing the existing helper to accept an explicit project argument and reuse it in both places to avoid triplicated curl invocation logic.Also applies to: 588-591
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/installed-aimux-shim.sh` around lines 538 - 542, Duplicate GET request construction is repeated in the worktree/graveyard list paths instead of reusing the existing request helper. Update aimux_curl_project_text_route to accept an explicit project argument (rather than always deriving it from pwd), then call that helper from the list-text and graveyard list blocks so both use the same curl logic and only vary by caller-provided project and path.src/daemon.ts (1)
487-531: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
getProjectServiceJsonduplicatespostProjectServiceJson.Endpoint lookup,
ensureProjectgating, status/okvalidation, and the catch-block error formatting are copied almost verbatim frompostProjectServiceJson(lines 533-573), differing only in HTTP method/body. Consider factoring the shared logic into one private helper taking method/body as parameters to avoid the two copies diverging over time (e.g. the timeout-vs-generic-error status code distinction already present in the/proxy/handler isn't applied to either of these).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/daemon.ts` around lines 487 - 531, getProjectServiceJson duplicates postProjectServiceJson; extract the shared endpoint lookup, ensureProject handling, requestJson status/ok validation, and catch-block error formatting into a private helper used by both methods. Keep the helper parameterized by method/body so getProjectServiceJson and postProjectServiceJson only supply their HTTP-specific differences, and preserve the existing error/status behavior in the shared path.src/core-text.ts (1)
240-252: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate worktree table formatting.
renderCoreWorktreeListLinesand the worktrees block insiderenderCoreGraveyardLinesbuild the same Name/Branch/Path table (samepadEndwidths, same separator length) with only the empty-field fallback (""vs"?") differing. Consider extracting a shared row-formatting helper to avoid the two copies drifting apart.♻️ Proposed shared helper
+function formatWorktreeRow(worktree: Record<string, unknown>, fallback = ""): string { + return ( + String(worktree.name ?? fallback).padEnd(30) + + String(worktree.branch ?? "").padEnd(35) + + String(worktree.path ?? fallback) + ); +} + export function renderCoreWorktreeListLines(payload: CoreWorktreeSummaryTextPayload): string[] { const worktrees = payload.worktrees.filter((entry) => entry && typeof entry === "object") as Array< Record<string, unknown> >; if (worktrees.length === 0) return ["No worktrees found."]; const lines = ["Name".padEnd(30) + "Branch".padEnd(35) + "Path", "-".repeat(95)]; - for (const worktree of worktrees) { - lines.push( - String(worktree.name ?? "").padEnd(30) + String(worktree.branch ?? "").padEnd(35) + String(worktree.path ?? ""), - ); - } + for (const worktree of worktrees) lines.push(formatWorktreeRow(worktree)); return lines; }Also applies to: 277-309
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/core-text.ts` around lines 240 - 252, Duplicate worktree table formatting exists in renderCoreWorktreeListLines and the worktrees block inside renderCoreGraveyardLines, so extract the shared Name/Branch/Path row and header formatting into a common helper using the same padEnd widths and separator length. Keep the only behavioral difference in the fallback values ("", "?") as an input to the helper, and update both call sites to use the shared formatting so the two tables cannot drift apart.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@scripts/installed-aimux-shim.sh`:
- Around line 609-643: The cleanup command in the command-dispatch block is
duplicating the `--project`/`--project=`/`--json` parsing already handled by
`aimux_parse_project_json_args`, so update `cleanup` to reuse that helper and
only handle the extra `--dry-run` flag locally. Keep `project_root` resolution
via `aimux_resolve_project_arg`, and make sure the cleanup branch still passes
both the parsed project value and `dryRun` to `aimux_post_query_text_route`
without reimplementing the shared parsing loop.
- Around line 538-542: Duplicate GET request construction is repeated in the
worktree/graveyard list paths instead of reusing the existing request helper.
Update aimux_curl_project_text_route to accept an explicit project argument
(rather than always deriving it from pwd), then call that helper from the
list-text and graveyard list blocks so both use the same curl logic and only
vary by caller-provided project and path.
In `@src/core-text.ts`:
- Around line 240-252: Duplicate worktree table formatting exists in
renderCoreWorktreeListLines and the worktrees block inside
renderCoreGraveyardLines, so extract the shared Name/Branch/Path row and header
formatting into a common helper using the same padEnd widths and separator
length. Keep the only behavioral difference in the fallback values ("", "?") as
an input to the helper, and update both call sites to use the shared formatting
so the two tables cannot drift apart.
In `@src/daemon.ts`:
- Around line 487-531: getProjectServiceJson duplicates postProjectServiceJson;
extract the shared endpoint lookup, ensureProject handling, requestJson
status/ok validation, and catch-block error formatting into a private helper
used by both methods. Keep the helper parameterized by method/body so
getProjectServiceJson and postProjectServiceJson only supply their HTTP-specific
differences, and preserve the existing error/status behavior in the shared path.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 85cac626-508b-4a54-bd87-101b5df48471
📒 Files selected for processing (8)
docs/command-ownership-inventory.mdscripts/installed-aimux-shim.shsrc/core-command-contract.tssrc/core-command-ownership.test.tssrc/core-text.tssrc/daemon.test.tssrc/daemon.tssrc/installed-shim.test.ts
|
Independent review finding fixed: |
1 participant
Summary
Verification
Summary by CodeRabbit
New Features
Bug Fixes
Documentation