docs(claude-ops): record the --all + install_new:all mass install - #4182
kyle-sexton wants to merge 2 commits into
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Claude finished @kyle-sexton's task in 3m 36s —— View job Code review complete
This is a documentation-only PR (no scripts changed), so I checked every command/selector the new prose introduces against the actual script contracts rather than treating it as prose-only. 1 correctness bug, found in two places (inline comments posted):
Verified as correct, not flagged:
No security concerns (docs-only change, no code execution paths added). |
|
Claude finished @kyle-sexton's task in 45s —— View job Security review: no issues found
Skip gate hit: the diff has no security-relevant surface. Both changed files are prose additions to skill documentation:
No shell scripts, code, workflow files, or executable logic are touched — only Markdown guidance describing operational policy (requiring a human confirmation count before a mass install). There is no injection surface, no credential/token handling, no authorization logic, and no GitHub Actions change in this diff. Per this lane's skip gate ("change has no security-relevant surface after reading the diff"), no findings to report. |
|
Last security-reviewed head: |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 805c753749
ℹ️ 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".
| `claude plugin uninstall <id> -s user -y` removes both the `installed_plugins.json` record and the | ||
| user-scope `enabledPlugins` entry, but it costs ~6s per plugin, so a four-figure revert runs for | ||
| hours. There is no bulk uninstall verb; `marketplace remove` is not one (see its own section above, |
There was a problem hiding this comment.
The revert guidance introduces volatile Claude CLI specifics—uninstall side effects, timing, and the absence of a bulk verb—without an external upstream source and a claim-specific basis/as-of/recheck record. The repository requires that record whenever a skill body restates volatile behavior; otherwise this recovery procedure can silently become unsafe as the CLI changes.
AGENTS.md reference: AGENTS.md:L20-L21
Useful? React with 👍 / 👎.
| the per-marketplace install gap first (`fleet-state.sh --ids install-gap` per marketplace, or one | ||
| `audit --all`), present the total, and proceed only on a yes. An agent running this unattended |
There was a problem hiding this comment.
When an operator follows this newly required safety check, neither command shown here is valid as written: fleet-state.sh accepts the selector missing-user-install, not install-gap, and the skill's Action Router accepts the marketplace target as the second bare token (audit all), not audit --all. Thus this gotcha's counted-confirmation recipe fails before producing the count it requires; use the documented selector with --marketplace <name> and the skill-level audit all spelling.
Useful? React with 👍 / 👎.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| [context/gotchas.md](context/gotchas.md)'s "`--all` with `install_new: all` is a mass install of | ||
| every catalog". **When the marketplace target is `all` and the rendered policy is `all`, do not | ||
| proceed unattended:** resolve the total install gap first (one `audit all`, or `fleet-state.sh | ||
| --ids install-gap` per marketplace), state the number, and get an explicit human yes. Treat the |
There was a problem hiding this comment.
fleet-state.sh --ids install-gap is not a valid invocation — install-gap isn't one of the accepted --ids selectors. fleet-state.sh's ids_selector_valid() only accepts installed-user | update-candidates-user | update-candidates-project | downgrade-candidates | current-project | missing-user-install | missing-enabled | user-scope-orphans (fleet-state.sh#L458-472). Running the documented command fails with ERROR: unknown --ids selector: install-gap.
"install-gap" only appears as a human-readable label in an error message in sync-run.sh (mp_error "install-gap projection failed...", sync-run.sh#L911); the actual selector it passes is missing-user-install (sync-run.sh#L908), which is also what context/sync-install-enable.md#L36 already uses. This line should say --ids missing-user-install.
|
|
||
| **The rule, until the skill enforces a volume gate itself:** treat `--all` combined with a rendered | ||
| `install_new` of `all` as requiring an explicit human confirmation that names the number. Resolve | ||
| the per-marketplace install gap first (`fleet-state.sh --ids install-gap` per marketplace, or one |
There was a problem hiding this comment.
Same issue as the parallel text added in SKILL.md: install-gap is not a valid --ids selector — fleet-state.sh's ids_selector_valid() only accepts installed-user | update-candidates-user | update-candidates-project | downgrade-candidates | current-project | missing-user-install | missing-enabled | user-scope-orphans. This command as written errors with unknown --ids selector: install-gap; it should read --ids missing-user-install (the selector sync-run.sh itself actually projects for this purpose, per its project_ids missing-user-install ... call).
|
Claude has reviewed this PR 1 time. The lane skips further automatic reviews after 5; deleting this comment resets the count. |
|
PR body contract — issue linkage This PR body does not yet satisfy the issue-linkage contract:
Edit the body and this comment updates itself on the next run. |
On 2026-09-14 `/claude-ops:plugins sync --all` run with the `install_new` userConfig set to `all` installed 2,231 plugins across every known marketplace in 22 minutes, ~49 GB of cache, and left 22 third-party MCP servers running for 8.5 hours. Against the one curated marketplace the policy was written for, `all` means "install the handful I am missing"; `--all` multiplies it across every catalog the machine knows about, and the wording does not change between the two cases. gotchas.md: new section recording the incident, why the downgrade guard and `audit` both miss this combination, the `claude --bare` requirement for the revert (a plain `plugin uninstall` spawns a session that loads every still- enabled plugin and boots its MCP servers), and the revert recipe. SKILL.md: the marketplace-resolution list now warns that `all` multiplies the install policy across catalogs, and the `install_new` section requires a counted human confirmation before that combination runs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XD4Hr2S7RA2G8PR2CadLP1
805c753 to
1c1f516
Compare
check-changelog-parity --check-bump refuses to reuse a published version when a change set touches shipped plugin files, documentation included. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XD4Hr2S7RA2G8PR2CadLP1
1 participant
What
Two edits to the
claude-ops:pluginsskill recording an incident the skill's existing safeguards do not catch.skills/plugins/context/gotchas.md— new section: "--allwithinstall_new: allis a mass install of every catalog, and nothing warns you."skills/plugins/SKILL.md— the marketplace-resolution list now warns thatallmultiplies the install policy across catalogs; theinstall_newsection requires a counted human confirmation before that combination runs.Why
On 2026-09-14,
/claude-ops:plugins sync --allwith theinstall_newuserConfig set toallinstalled 2,231 plugins (1,953claude-community, 269claude-plugins-official, 5dotnet-agent-skills, 4skills) between 14:43 and 15:05 — roughly 49 GB of plugin cache.A logon-triggered session then loaded them and started their MCP servers: 22 third-party processes ran for ~8.5 hours, one binding TCP 8099 on all interfaces, at least 11 running
npm install.The skill did exactly what it was told. Against one curated marketplace,
install_new: allmeans "install the handful I am missing" — a correct policy for a marketplace you maintain.--allsilently changes the scope that policy applies over, and neither the downgrade guard norauditsurfaces the resulting count before it runs.What the gotcha records
claude --barerequirement for the revert. A plainclaude plugin uninstallspawns a session that loads every still-enabled plugin and boots its MCP servers — so the first revert attempt was re-executing the bad plugin set on every iteration.--barewas verified over six uninstalls to spawn nothing.🤖 Generated with Claude Code
https://claude.ai/code/session_01XD4Hr2S7RA2G8PR2CadLP1