Align GUI project-service CLI route surfaces#35
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds worktree graveyard lifecycle and task-browsing endpoints; converts CLI and desktop commands to prefer project service with safe local fallbacks; extends Graveyard UI to manage graveyarded worktrees; implements metadata-server task read endpoints and thread mark-seen handling; updates tests and docs. ChangesWorktree Graveyard Management and Task Browsing
Sequence Diagram(s)sequenceDiagram
participant Client
participant MetadataServer
participant Storage
Client->>MetadataServer: GET /tasks?session=s1&status=pending
MetadataServer->>Storage: readAllTasks()
Storage-->>MetadataServer: tasks[]
MetadataServer-->>Client: { ok, tasks }
Client->>MetadataServer: GET /tasks/t1
MetadataServer->>Storage: readTask(t1)
Storage-->>MetadataServer: task
MetadataServer->>Storage: readThread(task.threadId)
Storage-->>MetadataServer: thread, messages
MetadataServer-->>Client: { ok, task, thread, messages }
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly Related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main.ts (1)
2665-2690:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon't translate project-service errors into an empty graveyard.
If
/graveyardfails on a live service, thiscatchreports an empty graveyard instead of falling back tolistTopologySessionStates()/listTopologyWorktreeGraveyard(). That makes the CLI lie about recoverable state.Suggested fix
+ const localGraveyard = () => ({ + ok: true, + entries: listTopologySessionStates({ statuses: ["graveyard"] }), + worktrees: listTopologyWorktreeGraveyard(), + }); try { - const graveyard = await getLiveProjectServiceJsonOrLocal(projectRoot, "/graveyard", () => ({ - ok: true, - entries: listTopologySessionStates({ statuses: ["graveyard"] }), - worktrees: listTopologyWorktreeGraveyard(), - })); + const graveyard = await getLiveProjectServiceJsonOrLocal(projectRoot, "/graveyard", localGraveyard); if (opts.json) { console.log( JSON.stringify( @@ } printGraveyard(graveyard); } catch { + const graveyard = localGraveyard(); if (opts.json) { - console.log(JSON.stringify({ entries: [], worktrees: [] }, null, 2)); + console.log(JSON.stringify({ entries: graveyard.entries, worktrees: graveyard.worktrees }, null, 2)); return; } - console.log("Graveyard is empty."); + printGraveyard(graveyard); }🤖 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 2665 - 2690, The catch block currently swallows failures from getLiveProjectServiceJsonOrLocal and returns an empty graveyard; change it to fallback to the local providers instead: on error call listTopologySessionStates({ statuses: ["graveyard"] }) and listTopologyWorktreeGraveyard() to build the same graveyard shape used above, then proceed to JSON output or printGraveyard like the success path; reference getLiveProjectServiceJsonOrLocal, listTopologySessionStates, listTopologyWorktreeGraveyard, printGraveyard and opts.json when implementing the fallback so failures from the live service don't falsely report an empty graveyard.
🤖 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 `@docs/current-architecture.md`:
- Around line 187-190: The project-service HTTP endpoint list is missing the
endpoints corresponding to the new CLI commands; add the following POST
endpoints to the service endpoint list to match the CLI and the desktop UI
contract: POST /worktrees/remove (for "aimux worktree remove"), POST
/worktrees/graveyard (for "aimux worktree graveyard"), POST
/graveyard/worktrees/resurrect (for "aimux worktree resurrect"), and POST
/graveyard/worktrees/delete (for "aimux worktree delete-graveyard"); ensure they
are formatted and ordered consistently with the existing endpoint list and match
the definitions in the desktop UI contract.
In `@docs/desktop-ui-contract.md`:
- Around line 52-56: Update the project-service HTTP endpoint list in
docs/current-architecture.md to include the notification inbox endpoints GET
/notifications, POST /notifications/read, and POST /notifications/clear (or add
an explicit pointer to the notification inbox surface described in
docs/desktop-ui-contract.md and docs/notification-system.md). Reference the
actual implementation in src/metadata-server.ts and the client helpers in
app/lib/api.ts so readers can find the server and client code and associated
tests.
In `@src/main.ts`:
- Around line 2435-2438: prepareProjectContext() changes process.cwd(), so
calling pathResolve(targetPath) after it causes relative targetPath to be
resolved against the repo root instead of the caller's cwd; move the resolution
of the worktree path (the pathResolve(targetPath) call that produces
resolvedPath) to run before prepareProjectContext(opts.project) (and likewise
before any prepareProjectContext calls near the other occurrences) so that
targetPath is resolved against the original working directory; update the blocks
containing prepareProjectContext, ensureDaemonProjectReadyForFallback, and
postLiveProjectServiceJsonOrLocal to compute resolvedPath first and then call
prepareProjectContext.
- Around line 2438-2446: postLiveProjectServiceJsonOrLocal currently only falls
back when the endpoint is absent, causing hard failures when a reachable service
returns 404/405/501; change its error-handling so that when the HTTP response
status is exactly 404, 405, or 501 it invokes the provided local fallback
callback (the function that constructs a Multiplexer and calls methods like
Multiplexer.removeDesktopWorktree), but for other HTTP errors (5xx) or network
errors it should rethrow/propagate the error as before; update
postLiveProjectServiceJsonOrLocal so callers such as the calls that pass () => {
const mux = new Multiplexer(); return mux.removeDesktopWorktree(resolvedPath); }
(and the other similar Multiplexer fallbacks) will only be used for
route-not-found/unsupported responses.
In `@src/metadata-server.ts`:
- Around line 1238-1240: The code currently calls decodeURIComponent on
url.pathname for the "/tasks/" (and similarly "/threads/") routes without
guarding against malformed percent-encoding; wrap the decodeURIComponent call
used to produce taskId (and threadId) in a try/catch and if decodeURIComponent
throws (URIError) respond with a 400 Bad Request (e.g., set status 400 and end
the response) instead of letting the exception bubble; update the handler around
the existing req.method === "GET" && url.pathname.startsWith("/tasks/") logic
where taskId is derived and likewise for the "/threads/" branch where threadId
is derived before calling readTask/readThread so that readTask/readThread are
only invoked with a valid decoded id.
---
Outside diff comments:
In `@src/main.ts`:
- Around line 2665-2690: The catch block currently swallows failures from
getLiveProjectServiceJsonOrLocal and returns an empty graveyard; change it to
fallback to the local providers instead: on error call
listTopologySessionStates({ statuses: ["graveyard"] }) and
listTopologyWorktreeGraveyard() to build the same graveyard shape used above,
then proceed to JSON output or printGraveyard like the success path; reference
getLiveProjectServiceJsonOrLocal, listTopologySessionStates,
listTopologyWorktreeGraveyard, printGraveyard and opts.json when implementing
the fallback so failures from the live service don't falsely report an empty
graveyard.
🪄 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 Plus
Run ID: 6dd8f0a5-7494-4aad-b6f0-bcf6fdc0a384
📒 Files selected for processing (8)
app/app/(main)/(tabs)/(dashboard)/graveyard.tsxapp/lib/api.test.tsapp/lib/api.tsdocs/current-architecture.mddocs/desktop-ui-contract.mdsrc/main.tssrc/metadata-server.test.tssrc/metadata-server.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@src/main.ts`:
- Around line 396-398: getLiveProjectServiceJsonOrLocal currently throws on any
HTTP error; modify it to mirror the POST variant by catching HTTP errors and
returning fallback() when the response status is one of the route-not-found
statuses (at least 404, 405, 501 and also handle the broader 405–422 range as
appropriate). Locate getLiveProjectServiceJsonOrLocal, wrap its fetch/axios call
in a try/catch (or inspect the response status) and if error.status (or
response.status) is in the allowed fallback set call and return fallback();
otherwise rethrow or propagate the error as before.
🪄 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 Plus
Run ID: 4e341c97-bd56-4839-8e3e-0ab5267eb4ff
📒 Files selected for processing (4)
docs/current-architecture.mdsrc/main.tssrc/metadata-server.test.tssrc/metadata-server.ts
✅ Files skipped from review due to trivial changes (1)
- docs/current-architecture.md
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@src/main.ts`:
- Around line 415-420: The code calls requestJson(...) inside
ensureDaemonProjectReadyForFallback() and only checks status codes 404/405/501,
but if requestJson() rejects (stale endpoint/transport failure) the function
hard-fails; wrap the requestJson call in a try/catch around the GET to catch
rejections and in the catch branch call and return fallback() (same as when
status is 404/405/501), using the same endpoint/ path variables and preserving
existing behavior for successful responses.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
Summary
Verification
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests