Skip to content

refactor!: Standardise all controllers and services#3922

Merged
Mrtenz merged 17 commits intomainfrom
controller-refactors
Mar 26, 2026
Merged

refactor!: Standardise all controllers and services#3922
Mrtenz merged 17 commits intomainfrom
controller-refactors

Conversation

@Mrtenz
Copy link
Member

@Mrtenz Mrtenz commented Mar 25, 2026

Feature branch of controller/service standardisation.


Note

High Risk
Large refactor across core controller/service messaging and type exports with multiple breaking renames; integration risk is mainly from missed action/event name updates or mismatched generated types at runtime/build time.

Overview
This PR standardizes controller/service messaging APIs by introducing auto-generated *-method-action-types.ts files and switching controllers/services (e.g., CronjobController, SnapInterfaceController, MultichainRoutingService, ExecutionService) to registerMethodActionHandlers + MESSENGER_EXPOSED_METHODS instead of manually registering action handlers.

It also includes several breaking renames and call-site updates: MultichainRouterMultichainRoutingService, SnapController:getAll usages → SnapController:getRunnableSnaps, and SnapController:getSnapController:getSnap, with corresponding test/index export updates. ExecutionService is reworked from an interface + AbstractExecutionService base into a single abstract ExecutionService class, extracting setupMultiplex into its own module.

Tooling is updated to enforce consistency: adds a workspace generate-method-action-types script (run during root lint with --check) and documents the breaking API/type changes in @metamask/snaps-controllers’ changelog.

Written by Cursor Bugbot for commit 3f024e3. This will update automatically on new commits. Configure here.

Mrtenz and others added 10 commits March 23, 2026 14:08
…3907)

This renames all `SnapController` action and event names and types to
follow the `Controller...Action` pattern used in most other controllers.
I've also added the `generate-method-actions` script used in
`MetaMask/core` to automatically generate these types.

I found numerous unrelated type errors in test files that I fixed in
this pull request as well, since it was a bit difficult to determine if
a type error was caused by this refactor or not, in some cases.

## Breaking changes

- All `SnapController` action types were renamed from `DoSomething` to
`SnapControllerDoSomethingAction`.
   - `GetSnap` is now `SnapControllerGetSnapAction`.
      - Note: The method is now called `getSnap` instead of `get`.
   - `HandleSnapRequest` is now `SnapControllerHandleRequestAction`.
   - `GetSnapState` is now `SnapControllerGetSnapStateAction`.
   - `HasSnap` is now `SnapControllerHasSnapAction`.
      - Note: The method is now called `hasSnap` instead of `has`.
   - `UpdateSnapState` is now `SnapControllerUpdateSnapStateAction`.
   - `ClearSnapState` is now `SnapControllerClearSnapStateAction`.
   - `UpdateRegistry` is now `SnapControllerUpdateRegistryAction`.
   - `EnableSnap` is now `SnapControllerEnableSnapAction`.
      - Note: The method is now called `enableSnap` instead of `enable`.
   - `DisableSnap` is now `SnapControllerDisableSnapAction`.
- Note: The method is now called `disableSnap` instead of `disable`.
   - `RemoveSnap` is now `SnapControllerRemoveSnapAction`.
      - Note: The method is now called `removeSnap` instead of `remove`.
   - `GetPermittedSnaps` is now `SnapControllerGetPermittedSnapsAction`.
- Note: The method is now called `getPermittedSnaps` instead of
`getPermitted`.
   - `GetAllSnaps` is now `SnapControllerGetAllSnapsAction`.
- Note: The method is now called `getAllSnaps` instead of `getAll`.
   - `GetRunnableSnaps` is now `SnapControllerGetRunnableSnapsAction`.
   - `StopAllSnaps` is now `SnapControllerStopAllSnapsAction`.
- `IncrementActiveReferences` is now
`SnapControllerIncrementActiveReferencesAction`.
- `DecrementActiveReferences` is now
`SnapControllerDecrementActiveReferencesAction`.
   - `InstallSnaps` is now `SnapControllerInstallSnapsAction`.
