Repository navigation
refactor(runtime): import runtime helpers from their owning modules, not re-export shims - #30373
Merged
Merged
Conversation
…not re-export shims Removes the re-export lines in 22 src/main/runtime modules (rpc schemas, skill surface, worker identity, terminal viewport, teardown, git, e2ee crypto and more); 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 3 functions across 50 files. (50 skipped: … | 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: replacing imports from runtime re-export shims with imports from owning modules. Description check The description includes the core template sections, explains the refactor and rationale, documents skipped modules, provides visual-proof rationale, and reports detailed testing. Some optional or non… 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 includes the core template sections, explains the refactor and rationale, documents skipped modules, provides visual-proof rationale, and reports detailed testing. Some optional or non-critical sections, such as Review, Notes, and Checklist, are not completed.
-
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 3 functions across 50 files. (50 skipped: 50 over the file limit.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
✨ 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.
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
22 modules in
src/main/runtimepartly forwarded exports that live in other files. Code now imports each name from the file that defines it, and the forwarding lines are gone. Nothing the app does changes.What Changed
export {…} from,export * from, andimport+export { … }pairs) from:agent-prompt-submission-verification,agent-session-recovery-capsuleclaude-agent-teams-servicemobile-notification-dismissal-store,mobile-session-terminal-retirement-prooforca-runtime-gitorchestration/worker-terminal-process-liveness,worker-transcript-payloadrpc/e2ee-crypto,rpc/orchestration-mutation-executorrpc/methods/browser-schemas,client-ui-schemas,clipboard,workspace-cleanup-ui-schemarpc/methods/orchestration/worker/worker-release-completion,worker-start-schemaruntime-rpc,runtime-skill-command-surface,runtime-skill-typesstructured-worker-identity,terminal-viewport,worktree-teardownsrc/main/runtime,server,native-chatand one cross-version e2e unit test. Duplicate imports are merged.Why
The user's rule is: no barrel or re-export modules.
Skipped, because open PRs edit them:
orca-runtime.ts(feat(native-chat): + menu shows chat, disabled chat and CLI rows #29074, feat(cli): create workspaces on environment recipes #28439)runtime-worktree-agent-rows.ts(feat(agent-status): read OSC 7501 program status beside hooks #30261, feat(agent-status): show OSC 7501 program status from agents (part 2 of 2) #30145)sidebar-inputs/sort-stamps-producer.ts(feat(sidebar): sort Recent and Smart by the owning host's stamps, hold order under the pointer (dark) #30284)orchestration/{mailbox-pointer-delivery,federation-sync,db}.ts(fix(orchestration): point a terminal-only pane's own direct mailbox #29925, feat(orchestration):send --deliver nowsteers a busy chat-mode worker #29540, fix(orchestration): wake a chat coordinator whose ended turn left a batch unacknowledged #29006)rpc/methods/orchestration/schemas.ts(feat(orchestration):send --deliver nowsteers a busy chat-mode worker #29540)rpc/methods/structured-agent-session-schemas.ts(feat(native-chat): + menu shows chat, disabled chat and CLI rows #29074, Sleep stops a workspace's native chat agents #28445)structured-agent-runtime-registrations.ts(feat(native-chat): chat agents are offered wherever their chat program runs, not only where their CLI is installed #28709)Skipped on purpose:
orca-runtime-create-terminal-dependencies.tsandruntime-linear-command-dependencies.ts. They re-exportnode:cryptoas an injectable dependency seam, and tests mock that seam. Removing it changes how those tests inject randomness, which is a design change rather than a de-barrel.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/runtime, plus the changedserver,native-chatand e2e unit test folders, single worker: 1,507 files, 15,461 tests, all passing.