chore(apps): consolidate accessor implementation to runtime (3/4) - #41378
Merged
Conversation
Contributor
|
Looks like this PR is ready to merge! 🎉 |
|
|
|
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (124)
💤 Files with no reviewable changes (106)
📜 Recent review details⏰ Context from checks skipped due to timeout. (20)
🧰 Additional context used📓 Path-based instructions (1)**/*.{ts,tsx,js}📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
Files:
🧠 Learnings (4)📚 Learning: 2026-02-26T19:25:44.063ZApplied to files:
📚 Learning: 2026-02-26T19:25:44.063ZApplied to files:
📚 Learning: 2026-05-06T12:21:44.083ZApplied to files:
📚 Learning: 2026-05-11T21:46:23.471ZApplied to files:
🪛 ast-grep (0.45.0)packages/apps/src/server/runtime/base/BaseRuntimeSubprocessController.ts[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec. (detect-child-process-typescript) 🔇 Additional comments (18)
WalkthroughThe PR removes host-side accessor implementations and ChangesAccessor consolidation
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (2)
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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #41378 +/- ##
===========================================
+ Coverage 68.72% 68.73% +0.01%
===========================================
Files 4151 4151
Lines 159513 159513
Branches 27988 27972 -16
===========================================
+ Hits 109621 109647 +26
+ Misses 44721 44686 -35
- Partials 5171 5180 +9
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
d-gubert
force-pushed
the
chore/apps-accessor-consolidation-2
branch
from
July 15, 2026 18:00
799c45f to
e1698b5
Compare
d-gubert
force-pushed
the
chore/apps-accessor-consolidation-3
branch
from
July 15, 2026 18:00
99e5b50 to
c8c10fe
Compare
d-gubert
force-pushed
the
chore/apps-accessor-consolidation-2
branch
from
July 15, 2026 18:49
e1698b5 to
91104bd
Compare
d-gubert
force-pushed
the
chore/apps-accessor-consolidation-3
branch
from
July 15, 2026 18:50
c8c10fe to
1658bb9
Compare
d-gubert
force-pushed
the
chore/apps-accessor-consolidation-2
branch
from
July 15, 2026 19:08
91104bd to
9630cd4
Compare
d-gubert
force-pushed
the
chore/apps-accessor-consolidation-3
branch
from
July 15, 2026 19:08
1658bb9 to
8bcb150
Compare
d-gubert
force-pushed
the
chore/apps-accessor-consolidation-2
branch
2 times, most recently
from
July 16, 2026 18:18
61c16fd to
8bc03d6
Compare
d-gubert
force-pushed
the
chore/apps-accessor-consolidation-3
branch
from
July 16, 2026 19:05
8bcb150 to
2e441d7
Compare
d-gubert
force-pushed
the
chore/apps-accessor-consolidation-2
branch
from
July 28, 2026 18:58
8bc03d6 to
c0d24d2
Compare
d-gubert
force-pushed
the
chore/apps-accessor-consolidation-3
branch
from
July 28, 2026 18:59
2e441d7 to
465c6a2
Compare
ggazzo
force-pushed
the
chore/apps-accessor-consolidation-2
branch
from
July 29, 2026 18:35
624af84 to
94bca38
Compare
ggazzo
force-pushed
the
chore/apps-accessor-consolidation-3
branch
from
July 29, 2026 18:35
465c6a2 to
f43eafd
Compare
d-gubert
force-pushed
the
chore/apps-accessor-consolidation-2
branch
from
July 29, 2026 20:16
94bca38 to
7deb704
Compare
d-gubert
force-pushed
the
chore/apps-accessor-consolidation-3
branch
from
July 29, 2026 20:16
f43eafd to
59d3fa4
Compare
Completes the primary objective: handleAccessorMessage and the entire accessor:* JSON-RPC category are gone. BaseRuntimeSubprocessController now dispatches only bridges:* (plus ready/log/error notifications), so there is a single app-originated RPC channel, a single permission model (the bridge do* wrappers), and accessor behavior lives in exactly one place - the base-runtime. - BaseRuntimeSubprocessController: delete handleAccessorMessage, ALLOWED_ACCESSOR_METHODS, isValidOrigin, getAccessorForOrigin, the accessor: branch in handleIncomingMessage, and the now-unused accessors/api fields (and their manager.getAccessorManager()/getApiManager() reads). The restart-time registration guard now lives in handleBridgeMessage via AppResourceBridge.REGISTRATION_METHODS (Phase 3). JSONRPC_METHOD_NOT_FOUND is kept - it is imported by ProxiedApp, AppListenerManager, AppVideoConfProvider. - AppListenerManager.executePostMessageSent: the one load-bearing host-side accessor consumer outside the RPC path now calls manager.getBridges().getUserBridge().doGetAppUser(appId) directly; the class no longer references AppAccessorManager. Physical deletion of the now-unreachable host accessor classes + AppAccessorManager is folded into follow-up #4 (documented), because it requires first un-threading the ignored accessors parameter from the sandbox-execution core (runTheCode) - a separate, behavior-neutral change the plan already carved out. The host accessors remain as dead code reachable by nothing on the subprocess path. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018FbxFGJWHiroxrdNPJRL3P
…oller test Phase 4 removed the accessor:* dispatch path (handleAccessorMessage) from BaseRuntimeSubprocessController, which DenoRuntimeSubprocessController extends. Four cases in DenoRuntimeSubprocessController.test.ts still called the deleted method and failed. Their behavior is now covered by the surviving handleBridgeMessage case plus the base-runtime accessor/RemoteBridges suites. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018FbxFGJWHiroxrdNPJRL3P
The base-runtime is the single source of truth for accessor behavior and the accessor:* message category is gone (Phase 4), so the host copies of the accessors are unreachable. This removes them and un-threads the vestigial accessor manager from the sandbox-execution core. - Drop the ignored `accessors: AppAccessorManager` parameter from the run*/ runTheCode methods of AppApi, AppSlashCommand, AppVideoConfProvider and AppOutboundCommunicationProvider, and the `accessors` field from their four managers (the app executes in the subprocess via sendRequest; the host accessor manager was never consulted). - Delete AppAccessorManager, its managers/index export, and AppManager's accessorManager field/constructor/getAccessorManager/purifyApp usage (purifyApp only cleared caches that were never populated). - Delete src/server/accessors/ (64 files) and the now-orphaned src/server/misc/UIHelper.ts. - Adapt tests: delete tests/server/accessors/ and the AppAccessorManager test; strip getAccessorManager/.accessors setup and assertions from the affected manager, AppManager and Deno-runtime tests. Behavior-neutral: host tsc error-set unchanged vs baseline; host node suites green (excluding the two deno-spawn integration suites, which require the deno binary and run in CI). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018FbxFGJWHiroxrdNPJRL3P
d-gubert
force-pushed
the
chore/apps-accessor-consolidation-3
branch
from
July 29, 2026 23:22
59d3fa4 to
ac77da9
Compare
d-gubert
marked this pull request as ready for review
July 29, 2026 23:33
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
ggazzo
approved these changes
Jul 30, 2026
ggazzo
added
the
stat: QA assured
dionisio-bot
Bot
added
the
stat: ready to merge
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
4 participants
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.
accessor:*message categoryProposed changes (including videos or screenshots)
Issue(s)
Related Jira task: ARCH-2185: reunite duplicated accessor implementations into a shared location
Steps to test or reproduce
Further comments
Summary by CodeRabbit