- Note: The method is now called `installSnaps` instead of `install`.
- `DisconnectOrigin` is now `SnapControllerRemoveSnapFromSubjectAction`.
- `RevokeDynamicPermissions` is now
`SnapControllerRevokeDynamicSnapPermissionsAction`.
   - `GetSnapFile` is now `SnapControllerGetSnapFileAction`.
- `IsMinimumPlatformVersion` is now
`SnapControllerIsMinimumPlatformVersionAction`.
   - `SetClientActive` is now `SnapControllerSetClientActiveAction`.
- All `SnapController` event types were renamed from `OnSomething` to
`SnapControllerOnSomethingEvent`.
- `SnapStateChange` was removed in favour of
`SnapControllerStateChangeEvent`.
   - `SnapBlocked` is now `SnapControllerSnapBlockedEvent`.
- `SnapInstallStarted` is now `SnapControllerSnapInstallStartedEvent`.
   - `SnapInstallFailed` is now `SnapControllerSnapInstallFailedEvent`.
   - `SnapInstalled` is now `SnapControllerSnapInstalledEvent`.
   - `SnapUninstalled` is now `SnapControllerSnapUninstalledEvent`.
   - `SnapUnblocked` is now `SnapControllerSnapUnblockedEvent.
   - `SnapUpdated` is now `SnapControllerSnapUpdatedEvent`.
   - `SnapRolledback` is now `SnapControllerSnapRolledbackEvent`.
   - `SnapTerminated` is now `SnapControllerSnapTerminatedEvent`.
   - `SnapEnabled` is now `SnapControllerSnapEnabledEvent`.
   - `SnapDisabled` is now `SnapControllerSnapDisabledEvent`.

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> **High Risk**
> Large breaking rename of `SnapController` messenger action/event names
(e.g., `get`→`getSnap`, `getAll`→`getAllSnaps`) and their exported
TypeScript types across multiple controllers/tests, which can easily
break downstream integrations. Adds a generated action-type source file
and enforces it via lint, so CI failures are likely if regeneration is
missed.
> 
> **Overview**
> **Standardizes `SnapController` messenger API naming** by renaming
action/event type aliases to the `SnapController…Action` /
`SnapController…Event` pattern and updating call sites (notably
`SnapController:get`→`SnapController:getSnap` and
`SnapController:getAll`→`SnapController:getAllSnaps`) across controllers
and tests.
> 
> **Introduces generated method action types** by adding
`SnapController-method-action-types.ts` (auto-generated union of method
action types), wiring workspace scripts (`generate-method-action-types`)
and enforcing it in root `lint`.
> 
> Also includes small cleanup/consistency fixes in tests (e.g., metadata
key `anonymous`→`includeInDebugSnapshot`) and removes now-unneeded lint
suppression comments in execution services.
> 
> <sup>Written by [Cursor
Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit
7386c7f. This will update automatically
on new commits. Configure
[here](https://cursor.com/dashboard?tab=bugbot).</sup>
<!-- /CURSOR_SUMMARY -->
)

This renames all `CronjobController` action and event names and types to
follow the `Controller...Action` pattern used in most other controllers.

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> **Medium Risk**
> Primarily a type-level refactor but marked breaking because exported
action type names and cronjob exports change; downstream TypeScript code
and messenger typings may need updates.
> 
> **Overview**
> **Standardizes `CronjobController` messenger action typing.** The PR
moves cronjob method action definitions into a new auto-generated
`CronjobController-method-action-types.ts`, renaming the exported action
types to the `CronjobController…Action` convention.
> 
> `CronjobController` now wires its messenger via
`registerMethodActionHandlers` with an explicit
`MESSENGER_EXPOSED_METHODS` list, and narrows messenger generics by
separating controller actions/events from externally *allowed* ones.
Public exports from `cronjob/index.ts` are adjusted accordingly, and the
changelog is updated to document the new breaking action type names
(including `CronjobControllerScheduleAction`,
`CronjobControllerCancelAction`, `CronjobControllerGetAction`).
> 
> <sup>Written by [Cursor
Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit
e5867a2. This will update automatically
on new commits. Configure
[here](https://cursor.com/dashboard?tab=bugbot).</sup>
<!-- /CURSOR_SUMMARY -->
…es (#3912)

This renames all `SnapInterfaceController` action and event names and
types to follow the `Controller...Action` pattern used in most other
controllers.

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> **Medium Risk**
> Breaking change that renames exported `SnapInterfaceController` action
types and adjusts messenger handler registration; downstream packages
relying on the old type names or exports will fail to compile. Runtime
behavior should be unchanged but messaging wiring changes could affect
integration if method exposure lists drift.
> 
> **Overview**
> **BREAKING:** Renames `SnapInterfaceController` action type aliases to
the standardized `SnapInterfaceController...Action` naming scheme,
updates `CHANGELOG`, and adjusts consumers (`SnapController`,
`SnapInsightsController`, and `snaps-simulation`) to use the new types.
> 
> Moves `SnapInterfaceController` method action type definitions into a
new auto-generated `SnapInterfaceController-method-action-types.ts`,
updates `interface/index.ts` exports, and replaces per-action
`registerActionHandler` calls with `registerMethodActionHandlers` driven
by a single `MESSENGER_EXPOSED_METHODS` list.
> 
> <sup>Written by [Cursor
Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit
4cd791b. This will update automatically
on new commits. Configure
[here](https://cursor.com/dashboard?tab=bugbot).</sup>
<!-- /CURSOR_SUMMARY -->
…d use messenger exposed methods pattern (#3913)

This updates the `MultichainRouter` to use messenger exposed methods.
Actions were already named properly.

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> **Medium Risk**
> Breaking rename and action-type reshaping will require downstream
updates, and the switch to `SnapController:getRunnableSnaps` changes the
dependency contract for protocol snap routing. Runtime logic appears
largely unchanged but touches request routing paths.
> 
> **Overview**
> Renames the multichain RPC router from `MultichainRouter` to
`MultichainRoutingService` and updates exported types/namespace action
strings accordingly (**breaking change**).
> 
> Refactors messenger integration to use `registerMethodActionHandlers`
with auto-generated method action types, and changes protocol-snap
discovery to call `SnapController:getRunnableSnaps` directly (no longer
requires `SnapController:getAllSnaps`). Tests and controller test-utils
are updated to use the new messenger helpers and action names, and the
changelog documents both breaking changes.
> 
> <sup>Written by [Cursor
Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit
5b0d482. This will update automatically
on new commits. Configure
[here](https://cursor.com/dashboard?tab=bugbot).</sup>
<!-- /CURSOR_SUMMARY -->

---------

Co-authored-by: Frederik Bolding <frederik.bolding@gmail.com>
…RunnableSnaps` (#3915)

This updates `SnapInsightsController` to use
`SnapController:getRunnableSnaps` instead of
`SnapController:getAllSnaps`.

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> **Medium Risk**
> Introduces a breaking change to `SnapInsightsController`'s messenger
dependencies by switching from `SnapController:getAllSnaps` to
`SnapController:getRunnableSnaps`, which could affect clients/wiring and
which snaps are queried for insights.
> 
> **Overview**
> `SnapInsightsController` now queries Snaps via
`SnapController:getRunnableSnaps` instead of fetching all Snaps and
filtering locally, updating its allowed action types accordingly.
> 
> Tests and controller test-utils were updated to delegate/mock
`getRunnableSnaps`, and the `SignatureController` state-change event
type was renamed/propagated to `SignatureControllerStateChangeEvent`.
The changelog documents the **breaking** dependency update, and
`insights/index.ts` now uses explicit type/value exports instead of
`export *`.
> 
> <sup>Written by [Cursor
Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit
6678cea. This will update automatically
on new commits. Configure
[here](https://cursor.com/dashboard?tab=bugbot).</sup>
<!-- /CURSOR_SUMMARY -->
This renames all `ExecutionService` action and event names and types to
follow the `Service...Action` pattern used in most other services.


<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> **Medium Risk**
> Breaking API/type changes to `ExecutionService` and its messenger
actions/events may require coordinated updates across consumers; runtime
logic is largely moved/renamed but touches snap execution/termination
pathways.
> 
> **Overview**
> Refactors `ExecutionService` to match the standard
`ServiceName...Action` / `ServiceName...Event` naming pattern, including
new auto-generated `ExecutionService-method-action-types.ts` and renamed
event types (e.g. `ExecutionServiceUnhandledErrorEvent`,
`ExecutionServiceOutboundRequestEvent`).
> 
> Replaces the old `ExecutionService` interface +
`AbstractExecutionService` base class by making `ExecutionService` the
abstract base class, updating all concrete implementations
(iframe/offscreen/proxy/webview/node) and public exports accordingly,
and adjusting tests/simulation/jest helpers to use the new types and
messenger handler registration.
> 
> <sup>Written by [Cursor
Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit
deff471. This will update automatically
on new commits. Configure
[here](https://cursor.com/dashboard?tab=bugbot).</sup>
<!-- /CURSOR_SUMMARY -->

---------

Co-authored-by: Frederik Bolding <frederik.bolding@gmail.com>
#3917)

This refactors the `WebSocketService` to use the messenger exposed
methods.

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> **Medium Risk**
> Moderate risk because it changes how `WebSocketService` actions are
registered/exposed on the messenger and makes previously private methods
public, which could break callers relying on old action types or handler
names.
> 
> **Overview**
> Refactors `WebSocketService` to register messenger actions via
`registerMethodActionHandlers` using an explicit
`MESSENGER_EXPOSED_METHODS` list, replacing per-action
`registerActionHandler` wiring.
> 
> Adds an auto-generated `WebSocketService-method-action-types.ts` file
for strongly-typed method action definitions, updates `WebSocketService`
to use these types, and adjusts exports in `websocket/index.ts` to
export the service class plus selected types (instead of `export *`).
> 
> <sup>Written by [Cursor
Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit
44b0f39. This will update automatically
on new commits. Configure
[here](https://cursor.com/dashboard?tab=bugbot).</sup>
<!-- /CURSOR_SUMMARY -->
This removes `activeReferences` from the Snap runtime data, including
the methods to increment and decrement it. This was never used in
production.

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> **Medium Risk**
> Removes public `SnapController` APIs
(`incrementActiveReferences`/`decrementActiveReferences`) and drops the
`activeReferences` runtime guard, which could change idle-time
termination behavior for any downstream code that relied on keeping
snaps alive.
> 
> **Overview**
> Removes `activeReferences` tracking from `SnapController` runtime
data, including deleting the
`incrementActiveReferences`/`decrementActiveReferences` methods and the
associated idle-timeout exclusion.
> 
> Updates tests by removing the scenario that asserted snaps with open
sessions are not terminated, and documents the breaking removal in the
`snaps-controllers` changelog.
> 
> <sup>Written by [Cursor
Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit
f4474b4. This will update automatically
on new commits. Configure
[here](https://cursor.com/dashboard?tab=bugbot).</sup>
<!-- /CURSOR_SUMMARY -->
This renames the `JsonSnapsRegistry` class to `SnapsRegistryController`
and updates actions and events to standardise them.

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> **Medium Risk**
> Medium risk because it is a breaking rename of registry controller
class/namespace and messenger action/event names, which can silently
break any callers or tests that still use the old `SnapsRegistry:*` API.
> 
> **Overview**
> **BREAKING refactor of the snaps registry controller API.** Renames
`JsonSnapsRegistry`/`SnapsRegistry` to `SnapRegistryController`, moves
registry types into `registry/types.ts`, and switches messenger wiring
to `registerMethodActionHandlers` with new method-based action types
(including `requestUpdate` replacing `update`).
> 
> Updates `SnapController`, tests, and test utilities/mocks to use the
new `SnapRegistryController:*` action/event names and
`SnapRegistryStatus` enum, and refreshes the changelog to document the
rename.
> 
> <sup>Written by [Cursor
Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit
7a64b38. This will update automatically
on new commits. Configure
[here](https://cursor.com/dashboard?tab=bugbot).</sup>
<!-- /CURSOR_SUMMARY -->
<!-- CURSOR_SUMMARY -->
> [!NOTE]
> **Low Risk**
> Primarily TypeScript type/export refactors; runtime behavior should be
unchanged, but downstream packages may require import/type updates due
to renamed or newly exported messenger/action/event types.
> 
> **Overview**
> Standardizes public exports across `snaps-controllers` by re-exporting
each controller/service’s `*Args`, `*Actions`, `*Events`, and
`*Messenger` types (e.g., `CronjobController`, `SnapInsightsController`,
`SnapInterfaceController`, `MultichainRoutingService`,
`ExecutionService`, `SnapController`).
> 
> Tightens internal messenger typing by making previously exported
`AllowedActions`/`AllowedEvents` helper unions internal (renamed to
local `Allowed*` types) and updating `snaps-simulation` to use the
concrete `*Messenger` types and a strongly-typed
`RootControllerMessenger` based on `@metamask/messenger`’s
`MessengerActions`/`MessengerEvents`.
> 
> <sup>Written by [Cursor
Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit
e1c676d. This will update automatically
on new commits. Configure
[here](https://cursor.com/dashboard?tab=bugbot).</sup>
<!-- /CURSOR_SUMMARY -->
@codecov
Copy link

codecov bot commented Mar 25, 2026

Codecov Report

❌ Patch coverage is 96.64179% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 98.56%. Comparing base (17136ba) to head (3f024e3).
⚠️ Report is 5 commits behind head on main.

Files with missing lines Patch % Lines
...snaps-controllers/src/services/ExecutionService.ts 93.93% 8 Missing ⚠️
...ckages/snaps-controllers/src/services/multiplex.ts 90.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3922   +/-   ##
=======================================
  Coverage   98.55%   98.56%           
=======================================
  Files         425      426    +1     
  Lines       12358    12316   -42     
  Branches     1935     1935           
=======================================
- Hits        12180    12139   -41     
+ Misses        178      177    -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 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.

…3923)

This changes the `getTruncatedSnap` and `getTruncatedSnapExpect` methods
on `SnapController` to be private.

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> **Medium Risk**
> Medium risk because it is a breaking API surface change: external
consumers can no longer call `SnapController.getTruncatedSnap*` and must
switch to other helpers, though runtime behavior is otherwise unchanged.
> 
> **Overview**
> Makes `SnapController`’s `getTruncatedSnap`/`getTruncatedSnapExpect`
methods private (converted to
`#getTruncatedSnap`/`#getTruncatedSnapExpect`) and updates all internal
event payloads and flows (`snapInstalled`, `snapUpdated`,
`snapEnabled/disabled`, `snapTerminated`, rollback, etc.) to use the new
private methods.
> 
> Updates tests to stop calling the controller methods directly and
instead assert against `getTruncatedSnap(...)`, and documents the
breaking removal in the `CHANGELOG`.
> 
> <sup>Written by [Cursor
Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit
f728514. This will update automatically
on new commits. Configure
[here](https://cursor.com/dashboard?tab=bugbot).</sup>
<!-- /CURSOR_SUMMARY -->
@Mrtenz Mrtenz marked this pull request as ready for review March 26, 2026 09:44
@Mrtenz Mrtenz requested a review from a team as a code owner March 26, 2026 09:44
Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

@Mrtenz Mrtenz added this pull request to the merge queue Mar 26, 2026
Merged via the queue into main with commit 43e58e6 Mar 26, 2026
128 of 129 checks passed
@Mrtenz Mrtenz deleted the controller-refactors branch March 26, 2026 13:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants