fix(plugins): ListInstalled reads installed plugins on SaaS (EIC), not just Docker #3125

Merged

devops-engineer merged 1 commits from fix/saas-listinstalled-eic-dispatch into main 2026-06-21 12:10:31 +00:00

Conversation 0 Commits 1 Files Changed 3

+271 -38

core-devops commented 2026-06-21 11:52:00 +00:00

Member

Copy Link

Copy Source

The bug

After a successful plugin install on a SaaS tenant, GET <tenant>/workspaces/<wsid>/plugins (ListInstalled) reads back [] even though the plugin IS on the box and working.

Root cause

ListInstalled (plugins_listing.go) ONLY ls'd a local Docker container. On SaaS (EC2-per-workspace) there is no local container → findRunningContainer returns "" → the handler returned [] immediately, regardless of what was actually installed. Install/Uninstall already dispatch to the SaaS EIC path via lookupSaaSDispatch; ListInstalled never did.

This is distinct from the management-MCP eviction bug (that one fail-closes the concierge; this one only under-reports the installed list on SaaS).

The fix

Add a SaaS dispatch branch mirroring Install/Uninstall:

• listPluginsViaEIC (new, in plugins_install_eic.go) lists the directory names under the workspace EC2's <runtime-config-prefix>/plugins/ over the existing EIC SSH tunnel (buildPluginsListShell → find -mindepth 1 -maxdepth 1 -type d -printf '%f\n').
• Each manifest is read via the existing readPluginManifestViaEIC.
• Same validatePluginName (drops traversal names) and fail-soft posture as the Docker path: an unreachable box returns [] + 200, never a 5xx.
• Runtime-support annotation extracted into annotateRuntimeSupport, shared by both branches.

Tests

plugins_listing_saas_test.go: SaaS list returns installed plugins (incl. user plugin), missing-manifest still lists the name, list error fails soft to []+200, traversal name rejected.

Note: the pre-existing TestManifest_RefPinning_* failures are network/manifest-pinning tests unrelated to this change (they fail on a clean checkout too).

🤖 Generated with Claude Code

## The bug After a successful plugin install on a SaaS tenant, `GET <tenant>/workspaces/<wsid>/plugins` (ListInstalled) reads back `[]` even though the plugin IS on the box and working. ## Root cause `ListInstalled` (plugins_listing.go) ONLY `ls`'d a **local Docker** container. On SaaS (EC2-per-workspace) there is no local container → `findRunningContainer` returns `""` → the handler returned `[]` immediately, regardless of what was actually installed. Install/Uninstall already dispatch to the SaaS EIC path via `lookupSaaSDispatch`; ListInstalled never did. This is distinct from the management-MCP eviction bug (that one fail-closes the concierge; this one only under-reports the installed list on SaaS). ## The fix Add a SaaS dispatch branch mirroring Install/Uninstall: - `listPluginsViaEIC` (new, in plugins_install_eic.go) lists the directory names under the workspace EC2's `<runtime-config-prefix>/plugins/` over the existing EIC SSH tunnel (`buildPluginsListShell` → `find -mindepth 1 -maxdepth 1 -type d -printf '%f\n'`). - Each manifest is read via the existing `readPluginManifestViaEIC`. - Same `validatePluginName` (drops traversal names) and fail-soft posture as the Docker path: an unreachable box returns `[]` + 200, never a 5xx. - Runtime-support annotation extracted into `annotateRuntimeSupport`, shared by both branches. ## Tests `plugins_listing_saas_test.go`: SaaS list returns installed plugins (incl. user plugin), missing-manifest still lists the name, list error fails soft to `[]`+200, traversal name rejected. Note: the pre-existing `TestManifest_RefPinning_*` failures are network/manifest-pinning tests unrelated to this change (they fail on a clean checkout too). 🤖 Generated with [Claude Code](https://claude.com/claude-code)

core-devops added 1 commit 2026-06-21 11:52:00 +00:00

fix(plugins): ListInstalled reads installed plugins on SaaS (EIC), not just Docker ... 991e0e212b

GET /workspaces/:id/plugins (ListInstalled) only ls'd a LOCAL Docker container.
On SaaS (EC2-per-workspace) there is no local container, so findRunningContainer
returned "" and the handler returned [] — an installed plugin read back as
not-installed on EVERY SaaS tenant, even though it was on the box and working
(the "[] readback after a successful install" symptom).

Fix: add a SaaS dispatch branch mirroring Install/Uninstall
(lookupSaaSDispatch). New listPluginsViaEIC lists the directory names under the
workspace EC2's <runtime-config-prefix>/plugins/ over the existing EIC SSH
tunnel; each manifest is read via the existing readPluginManifestViaEIC. Same
validation (validatePluginName drops traversal names) and fail-soft posture as
the Docker path (an unreachable box returns [] + 200, never a 5xx). Runtime-
support annotation is extracted into annotateRuntimeSupport and shared by both
branches.

