chore(apps): consolidate accessor implementation to runtime (1/4)

[d-gubert]

Proposed changes (including videos or screenshots)

This PR adds a comprehensive design document (base-runtime-accessor-consolidation.md) that outlines a phased plan to consolidate accessor logic from packages/apps/src/server/accessors/ into the base-runtime, eliminating the need for the accessor:* JSON-RPC message category and making the base-runtime the single source of truth for accessor behavior.

Current state: Accessor calls from apps take three paths:

1. Pure proxy via accessor:* messages (forwarded to host)
2. Local implementation + direct bridge calls
3. Fully local implementation

Target state: Consolidate all accessor logic into the base-runtime, with side effects flowing through the existing bridges:* RPC channel.

This document represents significant architectural work to improve code organization and eliminate technical debt. It includes:

• Detailed analysis of all accessor classes and their dependencies
• Identification and resolution of behavioral drift between runtime and host implementations
• Comprehensive design for the new AppResourceBridge to close gaps
• Clear phased implementation plan that keeps both test suites green
• Explicit trade-offs and follow-up work items

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

• New Features
  • Added a centralized bridge communication layer for app runtime operations.
  • Added automatic UI block, element, and action ID assignment.
  • Standardized app-identity handling for bridge “do” calls.
• Documentation
  • Added a proposal outlining accessor consolidation and migration rules.
  • Documented “APP_ID” normalization exceptions and limitations.
• Tests
  • Expanded coverage for bridge routing, request recording, and accessor parity.
  • Updated accessor-related unit test expectations for normalized app identity.

[changeset-bot]

⚠️ No Changeset found

Latest commit: 5b74754

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[CLAassistant]

All committers have signed the CLA.

[dionisio-bot]

Looks like this PR is not ready to merge, because of the following issues:

• This PR is targeting the wrong base branch. It should target 8.8.0, but it targets 8.7.0

Please fix the issues and try again

If you have any trouble, please check the PR guidelines

[coderabbitai]

Walkthrough

The PR documents accessor consolidation, adds RemoteBridges and a local UIHelper, migrates HTTP, modify, notifier, and room paths to bridge calls, introduces APP_ID normalization guidance, and adds bridge and parity tests.

Changes

Accessor consolidation

