fix(plugins): harden bundled install/uninstall sweep

[vincentkoc]

Summary

• Problem: bundled plugin install/uninstall coverage exposed that config-gated bundled plugins could not be installed cleanly before required config existed.
• Why it matters: packaged sweeps need to cover plugins such as memory-lancedb without forcing users or tests to seed credentials first.
• What changed: config-gated bundled installs now persist the install record and load path without writing invalid placeholder plugin config, skip slot selection while disabled, and add a Docker E2E sweep lane.
• What did NOT change: user-provided valid plugin config still enables normally; uninstall semantics still remove plugin records, entries, allow/deny references, load paths, and managed files.

Change Type (select all)

• Bug fix
• Feature
• Refactor required for the fix
• Docs
• Security hardening
• Chore/infra

Scope (select all touched areas)

• Gateway / orchestration
• Skills / tool execution
• Auth / tokens
• Memory / storage
• Integrations
• API / contracts
• UI / DX
• CI/CD / infra

Linked Issue/PR

• Related [Bug]: Stale plugin config entry emits noisy warning on every CLI invocation with no way to clean it up #59448
• Related [Feature]: Add CLI command to remove stale plugin config entries (currently requires manual JSON editing) #59455
• This PR fixes a bug or regression

Root Cause (if applicable)

• Root cause: bundled plugin install persistence assumed install meant enable, but required-schema plugin config validation still runs for entries carrying config even when the entry is disabled.
• Missing detection / guardrail: there was no packaged sweep proving bundled plugin install and uninstall for config-gated plugins.
• Contributing context: memory-lancedb requires embedding config, so an empty placeholder entry is not write-safe.

Regression Test Plan (if applicable)

• Coverage level that should have caught this:
  • Unit test
  • Seam / integration test
  • End-to-end test
  • Existing coverage already sufficient
• Target test or file: src/cli/plugins-cli.install.test.ts, src/cli/plugins-install-persist.test.ts, scripts/e2e/bundled-plugin-install-uninstall-docker.sh
• Scenario the test should lock in: installing a bundled required-config plugin persists install metadata without invalid config, then uninstall removes records and load paths.
• Why this is the smallest reliable guardrail: unit tests cover config mutation and the Docker smoke proves packaged dist behavior.
• Existing test that already covers this (if any): partial plugin install/uninstall tests existed, but not bundled required-config packaged sweep.
• If no new test is added, why not: N/A

User-visible / Behavior Changes

Bundled plugins that require config can be installed before credentials/config are present; they remain disabled until configured and explicitly enabled.

Diagram (if applicable)

Before:
plugins install memory-lancedb -> writes incomplete config entry -> config validation failure

After:
plugins install memory-lancedb -> records bundled install + load path -> user configures -> plugins enable memory-lancedb

Security Impact (required)

• New permissions/capabilities? No
• Secrets/tokens handling changed? No
• New/changed network calls? No
• Command/tool execution surface changed? No
• Data access scope changed? No
• If any Yes, explain risk + mitigation: N/A

Repro + Verification

Environment

• OS: Testbox Linux runner
• Runtime/container: Node 22 / packaged Docker E2E image
• Model/provider: N/A
• Integration/channel (if any): bundled plugin install/uninstall CLI
• Relevant config (redacted): clean HOME plus bundled memory-lancedb and telegram install smoke

Steps

1. Install bundled memory-lancedb from packaged dist without embedding config.
2. Verify install record and load path are written but invalid placeholder config is not.
3. Uninstall memory-lancedb and verify records, load path, allow/deny entries, and config entry are removed.
4. Repeat with a non-config-gated bundled plugin.

Expected

• Config-gated install succeeds without invalid config.
• Uninstall fully cleans the persisted install state.

Actual

• Matches expected after this PR.

Evidence

• Failing test/log before + passing after
• Trace/log snippets
• Screenshot/recording
• Perf numbers (if relevant)

Human Verification (required)

• Verified scenarios: clean rebase, targeted plugin/tooling tests, package formatting, Testbox changed gate, rebuilt Docker smoke for memory-lancedb and telegram.
• Edge cases checked: stale incomplete config entry, restrictive allow/deny lists, required-config bundled source metadata, uninstall cleanup assertions.
• What you did not verify: the full all-bundled-plugin Docker sweep across every bundled plugin shard.

Review Conversations

• I replied to or resolved every bot review conversation I addressed in this PR.
• I left unresolved only the conversations that still need reviewer or maintainer judgment.

Compatibility / Migration

• Backward compatible? Yes
• Config/env changes? No
• Migration needed? No
• If yes, exact upgrade steps: N/A

Risks and Mitigations

• Risk: config-gated plugins may appear installed but disabled until configured.
  • Mitigation: install prints a warning and points users to configure then enable the plugin.

[greptile-apps]

Greptile Summary

This PR hardens bundled plugin install/uninstall by separating "persist install record + load path" from "enable plugin entry" for config-gated plugins (e.g., memory-lancedb). It also adds configSchema/requiresConfig fields to BundledPluginSource, introduces an enable flag to persistPluginInstall, passes updateChannelConfig: false to enablePluginInConfig during installs, and backs everything with a new Docker E2E sweep lane and targeted unit tests.

Confidence Score: 4/5

Safe to merge; all findings are P2 and the author explicitly smoke-tested the telegram and memory-lancedb paths.

No P0/P1 issues found. The three P2 comments cover: a behavioral change for channel-plugin installs that skips setting channels.* (intentional per smoke test, but not explicitly documented), an empty entries: {} side-effect in prepareConfigForDisabledBundledInstall, and a duplicate isRecord helper.