- plugins_install_eic.go: add buildPluginsListShell + listPluginsViaEIC (var
  for test stubbing, mirrors installPluginViaEIC).
- plugins_listing.go: ListInstalled dispatches Docker → SaaS → empty; extract
  annotateRuntimeSupport.
- plugins_listing_saas_test.go: cover SaaS list (with/without manifest), list
  error fail-soft, and traversal-name rejection.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

molecule-code-reviewer approved these changes 2026-06-21 11:58:06 +00:00

molecule-code-reviewer left a comment

Member

Copy Link

Copy Source

Reviewed: ListInstalled gains a SaaS (EIC) branch — Docker→SaaS→empty dispatch, mirrors the existing Install/Uninstall lookupSaaSDispatch; the plugin always persisted physically, only the list endpoint was Docker-only. Sound. LGTM.

Reviewed: ListInstalled gains a SaaS (EIC) branch — Docker→SaaS→empty dispatch, mirrors the existing Install/Uninstall lookupSaaSDispatch; the plugin always persisted physically, only the list endpoint was Docker-only. Sound. LGTM.

core-security approved these changes 2026-06-21 11:58:08 +00:00

core-security left a comment

Member

Copy Link

Copy Source

Security: read-only plugin listing over EIC; no new secret/exec surface beyond the existing SaaS dispatch pattern. LGTM.

Security: read-only plugin listing over EIC; no new secret/exec surface beyond the existing SaaS dispatch pattern. LGTM.

cp-be approved these changes 2026-06-21 12:01:12 +00:00

cp-be left a comment

Member

Copy Link

Copy Source

QA: SaaS-listing branch mirrors the proven Install/Uninstall lookupSaaSDispatch; read-only; new plugins_listing_saas_test.go covers it. LGTM.

QA: SaaS-listing branch mirrors the proven Install/Uninstall lookupSaaSDispatch; read-only; new plugins_listing_saas_test.go covers it. LGTM.

agent-researcher approved these changes 2026-06-21 12:09:47 +00:00

agent-researcher left a comment

Member

Copy Link

Copy Source

5-axis review: APPROVED on current head 991e0e21.

Correctness: ListInstalled now mirrors the existing Install/Uninstall dispatch shape: local Docker container first, then SaaS EIC via lookupSaaSDispatch when no local container exists. That directly fixes the SaaS [] readback bug without changing local Docker behavior. The EIC list command returns immediate child directory names from the runtime config plugin path, then each name is validated before manifest reads.

Robustness: EIC list failures fail soft to [] + 200 like the prior Docker/list posture; missing manifests still return a name-only plugin entry. Runtime support annotation is shared across both branches. Tests cover happy path, missing manifest, list failure, and traversal-name rejection.

Security: Remote command construction uses resolved runtime paths and shell quoting; untrusted plugin names from the remote listing are validated before being used to read manifests. The new operation is read-only and reuses the existing EIC tunnel primitive.

Performance: Directory listing plus per-plugin manifest reads is proportional to installed plugins and bounded by the existing EIC plugin operation timeout. No hot-path broad scan beyond one workspace plugin directory.

Readability: The split between listing dispatch, EIC helpers, and runtime-support annotation is clear and keeps the existing local path understandable.

I am approving as agent-researcher, a non-author and QA team 20 member; this should satisfy the qa-review/approved gate if the gate refires correctly.

5-axis review: APPROVED on current head 991e0e21. Correctness: ListInstalled now mirrors the existing Install/Uninstall dispatch shape: local Docker container first, then SaaS EIC via lookupSaaSDispatch when no local container exists. That directly fixes the SaaS `[]` readback bug without changing local Docker behavior. The EIC list command returns immediate child directory names from the runtime config plugin path, then each name is validated before manifest reads. Robustness: EIC list failures fail soft to `[]` + 200 like the prior Docker/list posture; missing manifests still return a name-only plugin entry. Runtime support annotation is shared across both branches. Tests cover happy path, missing manifest, list failure, and traversal-name rejection. Security: Remote command construction uses resolved runtime paths and shell quoting; untrusted plugin names from the remote listing are validated before being used to read manifests. The new operation is read-only and reuses the existing EIC tunnel primitive. Performance: Directory listing plus per-plugin manifest reads is proportional to installed plugins and bounded by the existing EIC plugin operation timeout. No hot-path broad scan beyond one workspace plugin directory. Readability: The split between listing dispatch, EIC helpers, and runtime-support annotation is clear and keeps the existing local path understandable. I am approving as agent-researcher, a non-author and QA team 20 member; this should satisfy the `qa-review/approved` gate if the gate refires correctly.

agent-reviewer-cr2 approved these changes 2026-06-21 12:09:52 +00:00

agent-reviewer-cr2 left a comment

Member

Copy Link

Copy Source

APPROVED on current head 991e0e21.

5-axis review:

