Repository navigation
[3545 by Claude Opus 5.5] Plugins page: catalog, install with permission review, connect, enable, disable and uninstall - #3567
Conversation
… plugin goes through install, connect, limit to a repository, enable, disable and uninstall entirely from the page, and the state badge changes after each step. Tests: typecheck and zero-warning lint pass for `propr-ui`. The full UI suite passes (349 files, 3213 tests), and so does the new Playwright spec `e2e/plugins.pw.ts` (2 tests). The browser runs use a mocked plugin API that keeps state between steps, not a real server. No backend code changed. What's on the page (`/plugins` and `/plugins/<id>`) - Access: only administrators with `instance.manage_settings` see it. It's in the sidebar, under More on mobile, and there's an "Open Plugins" link above Skills & MCP servers in Settings. Anyone else who opens the address gets "Administrator access required". - Catalog: one card per plugin with icon, name, description, category and state badge, a category filter and search by name. The badges are Not installed, Off, Needs connection, Enabled, Needs approval and Degraded. - Detail: four steps in fixed order, each marked Done or To do: 1. Install lists what the plugin adds in plain words from the manifest and each permission with its explanation. When a package server gets secret variables, it shows the package, exact version and variable names, and Install stays disabled until that acknowledgement is ticked. After an upgrade that needs approval, this step shows what's new and an Approve button. 2. Connect has one row per connection with the manifest's help text and link. OAuth reuses the existing Connect / Reconnect / Disconnect component from Skills & MCP servers. Tokens and signing secrets have a write-only field; once saved, the page only says who stored it and when. 3. Configure only offers Limit to repositories for now, as the issue asked. 4. Enable is one checkbox. While something is missing it can't be ticked, and the step lists the API's reasons, for example the missing required connection. - What it added lists the managed servers and skill, linking to Skills & MCP servers, and the template. - Uninstall is a separate action. Its confirmation lists what will be removed, and if the API refuses, the page shows the API's message word for word. - Safety: manifest text is shown as plain text, only `https` links are clickable, and nothing supplied by the plugin runs in the browser. Code: the API client is `propr-ui/src/api/pluginsApi.ts`, typed from the generated OpenAPI types. The page is in `propr-ui/src/pages/PluginsPage/`, split into small components that stay under the line limit. Component tests cover the catalog, each step and the error states. `docs/docs/features/plugins.md` now has a Plugins page section with a walkthrough, and `docs/docs/features/web-ui.md` has a Plugins section. Changes outside the brief: - I put the `/mcp-logs` route on one line in `App.tsx`, written the same way as the new Plugins route, so the file stays under the 400-line limit. - The Enabled checkbox shows the new state while the request is in progress. Without that, a click looked like it did nothing until the API answered. Decision for you: - Plugin icons: the API doesn't serve a plugin's icon file, so each card shows an icon chosen by category instead. Showing the real icon needs an icon route on the API. Previews of the catalog, the install acknowledgement, the blocked Enable step, uninstall with the API refusal, and two phone-width views are attached; they use mocked API data.
|
/ultrafix goal=8 max=10 |
|
🔄 Ultrafix loop started (goal: 8/10, max cycles: 10) First action:
|
|
✅ AI Code Review Complete requested by @propr-ultrafix Posted 1 review: |
🔍 AI Code Review — codex:gpt-6-astraOverall EvaluationThe PR implements the catalog and four-step management flow with clear component boundaries. It needs a localized asynchronous state fix before merge. ✅ API contract alignment — Mutations send the required JSON content type and preflight header, and API refusal text is preserved. ✅ Safe manifest rendering — Manifest text uses React text rendering, external manifest links require HTTPS, and package-secret acknowledgements identify the package, version and variables. ✅ Useful workflow coverage — Component tests cover installation, repository limits, enablement and refusal handling; browser tests exercise the token-based lifecycle. This review uses static analysis of the supplied code; no commands were run. The authoritative checks show zero failures and five pending checks. Merge blockersEvery finding below was introduced by this PR and must be resolved before merging. F1: 🔴 Prevent stale responses from replacing newer state
SuggestionsThese are optional follow-ups and are not sent to S1: 🟢 Exercise browser OAuth completionThe Playwright lifecycle sets ScoreThe implementation is well scoped and has useful coverage, but stale asynchronous responses can misrepresent the outcome of administrative actions. The remaining correction is localized; current checks have no reported failures. Score: 6/10 🤖 Review Details
Copy and edit to choose records from this review: 🤖 Review by ProPR |
|
✅ Applied the requested follow-up changes in commit 329623d
Summary of ChangesI fixed F1: a background read that started before a change can no longer overwrite the state that change returned. The plugin tests (21), typecheck, zero-warning lint and the Playwright spec What changed in
The sibling path in New tests in
All three fail against the old hook and pass against the new one. I made no previews, since this is a fix for out-of-order responses rather than something visible on the page. The only remaining case is two different administrators changing the same plugin at once. One page can't order that, and it is outside this finding. 🤖 Implementation Details
💡 ProPR Slash Commands
🤖 Generated by ProPR |
…can no longer overwrite the state that change returned. The plugin tests (21), typecheck, zero-warning lint and the Playwright spec `e2e/plugins.pw.ts` (2 tests) all pass. I fixed F1: a background read that started before a change can no longer overwrite the state that change returned. The plugin tests (21), typecheck, zero-warning lint and the Playwright spec `e2e/plugins.pw.ts` (2 tests) all pass. **What changed in `usePluginDetail.ts`:** - **Old reads are discarded.** Each page read and each change takes a running number. A read only updates the page if it is still the newest one when it answers and no change is in progress. Starting a change and finishing one both bump the number, so reads from before or during a change are dropped. Overlapping reads are covered too: an older one can't overwrite a newer one. - **One change at a time.** The hook now refuses to start a second change while one is running. It doesn't wait for the page to redraw, so a quick double click can't start two. OAuth Disconnect goes through the same check. - **Re-reading after a failed change.** A refused change returns no plugin, and any read that came back meanwhile was dropped, so the page reads the plugin again after a failure. **The sibling path in `PluginConnections.tsx`:** the hook now marks the OAuth Connect, Reconnect and Disconnect controls busy while any other change is running, not just their own connection's request. So a disconnect can no longer run alongside, say, a disable. One visible side effect: those controls show "Working…" during any change on the page. **New tests in `PluginDetail.test.tsx`:** - An Enabled read answers after a successful disable: the badge stays Off, the checkbox stays unticked and the Disabled notice stays. - A read that answers while a disable is in progress is ignored, and the OAuth buttons stay disabled until the disable returns. - A refused change triggers a fresh read. All three fail against the old hook and pass against the new one. I made no previews, since this is a fix for out-of-order responses rather than something visible on the page. The only remaining case is two different administrators changing the same plugin at once. One page can't order that, and it is outside this finding. PR: #3567 Comment by: @propr-ultrafix (ID: 0) Model: Claude Opus 5.5
|
✅ AI Code Review Complete requested by @propr-ultrafix Posted 1 review: |
|
CI failed: Full Test Suite Shard 2/4 Please investigate and fix this CI failure.
Failure evidence
|
|
✅ Applied the requested follow-up changes in commit ca42dab Summary of ChangesI fixed the Shard 2/4 failure. The Plugins work didn't cause it: GitHub's What rpm 6 changed. I installed the actual rpm 6.0.1 and reproduced all three CI failures. It found two problems:
The fix.
Tests. For you:
The change can't be seen in the UI, so there's no preview. 🤖 Implementation Details
Undo Changes • View Task Execution 💡 ProPR Slash Commands
🤖 Generated by ProPR |
…e it: GitHub's `ubuntu-latest` runner moved to Ubuntu 26.04 this morning, which ships rpm 6.0.1. The RPM signature check in `apps/desktop/scripts/rpm-signing.mjs` only understood rpm 4's output. Every shard-2 run since about 09:00 fails in `rpm-signing.test.mjs`, including on other branches like `ci/faster-validate`, and the last run on 24.04 passed it.
I fixed the Shard 2/4 failure. The Plugins work didn't cause it: GitHub's `ubuntu-latest` runner moved to Ubuntu 26.04 this morning, which ships rpm 6.0.1. The RPM signature check in `apps/desktop/scripts/rpm-signing.mjs` only understood rpm 4's output. Every shard-2 run since about 09:00 fails in `rpm-signing.test.mjs`, including on other branches like `ci/faster-validate`, and the last run on 24.04 passed it.
**What rpm 6 changed.** I installed the actual rpm 6.0.1 and reproduced all three CI failures. It found two problems:
- Imported keys are now named with the key's full fingerprint, not its last 8 characters.
- rpm 6 reports the full fingerprint in its signature line and signs only the package header. The second signature line that rpm 4 printed no longer exists. The checker required both lines, so it rejected every package rpm 6 signs, not just the test fixtures.
**The fix.**
- **`rpm-signing.mjs`**: the check now accepts either form.
- The rpm 4 rule is unchanged: both signatures from the pinned key ID.
- The new rpm 6 rule needs two things: a verified header signature whose full fingerprint matches the pinned one, and a verified payload digest line. The signed header contains the payload digests, so the whole package is still covered.
- Any failed status anywhere in the output still rejects the package.
- **`rpm-signing.test.mjs`**:
- The two key-name checks accept either naming form, and still require exactly one key.
- A new test covers rpm 6 output. It rejects a missing or failed header signature, a missing payload digest, and a fingerprint that only matches in its last 8 characters.
**Tests.** `rpm-signing.test.mjs` passes 10/10 with no skips under both rpm 6.0.1 (as on 26.04) and rpm 4.18 (Debian's build, as a stand-in for 24.04). `linux-preview-release.test.mjs` and `signing-key-verification.test.mjs` pass 19/19 under rpm 6.
**For you:**
- The jobs that sign releases are pinned to `ubuntu-24.04`, so release packages still carry both signatures. That matters because the openSUSE Leap 16 install check runs rpm 4.20.
- The user-facing install notes in `linux-preview-release.mjs` and the docs (`linux-rpm-signing.md`, `linux-release-key.md`) still describe rpm 4 output ("both key IDs", key ID `cc4e1599`). Someone on an rpm 6 system will see one header line with the full fingerprint instead. I left them alone because the request was only the CI fix, and a test checks the install-note text.
The change can't be seen in the UI, so there's no preview.
PR: #3567
Comment by: @propr-dev[bot] (ID: 6875619497)
Model: Claude Opus 5.5
|
Please revert commit ca42dab in this PR: restore This PR is the Plugins page (issue #3545) and must not change the desktop release signature verification. The Ubuntu 26.04 runner problem is handled separately: #3569 pins the hosted CI jobs on this epic branch to Do not change anything under |
|
✅ Applied the requested follow-up changes in commit 95f9325 Summary of ChangesI reverted commit ca42dab.
There are no visual previews because nothing on the page changed. 🤖 Implementation Details
Undo Changes • View Task Execution 💡 ProPR Slash Commands
🤖 Generated by ProPR |
…g.mjs` and `apps/desktop/scripts/rpm-signing.test.mjs` now match the base branch `3542-epic-plugin-system-xu8` byte for byte. I reverted commit ca42dab. `apps/desktop/scripts/rpm-signing.mjs` and `apps/desktop/scripts/rpm-signing.test.mjs` now match the base branch `3542-epic-plugin-system-xu8` byte for byte. - **How:** the workspace's git directory is read-only, so `git revert` and `git checkout` failed. Instead I wrote both files back from their contents in the commit before ca42dab. - **Check:** comparing against the base branch, nothing under `apps/desktop/` or `.github/` differs, and these two files are the only changes in the working tree. - **F1:** the fix from 329623d is unchanged. - **Tests:** I didn't run any. Until #3569 is merged into the epic branch, `rpm-signing.test.mjs` and `rpm-maker.test.ts` will keep failing on Ubuntu 26.04 runners, as you expected. There are no visual previews because nothing on the page changed. PR: #3567 Comment by: @integry (ID: 6875804297) Model: Claude Opus 5.5
|
CI failed: Full Test Suite Shard 2/4 Please investigate and fix this CI failure.
Failure evidence
|
|
CI failed: Full Test Suite Shard 4/4 Please investigate and fix this CI failure.
Failure evidence
|
|
CI failed: Run Full Test Suite Please investigate and fix this CI failure.
Failure evidence
|
Full Test Suite ResultsValidation failed during: Test shard (shard 2), Test shard (shard 4).
View shard 2/4 outputView shard 4/4 outputDetails truncated; see the uploaded artifacts for complete failure rows and logs. |
|
ℹ️ Analyzed the follow-up request by @propr-dev[bot], @github-actions[bot] Analysis SummaryI didn't change any code for this CI failure. You asked me not to fix it in this PR, and it isn't caused by the Plugins work.
There were no visual changes, so I didn't make previews. No code changes were necessary based on the current state of the branch. 🤖 Analysis Details
💡 ProPR Slash Commands
🤖 Generated by ProPR |
…plugins-page-catalog-inst-20261114-0919-mwu
🔍 AI Code Review — codex:gpt-6-astraOverall EvaluationThe PR implements the Plugins catalog and lifecycle controls with focused components and API-backed state. It needs a localized correction before merge: secret acknowledgements can remain checked after the package details change. ✅ API contract alignment — Mutations use the required JSON bodies and preflight header, and API refusal messages are preserved. ✅ State reconciliation — Mutation sequencing prevents older background reads from overwriting confirmed changes, with targeted regression tests. ✅ Scope discipline — The current diff contains no desktop release or workflow changes. All supplied current-head checks passed or were skipped. Review was static; no commands or tests were run. Merge blockersEvery finding below was introduced by this PR and must be resolved before merging. F2: 🔴 Reset consent when acknowledgement details change
SuggestionsThese are optional follow-ups and are not sent to S2: 🟢 Exercise browser OAuth completionThe Playwright lifecycle test sets ScoreThe implementation is focused and current checks are green, but acknowledgement state must track the exact secret exposure being approved before merge. Score: 6/10 🤖 Review Details
Copy and edit to choose records from this review: 🤖 Review by ProPR |
|
✅ Applied the requested follow-up changes in commit ab177d5
Summary of ChangesI fixed F2. A ticked secret acknowledgement now stays ticked only while the server, package, exact version and secret-variable names are all unchanged. If a refreshed read changes any of them, the checkbox shows unticked again and Approve or Install is disabled until the administrator ticks it again. Change (
Regressions (
Verification: with the previous component, both new tests fail. With the change, all 23 Plugins tests pass, and typecheck and zero-warning lint pass for One window remains, and the UI can't close it. If an upgrade lands between the Approve click and the API handling it, the backend still records the then-current binding for the names it received. Closing that would need the API to accept and check the version being acknowledged, which is outside F2. I didn't capture previews: the only visible difference is a checkbox showing unticked after a refresh, and the tests cover that. Review digestRisk-tiered paths
Validation evidence
Diff size: 2 files changed, +82/−20 lines 🤖 Implementation Details
💡 ProPR Slash Commands
🤖 Generated by ProPR |
…d only while the server, package, exact version and secret-variable names are all unchanged. If a refreshed read changes any of them, the checkbox shows unticked again and Approve or Install is disabled until the administrator ticks it again. I fixed F2. A ticked secret acknowledgement now stays ticked only while the server, package, exact version and secret-variable names are all unchanged. If a refreshed read changes any of them, the checkbox shows unticked again and Approve or Install is disabled until the administrator ticks it again. **Change** (`propr-ui/src/pages/PluginsPage/PluginInstallStep.tsx`) - Each tick is now stored against the full set of details it confirms, not just the server name. - Approve and Install now send only the servers whose current details match what was ticked, so an old tick can no longer go out under a new binding. - The approval form and the install form share this logic, so both are fixed. **Regressions** (`PluginDetail.test.tsx`) - **Approval:** tick for 1.1.0, start OAuth sign-in, and refocus the page; the refreshed read brings 1.2.0. The box shows unticked and Approve is disabled. After ticking again, a second refresh adds a new secret variable at the same version, and the box is cleared again. Approve is called once, only after the latest tick. - **Install:** tick for 1.0.0; Install is refused, and the follow-up read brings 1.0.1. The box shows unticked and Install is disabled. **Verification:** with the previous component, both new tests fail. With the change, all 23 Plugins tests pass, and typecheck and zero-warning lint pass for `propr-ui`. One window remains, and the UI can't close it. If an upgrade lands between the Approve click and the API handling it, the backend still records the then-current binding for the names it received. Closing that would need the API to accept and check the version being acknowledged, which is outside F2. I didn't capture previews: the only visible difference is a checkbox showing unticked after a refresh, and the tests cover that. PR: #3567 Comment by: @propr-ultrafix (ID: 0) Model: Claude Opus 5.5
|
✅ AI Code Review Complete requested by @propr-ultrafix Posted 1 review: |
🔍 AI Code Review — codex:gpt-6-astraOverall EvaluationReady to merge within the reviewed scope. The implementation delivers the Plugins catalog and management flow with appropriately separated components. The supplied current-head checks report 16 passed and no failures or pending checks. ✅ API contract alignment — Mutation requests use the required JSON bodies and preflight header, and API refusal messages reach the UI. ✅ State protection — The detail hook prevents overlapping mutations and rejects stale reads across mutation boundaries. Secret acknowledgements are bound to the displayed package, version and variable names. ✅ Scope preserved — The current diff contains no changes under This assessment is based on static tracing of the supplied diff and repository excerpts; no commands or tests were run. Merge blockersNo merge blockers. SuggestionsThese are optional follow-ups and are not sent to S3: 🟢 Exercise browser OAuth completionThe Playwright lifecycle test in ScoreThe implementation meets the effective objective, with useful regression coverage and passing current-head checks. Static review found no verified merge-blocking defects. Score: 9/10 🤖 Review Details
Copy and edit to choose records from this review: 🤖 Review by ProPR |
|
ℹ️ GitHub auto-merge is turned off for this repository, so ProPR merges this PR itself once it is ready (checks passed or no CI configured, and the review goal met). To let GitHub merge it the moment required checks pass, enable Settings → General → Pull Requests → Allow auto-merge. |
AI Implementation Summary
Closes #3545
Branch:
3545/claude-opus-5-5-plugins-page-catalog-inst-20261114-0919-mwuCommits: ✅ Changes committed (6da8a07)
AI Processing Completed
Execution Details:
Summary:
I've built the Plugins page for #3545. In a browser test, the example plugin goes through install, connect, limit to a repository, enable, disable and uninstall entirely from the page, and the state badge changes after each step.
Tests: typecheck and zero-warning lint pass for
propr-ui. The full UI suite passes (349 files, 3213 tests), and so does the new Playwright spece2e/plugins.pw.ts(2 tests). The browser runs use a mocked plugin API that keeps state between steps, not a real server. No backend code changed.What's on the page (
/pluginsand/plugins/<id>)instance.manage_settingssee it. It's in the sidebar, under More on mobile, and there's an "Open Plugins" link above Skills & MCP servers in Settings. Anyone else who opens the address gets "Administrator access required".httpslinks are clickable, and nothing supplied by the plugin runs in the browser.Code: the API client is
propr-ui/src/api/pluginsApi.ts, typed from the generated OpenAPI types. The page is inpropr-ui/src/pages/PluginsPage/, split into small components that stay under the line limit. Component tests cover the catalog, each step and the error states.docs/docs/features/plugins.mdnow has a Plugins page section with a walkthrough, anddocs/docs/features/web-ui.mdhas a Plugins section.Changes outside the brief:
/mcp-logsroute on one line inApp.tsx, written the same way as the new Plugins route, so the file stays under the 400-line limit.Decision for you:
Previews of the catalog, the install acknowledgement, the blocked Enable step, uninstall with the API refusal, and two phone-width views are attached; they use mocked API data.
Detailed Logs:
9d0ba48f-485d-4a7c-8f5a-14614f27419cLog files stored at:
/usr/src/app/logs/executions/issue-3545-2026-11-14T09-38-50-033Z-conversation.json/usr/src/app/logs/executions/issue-3545-2026-11-14T09-38-50-033Z-output.txtLatest Conversation Messages
This PR was created automatically by ProPR after processing issue #3545.
💡 Need changes?
Comment on this PR to request refinements — the AI agent monitors comments and will update the implementation based on your feedback. Keep iterating until you're satisfied!
Visual preview
Plugins catalog
The new Plugins page in the sidebar: one card per plugin with category icon, description, category and state badge, plus search and category filter (mocked API data)
Connect sign-in required. Original retained until 2027-02-12T09:38:54.506Z.
View the full-resolution original in ProPR Connect
Connect step after install
After install the badge reads Needs connection; the OAuth connection reuses the Skills & MCP servers Connect component
Connect sign-in required. Original retained until 2027-02-12T09:38:55.700Z.
View the full-resolution original in ProPR Connect
Plugin detail at phone width
The plugin header and Install step at 390px
Connect sign-in required. Original retained until 2027-02-12T09:38:56.846Z.
View the full-resolution original in ProPR Connect
Enable blocked at phone width
The Enabled checkbox is blocked with the missing required connection named, followed by What it added and Uninstall
Connect sign-in required. Original retained until 2027-02-12T09:38:57.941Z.
View the full-resolution original in ProPR Connect
Limited to a repository and enabled
The token is stored (presence only), the plugin is limited to one repository and enabled
Connect sign-in required. Original retained until 2027-02-12T09:38:59.668Z.
View the full-resolution original in ProPR Connect
Install step with secret acknowledgement
Install shows the permissions and the package, exact version and secret variable names; Install stays disabled until the acknowledgement is ticked
Connect sign-in required. Original retained until 2027-02-12T09:39:00.821Z.
View the full-resolution original in ProPR Connect
Uninstall confirmation and API refusal
What it added, the uninstall confirmation listing what will be removed, and the API refusal text when an automation still uses a managed item
Connect sign-in required. Original retained until 2027-02-12T09:39:02.650Z.
View the full-resolution original in ProPR Connect