Repository navigation
refactor(ipc): import ipc helpers from their owning modules, not re-export shims - #30369
Merged
Merged
Conversation
…xport shims Deletes local-agent-install-dir-detection, notification-burst-cooldown, terminal-git-credential-guard and tui-agent-detection-commands (pure re-export modules) and the re-export lines in seven more ipc modules; importers and test mocks name the real modules.
Contributor
|
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 |
|---|---|---|---|
| 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 29 files. | 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 IPC helpers from their owning modules instead of re-export shims. Description check The description covers the core template sections, including the plain-language summary, changes, rationale, linked-issue status, visual proof, and testing. It omits the Review, Agent skill upstream b… 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 covers the core template sections, including the plain-language summary, changes, rationale, linked-issue status, visual proof, and testing. It omits the Review, Agent skill upstream boundary, Notes, and Checklist sections, but the main change and verification details are clear.
-
- 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.
4 tasks done
frbarbre
added a commit
to frbarbre/orca
that referenced
this pull request
Oct 18, 2026
…tup split Upstream dropped the watcher type re-exports from parcel-watcher-process.ts (stablyai#30369) and the setupScriptLaunchMode setting (stablyai#29819: setup always gets its own tab), so the language-server watcher imports the owning modules and the fork's background-terminal launcher no longer splits setup into a pane.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ELI5
Eleven files in
src/main/ipcpartly or entirely forwarded exports that live in other files. This PR changes the code to import each name from the file that defines it and removes the forwarding. Nothing the app does changes.What Changed
ipc/local-agent-install-dir-detection.ts,notification-burst-cooldown.ts,terminal-git-credential-guard.tsandtui-agent-detection-commands.ts. The last was anexport *fromshared/.export {…} from,export * from, andexport type * from) in:feedback-image-attachments,feedback-requestparcel-watcher-process,pluginspty-pending-data-drain-queue,pty/host-env/pi-agentruntime-watcher-process-poolipc,preflight,plugins,notifications,runtimeandagent-hooks, merging duplicate imports where needed.ipc/local-agent-install-dir-detectionpath, so after the rewrite their mocks no longer reached the code under test. They now mockshared/local-agent-install-dir-detection. Two of the three tests failed until this was fixed.Why
The user's rule is that there are no barrel or re-export modules. These files hide which module owns a function and give a function two import paths. As those mocks showed, that lets a test mock the wrong path without noticing.
Seven more
ipcmodules re-export names that existing test mocks depend on:filesystem-auth,filesystem-watcher,markdown-documents,preflight,runtime-environment-transport-routing,worktree-base-directory-pollerandworktree-logic. They get a separate PR so this one stays mechanical.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:ci: clean.src/main/ipc,plugins,preflight,notificationsandagent-hooks, plus every changed test file, single worker: 718 files and 6,961 tests, all passing.src/cli/skills.test.tsandruntime/file-watcher-host.test.tsalso pass.