Hard cut services and worktrees to runtime topology#32
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)
📝 WalkthroughWalkthroughMigrates worktree graveyard and service lifecycle to a runtime topology store, adding topology worktree/service modules, preserving service node/binding state, and wiring topology-backed APIs into persistence, metadata-server, multiplexer services, CLI, and debug reporting. ChangesWorktree and Service Lifecycle Management via Runtime Topology
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 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: 3
🤖 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 1600-1603: The calls to ensureDaemonProjectReady are preventing
the getLiveProjectServiceJsonOrLocal fallback from running for worktree
commands; modify the three spots (the call sites around prepareProjectContext →
ensureDaemonProjectReady → getLiveProjectServiceJsonOrLocal) so that
getLiveProjectServiceJsonOrLocal is invoked without preemptive
ensureDaemonProjectReady (or only call ensureDaemonProjectReady after checking
the live-vs-local result), specifically update the invocation sequences that
involve prepareProjectContext, ensureDaemonProjectReady, and
getLiveProjectServiceJsonOrLocal used by the worktree, worktree list, and
worktree create flows so the local-path fallback can execute when the daemon is
not ready.
In `@src/multiplexer/persistence-methods.ts`:
- Around line 747-753: The code currently sets the topology worktree state to
"removing" via upsertTopologyWorktree({path, name: path.split("/").pop() ??
path}, "removing") before running validations, which can leave stale "removing"
state if validation throws; move the upsertTopologyWorktree(...) call so it runs
only after all validation guards succeed, or wrap the validation+removal
sequence in a try/catch and on any failure call upsertTopologyWorktree(...) to
revert the state (e.g., clear or set back to the previous status) to ensure no
stale "removing" remains; apply the same change for the other occurrences of
upsertTopologyWorktree in the surrounding removal logic (the other similar call
sites mentioned).
In `@src/multiplexer/worktree-graveyard.ts`:
- Line 19: Replace the POSIX-only split call used to derive worktree names with
Node's cross-platform path basename: change the assignment of name (currently
"entry.name ?? entry.path.split(\"/\").pop() ?? entry.path") to use
path.basename(entry.path) (e.g., "entry.name ?? path.basename(entry.path) ??
entry.path"), and add the proper import for the path module (import path from
'path' or const path = require('path')) so entry.path is handled correctly on
Windows and POSIX.
🪄 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: 90efe6bd-523d-419e-90e9-b7d90756b7b5
📒 Files selected for processing (22)
src/debug-state.test.tssrc/debug-state.tssrc/main.tssrc/metadata-server.test.tssrc/metadata-server.tssrc/multiplexer/dashboard-model.tssrc/multiplexer/persistence-methods.test.tssrc/multiplexer/persistence-methods.tssrc/multiplexer/runtime-state.test.tssrc/multiplexer/runtime-state.tssrc/multiplexer/services.test.tssrc/multiplexer/services.tssrc/multiplexer/worktree-graveyard.tssrc/paths.tssrc/runtime-core/topology-services.test.tssrc/runtime-core/topology-services.tssrc/runtime-core/topology-sessions.test.tssrc/runtime-core/topology-sessions.tssrc/runtime-core/topology-store.test.tssrc/runtime-core/topology-store.tssrc/runtime-core/topology-worktrees.test.tssrc/runtime-core/topology-worktrees.ts
💤 Files with no reviewable changes (1)
- src/paths.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 1599-1603: The fallback helper ensureDaemonProjectReadyForFallback
currently swallows all errors; update its catch to rethrow
ProjectServiceVersionError so it isn't suppressed: replace the empty catch with
catch (err) { if (err instanceof ProjectServiceVersionError) throw err; /*
otherwise ignore/allow fallback */ } and ensure ProjectServiceVersionError is
imported where ensureDaemonProjectReadyForFallback is defined so the instanceof
check works against the correct class.
🪄 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: 9edf4669-88a6-419e-8d55-614ea76b88e1
📒 Files selected for processing (3)
src/main.tssrc/multiplexer/persistence-methods.tssrc/multiplexer/worktree-graveyard.ts
Summary
Verification
Summary by CodeRabbit
New Features
Refactor
Tests