src/cli/plugins-install-persist.ts — the updateChannelConfig: false option now applies to all installs, not just config-gated ones; worth verifying channel plugin behavior end-to-end if the system reads channels..enabled at startup.

Comments Outside Diff (2)

1. src/cli/plugins-install-command.ts, line 397-410 (link)

   prepareConfigForDisabledBundledInstall injects empty entries: {}

   When config.plugins?.entries is undefined, the function falls back to {}, spreads nothing out of it, and writes entries: nextEntries (i.e., entries: {}) into the returned config. This means configs that previously had no plugins.entries key at all will end up with an empty entries: {} in the persisted file — adding noise without purpose. A simple guard avoids this:

   return {
     ...config,
     plugins: {
       ...config.plugins,
       ...(Object.keys(nextEntries).length > 0 || config.plugins?.entries !== undefined
         ? { entries: nextEntries }
         : {}),
     },
   };

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: src/cli/plugins-install-command.ts
   Line: 397-410
   
   Comment:
   **`prepareConfigForDisabledBundledInstall` injects empty `entries: {}`**
   
   When `config.plugins?.entries` is `undefined`, the function falls back to `{}`, spreads nothing out of it, and writes `entries: nextEntries` (i.e., `entries: {}`) into the returned config. This means configs that previously had no `plugins.entries` key at all will end up with an empty `entries: {}` in the persisted file — adding noise without purpose. A simple guard avoids this:
   
   ```ts
   return {
     ...config,
     plugins: {
       ...config.plugins,
       ...(Object.keys(nextEntries).length > 0 || config.plugins?.entries !== undefined
         ? { entries: nextEntries }
         : {}),
     },
   };
   ```
   
   How can I resolve this? If you propose a fix, please make it concise.

2. src/cli/plugins-install-command.ts, line 364-370 (link)

   Duplicate isRecord helper

   An identical isRecord function is also defined in src/plugins/bundled-sources.ts. Extracting it to a shared utility (e.g., src/utils.ts or a dedicated type-guard module) would remove the duplication and keep the implementations in sync if the definition ever needs to change.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: src/cli/plugins-install-command.ts
   Line: 364-370
   
   Comment:
   **Duplicate `isRecord` helper**
   
   An identical `isRecord` function is also defined in `src/plugins/bundled-sources.ts`. Extracting it to a shared utility (e.g., `src/utils.ts` or a dedicated type-guard module) would remove the duplication and keep the implementations in sync if the definition ever needs to change.
   
   How can I resolve this? If you propose a fix, please make it concise.

Prompt To Fix All With AI

This is a comment left during a code review.
Path: src/cli/plugins-install-persist.ts
Line: 78-80

Comment:
**`updateChannelConfig: false` silently changes behavior for all installs**

`updateChannelConfig: false` is passed for every non-disabled install, not just config-gated ones. Previously, `persistPluginInstall` for a built-in channel plugin (e.g., `telegram`) would write `channels.telegram.enabled = true` via `setPluginEnabledInConfig`; with this option the branch is skipped and `channels.*` is never touched during install. If the runtime reads `channels.<id>.enabled` to gate channel startup, channel plugins installed via `plugins install telegram` will no longer be activated until `plugins enable telegram` is run explicitly.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: src/cli/plugins-install-command.ts
Line: 397-410

Comment:
**`prepareConfigForDisabledBundledInstall` injects empty `entries: {}`**

When `config.plugins?.entries` is `undefined`, the function falls back to `{}`, spreads nothing out of it, and writes `entries: nextEntries` (i.e., `entries: {}`) into the returned config. This means configs that previously had no `plugins.entries` key at all will end up with an empty `entries: {}` in the persisted file — adding noise without purpose. A simple guard avoids this:

```ts
return {
  ...config,
  plugins: {
    ...config.plugins,
    ...(Object.keys(nextEntries).length > 0 || config.plugins?.entries !== undefined
      ? { entries: nextEntries }
      : {}),
  },
};
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: src/cli/plugins-install-command.ts
Line: 364-370

Comment:
**Duplicate `isRecord` helper**

An identical `isRecord` function is also defined in `src/plugins/bundled-sources.ts`. Extracting it to a shared utility (e.g., `src/utils.ts` or a dedicated type-guard module) would remove the duplication and keep the implementations in sync if the definition ever needs to change.

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (1): Last reviewed commit: "docs(changelog): credit bundled plugin s..." | Re-trigger Greptile}

[greptile-apps]

updateChannelConfig: false silently changes behavior for all installs

updateChannelConfig: false is passed for every non-disabled install, not just config-gated ones. Previously, persistPluginInstall for a built-in channel plugin (e.g., telegram) would write channels.telegram.enabled = true via setPluginEnabledInConfig; with this option the branch is skipped and channels.* is never touched during install. If the runtime reads channels.<id>.enabled to gate channel startup, channel plugins installed via plugins install telegram will no longer be activated until plugins enable telegram is run explicitly.

Prompt To Fix With AI

This is a comment left during a code review.
Path: src/cli/plugins-install-persist.ts
Line: 78-80

Comment:
**`updateChannelConfig: false` silently changes behavior for all installs**

`updateChannelConfig: false` is passed for every non-disabled install, not just config-gated ones. Previously, `persistPluginInstall` for a built-in channel plugin (e.g., `telegram`) would write `channels.telegram.enabled = true` via `setPluginEnabledInConfig`; with this option the branch is skipped and `channels.*` is never touched during install. If the runtime reads `channels.<id>.enabled` to gate channel startup, channel plugins installed via `plugins install telegram` will no longer be activated until `plugins enable telegram` is run explicitly.

How can I resolve this? If you propose a fix, please make it concise.