Repository navigation
refactor(ipc): import worktree, watcher and runtime-environment helpers from their owning modules - #30375
Conversation
…rs from their owning modules Removes the re-export lines in worktree-logic, worktree-base-directory-poller, filesystem-watcher and runtime-environment-transport-routing. Test mocks that supplied a re-exported name now mock the module that defines it, so they still reach the code under test.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Description check |  |
The description explains the refactor, motivation, scope, visual impact, and testing results. However, it lists N/A under Linked Issue despite the template requiring an actual issue link, and it omi… |
Add the required linked issue, or document the applicable maintainer exception. Include the Review, Notes, and Checklist sections from the repository template, with completed items or explicit explanations where applicable. |
| Docstring Coverage |  |
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 50 files. (5 skipped: 5… | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Title check The title clearly and concisely describes the main change: importing worktree, watcher, and runtime-environment helpers from their defining modules. Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request.
Full details: Description check
-
Explanation
The description explains the refactor, motivation, scope, visual impact, and testing results. However, it lists
N/Aunder Linked Issue despite the template requiring an actual issue link, and it omits the Review, Notes, and Checklist sections.Resolution
Add the required linked issue, or document the applicable maintainer exception. Include the Review, Notes, and Checklist sections from the repository template, with completed items or explicit explanations where applicable.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 50 files. (5 skipped: 5 over the file limit.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
- Fix all pre-merge checks with AI
✨ Finishing Touches: 💡 1
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 @coderabbitai help to get the list of available commands.
1 participant
ELI5
Four IPC modules were forwarding names that are defined in other files. Some tests mocked those names through the forwarding module. This PR imports each name from the file that defines it, moves those mocks to the same file, and removes the forwarding lines. Nothing the app does changes.
What Changed
ipc/worktree-logic.ts:computeBranchNameand related names, plusmergeWorktreeand the display-name helpersipc/worktree-base-directory-poller.ts:WORKTREE_BASE_BACKSTOP_TICKSipc/filesystem-watcher.ts: the local/remote removal helpers,registerFilesystemWatcherHandlersandcloseAllWatchersipc/runtime-environments/runtime-environment-transport-routing.ts:getRuntimeEnvironmentStatusipc,agent-hooks,skills,startup,runtimeand two top-levelsrc/mainmodules.worktree-branch-name,filesystem-watcher-handlersandruntime-environment-status-probe, across six tests.vi.mockcalls:runtime-environment-host-dispatch,runtime-environment-managed-host-disconnectandfolder-repo-git-upgrade-lifetime. The new marker-poller mock keeps the real module and overrides only the constant.Why
The user's rule is that no module re-exports another module's names. These four needed the mock moves, so they got their own PR instead of riding along with the mechanical one (#30369).
Three ipc modules are still pending, all for the same mock reason:
filesystem-auth, which also hasimport * asnamespace users in tests,markdown-documentsandpreflight.dashboard-payload-validationis skipped because open PRs #30145, #29953 and #28445 edit it.Linked Issue
N/A (SSH hardening round 2, de-barrel follow-up).
Visual Proof
N/A: an import-only refactor.
Testing
typecheck:ciis clean.src/main/ipc,agent-hooks,skills,startupandruntime/workspaces, plus every touched runtime and top-level module's tests and every changed test file, pass single-worker: 852 files, 8,091 tests.