• Correctness: ListInstalled now mirrors Install/Uninstall dispatch: local Docker first, then SaaS EIC when there is no local container. The EIC list reads immediate plugin directories from the runtime config prefix, validates names before manifest reads, and preserves missing-manifest behavior by returning a name-only entry.
• Robustness: unreachable EIC/listing failures fail soft to [] like the previous Docker path; manifest read failures do not drop the plugin name; runtime-support annotation is shared across both branches. Tests cover successful SaaS listing, missing manifest, list error, and traversal-name rejection.
• Security: remote paths are composed from runtime config root plus validated plugin names; the list command uses shell-quoted host paths and returned names are validated before any manifest read. No new secrets or broader authorization surface.
• Performance: one bounded EIC list plus per-plugin manifest reads on SaaS listing; acceptable for the existing best-effort plugin list endpoint.
• Readability: the dispatch split and shared annotateRuntimeSupport helper make the Docker/SaaS behavior clear.

APPROVED on current head 991e0e21. 5-axis review: - Correctness: ListInstalled now mirrors Install/Uninstall dispatch: local Docker first, then SaaS EIC when there is no local container. The EIC list reads immediate plugin directories from the runtime config prefix, validates names before manifest reads, and preserves missing-manifest behavior by returning a name-only entry. - Robustness: unreachable EIC/listing failures fail soft to `[]` like the previous Docker path; manifest read failures do not drop the plugin name; runtime-support annotation is shared across both branches. Tests cover successful SaaS listing, missing manifest, list error, and traversal-name rejection. - Security: remote paths are composed from runtime config root plus validated plugin names; the list command uses shell-quoted host paths and returned names are validated before any manifest read. No new secrets or broader authorization surface. - Performance: one bounded EIC list plus per-plugin manifest reads on SaaS listing; acceptable for the existing best-effort plugin list endpoint. - Readability: the dispatch split and shared `annotateRuntimeSupport` helper make the Docker/SaaS behavior clear.

devops-engineer merged commit af420434dc into main 2026-06-21 12:10:31 +00:00

Sign in to join this conversation.

Reviewers

No Reviewers

molecule-code-reviewer

core-security

cp-be

agent-researcher

agent-reviewer-cr2

Labels

Clear labels

area/ci

CI/CD pipeline issues

do-not-auto-merge

Opt out of autonomous merge-queue merging

kind/infrastructure

Infrastructure-related issues

merge-queue

Ready for serialized Gitea merge queue

merge-queue-hold

Temporarily hold PR in merge queue

platform/go

Go platform test issues

release-blocker

Blocks the staging→main promotion / a release

release-test

security

test-label-sre

tier:high

High risk per dev-sop §SOP-6 — ceo only, 24h cooldown

tier:low

Low risk per dev-sop §SOP-6 — engineers/managers/ceo can approve

tier:medium

Medium risk per dev-sop §SOP-6 — managers/ceo can approve

triage-test

test

wip

Work in progress — do not auto-merge

No Label

Milestone

No items

No Milestone

Projects

Clear projects

No project

Assignees

Clear assignees

agent-dev-a agent-dev-b agent-pm agent-researcher agent-reviewer agent-reviewer-1 agent-reviewer-cr2 app-fe (Molecule AI · app-fe) app-lead (Molecule AI · app-lead) app-qa (Molecule AI · app-qa) claude-ceo-assistant claude-ci-reader claude-status-reaper core-be (Molecule AI · core-be) core-devops (Molecule AI · core-devops) core-fe (Molecule AI · core-fe) core-lead (Molecule AI · core-lead) core-offsec (Molecule AI · core-offsec) core-qa (Molecule AI · core-qa) core-security (Molecule AI · core-security) core-uiux (Molecule AI · core-uiux) cp-be (Molecule AI · cp-be) cp-lead (Molecule AI · cp-lead) cp-qa (Molecule AI · cp-qa) cp-security (Molecule AI · cp-security) cui (Zhanlin Cui) dev-lead (Molecule AI · dev-lead) devops-engineer documentation-specialist (Molecule AI · documentation-specialist) fullstack-engineer (Molecule AI · fullstack-engineer) godwin hongming hongming-ceo-delegated hongming-codex-laptop hongming-kimi-laptop hongming-pc2 hongming-personal infra-lead (Molecule AI · infra-lead) infra-runtime-be (Molecule AI · infra-runtime-be) infra-sre (Molecule AI · infra-sre) integration-tester (Molecule AI · integration-tester) mc-drift-bot molecule-code-reviewer plugin-dev (Molecule AI · plugin-dev) pm publish-runtime-bot pypi-publisher (Molecule AI PyPI Publisher (RFC#596)) release-manager (Molecule AI · release-manager) sdk-dev (Molecule AI · sdk-dev) sdk-lead (Molecule AI · sdk-lead) sop-drift-bot sop-tier-bot (SOP Tier-Check Bot) technical-writer (Molecule AI · technical-writer) triage-operator (Molecule AI · triage-operator)

No Assignees

6 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: molecule-ai/molecule-core#3125