Conversation
rekmarks
had a problem deploying
to
default-branch
GitHub Actions
Failure
rekmarks
force-pushed
the
rekm/restricted-method-messengers
branch
from
79ac65f to
1564307
Compare
Introduce a `messenger` option on permission specification builders and an `actionNames` field on the builder export, letting restricted-method specs receive a scoped messenger in place of `methodHooks`. Add `createRestrictedMethodMessenger`, a utility that builds a minimally-scoped child messenger with the spec's declared actions delegated from a root messenger. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
rekmarks
changed the title
Member
Author
|
@cursor review |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit a44dfea. Configure here.
…ger types Overload createRestrictedMethodMessenger so callers that pass actionNames get a non-nullable scoped messenger, require actionNames to be a non-empty tuple of the root messenger's action types, and thread the namespace through as a string literal. Tighten the spec builder export constraint's actionNames to a non-empty tuple and improve related JSDoc and tests. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Member
There was a problem hiding this comment.
More generally do we still want to keep PermittedHandlerExport or should that be replaced by MethodHandler?
Member
Author
There was a problem hiding this comment.
This would be an improvement but it's orthogonal to this PR. Also not entirely clear to me if now is the time to force these methods onto JsonRpcEngineV2.
Member
There was a problem hiding this comment.
I think it is worth it to decide on this before we roll out this change. It can be done in a follow-up PR though
Member
Author
There was a problem hiding this comment.
We should probably just ship a non-breaking /v2 version of these methods if we do it.
| * delegated to it; {@link createRestrictedMethodMessenger} is the canonical | ||
| * way to construct it. | ||
| */ | ||
| messenger?: SpecMessenger; |
Member
There was a problem hiding this comment.
It would be nice if we could infer the messenger ala createMethodMiddleware, but it may be too much work to refactor. WDYT?
Member
Author
There was a problem hiding this comment.
It's too much work to refactor. We should just do #4238 instead.
FrederikBolding
approved these changes
Apr 24, 2026
Member
|
@metamaskbot publish-preview |
Contributor
|
Preview builds have been published. Learn how to use preview builds in other projects. Expand for full list of packages and versions. |
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
2 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.
Explanation
Adds a
messengeroption to permission specification builders as an alternative tomethodHooks, aligning spec builders with the messenger-based method middleware introduced in #8506. Restricted-method specs can now declare the root-messenger actions they need viaactionNamesand consume a minimally-scopedMessenger.Incidentally, also removes the
factoryHooksandvalidatorHooksfields from the spec builder surface. Both of these were unused in practice.References
createMethodMiddlewarefunction #8506Checklist
Note
Medium Risk
Breaking type-surface changes to permission specification builders (removing
factoryHooks/validatorHooksand adding optional scopedmessenger) may require downstream updates; new messenger delegation logic could impact how restricted-method specs invoke host actions.Overview
Restricted-method permission specification builders can now receive an optional scoped
messenger(constructed from spec-declaredactionNames) as an alternative to relying onmethodHooks, enabling specs to call only explicitly-delegated host actions.Introduces
createRestrictedMethodMessenger(exported from the package) to create a childMessengerwith a minimal delegated action surface, and adds tests covering delegation behavior and the new builder option. BREAKING: removesfactoryHooks,validatorHooks, and related export fields from the permission specification builder API surface.Reviewed by Cursor Bugbot for commit 1e2fe74. Bugbot is set up for automated code reviews on this repo. Configure here.