Layer / File(s)	Summary
Protocol and migration design docs/proposals/apps-accessor-consolidation/*	The proposal defines accessor dispositions, bridge contracts, APP_ID normalization and exceptions, drift handling, and phased migration steps.
Remote bridge foundation packages/apps/base-runtime/src/lib/bridges/RemoteBridges.ts, packages/apps/base-runtime/src/lib/UIHelper.ts, packages/apps/base-runtime/src/lib/bridges/tests/RemoteBridges.test.ts	RemoteBridges validates do* calls, builds bridge JSON-RPC methods, unwraps results, maps errors, and exposes typed bridge getters; UIHelper assigns missing UI identifiers.
Accessor bridge migration packages/apps/base-runtime/src/lib/accessors/http.ts, packages/apps/base-runtime/src/lib/accessors/modify/*, packages/apps/base-runtime/src/lib/accessors/notifier.ts, packages/apps/base-runtime/src/lib/roomFactory.ts	Accessor operations and the room mock route through RemoteBridges, using APP_ID placeholders for caller-identity parameters.
Migration validation packages/apps/base-runtime/src/lib/accessors/tests/*	Existing request expectations are updated, and the parity harness records ordered calls, resolves canned responses, and verifies migrated traffic.

Estimated code review effort: 3 (Moderate) | ~30 minutes

Suggested labels: type: chore

Suggested reviewers: ggazzo, ricardogarim

Sequence Diagram(s)

sequenceDiagram
  participant Accessor
  participant RemoteBridges
  participant Messenger
  participant HostBridge
  Accessor->>RemoteBridges: invoke do* operation
  RemoteBridges->>Messenger: send bridges:<bridge>:<doMethod>
  Messenger->>HostBridge: dispatch request
  HostBridge-->>Messenger: return JSON-RPC result
  Messenger-->>RemoteBridges: return response
  RemoteBridges-->>Accessor: return unwrapped result

Loading

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title matches the main change: consolidating accessor implementation into base-runtime.

Warning

Review ran into problems

🔥 Problems

Errors were encountered while retrieving linked issues.

Errors (1)

• CONSOLIDATION-1: Request failed with status code 401

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 68.46%. Comparing base (abab8a0) to head (5b74754).
⚠️ Report is 99 commits behind head on develop.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop   #41376      +/-   ##
===========================================
- Coverage    68.47%   68.46%   -0.01%     
===========================================
  Files         4092     4092              
  Lines       158285   158285              
  Branches     28622    28667      +45     
===========================================
- Hits        108379   108373       -6     
- Misses       44874    44877       +3     
- Partials      5032     5035       +3     

Flag	Coverage Δ
e2e	59.20% <ø> (-0.14%)	⬇️
e2e-api	45.77% <ø> (+0.27%)	⬆️
unit	70.32% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[coderabbitai]

Actionable comments posted: 4

🧹 Nitpick comments (2)

docs/proposals/apps-accessor-consolidation/README.md (1)

261-261: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Specify a language for the fenced code block.

Static analysis tool markdownlint-cli2 flagged that this fenced code block lacks a specified language. Consider adding text or plaintext for better formatting.

📝 Proposed fix

-```
+```text
 AppResourceBridge (host-side, internal — not part of the app-facing definition surface)

🤖 Prompt for 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.

In `@docs/proposals/apps-accessor-consolidation/README.md` at line 261, Specify an
explicit language for the fenced code block containing the AppResourceBridge
text, using text or plaintext as appropriate, while preserving the block’s
existing content.

Source: Linters/SAST tools

packages/apps/base-runtime/src/lib/accessors/modify/ModifyExtender.ts (1)

24-25: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Remove the repeated implementation comments.

The late-bound callback is already concise and self-explanatory.

• packages/apps/base-runtime/src/lib/accessors/modify/ModifyExtender.ts#L24-L25: remove the constructor comment.
• packages/apps/base-runtime/src/lib/accessors/modify/ModifyUpdater.ts#L31-L32: remove the duplicate comment.
• packages/apps/base-runtime/src/lib/accessors/notifier.ts#L19-L20: remove the duplicate comment.

As per coding guidelines, “Avoid code comments in the implementation.” <coding_guidelines>

🤖 Prompt for 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.

In `@packages/apps/base-runtime/src/lib/accessors/modify/ModifyExtender.ts` around
lines 24 - 25, Remove the repeated late-bound callback implementation comments
from the constructors in ModifyExtender.ts (lines 24-25), ModifyUpdater.ts
(lines 31-32), and notifier.ts (lines 19-20). Leave the callback implementations
unchanged.

Source: Coding guidelines

🤖 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 `@packages/apps/base-runtime/src/lib/accessors/notifier.ts`:
- Around line 63-64: Update getAppUser to return Promise<IUser>, validate the
result of doGetAppUser, and reject or throw when no app user is returned instead
of propagating undefined. Then remove the downstream as IUser casts and optional
chaining that are no longer needed, preserving the required sender and typing
username behavior.

In
`@packages/apps/base-runtime/src/lib/accessors/tests/helpers/parityHarness.test.ts`:
- Around line 46-59: Update the parity assertion in the
ModifyExtender.extendMessage/finish test to validate the recorded calls’
parameters as well as their method names. Use emitted() or equivalent call
inspection to assert the expected ordered arguments, including APP_ID
normalization, while preserving the existing bridge-method ordering check.

In `@packages/apps/base-runtime/src/lib/accessors/tests/helpers/parityHarness.ts`:
- Line 6: Update the documentation reference in the parityHarness comment to
point to docs/proposals/apps-accessor-consolidation/README.md, replacing the
outdated docs/base-runtime-accessor-consolidation.md path.
- Around line 64-67: Update the sender callback that builds each RecordedCall to
snapshot requestDescriptor.params, including nested objects and arrays, before
pushing it into calls. Preserve the existing normalization of non-array params
while ensuring later accessor mutations cannot change previously recorded calls.

---

Nitpick comments:
In `@docs/proposals/apps-accessor-consolidation/README.md`:
- Line 261: Specify an explicit language for the fenced code block containing
the AppResourceBridge text, using text or plaintext as appropriate, while
preserving the block’s existing content.

In `@packages/apps/base-runtime/src/lib/accessors/modify/ModifyExtender.ts`:
- Around line 24-25: Remove the repeated late-bound callback implementation
comments from the constructors in ModifyExtender.ts (lines 24-25),
ModifyUpdater.ts (lines 31-32), and notifier.ts (lines 19-20). Leave the
callback implementations unchanged.

🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: bb9acd44-1e1a-4550-b712-91c0c2d66aad

📥 Commits

Reviewing files that changed from the base of the PR and between 012dd32 and 477d50c.

📒 Files selected for processing (16)

• docs/proposals/apps-accessor-consolidation/README.md
• docs/proposals/apps-accessor-consolidation/base-runtime-app-id-exceptions.md
• packages/apps/base-runtime/src/lib/UIHelper.ts
• packages/apps/base-runtime/src/lib/accessors/http.ts
• packages/apps/base-runtime/src/lib/accessors/modify/ModifyCreator.ts
• packages/apps/base-runtime/src/lib/accessors/modify/ModifyExtender.ts
• packages/apps/base-runtime/src/lib/accessors/modify/ModifyUpdater.ts
• packages/apps/base-runtime/src/lib/accessors/notifier.ts
• packages/apps/base-runtime/src/lib/accessors/tests/ModifyCreator.test.ts
• packages/apps/base-runtime/src/lib/accessors/tests/ModifyExtender.test.ts
• packages/apps/base-runtime/src/lib/accessors/tests/ModifyUpdater.test.ts
• packages/apps/base-runtime/src/lib/accessors/tests/helpers/parityHarness.test.ts
• packages/apps/base-runtime/src/lib/accessors/tests/helpers/parityHarness.ts
• packages/apps/base-runtime/src/lib/bridges/RemoteBridges.ts
• packages/apps/base-runtime/src/lib/bridges/tests/RemoteBridges.test.ts
• packages/apps/base-runtime/src/lib/roomFactory.ts

📜 Review details

⏰ Context from checks skipped due to timeout. (4)

• GitHub Check: cubic · AI code reviewer
• GitHub Check: Hacktron Security Check
• GitHub Check: 📦 Build Packages
• GitHub Check: CodeQL-Build

🧰 Additional context used

📓 Path-based instructions (1)

**/*.{ts,tsx,js}

📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)

**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation

Files:

• packages/apps/base-runtime/src/lib/accessors/tests/helpers/parityHarness.test.ts
• packages/apps/base-runtime/src/lib/accessors/tests/helpers/parityHarness.ts
• packages/apps/base-runtime/src/lib/accessors/tests/ModifyCreator.test.ts
• packages/apps/base-runtime/src/lib/bridges/tests/RemoteBridges.test.ts
• packages/apps/base-runtime/src/lib/UIHelper.ts
• packages/apps/base-runtime/src/lib/roomFactory.ts
• packages/apps/base-runtime/src/lib/accessors/tests/ModifyUpdater.test.ts
• packages/apps/base-runtime/src/lib/accessors/notifier.ts
• packages/apps/base-runtime/src/lib/accessors/modify/ModifyExtender.ts
• packages/apps/base-runtime/src/lib/accessors/http.ts
• packages/apps/base-runtime/src/lib/accessors/tests/ModifyExtender.test.ts
• packages/apps/base-runtime/src/lib/bridges/RemoteBridges.ts
• packages/apps/base-runtime/src/lib/accessors/modify/ModifyUpdater.ts
• packages/apps/base-runtime/src/lib/accessors/modify/ModifyCreator.ts

🧠 Learnings (3)

📚 Learning: 2026-02-26T19:25:44.063Z

Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In the Rocket.Chat repository, do not reference Biome lint rules in code review feedback. Biome is not used even if biome.json exists; only reference Biome rules if there is explicit, project-wide usage documented. For TypeScript files, review lint implications without Biome guidance unless the project enables Biome rules.

Applied to files:

• packages/apps/base-runtime/src/lib/accessors/tests/helpers/parityHarness.test.ts
• packages/apps/base-runtime/src/lib/accessors/tests/helpers/parityHarness.ts
• packages/apps/base-runtime/src/lib/accessors/tests/ModifyCreator.test.ts
• packages/apps/base-runtime/src/lib/bridges/tests/RemoteBridges.test.ts
• packages/apps/base-runtime/src/lib/UIHelper.ts
• packages/apps/base-runtime/src/lib/roomFactory.ts
• packages/apps/base-runtime/src/lib/accessors/tests/ModifyUpdater.test.ts
• packages/apps/base-runtime/src/lib/accessors/notifier.ts
• packages/apps/base-runtime/src/lib/accessors/modify/ModifyExtender.ts
• packages/apps/base-runtime/src/lib/accessors/http.ts
• packages/apps/base-runtime/src/lib/accessors/tests/ModifyExtender.test.ts
• packages/apps/base-runtime/src/lib/bridges/RemoteBridges.ts
• packages/apps/base-runtime/src/lib/accessors/modify/ModifyUpdater.ts
• packages/apps/base-runtime/src/lib/accessors/modify/ModifyCreator.ts

📚 Learning: 2026-02-26T19:25:44.063Z

Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In this repository (RocketChat/Rocket.Chat), Biome lint rules are not used even if a biome.json exists. When reviewing TypeScript files (e.g., packages/ui-voip/src/providers/useMediaSession.ts), ensure lint suggestions do not reference Biome-specific rules. Rely on general ESLint/TypeScript lint rules and project conventions instead.

Applied to files:

• packages/apps/base-runtime/src/lib/accessors/tests/helpers/parityHarness.test.ts
• packages/apps/base-runtime/src/lib/accessors/tests/helpers/parityHarness.ts
• packages/apps/base-runtime/src/lib/accessors/tests/ModifyCreator.test.ts
• packages/apps/base-runtime/src/lib/bridges/tests/RemoteBridges.test.ts
• packages/apps/base-runtime/src/lib/UIHelper.ts
• packages/apps/base-runtime/src/lib/roomFactory.ts
• packages/apps/base-runtime/src/lib/accessors/tests/ModifyUpdater.test.ts
• packages/apps/base-runtime/src/lib/accessors/notifier.ts
• packages/apps/base-runtime/src/lib/accessors/modify/ModifyExtender.ts
• packages/apps/base-runtime/src/lib/accessors/http.ts
• packages/apps/base-runtime/src/lib/accessors/tests/ModifyExtender.test.ts
• packages/apps/base-runtime/src/lib/bridges/RemoteBridges.ts
• packages/apps/base-runtime/src/lib/accessors/modify/ModifyUpdater.ts
• packages/apps/base-runtime/src/lib/accessors/modify/ModifyCreator.ts

📚 Learning: 2026-05-06T12:21:44.083Z

Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 40256
File: apps/meteor/client/components/CreateDiscussion/CreateDiscussion.tsx:121-149
Timestamp: 2026-05-06T12:21:44.083Z
Learning: Field wrappers in rocket.chat/fuselage-forms (Field, FieldLabel, FieldRow, FieldError, FieldHint) auto-create htmlFor/id associations, aria-describedby, and role="alert" for errors. Do not manually set htmlFor, id, aria-describedby, or role attributes when using these wrappers. This automatic wiring does not apply to plain rocket.chat/fuselage components, which require explicit ID wiring per the accessibility docs. In code reviews, prefer using fuselage-forms wrappers for form fields and verify there is no unnecessary manual ID/aria wiring in files that use these wrappers. If a component uses plain fuselage components, ensure proper id wiring as per docs.

Applied to files:

• packages/apps/base-runtime/src/lib/accessors/tests/helpers/parityHarness.test.ts
• packages/apps/base-runtime/src/lib/accessors/tests/helpers/parityHarness.ts
• packages/apps/base-runtime/src/lib/accessors/tests/ModifyCreator.test.ts
• packages/apps/base-runtime/src/lib/bridges/tests/RemoteBridges.test.ts
• packages/apps/base-runtime/src/lib/UIHelper.ts
• packages/apps/base-runtime/src/lib/roomFactory.ts
• packages/apps/base-runtime/src/lib/accessors/tests/ModifyUpdater.test.ts
• packages/apps/base-runtime/src/lib/accessors/notifier.ts
• packages/apps/base-runtime/src/lib/accessors/modify/ModifyExtender.ts
• packages/apps/base-runtime/src/lib/accessors/http.ts
• packages/apps/base-runtime/src/lib/accessors/tests/ModifyExtender.test.ts
• packages/apps/base-runtime/src/lib/bridges/RemoteBridges.ts
• packages/apps/base-runtime/src/lib/accessors/modify/ModifyUpdater.ts
• packages/apps/base-runtime/src/lib/accessors/modify/ModifyCreator.ts

🪛 LanguageTool

docs/proposals/apps-accessor-consolidation/README.md

[style] ~203-~203: ‘exactly the same’ might be wordy. Consider a shorter alternative.
Context: ...gistry`. The new bridge methods receive exactly the same serialized shapes, so **the wire semant...

(EN_WORDINESS_PREMIUM_EXACTLY_THE_SAME)

🪛 markdownlint-cli2 (0.23.0)

docs/proposals/apps-accessor-consolidation/README.md

[warning] 261-261: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

🔇 Additional comments (8)

packages/apps/base-runtime/src/lib/accessors/modify/ModifyExtender.ts (1)

11-23: LGTM!

Also applies to: 26-30, 39-47, 57-65

packages/apps/base-runtime/src/lib/accessors/modify/ModifyUpdater.ts (1)

14-30: LGTM!

Also applies to: 33-33, 75-87, 118-118, 148-148

packages/apps/base-runtime/src/lib/accessors/notifier.ts (1)

9-18: LGTM!

Also applies to: 21-22, 59-61

packages/apps/base-runtime/src/lib/accessors/tests/ModifyCreator.test.ts (1)

59-59: LGTM!

packages/apps/base-runtime/src/lib/accessors/tests/ModifyExtender.test.ts (1)

40-40: LGTM!

Also applies to: 51-51, 66-66, 77-77, 92-92, 103-103

packages/apps/base-runtime/src/lib/accessors/tests/ModifyUpdater.test.ts (1)

45-45: LGTM!

Also applies to: 64-64, 79-79, 101-101

packages/apps/base-runtime/src/lib/accessors/tests/helpers/parityHarness.test.ts (1)

1-44: LGTM!

packages/apps/base-runtime/src/lib/accessors/tests/helpers/parityHarness.ts (1)

23-53: LGTM!

Also applies to: 61-63, 69-80

[cubic-dev-ai]

All reported issues were addressed across 16 files

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>