[Feat] Add plugin management RPCs#75186
Conversation
Signed-off-by: samzong <samzong.lu@gmail.com>
|
Codex review: needs real behavior proof before merge. Reviewed May 30, 2026, 12:59 AM ET / 04:59 UTC. Summary PR surface: Source +1329, Tests +644, Docs +15, Other +336. Total +2324 across 37 files. Reproducibility: unclear. The review failed before ClawSweeper could establish a reproduction path. Review metrics: none identified. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Risk before merge
Maintainer options:
Next step before merge
Review detailsBest possible solution: Retry the Codex review after fixing the execution failure. Do we have a high-confidence way to reproduce the issue? Unclear. The review failed before ClawSweeper could establish a reproduction path. Is this the best way to solve the issue? Unclear. Retry the review first so ClawSweeper can evaluate the actual issue and fix direction. AGENTS.md: unclear because the file could not be read completely. Codex review notes: model gpt-5.5, reasoning high; reviewed against b352cb2d8e7f. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +1329, Tests +644, Docs +15, Other +336. Total +2324 across 37 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a72af0baca
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Signed-off-by: samzong <samzong.lu@gmail.com>
|
@clawsweeper re-review |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e37da0ba3d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ec35c9f4be
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (!code) { | ||
| return "unavailable"; | ||
| } |
There was a problem hiding this comment.
Treat uncoded install failures as invalid requests
Map failures without an explicit install error code to INVALID_REQUEST instead of UNAVAILABLE. installPluginFromDir/installPluginFromFile return uncoded failures for user input problems like missing paths (directory not found, file not found), so the current branch marks deterministic client errors as transient service outages and can trigger incorrect retry/error handling in Gateway clients.
Useful? React with 👍 / 👎.
|
Thanks for putting this together. We are considering this seriously because remote plugin management is an important capability, and this PR is pointed at the right surface area. That said, it is also tricky enough that we are not ready to land it yet. Exposing plugin install/update/uninstall over the Gateway control plane touches protocol shape, admin/security boundaries, host-path semantics, registry state, and app compatibility. We need to review those details carefully, make sure the branch is current with main, and likely tighten the changelog/API story before this can merge. So: still interested, but not an immediate land as-is. |
|
@steipete thanks for your review it. Agree with u. Just let me know what you'd like me to focus on first. Happy to keep pushing this forward whenever you're ready. |
|
This pull request has been automatically marked as stale due to inactivity. |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
Summary
Describe the problem and fix in 2–5 bullets:
If this PR fixes a plugin beta-release blocker, title it
fix(<plugin-id>): beta blocker - <summary>and link the matchingBeta blocker: <plugin-name> - <summary>issue labeledbeta-blocker. Contributors cannot label PRs, so the title is the PR-side signal for maintainers and automation.plugins.*Gateway RPC params/validators/handlers, extracted CLI plugin-management persistence and registry refresh logic into reusablesrc/plugins/*services, updated Operator scopes/rate limits, regenerated Swift protocol models, and documented the new protocol surface.plugins.installpath requests resolve on the Gateway host.Change Type (select all)
Scope (select all touched areas)
Linked Issue/PR
Root Cause (if applicable)
For bug fixes or regressions, explain why this happened, not just what changed. Otherwise write
N/A. If the cause is unclear, writeUnknown.Regression Test Plan (if applicable)
For bug fixes or regressions, name the smallest reliable test coverage that should catch this. Otherwise write
N/A.src/gateway/server-methods/plugins.test.ts,src/plugins/management.test.ts,src/gateway/method-scopes.test.ts,src/gateway/server-methods.control-plane-rate-limit.test.ts.User-visible / Behavior Changes
Operators can now call Gateway plugin management RPCs:
plugins.list,plugins.inspect,plugins.doctor,plugins.registry.status,plugins.registry.refresh,plugins.install,plugins.update,plugins.uninstall,plugins.enable, andplugins.disable.Diagram (if applicable)
For UI changes or non-trivial logic flows, include a small ASCII diagram reviewers can scan quickly. Otherwise write
N/A.Security Impact (required)
Yes/No) Yes.Yes/No) No.Yes/No) Yes.Yes/No) Yes.Yes/No) Yes.Yes, explain risk + mitigation: The new RPCs expose plugin management over the Gateway control plane. Mutating methods are scoped tooperator.admin, covered by the control-plane write rate limit, and use the existing plugin installer safety checks. Metadata-returning list/inspect/registry methods are admin-scoped exceptplugins.doctor, because they can expose Gateway-host plugin paths or diagnostics.plugins.installlocal paths resolve on the Gateway host and do not upload client files.Repro + Verification
Environment
pnpm.Steps
plugins.*Gateway RPC methods with validated params.Expected
operator.admin, are rate-limited, and persist plugin config/install-record changes through the shared helpers.Actual
Evidence
Attach at least one:
Human Verification (required)
What you personally verified (not just CI), and how:
pnpm test src/gateway/server-methods/plugins.test.ts src/plugins/management.test.ts src/gateway/method-scopes.test.ts src/gateway/server-methods.control-plane-rate-limit.test.ts;pnpm protocol:check; targetedpnpm exec oxfmt --check --threads=1on changed TypeScript files;pnpm format:docs:check;git diff --cached --checkbefore commit; staged CJK added-line gate; pre-ship committed-diff review.pnpm check:changedlocally. External contributors on this machine do not have Blacksmith Testbox org access; CI parity is expected to run from GitHub Actions on this PR branch.Review Conversations
If a bot review conversation is addressed by this PR, resolve that conversation yourself. Do not leave bot review conversation cleanup for maintainers.
Compatibility / Migration
Yes/No) Yes.Yes/No) No.Yes/No) No.Risks and Mitigations
List only real risks for this PR. Add/remove entries as needed. If none, write
None.