feat(plugins): read-only catalog view with search and filters (#15)

[pullfrog]

Catalog UI 1/5 for #4. Prerequisite check first: the catalog/schema issue named in #4 is #2, and it has landed — resources/plugin-catalog.json, resources/plugin-catalog.schema.json and scripts/validate-plugin-catalog.js are all on main, so no schema was invented here.

Read-only browsing only. No install action (4/5), no card contents (2/5), no selection or dependency behaviour (3/5).

What it does

The Plugin Manager screen gains a Browse catalog section above the existing install-from-git form:

• Text search over name, id, description, category, source, stability, version and instrument names. Terms are ANDed, so piano keys narrows rather than widens.
• Five filters as checkbox groups: category, instrument, source/maintainer, stability, and status (installed / available / update available).
• Combine freely. Values inside one group are ORed, groups are ANDed, and search composes with them. A Clear filters button resets the query and every group and disables itself when nothing is active.
• Facet options are derived from the whole catalog, never from the current results, so ticking one value never makes its siblings unreachable.
• Loading, empty, no-match and error states are distinct. Rows show name, description, version and a status badge — deliberately minimal, since card contents are 2/5.

Search and filtering run in the renderer over the rows already fetched from disk, so browsing is offline by construction: plugins:catalog reads resources/plugin-catalog.json (or process.resourcesPath) with no fetch anywhere on this path.

One contract change

plugins:catalog returned a bare array, and on a missing or damaged bundled file listCatalog() returned [] — indistinguishable from a catalog that genuinely offers nothing. It now answers { ok, entries, message? }, and the view renders message as an error state. preload.ts and docs/PLUGIN_CATALOG.md are updated to match. The error is sticky: typing or ticking after a failure cannot overwrite it with "the bundled catalog has no plugins".

This is a deliberate change to an IPC answer shape, made now while no renderer consumed it yet. installCatalog and rollbackCatalog are untouched.

Tests

tests/plugin-catalog-view.test.js, 17 tests, node --test, no new dependency. It lifts the four IIFE-free functions (pmCatalogState, pmCatalogHaystack, pmCatalogFacets, pmFilterCatalog) out of screen.js by source — the same technique tests/monitor-mute-suppression.test.js already uses for the audio screen — which avoids standing up a DOM harness. Covered: search by each searchable field, case-insensitivity, ANDed terms, a blank query not filtering, each of the five filters, OR-within / AND-across, clearing, non-mutation, the install-state derivation, facet derivation, and the real bundled resources/plugin-catalog.json listing and filtering with no network.

Added as a named step in .github/workflows/ci.yml — npm test globs suites that need the JUCE submodule and native build, so a new tests/*.test.js never runs in CI unless named explicitly.

Verification

• node --test tests/plugin-catalog-view.test.js — 17 pass.
• node --test tests/plugin-installer.test.js (48), tests/plugin-catalog.test.js, tests/contract-check.test.js, tests/renderer-capability-migration.test.js — all pass.
• Targeted tsc --noEmit over src/main/plugin-manager.ts and src/main/preload.ts — clean. Full npm run typecheck was not run: the JUCE submodule is absent from a clean checkout, so it cannot run locally.
• The catalog lock and remote manifest checks were not run; no catalog file changed.

Not in this PR

One thing the reviewer surfaced that I left alone to keep the PR scoped: the pre-existing git-installed list in refreshList() interpolates manifest.name, manifest.description and plugin.name into innerHTML unescaped, under webSecurity: false (src/main/main.ts:413). Those values come from an arbitrary cloned repository's plugin.json. It is not a regression from this change, but the new esc() comment now points at it — worth a separate fix.

  ^{｜ View workflow run ｜ via Pullfrog ｜ Using big-pickle (free) ｜ 𝕏}

[codacy-production]

Not up to standards ⛔

🔴 Issues 6 critical · 15 high

Alerts:
⚠ 21 issues (≤ 0 issues of at least minor severity)

Results:
21 new issues

Category	Results
Security	6 critical 15 high

View in Codacy

🟢 Metrics 6 complexity · 2 duplication

Metric	Results
Complexity	6
Duplication	2

View in Codacy

_{NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.}

[carochacs]

Review for #15 (read-only catalog view). No blocking findings.

Checked

• node --test tests/plugin-catalog-view.test.js → 17/17 pass. Not run: the full typecheck (no node_modules here), so CI is the first whole-project check.
• Escaping: every catalog field and error string rendered through innerHTML goes through esc() (screen.js: names, descriptions, versions, facet values, data-value attributes). esc covers & < > " ', so the attribute context is safe. This matters because the renderer runs with webSecurity:false, as the code comment notes.
• IPC contract change: plugins:catalog now answers { ok, entries, message? } instead of a bare array, so a missing or damaged catalog is distinguishable from an empty one. I found exactly one consumer (screen.js ~447) and it uses the new shape; preload typing is updated and the test pins the row fields.
• The new CI step and PLUGIN_CATALOG.md change are small and consistent with the explicit-suite convention.

Notes (non-blocking)

• The comment in screen.js correctly flags that the git-installed list further down still interpolates plugin.manifest.description unescaped under the same webSecurity:false. That is pre-existing and out of this PR's scope, but it is worth its own issue.
• Entries the loader rejects individually are still dropped silently, so a catalog whose entries all fail validation answers ok with no entries; the comment on listCatalog documents it.

This PR does not touch a decision tagged to @itsjustroxi. Not approving or merging.

---

Generated by Claude Code

[pullfrog]

Thanks for the review — no code changes made, since there were no blocking findings and both notes called for separate work.

Note 1 — unescaped git-installed list. Agreed it is real and pre-existing, so it stays out of this PR. Filed as #29 to make it a tracked fix rather than a comment: refreshList() interpolates manifest.name, manifest.description, plugin.version and plugin.name (including inside a data-name attribute) into innerHTML unescaped, and the renderer runs with webSecurity: false (src/main/main.ts:413). The issue proposes reusing the esc() helper this PR adds, prefers textContent for the plain-text fields, and asks for a sweep of the other renderers while it is fresh.

Note 2 — individually rejected entries. Verified against loadCatalog() (src/main/plugin-installer.ts:151): invalid and duplicate entries are dropped with a console.warn and never reach catalog.entries, so a catalog whose every entry fails the runtime gate does answer ok: true with no entries. That is the pre-existing design, and the new comment on listCatalog states it, so I left the behaviour alone. If the intent is to surface per-entry rejection to the user rather than only to the main-process log, #24 already owns the "handle blocked catalog entries" surface and is the better home for it than a new issue here.

One clarification for the record: this PR does not touch a decision tagged to you, and it is not asking for approval — it stays open on that basis.

Task list (6/6 completed)

• Checkout PR branch feat(plugins): read-only catalog view with search and filters (#15) #28
• Fetch review comments for review_id 5396785600
• Verify each finding against actual code and implement fixes
• Re-check review comments fresh, test, commit, push
• File follow-up issue for the pre-existing unescaped plugin list
• Reply to threads and report progress

  ^{｜ View workflow run ｜ via Pullfrog ｜ Using big-pickle (free) ｜ 𝕏}

[kilo-code-bot]

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Files Reviewed (2 files)

• src/renderer/plugin-manager/screen.js
• tests/plugin-catalog-view.test.js

Previous Review Summaries (2 snapshots, latest commit 54d89f4)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit 54d89f4)

Status: 1 Issue Found | Recommendation: Address before merge

Overview

Severity	Count
CRITICAL	1
WARNING	0
SUGGESTION	0

Issue Details (click to expand)

CRITICAL

File	Line	Issue
src/renderer/plugin-manager/screen.js	18	Duplicate const esc declaration causes SyntaxError in strict mode

Files Reviewed (7 files)

• .github/workflows/ci.yml
• docs/PLUGIN_CATALOG.md
• src/main/plugin-manager.ts
• src/main/preload.ts
• src/renderer/plugin-manager/screen.html
• src/renderer/plugin-manager/screen.js - 1 issue
• tests/plugin-catalog-view.test.js

Fix these issues in Kilo Cloud

Previous review

This review did not finish. The model reached its output limit before it
could write the review — a reasoning model can spend the whole budget thinking.
Re-run the review, or lower the model's thinking effort, and it should get
further. Any inline comments below are from an earlier review.

---

_{Reviewed by free · Input: 34.4K · Output: 3.9K · Cached: 131.7K}

[pullfrog]

No code fix needed — the failure is a third-party review bot hitting its output limit

I investigated check suite 100393185274 and could not find a defect to fix. Nothing was changed; the working tree is clean at bfbb3ca.

What actually failed

The suite's only failing check run is Kilo Code Review (run id 111021199752):

Kilo Code Review hit the output limit
The model reached its output limit before finishing the review, so no complete review was produced.

That is a capacity failure inside the reviewing model, not a test, build or lint failure. get_check_suite_logs returned "no failed workflow runs found for this check suite" because Kilo is a GitHub App check, not a workflow run — which is why the logs endpoint looked empty.

Kilo Code Review is configured entirely outside this repository: grep -ril kilo .github/ finds no reference, and there is no Kilo workflow, action or config file in the tree. There is no repo-side change that makes a model stop hitting its token ceiling.

Every check the repo owns is green

CI / check (run 37062182746) succeeded on all 12 steps — TypeScript type check, plugin catalog tests, plugin installer tests, the new plugin catalog view tests, catalog lock check, verify pinned manifests/archives, and npm audit. The three addon matrix jobs on ubuntu-22.04 / macos-14 / windows-latest also succeeded.

I reproduced those steps locally on the PR head:

Suite	Result
node --test tests/plugin-catalog-view.test.js	17/17 pass
node --test tests/plugin-installer.test.js	32/32 pass
node --test tests/plugin-catalog.test.js	4/4 pass
node --test tests/renderer-capability-migration.test.js	6/6 pass
node --test tests/contract-check.test.js	2 pass, 1 skip (addon not built)
npm run catalog:lock:check	current, 6 plugins

(plugin-installer needed typescript on NODE_PATH — an environment artifact of the missing node_modules, not a test failure. Full npm run typecheck still cannot run locally: the JUCE submodule is absent.)

The contract change is safe

The one behavioural change — plugins:catalog answering { ok, entries, message? } instead of a bare array — has exactly one consumer, src/renderer/plugin-manager/screen.js:447, and it is updated to match. src/main/plugin-manager.ts:486 and src/main/preload.ts:509 are consistent, and tests/contracts/ has no snapshot covering this channel, so nothing was left stale.

Two things worth a maintainer's attention

Neither blocks this PR, and I deliberately did not "fix" them by editing code:

1. Codacy reports 21 new issues on the PR head, all in src/renderer/plugin-manager/screen.js (its own suite, conclusion action_required). Seven are failure-level "user controlled data in innerHTML" on lines 367, 371 and 397. These look like false positives — every catalog value on those paths goes through the esc() helper at line 18 — but Codacy's flow analysis does not see through the escaping, so the noise is real even if the finding is not. Per the repo's own convention these would want narrowly-scoped suppressions with a stated reason rather than a file-level disable.
2. The pre-existing unescaped innerHTML in refreshList() (lines 55–65, interpolating manifest.name / manifest.description / plugin.name from an arbitrary cloned repo's plugin.json under webSecurity: false) is untouched, as the PR body already flags. Not a regression; still a separate fix.

Suggested unblock

Re-run the Kilo Code Review check from the GitHub UI, or dismiss it if the bot is not a required status in the rulesets. I did not push a no-op commit to re-trigger it — that would be gaming the check rather than fixing anything.

Task list (5/7 completed)

• Checkout PR feat(plugins): read-only catalog view with search and filters (#15) #28 branch
• Fetch check suite logs and diagnose failure
• Verify failure was introduced by this PR
• Reproduce CI steps locally
• Implement fix
• Verify fix with same CI command
• Commit, push to PR branch, report progress

  ^{｜ View workflow run ｜ via Pullfrog ｜ Using big-pickle (free) ｜ 𝕏}

[pullfrog]

Fixed the TypeScript syntax error in src/main/plugin-manager.ts where listCatalog had conflicting declarations (merge artifact from the signature change). Corrected listCatalog to a single export with the new return shape { ok, entries, message? }, removed unreachable code, and updated catalogSelectionEntries() to handle both the new shape and any legacy array shape defensively. The relevant catalog-view tests still pass locally (17/17). Changes committed and pushed to the PR branch.

  ^{｜ View workflow run ｜ via Pullfrog ｜ Using big-pickle (free) ｜ 𝕏}