Route workflow commands through core daemon - #310
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: 46 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 (2)
📝 WalkthroughWalkthroughThis PR adds workflow commands ( ChangesWorkflow commands cohort
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant Shim as Installed Shim
participant Daemon
participant ProjectService
CLI->>Shim: aimux task/handoff/review <args>
Shim->>Daemon: GET/POST /core/{handoff|task|review}/*-text
Daemon->>ProjectService: requestJson (send/accept/complete/list/show/assign/action)
ProjectService-->>Daemon: task/thread/message data
Daemon-->>Shim: rendered text lines
Shim-->>CLI: stdout output
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
src/daemon.ts (1)
960-1181: 🧹 Nitpick | 🔵 TrivialNew route handlers look correct; verify end-to-end via built daemon.
Handler logic, validation, and dispatch wiring for handoff/task/review routes are consistent with the route contract and covered by
daemon.test.ts. As per coding guidelines, for this kind of daemon runtime/CLI behavior, build and install the local release asset and runaimux restartto confirm the daemon actually picks up these new routes end-to-end, rather than relying on source-level test mocks alone.🤖 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 960 - 1181, The new route handlers in daemon.ts look fine, but this change needs end-to-end verification against the built daemon rather than only source-level tests. Build and install the local release asset, then use aimux restart and exercise the new handoff/task/review routes to confirm the runtime actually picks up the updated handler methods such as handoffSendTextRoute, handoffMutationTextRoute, taskAssignTextRoute, taskMutationTextRoute, reviewRequestChangesTextRoute, and taskMutationResponse.Source: Coding guidelines
scripts/installed-aimux-shim.sh (2)
1030-1058: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
aimux_parse_thread_action_argsandaimux_parse_task_action_argsare near-duplicates.Both functions parse identical
--project/--from/--body/--jsonoptions and differ only in the id variable name (AIMUX_PARSED_THREADvsAIMUX_PARSED_TASK). Consider unifying into a single helper that takes the output variable name (or id kind) as a parameter to avoid maintaining two copies of the same parsing logic.♻️ Sketch of a shared helper
-aimux_parse_thread_action_args() { - ... - AIMUX_PARSED_THREAD="$thread_id" - ... -} - -aimux_parse_task_action_args() { - ... - AIMUX_PARSED_TASK="$task_id" - ... -} +aimux_parse_id_action_args() { + id_var_name="$1" + shift + [ "$#" -gt 0 ] || return 1 + the_id="$1" + case "$the_id" in -*) return 1 ;; esac + shift + ... + eval "$id_var_name=\$the_id" +}Also applies to: 1177-1205
🤖 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 1030 - 1058, The parsing logic in aimux_parse_thread_action_args and aimux_parse_task_action_args is duplicated, differing only in the output id field they set. Refactor the shared option parsing for --project, --from, --body, and --json into one helper that accepts the id kind or destination variable name, then have both aimux_parse_thread_action_args and aimux_parse_task_action_args call it and assign the appropriate AIMUX_PARSED_* variable.
959-1028: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate option-parsing scaffolding vs.
aimux_try_message.The
sendbranch here re-implements almost the entire option table (--project,--from,--to,--assignee,--tool,--worktree,--title,--json) already present inaimux_try_message(lines 902-957). Consider extracting a shared option-parsing helper (e.g.,aimux_parse_dispatch_args) that bothaimux_try_messageandaimux_try_handoffcan call, reducing the risk of the two diverging silently as new flags are added.🤖 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 959 - 1028, The send branch in aimux_try_handoff duplicates the same option parsing already implemented in aimux_try_message, so refactor the shared flag handling into a common helper and have both functions call it. Extract the parsing for project/from/to/assignee/tool/worktree/title/json into a reusable function such as aimux_parse_dispatch_args, then use its parsed outputs in aimux_try_handoff and aimux_try_message to keep behavior consistent as flags evolve.src/main.ts (1)
2225-2559: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRepeated action-handler boilerplate across handoff/task/review mutation commands.
The
handoff accept,handoff complete,task accept/block/complete/reopen, andreview approve/request-changeshandlers all follow the identical shape: parse{project, from, body, json}, callprepareProjectContext, POST to a project-service endpoint, then optionally echo JSON. Consider factoring a small helper, e.g.runIdActionCommand(path, idField, opts), to reduce the ~9x duplicated wiring and make future endpoint additions less error-prone.🤖 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/main.ts` around lines 2225 - 2559, The handoff/task/review mutation command handlers repeat the same option parsing, project context setup, service POST, and JSON/output branching logic. Factor this shared flow out of the existing command actions into a small reusable helper (for example a function used by the `handoff`, `task`, and `review` command builders) and have `accept`, `complete`, `block`, `reopen`, `approve`, and `request-changes` call it with their endpoint and id field details. Keep the command-specific names and response printing, but centralize the common wiring to reduce duplication and make future commands easier to add.
🤖 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.
Inline comments:
In `@scripts/installed-aimux-shim.sh`:
- Around line 1135-1136: The `--diff` space-form parsing in the argument switch
is rejecting valid diff text that starts with dashes, causing it to miss the
shim fast path. Update the `--diff)` handling to treat the next argument as raw
diff content instead of passing it through `aimux_require_arg_value`, while
keeping `--diff=*` unchanged; use the `--diff` case in the same dispatch block
to apply the exception only for the space-separated form.
---
Nitpick comments:
In `@scripts/installed-aimux-shim.sh`:
- Around line 1030-1058: The parsing logic in aimux_parse_thread_action_args and
aimux_parse_task_action_args is duplicated, differing only in the output id
field they set. Refactor the shared option parsing for --project, --from,
--body, and --json into one helper that accepts the id kind or destination
variable name, then have both aimux_parse_thread_action_args and
aimux_parse_task_action_args call it and assign the appropriate AIMUX_PARSED_*
variable.
- Around line 959-1028: The send branch in aimux_try_handoff duplicates the same
option parsing already implemented in aimux_try_message, so refactor the shared
flag handling into a common helper and have both functions call it. Extract the
parsing for project/from/to/assignee/tool/worktree/title/json into a reusable
function such as aimux_parse_dispatch_args, then use its parsed outputs in
aimux_try_handoff and aimux_try_message to keep behavior consistent as flags
evolve.
In `@src/daemon.ts`:
- Around line 960-1181: The new route handlers in daemon.ts look fine, but this
change needs end-to-end verification against the built daemon rather than only
source-level tests. Build and install the local release asset, then use aimux
restart and exercise the new handoff/task/review routes to confirm the runtime
actually picks up the updated handler methods such as handoffSendTextRoute,
handoffMutationTextRoute, taskAssignTextRoute, taskMutationTextRoute,
reviewRequestChangesTextRoute, and taskMutationResponse.
In `@src/main.ts`:
- Around line 2225-2559: The handoff/task/review mutation command handlers
repeat the same option parsing, project context setup, service POST, and
JSON/output branching logic. Factor this shared flow out of the existing command
actions into a small reusable helper (for example a function used by the
`handoff`, `task`, and `review` command builders) and have `accept`, `complete`,
`block`, `reopen`, `approve`, and `request-changes` call it with their endpoint
and id field details. Keep the command-specific names and response printing, but
centralize the common wiring to reduce duplication and make future commands
easier to add.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c8d9a7a2-9224-4801-a61d-c83395204a44
📒 Files selected for processing (9)
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.tssrc/main.ts
Summary
Verification
Summary by CodeRabbit
New Features
Bug Fixes