Skip to content

feat(claude-ops): add plugins skill for plugin-fleet sync/audit/converge#248

Merged
kyle-sexton merged 23 commits into
mainfrom
feat/claude-ops-plugins-skill
Jul 17, 2026
Merged

feat(claude-ops): add plugins skill for plugin-fleet sync/audit/converge#248
kyle-sexton merged 23 commits into
mainfrom
feat/claude-ops-plugins-skill

Conversation

@kyle-sexton

@kyle-sexton kyle-sexton commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

No linked issue.

Summary

  • Adds /claude-ops:plugins — an action-routed skill (sync default / audit read-only / converge explicit scope consolidation) that brings a machine's Claude Code plugin fleet current: marketplace refresh, updates to plugins that actually load (including in-repo project/local-scope installs), new-catalog-plugin install per an install_new policy, and scope-divergence detection/reporting — never silently fixing drift.
  • Core is fleet-state.sh, a read-only, TDD-built state-inspection script (14 tests / 29 assertions) that reconciles installed_plugins.json / known_marketplaces.json against a project's cwd, handling Windows-vs-Git-Bash path normalization and separating benign same-version multi-scope installs from actionable version divergence.
  • claude-ops bumped 0.8.0 → 0.9.0; new install_new userConfig option.

Verification

  • /verification:confirm ran end-to-end: Stage 1 (bash + markdown ecosystems, full validate-plugins.sh, check-skill.sh, script test suite) all green; Stage 2 outcome verdict from a fresh-context (non-authoring) verifier agent: CONFIRMED, all 6 plan-phase acceptance criteria hold.
  • The verifier's one real finding (docs overclaimed enable-state-mismatch detection when fleet-state.sh only exposes the merged effective value, not each scope's raw map) is fixed in this PR — see context/converge.md / SKILL.md / evals.json.
  • Known, disclosed gap (documented in docs/topics/plugin-fleet-sync-skill/PLAN.md's Phase 6 evidence): audit is live-verified end-to-end against this repo and a second real repo. sync's in-repo update step (Step 2, the plan's own "primary value path") and converge were never exercised against real divergence this session — both would require mutating a real repo outside this one, which needs an explicit decision at that time, not a unilateral one here.
  • Locally mirrored every CI hygiene/plugin-gate lane: shellcheck, shfmt, markdownlint, typos, gitleaks, editorconfig, exec-bit, hook-utils-sync, cross-plugin-source-drift, full plugin contract-test suite, manifest/catalog validation — all pass.

Plan

Full phase-by-phase plan, evidence, and acceptance criteria: docs/topics/plugin-fleet-sync-skill/PLAN.md.

Related

None — self-contained plan/design/implementation cycle for a new skill, not tied to an existing issue, ADR, or decision-log entry.

kyle-sexton and others added 13 commits July 17, 2026 08:55
Locked contract for /claude-ops:plugins (sync/audit/converge): interview
Brief, design early-exit resolution, and the stress-tested six-phase
implementation plan. Phase 1 carries the USER-RESERVED go/no-go on
whether `plugin update -s project` writes committed settings.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…phase 1)

Empirically resolve the plan's three unknowns before authoring the
plugins skill: `plugin update -s project` does not write committed
settings (hash-verified in a real dual-scope repo), `/reload-plugins`
has no `--force` flag, and `userConfig` has no `enum` support. Snapshot
the live installed_plugins.json / known_marketplaces.json shapes into
scrubbed fixtures for phase 2.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…n (phase 2)

Read-only JSON state report over CC's internal plugin-state files:
catalog completeness, install completeness, and scope divergence per
marketplace, with --marketplace/--all and a dynamic default-marketplace
resolution that never hardcodes a marketplace name. Ships with an
11-case TDD suite covering divergence, missing-install, missing-enabled
vs explicit opt-out, marketplace-absent, malformed-shape fail-loud, the
native-Windows-path-vs-Git-Bash CRITICAL case, and jq-missing.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ivergence

fleet-state.sh's divergences flagged every multi-scope install record as a
conflict, but a project pin and a user pin at the identical version isn't
one — it's a normal, non-actionable state. A raw record count inflates the
report's "N behind, run converge" line with entries that need no action
(2 of 31 on this machine). Add versionsMatch so the report layer can filter
to the actionable subset (29 here) instead of the raw count.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… docs (phase 3)

SKILL.md action router (sync default / audit / converge) plus four
context files: the sync algorithm (marketplace refresh, in-repo update,
user-scope sweep, install-new per policy, enabledPlugins completeness),
the converge scope-consolidation flow with autonomous-session abort,
verified scope/version/reload semantics, and known failure modes.

Also fixes fleet-state.sh's missing_from_install, which didn't yet
exclude ids explicitly opted out (enabledPlugins: false) before ever
being installed — found while grounding sync.md's "Missing" definition
in the script's actual behavior.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
6 cases covering routing (bare invocation -> sync), audit's read-only
guarantee, install_new=ask batched-prompt behavior, converge's
autonomous-session abort, the divergence report's bulk-collapse vs
per-row-conflict shape, and the never-hand-edit-internal-state
anti-pattern. Passes check-skill.sh with 0 errors/0 warnings.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
version, description, CHANGELOG, README skills table + configuration
entry for the new plugins skill and its install_new userConfig option.
Regenerates the repo-root README's plugin catalog block, which went
stale against the updated claude-ops description.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…KILL.md

SKILL.md pointed at ${CLAUDE_PLUGIN_ROOT}/scripts/fleet-state.sh; the
script actually lives at skills/plugins/scripts/fleet-state.sh relative
to the plugin root. Caught by a real end-to-end /claude-ops:plugins
audit run against a --plugin-dir dev-loop session (phase 6).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…phase 6)

Found via real end-to-end execution against the actual skill (the
repo's --plugin-dir dev-loop convention, not a simulation):

- fleet-state.sh's currentProject stayed null in a real headless
  session because CLAUDE_PROJECT_DIR isn't reliably exported there.
  Falls back to the cwd's git toplevel now (CLAUDE_PROJECT_DIR stays
  authoritative when set), with a regression test for the fallback
  path that failed against the unpatched script and passes now.
- Documents that `claude plugin update` rejects a bare plugin name
  even when unambiguous -- always pass the fully-qualified id.
- Removes a hardcoded marketplace name that leaked into gotchas.md's
  worked example, caught by this repo's own plugin-contract validator.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… boundary

audit is live-verified end-to-end; sync's Step 2 (in-repo update) and
converge were never exercised against real divergence this session.
Caught by advisor review before declaring the plan complete.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… plugins skill

/verification:confirm's fresh-context verifier found that converge.md and
SKILL.md described an enable-state mismatch (true in one scope, false in
another) as detected-but-unresolved, when fleet-state.sh never exposes
each scope's raw enabledPlugins map and genuinely cannot detect it. Reword
both docs to state the real blind spot, and swap evals.json's eval 5 off
a scenario the model could only pass by echoing a fact handed to it in
the prompt.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Every other *.test.sh in this repo is 100755; these two new scripts
carry a shebang but were committed at 100644. CI's exec-bit lane
(scripts/aggregate-hygiene-results.sh via the shared exec-bit action)
would have flagged this.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
mis-attribution tripped the typos hygiene lane; reworded to wrong
attribution. Caught by locally mirroring CI's hygiene lane before PR
creation.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 96981c38bc

ℹ️ 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".

Comment thread plugins/claude-ops/skills/plugins/context/sync.md
Comment thread plugins/claude-ops/skills/plugins/context/sync.md Outdated
Comment thread plugins/claude-ops/skills/plugins/SKILL.md Outdated
Comment thread plugins/claude-ops/skills/plugins/scripts/fleet-state.test.sh Outdated
…e PR run

CI's machine-specific-paths lane flagged a hardcoded D:\repos\...\medley
path in PLAN.md's Phase 1 open questions; genericized to a placeholder.

CI's plugin-gate lane failed a test that passed locally: the "jq missing"
case ran the bare script against default file paths instead of routing
through run_state's fixture builder like every other case. On this dev
machine a real Claude Code install satisfies those defaults so the script
reached its jq check; on a clean CI runner the file-not-found check fires
first and the case never exercises what it claims to test. Fixed by
building a real fixture dir and only stripping jq from PATH, isolating
jq-absence as the sole variable.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 114e31845b

ℹ️ 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".

Comment thread plugins/claude-ops/skills/plugins/context/sync.md Outdated
Comment thread plugins/claude-ops/skills/plugins/context/converge.md Outdated
Comment thread plugins/claude-ops/skills/plugins/scripts/fleet-state.sh Outdated
command -v jq only reports the first PATH match. This machine has a
single jq, so stripping its directory worked locally, but the CI
runner's ubuntu-24.04 image resolves jq from a second PATH directory
too — the "jq missing" case silently ran with jq still present and
exercised the wrong branch. Walk all of PATH and drop every directory
that actually contains an executable jq.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f403c5207a

ℹ️ 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".

Comment thread plugins/claude-ops/skills/plugins/scripts/fleet-state.sh Outdated
Comment thread plugins/claude-ops/skills/plugins/scripts/fleet-state.test.sh Outdated
@claude

claude Bot commented Jul 17, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 4m 27s —— View job


Code Review — feat(claude-ops): plugins skill

  • Gather context and understand the request
  • Read changed files (diff + key implementation files)
  • Review fleet-state.sh (correctness, error handling, edge cases)
  • Review fleet-state.test.sh (test coverage, jq-path issues)
  • Review skill context files (sync.md, converge.md, SKILL.md)
  • Review plugin.json manifest and config changes
  • Assess CLAUDE.md design-rule compliance
  • Post findings

Overall the design is solid — fleet-state.sh is well-structured, the read-only/CLI-only discipline is maintained throughout, the Windows path normalization handling is a genuine gotcha that's explicitly addressed, and the explicit-confirm gate in converge is exactly right. The test suite is above average for this codebase.

That said, I found two confirmed bugs (one severe, one latent on certain CI configurations) and three correctness/documentation gaps that will cause silent misbehavior in production use. Details below, most-severe first.


🔴 Bug — require_json inside emit_marketplace silently corrupts the entire --all output

fleet-state.sh:212

When --all is running, each marketplace is collected via:

block=$(emit_marketplace "$n" || true)
result=$(jq -c --arg n "$n" --argjson b "$block" '. + {($n): $b}' <<<"$result")

The || true handles emit_marketplace returning non-zero via return 1 — and every early-return path in the function correctly emits a JSON error object first. But require_json at line 212 uses exit 2, not return 1. exit inside $(...) kills only the subshell, so || true catches the non-zero code — but $block is empty (the error went to stderr, nothing landed on stdout). Then --argjson b "" is invalid JSON, jq fails, $result is set to empty. On the next iteration <<<$result passes empty input to jq, which fails again. The cascade means one marketplace with a malformed marketplace.json silently wipes the entire --all output.

The existing --all test doesn't exercise this path — it tests a missing catalog fixture (the ! -f branch, which correctly returns 1 with a JSON error object), not a malformed catalog file that passes the file-exists check but fails jq empty.

The fix: replace the require_json "$catalog_json" call inside emit_marketplace with the same inline error-emit-and-return 1 pattern the other early paths use. The fail-loud guarantee (require_json's purpose) still holds for the pre-function top-level calls outside emit_marketplace; it's only inside the per-marketplace function that exit needs to become an error object + return 1.

Fix this →


🔴 Bug — jq PATH stripping removes bash on systems where both live in /usr/bin

fleet-state.test.sh:360-364

The f403c52 commit correctly fixed the "strip only the first jq directory" defect (where a second jq copy remained on GitHub Actions). The current logic:

[[ -n "$dir" && -x "$dir/jq" ]] && continue

…drops any PATH entry that contains a jq executable. On Debian/Ubuntu (apt install jq), jq lives in /usr/bin — which is also where bash lives. Dropping /usr/bin means bash "$SCRIPT" inside run_state fails with env: 'bash': No such file or directory, and the case never reaches the jq required message it's supposed to verify.

The fix needs to keep bash reachable while hiding jq. The cleanest approach for a test fixture: create a temp bin/ dir with a stub jq that exits non-zero, prepend it to PATH, and keep the real PATH intact:

fake_bin="$TEST_TMPDIR/no-jq-bin"
mkdir -p "$fake_bin"
printf '#!/usr/bin/env bash\nexit 127\n' > "$fake_bin/jq"
chmod +x "$fake_bin/jq"
out=$(run_state "$case_dir" "PATH=$fake_bin:$PATH")

This shadows jq without touching directories bash depends on. Fix this →


🟡 Correctness — missing_from_enabled includes installs from unrelated projects

fleet-state.sh:254-258

installed_ids (line 242) is built from ALL install records in installed_plugins.json, spanning every projectPath. But known_at_mp (line 254) only covers ids mentioned in user settings + the current project's settings. Any plugin installed at project/local scope in a different repo — but absent from user settings and the current project — falls into installed_ids - known_at_mp and appears in missing_from_enabled. This would prompt sync to run claude plugin enable from the wrong working tree.

Fix: compute missing_from_enabled only from install records where scope == "user" or currentProject == true, not from the full installed_ids. Fix this →


🟡 Algorithm gap — single-scope project installs never get updated by sync

sync.md:41-42

Step 2 of sync only updates currentProject: true entries that also appear in divergences[] with versionsMatch: false. A divergences[] entry requires length > 1 (multiple scope records for the same id). A plugin installed only at project scope in the current repo — no user-scope baseline, no other project scopes — has exactly one install record, so no divergence entry is produced. Step 3 only sweeps scope == "user". Result: a project-only install that's behind the marketplace version is never touched by either step, silently left stale.

The fix for the doc: Step 2's gate should be "any currentProject: true entry that is behind the marketplace's catalog version" — not "has a divergence entry." The comparison baseline is the catalog, not another scope. Fix this →


🟡 Doc inconsistency — sync Step 2 gate uses env var; fleet-state.sh uses git fallback

sync.md:32

Step 2 says "Only when CLAUDE_PROJECT_DIR is set." But fleet-state.sh explicitly handles the case where CLAUDE_PROJECT_DIR is absent (lines 119-123), falling back to git rev-parse --show-toplevel, and this is verified by the test at line 253-285. In a headless -p session without the env var, fleet-state.sh correctly sets currentProject: true — but the skill, following sync.md's gate, would skip Step 2 entirely.

The correct gate is: does fleet-state.sh's output contain any entry with currentProject: true? Not whether the env var is set. Fix this →


🟡 Design gap — converge cross-project rows lack cwd context

converge.md:37

The proposed claude plugin uninstall <id> -s project / claude plugin update <id> -s <scope> commands are scoped to a project directory. installed_plugins.json can contain divergence records from multiple repos. When a divergence record belongs to a different projectPath, running these commands from the current working directory operates on the current repo's install — not the row's projectPath. The result is either a no-op (plugin isn't installed at project scope here) or the wrong repo's pin is removed.

The doc should either (a) restrict converge to currentProject: true rows and note that cross-machine/cross-project rows require running the command from that project's directory, or (b) show the commands prefixed with cd <row.scopes[].projectPath>. The PR description's disclosed gap ("converge was never exercised against real divergence this session") means this is untested behavior. Fix this →


⚪ Unverifiable claim — /reload-plugins --force

scope-semantics.md:56-59 / sync.md:98 / SKILL.md:113

The skill asserts (in three places) that /reload-plugins has no --force flag, citing a verification against code.claude.com/docs/en/plugins-reference. The Codex automated review disputes this, citing the discover-plugins page as mentioning --force for plugins with MCP servers. Per CLAUDE.md's fresh-docs mandate, whichever page was fetched during development should be re-fetched before this lands to confirm the claim is still accurate — if --force exists, advising users not to pass it will give them incorrect reload guidance for MCP-backed plugins.


Minor notes

  • fleet-state.sh:100 — the jq -r 'has("plugins") and (...)' check validates the top-level shape but doesn't assert that .plugins values are arrays. If CC ever changes the per-id value from [{...}] to {...}, the script would silently misprocess it. Given the explicit fail-loud philosophy, this seems worth a more complete shape check, though it's a minor concern.
  • plugin.jsoninstall_new userConfig addition is clean. The "enum in prose" workaround for the missing enum manifest type is well-documented and consistent with scope-semantics.md.
  • CLAUDE.md compliance — All design rules observed: no hardcoded paths, ${CLAUDE_PLUGIN_ROOT}/${CLAUDE_PLUGIN_DATA} used correctly, no PII, version bumped (0.8.0 → 0.9.0). The repo-agnostic rule is satisfied by the dynamic marketplace resolution via fleet-state.sh.

Summary: Two confirmed bugs need fixes before merge (the --all corruption via require_json/exit 2 and the jq-test bash-path stripping). The three correctness/doc gaps are real but lower-urgency — the single-scope-project-install gap and the cross-project converge path both only manifest against divergence scenarios the PR author correctly flags as untested. The CLAUDE_PROJECT_DIR gate doc inconsistency is the most likely to confuse an LLM agent following the skill docs.

Directory-exclusion PATH filtering (drop every dir containing jq) is not
universally safe: on this dev machine jq lives in its own directory, so
excluding it is harmless, but a GitHub Actions ubuntu runner colocates
jq with bash and coreutils in /usr/bin, so excluding jq's directory
there also removed bash, breaking the case with "env: 'bash': No such
file or directory" before the script ever reached its jq check.
Verified empirically that a chmod-000 decoy jq earlier on PATH doesn't
help either — command -v skips it and still finds the real jq further
down PATH.

Fixed by building an isolated PATH containing copies (never symlinks —
those need elevation on Windows) of only the tools fleet-state.sh needs
before its jq check: dirname, and bash itself to launch the
interpreter, plus any colocated *.dll bash.exe needs on Windows/MSYS
(a no-op on Linux). This no longer depends on jq's location at all.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@claude

claude Bot commented Jul 17, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 3m 41s —— View job


Re-review after 88bd1ecfeat(claude-ops): plugins skill

  • Gather context — read latest commits and changed files
  • Check if 🔴 require_json/exit 2 bug in emit_marketplace was fixed
  • Check if 🔴 jq-PATH/bash-stripping bug in fleet-state.test.sh was fixed (88bd1ec)
  • Re-assess 🟡 correctness/doc gaps
  • Post updated findings

What the latest commits fixed

88bd1ec — jq-missing test (🔴 Bug 2) ✅ FIXED

The path-filtering approach is replaced with a clean isolated $safe_bin_dir that holds only copies of bash, dirname, and any colocated DLLs — jq is deliberately absent. hook-utils.sh sources safely under this minimal PATH (all top-level statements are function definitions + a return-based double-source guard; command -v jq inside it returns 0 on miss rather than erroring). The fleet-state.sh jq check fires correctly, the test verifies exit 2 and the "jq required" message. This is the right fix.

114e318, f403c52, 96981c3, 56ca78b, ecdd76c — CI hygiene, executable bits, enable-state-mismatch overclaiming correction. No substantive logic changes relevant to the prior review findings.


What is still open

🔴 Bug — require_json inside emit_marketplace still uses exit 2 (merge blocker)

fleet-state.sh:212 — unchanged. The original finding still holds verbatim:

  • require_json at lines 87–96 uses exit 2 on a failed jq empty check.
  • Inside emit_marketplace, this fires inside the $(emit_marketplace "$n" || true) subshell.
  • The subshell exits 2; || true catches the non-zero code; $block is empty string.
  • jq -c --argjson b "$block" gets an empty string as JSON input → jq fails → $result goes empty.
  • Every subsequent iteration fails the same way; the entire --all output is silently wiped by one malformed marketplace.json.

The --all test case at line 291–306 still only tests a missing catalog fixture (the [[ ! -f "$fixture" ]] branch, which correctly returns 1 with a JSON error object). It does not exercise a present-but-malformed catalog file hitting require_json. The bug path has no test coverage.

The fix: inside emit_marketplace, replace the require_json call with the same inline error-emit + return 1 pattern the other early-return paths use, and add a test case with a present-but-malformed catalog in --all mode. Fix this →


🟡 missing_from_enabled still spans all projects — fleet-state.sh:241-258

installed_ids (line 242) is still built from .[].id across all install records, including project-scope installs from other repos. known_at_mp (line 254) only covers ids mentioned in user settings + the current project's settings, so cross-project project-scope installs land in missing_from_enabled. Unchanged from the prior review.

🟡 sync.md Step 2 gate still uses CLAUDE_PROJECT_DIR instead of currentProject: truesync.md:32

fleet-state.sh falls back to git rev-parse when CLAUDE_PROJECT_DIR is absent, and the test at line 253–285 verifies this. The skill doc's gate should key on currentProject: true in the output, not on whether the env var is set. Unchanged.

🟡 sync.md Step 2 divergence gate misses single-scope project installs — sync.md:40

A plugin installed only at project scope in the current repo has exactly one install record → no divergences[] entry → Step 2's gate skips it → a stale project-only install is never updated. Unchanged.

🟡 converge.md Step 2 missing cwd context — converge.md:37

claude plugin uninstall/update -s project is directory-scoped. When a divergence row's scopes[].projectPath doesn't match cwd, the command targets the wrong repo or silently no-ops. The doc doesn't surface this. Unchanged.

/reload-plugins --force claim still needs fresh-doc verification — scope-semantics.md:55-60 / sync.md:99

The claim "no --force flag exists" is asserted in three places. CLAUDE.md's fresh-docs mandate requires this to be re-verified by fetching the current plugins-reference page before merge. If --force exists for MCP-backed plugin reloads (as the Codex review suggested), advising users to omit it gives incorrect guidance.


Summary

One commit since the prior review: 88bd1ec correctly fixes the jq-missing test case with a safe copy-based isolated PATH — that 🔴 is resolved. The other 🔴 (require_json/exit 2 inside emit_marketplace) is still present and still the blocking issue. The three 🟡 doc/logic gaps and the ⚪ unverified claim are unchanged.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 88bd1ecda7

ℹ️ 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".

Comment thread plugins/claude-ops/skills/plugins/context/sync.md Outdated
Comment thread plugins/claude-ops/skills/plugins/SKILL.md
kyle-sexton and others added 2 commits July 17, 2026 09:52
…missing_from_enabled false positive

Two real bugs found by Codex's PR review, both verified by direct
reproduction before fixing:

- emit_marketplace ran a marketplace's catalog JSON through the fatal
  require_json helper, which exit-2's the whole process. A malformed
  (not merely absent) catalog for one marketplace during --all silently
  killed the entire sweep before any {marketplaces: ...} report was
  emitted for any marketplace, not just the broken one. Fixed to match
  the sibling error branches: report the error inline and return 1.

- missing_from_enabled compared every project/local install machine-wide
  against only the current PROJECT_ROOT's settings files. A plugin
  already enabled in a different repo's own .claude/settings.json (which
  this script never reads) showed up as a false-positive
  missing_from_enabled, which sync's Step 5 could act on from the wrong
  repo. Fixed to only consider ids this invocation can actually verify:
  user scope (global) and the current repo's project/local scope;
  other-repo project/local installs are excluded, not asserted missing.

Both regression-tested (fixture-reproduced red, fixed, green): 17 cases,
0 failed.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…repo risk, stale --force claim, unrendered install_new

Four more findings from Codex's PR review, each independently verified
against current docs, actual CLI behavior, or the script's own output
before fixing:

- sync.md Step 2 gated in-repo update on CLAUDE_PROJECT_DIR being set
  directly, bypassing fleet-state.sh's own git-toplevel fallback — the
  exact case that fallback exists for. It also pre-filtered on
  divergences[] with versionsMatch:false, but divergences[] only holds
  ids with more than one scope record, so a single-scope project/local
  install (the common case) or a multi-scope install stale in every
  scope alike never appeared there and was silently never updated,
  defeating the skill's own documented "primary value path." Fixed to
  always call fleet-state.sh and unconditionally update every
  currentProject:true entry, mirroring Step 3's own pattern.

- converge.md's proposed -s project/-s local commands carried no path
  targeting, but the CLI always operates on the current directory's
  settings — a divergence row can legitimately belong to a different
  repo than the one converge is running from. Fixed to require an
  explicit cd into the row's own projectPath before every project/local
  command.

- "/reload-plugins has no --force flag" was wrong: current docs
  (code.claude.com/docs/en/discover-plugins) document --force
  (Claude Code >=2.1.163) for the case where an updated MCP-server
  plugin's non-deferred tools would otherwise invalidate the prompt
  cache. Fixed everywhere this claim appeared (SKILL.md, sync.md,
  scope-semantics.md, evals.json) to recommend --force only when that
  specific condition applies, not as a blanket default.

- install_new's userConfig value was never actually rendered anywhere
  in this skill's content via ${user_config.install_new} — Claude Code
  only text-substitutes a configured value where that literal
  placeholder appears, so the model had no way to read the configured
  policy at runtime despite sync.md's Step 4 branching on it. Fixed by
  rendering the value in SKILL.md's userConfig section and pointing
  sync.md's Step 4 at that render instead of the option's static prose.

The marketplace-CLI-syntax finding from the same review pass was
verified INCORRECT (claude plugin marketplace update <name> is
documented CLI syntax at plugin-marketplaces.md's "non-interactive
subcommands for scripting" section) and needs no fix, replied to on
GitHub with the citation instead.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 706633bba5

ℹ️ 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".

Comment thread plugins/claude-ops/skills/plugins/context/sync.md
Comment thread plugins/claude-ops/skills/plugins/context/converge.md Outdated
kyle-sexton and others added 2 commits July 17, 2026 10:08
…ssing_from_enabled

Codex's PR review (on the previous fix push) found that Step 5 treated a
plugin with no explicit enabledPlugins entry as a completeness gap
needing claude plugin enable, even when the marketplace entry declares
defaultEnabled:false — a publisher's deliberate opt-in-required default
(plugins-reference.md: takes precedence over plugin.json's own field,
"the user turns it on with claude plugin enable"). No entry anywhere is
the intended state for those ids, not something sync should auto-flip.

Fixed by reading defaultEnabled per catalog plugin from the same
marketplace.json fleet-state.sh already parses for the catalog list,
and excluding those ids from missing_from_enabled. Documented the one
residual gap this doesn't cover (a plugin.json-only defaultEnabled with
no mirrored marketplace-entry override — fleet-state.sh never reads an
installed plugin's own manifest). Regression-tested: confirmed red
before the fix, green after. 18 cases, 0 failed.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…onsolidation strategy

Codex's PR review found that Step 2 chose "uninstall project/local, fall
through to user scope" whenever a user-scope entry existed at all, with
no check that user scope actually held the newer version. divergences[]
only proves scopes disagree, not which one is ahead — a repo pinning
project:0.9.0 against a stale user:0.8.0 would have its project pin
uninstalled in favor of the older user record, regressing the effective
loaded version. That's the opposite of what converge exists to do, and
the one action that can touch committed settings is exactly where a
silent regression is worst.

Fixed: Step 2 now compares scopes[].version (fleet-state.sh already
emits it per scope) to find the actual highest version first, then
picks the strategy from that — user scope only wins when it's genuinely
the newest; otherwise every lagging scope, user included, updates to
match the highest version present.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0d6f1f73f5

ℹ️ 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".

Comment thread plugins/claude-ops/skills/plugins/scripts/fleet-state.sh Outdated
Comment thread plugins/claude-ops/skills/plugins/context/sync.md Outdated
@claude

claude Bot commented Jul 17, 2026

Copy link
Copy Markdown

Re-review after 0d6f1f7feat(claude-ops): plugins skill

  • Gather context — read latest commits and open issues
  • Review fleet-state.sh for case-sensitive path matching issue
  • Review sync.md Step 5 scope restriction gap
  • Check all previously-open findings against current HEAD
  • Verify CLAUDE.md fresh-docs mandate compliance
  • Post findings

What the latest commits fixed

0d6f1f7 — converge stale-user-pin risk ✅ FIXED

converge.md Step 2 now correctly compares scopes[].version to find the actual highest version before choosing a strategy. The old "does a user entry exist → uninstall project" heuristic that could regress a project: 0.9.0 / user: 0.8.0 divergence to the older user pin is replaced with proper semver-comparison-gated logic. The (cd "<scopes[].projectPath>" && ...) wrapping for out-of-cwd project commands is explicit throughout. Both findings from the prior review that drove this commit are resolved.

348dd52defaultEnabled: false auto-enable prevention ✅ FIXED

fleet-state.sh now reads defaultEnabled per catalog entry and excludes those ids from missing_from_enabled. Confirmed: the fix reads from the same $catalog_json already parsed for the catalog list, so no additional file access is needed. Test coverage added and confirmed red/green.


Two new open findings (from Codex review on 0d6f1f7)

🟡 ascii_downcase in the currentProject jq filter is unconditional — bypasses hook::normalize_path's OS gate

fleet-state.sh:252-254

hook::normalize_path (in hook-utils.sh:35-48) is explicitly OS-gated: it only folds case on OSTYPE = msys*/cygwin*/win32, and the comment explains exactly why — on a case-sensitive POSIX filesystem /c/Repo and /c/repo are distinct directories, and folding them collapses a legitimate boundary. On POSIX hosts, hook::normalize_path passes through unchanged.

But the jq filter that computes currentProject applies ascii_downcase to both sides unconditionally:

((.projectPath | gsub("\\\\";"/") | ascii_downcase) == ($cur | ascii_downcase))

$cur (built from hook::normalize_path "$(hook::physical_path "$PROJECT_ROOT")") is the OS-correctly-normalized path — no lowercasing on Linux. But this jq filter re-lowercases it anyway. On a case-sensitive Linux filesystem with two sibling directories that differ only by case (e.g. /work/repo vs /work/Repo), an install record whose projectPath is /work/Repo would be marked currentProject: true when the session is standing in /work/repo, which is a different directory. The consequence: verifiable_ids and missing_from_enabled include the wrong install record, and Step 2 / Step 5 of sync would target the current directory for a plugin belonging to a sibling.

resolve_default_marketplace (line 169-175) avoids this by trying an exact-match first before falling back to the case-folded form — only the currentProject filter uses the all-or-nothing downcase.

The fix: plumb OSTYPE through to jq (or detect Windows once in bash and pass a boolean flag), and apply ascii_downcase only when the flag is true. The existing hook::normalize_path logic already encodes the correct rule — this should mirror it rather than reinvent a looser version.

Fix this →%2C%20pass%20it%20as%20--argjson%20ci%20%22%24ci_compare%22%2C%20and%20replace%20the%20unconditional%20ascii_downcase%20comparison%20with%3A%20if%20%24ci%20then%20((.projectPath%20%7C%20gsub(%22%5C%5C%5C%5C%22%3B%22%2F%22)%20%7C%20ascii_downcase)%20%3D%3D%20(%24cur%20%7C%20ascii_downcase))%20else%20((.projectPath%20%7C%20gsub(%22%5C%5C%5C%5C%22%3B%22%2F%22))%20%3D%3D%20%24cur)%20end.%20Branch%3A%20feat%2Fclaude-ops-plugins-skill&repo=melodic-software/claude-code-plugins)


🟡 sync.md Step 5 iterates all scopes — must restrict to readable scopes

sync.md:107-111

fleet-state.sh correctly restricts missing_from_enabled to verifiable ids (line 283: scope == "user" OR currentProject == true) — so the list of ids is sound. But Step 5's instruction says: "for each remaining id, and for each scope where that id has an install record (from installed[]) but no raw entry in that scope's own enabledPlugins map."

installed[] contains ALL scope records, including currentProject: false records from other repos. Consider: plugin alpha@market1 has a user-scope install (correctly in missing_from_enabled) and a project-scope install in /other/repo (with currentProject: false). Step 5 would:

  1. See the scope: "project", currentProject: false, projectPath: "/other/repo" entry in installed[]
  2. Check whether it has a raw entry in "that scope's own enabledPlugins map" — project_map reflects only the current directory's .claude/settings.json, not /other/repo's, so the answer is false
  3. Conclude it should run claude plugin enable alpha@market1 -s project — which modifies the current directory's settings, not /other/repo's

The fix is one sentence added to Step 5: restrict "for each scope" to only records where scope == "user" OR currentProject == true — the same restriction verifiable_ids already applies. Records with currentProject: false belong to other repos whose settings files were never read; treating their absence from the local project_map as a "gap to fill" is a false positive.

Fix this →


Status of all prior findings

Finding Status
🔴 require_json/exit 2 corrupts --all output ✅ Fixed e5f1b37
🔴 jq-missing test strips bash from PATH ✅ Fixed 88bd1ec
🟡 missing_from_enabled spans all projects ✅ Fixed e5f1b37
🟡 sync.md Step 2 gates on CLAUDE_PROJECT_DIR env var ✅ Fixed 706633b
🟡 sync.md Step 2 divergence gate misses single-scope installs ✅ Fixed 706633b
🟡 converge.md missing cwd context for cross-project commands ✅ Fixed 706633b
/reload-plugins --force claim unverified ✅ Fixed 706633b
🟡 defaultEnabled: false plugins auto-enabled by Step 5 ✅ Fixed 348dd52
🟡 converge could regress to stale user pin ✅ Fixed 0d6f1f7
🟡 ascii_downcase unconditional in currentProject filter Open
🟡 sync.md Step 5 iterates all scopes (including other-repo) Open

Summary

Two 🟡 findings remain. Neither is a merge blocker at the severity of the prior two 🔴s — the case-sensitivity issue only manifests on POSIX systems with case-differing sibling directories (genuinely unusual), and the Step 5 scope-restriction gap is a doc-level fix for the scan already correctly scoping the missing_from_enabled list. Both are real and worth fixing before this skill sees broad use, but the core script logic and the previously-breaking test paths are all solid.

View job run | Branch

…risons

Codex's PR review found that currentProject and default-marketplace
resolution both applied ascii_downcase to path comparisons
unconditionally, mirroring hook::normalize_path's Windows-only case
fold everywhere else calls it. hook::normalize_path itself is already
platform-conditional (case-fold only on msys/cygwin/win32; pass
through unchanged on POSIX, where filesystems are case-sensitive), but
these two inline jq comparisons reimplemented the fold without the
platform check. On a case-sensitive host (e.g. GitHub's ubuntu CI
runner), two genuinely different sibling repos differing only by case
(/work/repo vs /work/Repo) would compare equal, and since sync now
unconditionally updates every currentProject:true entry, a false
match could steer a project-scope mutation at the wrong repo.

Fixed by computing the same $OSTYPE-based case-insensitive flag
hook::normalize_path uses and gating both comparisons on it instead of
folding case unconditionally. Regression-tested: same fixture run
twice, once forcing a POSIX OSTYPE (must not match) and once with this
host's real OSTYPE (must still match — no regression to the existing
Windows behavior). Confirmed red against the unpatched script before
fixing. 19 cases, 0 failed.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Codex's PR review found that even after the missing_from_enabled fix
excluded other-repo project/local installs from the missing set, an id
that legitimately IS missing_from_enabled (e.g. a genuine user-scope
gap) could still carry an unrelated other-repo project/local install
record in installed[]. Step 5's per-scope loop iterated every scope
with an install record for that id, including that unread other-repo
record, and would run claude plugin enable -s project|local for it —
risking a mutation at the current repo or an unread repo, the same
wrong-repo class of bug already fixed in converge.md and
missing_from_enabled itself.

Fixed: Step 5's loop is now explicitly restricted to user scope, or
project/local scope with currentProject:true — the exact same
restriction already applied to missing_from_enabled's computation,
extended to the downstream action that consumes it.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1de4658fe0

ℹ️ 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".

Comment thread plugins/claude-ops/skills/plugins/scripts/fleet-state.sh
Comment thread plugins/claude-ops/skills/plugins/scripts/fleet-state.test.sh Outdated
…ministic

The "no regression" half of the case-sensitivity test asserted the
differently-cased fixture would still match under "this host's real
OSTYPE," which only holds on the case-insensitive Windows dev machine
this was authored on — CI's ubuntu runner has a genuinely
case-sensitive OSTYPE, so the fix (working correctly) produced false
there too, and the test's own hardcoded "true" expectation was wrong
for that platform. Fixed by forcing both halves of the test explicitly
(OSTYPE=linux-gnu / OSTYPE=msys) instead of relying on either host's
ambient value, so the assertions hold identically everywhere. 19
cases, 0 failed, verified on this machine.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6d751174e0

ℹ️ 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".

Comment thread plugins/claude-ops/skills/plugins/scripts/fleet-state.sh
Comment thread plugins/claude-ops/skills/plugins/scripts/fleet-state.sh
@claude

claude Bot commented Jul 17, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 5m 12s —— View job


Re-review after 6d75117feat(claude-ops): plugins skill

  • Gather context — read latest commits and open findings
  • Check fleet-state.sh:273missing_from_install all-scope gap (Codex finding from 1de4658)
  • Check fleet-state.sh:102 — per-plugin shape drift (Codex finding on HEAD)
  • Check fleet-state.sh:352 — missing marketplace arg before shift (Codex finding on HEAD)
  • Verify 6d75117 case-fold test fix is correct
  • Compile full findings table
  • Post final review

What 6d75117 fixed

Case-fold test OS-awareness ✅ FIXED

The test at lines 316–333 now passes two explicit OSTYPE overrides rather than relying on the host's ambient value:

  • OSTYPE="linux-gnu" → asserts currentProject: false (case-sensitive POSIX: differently-cased sibling must NOT match)
  • OSTYPE="msys" → asserts currentProject: true (case-insensitive Windows: must still match, no regression)

This is the right fix and is deterministic across CI (ubuntu) and any dev host. The companion script logic in fleet-state.sh (lines 169–173) correctly gates case_insensitive_os on ${OSTYPE:-}, and both resolve_default_marketplace and the currentProject filter consume the $ci flag rather than applying ascii_downcase unconditionally.


Three open findings

🟡 missing_from_install uses all-scope installed_ids, not user-scope only — fleet-state.sh:272–282

installed_ids (line 273) is [.[].id] | unique from the full installed array, which contains records for ALL scopes — including a project-scope install in the current repo. So a plugin installed only at project scope (not at user scope) IS in installed_ids and therefore NOT in missing_from_install. Sync Step 4 (sync.md:77–85) uses missing_from_install to drive claude plugin install <id> -s user — but a project-only install is invisible to it, so the user-scope gap is never presented regardless of the install_new policy.

This has been open since the Codex review on 1de4658 and has had no response. Whether the fix belongs in the script (redefine missing_from_install as catalog - user_scope_installed - opted_out) or in sync.md (Step 4 should additionally consider ids installed only at non-user scope) is a design call — but the current behavior silently omits a real install gap from the output field sync consumes.

Fix this →


🟡 Per-plugin shape validation only checks top-level type — fleet-state.sh:100–102

The shape check at line 100 validates that .plugins is an object, but not that each plugin entry is an array. If an individual entry drifts to a non-array type, the jq filter at lines 252–270 runs .value[] on it and fails. Since the assignment is in a command substitution (installed=$(... | jq -cs '.')) and the script has set -uo pipefail but not set -e, the jq failure is swallowed: installed becomes empty and the script exits 0 with installed: [] and a spurious missing_from_install. The fail-loud guarantee the comment at line 82 promises doesn't hold below the top-level object check.

The fix is a deeper shape assertion at lines 100–103:

if [[ "$(jq -r 'has("plugins") and (.plugins | type == "object") and
     (.plugins | to_entries | all(.value | type == "array"))' "$INSTALLED_JSON")" != "true" ]]; then

Fix this →


🟡 --marketplace with no name silently infinite-loops — fleet-state.sh:347–353

When invoked as fleet-state.sh --marketplace (no name), $# = 1, $1 = "--marketplace", TARGET = "", and shift 2 fails (bash: "can't shift that many") — returning exit code 1 without modifying $@. Since the script has no set -e, execution continues, the while loop body exits, and the next iteration still sees $1 = "--marketplace". The loop repeats forever. The guard at line 375 (if [[ -z "$TARGET" ]]) is never reached.

The fix: check and exit early inside the --marketplace branch before any shift:

--marketplace)
  MODE="single"
  TARGET="${2:-}"
  if [[ -z "$TARGET" ]]; then
    echo "ERROR: --marketplace requires a name" >&2
    exit 2
  fi
  shift 2
  ;;

This is only triggered by malformed manual invocation (the skill always passes a name), but the infinite-loop failure mode is worse than a clean error exit. Fix this →


Full findings status

Finding Status
🔴 require_json/exit 2 corrupts --all output ✅ Fixed e5f1b37
🔴 jq-missing test strips bash from PATH ✅ Fixed 88bd1ec
🟡 missing_from_enabled spans all projects ✅ Fixed e5f1b37
🟡 sync.md Step 2 gates on CLAUDE_PROJECT_DIR env var ✅ Fixed 706633b
🟡 sync.md Step 2 divergence gate misses single-scope installs ✅ Fixed 706633b
🟡 converge.md missing cwd context for cross-project commands ✅ Fixed 706633b
/reload-plugins --force claim unverified ✅ Fixed 706633b
🟡 defaultEnabled: false plugins auto-enabled by Step 5 ✅ Fixed 348dd52
🟡 converge could regress to stale user pin ✅ Fixed 0d6f1f7
🟡 ascii_downcase unconditional in currentProject filter ✅ Fixed 6b42033
🟡 sync.md Step 5 iterates all scopes (including other-repo) ✅ Fixed 1de4658
🟡 Case-fold test fails on case-sensitive CI host ✅ Fixed 6d75117
🟡 missing_from_install uses all-scope ids, misses user-scope gap Open (from 1de4658 Codex, unanswered)
🟡 Per-plugin shape drift not caught by shape check Open (from HEAD Codex)
🟡 --marketplace with no name infinite-loops Open (from HEAD Codex)

Summary

6d75117 correctly closes the case-fold test OS-awareness finding with a clean explicit-OSTYPE approach. All prior 🔴 and 🟡 findings from the full review cycle are resolved. The remaining three are genuine but none rises to the severity of the earlier blockers — the infinite-loop is only reachable on malformed manual invocation, the shape-drift check is a defensive-robustness gap, and the missing_from_install semantic question is a design clarification. The core script logic, test suite (17/17 cases), and skill documents are solid.
| Branch

@kyle-sexton
kyle-sexton merged commit 180d46f into main Jul 17, 2026
19 checks passed
@kyle-sexton
kyle-sexton deleted the feat/claude-ops-plugins-skill branch July 17, 2026 15:09
kyle-sexton added a commit that referenced this pull request Jul 21, 2026
)

## Summary

Closes the three deferred, independently-corroborated (Codex +
claude[bot]) review findings from PR #248's review cycle against
`plugins/claude-ops/skills/plugins/scripts/fleet-state.sh`. Each was
verified VALID but deferred out of #248 to bound an already-long (6+
round) fix/review loop. All fixed here with TDD regression tests
(red-then-green).

Version bumped `0.15.4` → `0.16.0` (minor: adds a new consumable output
field).

## Fix

**1 — user-scope install completeness (additive, no behavior change to
the existing field).**
`fleet-state.sh` now emits a new field `missing_from_user_install` =
catalog minus *user-scope*-installed minus opted-out, alongside the
untouched all-scope `missing_from_install`. A plugin installed only at
`project`/`local` scope was present all-scope (absent from
`missing_from_install`) and so was never offered a `user`-scope install
by `sync` Step 4 — silently breaking the "usable from any directory"
guarantee for it. `sync.md` Step 4 now keys its user-scope install offer
off the new field. The existing `missing_from_install` field and its
all-scope semantics are unchanged.
Chosen approach: the additive option from the issue (new field), not the
redefinition option — no behavior change to any existing consumer.
Beyond the literal instruction: `SKILL.md`'s report template ("Action
needed") reference was also switched to `missing_from_user_install`, so
the report and Step 4 name the same signal (safe —
`missing_from_user_install` ⊇ `missing_from_install` always, since
user-installed ⊆ all-installed). `scope-semantics.md`'s reference was
deliberately left on `missing_from_install` — it documents the preserved
all-scope field's rename-coverage semantics.

**2 — per-plugin shape validation.**
The shape check validated `.plugins` is an object but not that each
entry is an array. A drifted non-array entry passed the check, then
failed inside the installed-flatten `jq` pipeline in a command
substitution; with `set -uo pipefail` but no `set -e` the failure was
swallowed and the script exited **0** with `installed: []` instead of
failing loud per its stated design. The check now also asserts
`(.plugins | to_entries | all(.value | type == "array"))` and exits 2 on
drift.

**3 — `--marketplace` with no name.**
`fleet-state.sh --marketplace` (no following arg) set `TARGET=""`, then
`shift 2` failed silently (only one positional param left, no `set -e`),
leaving `$1` unchanged so the arg loop re-read `--marketplace` forever;
the post-loop `-z "$TARGET"` guard was never reached. The empty check
now runs inside the `--marketplace` branch before `shift`, exiting 2.
The now-dead post-loop guard was removed.

## Verification

TDD: three regression cases were added and run **against the unfixed
script first** to prove each bug, then the fixes were applied.

Before (unfixed script — the three new cases red, proving each bug):

```
FAIL: user-scope: project-only + never-installed both surface, user-scope install excluded
FAIL: user-scope: opt-out excluded from missing_from_user_install too
FAIL: non-array plugin entry: exit 2
  detail: expected exit 2, got 0            # #2: swallowed failure — exited 0 with installed:[]
FAIL: --marketplace no arg: exit 2, not an infinite loop
  detail: expected exit 2, got 124          # #3: 124 = timeout killed the infinite loop
FAIL: --marketplace no arg: actionable error
```

After (fixed script — full suite green):

```
PASS: --marketplace no arg: exit 2, not an infinite loop
PASS: --marketplace no arg: actionable error

23 cases, 0 failed
```

`shellcheck` (both scripts) and `markdownlint-cli2` (all changed
markdown) report clean:

```
SHELLCHECK CLEAN
Summary: 0 error(s)
```

Closes #254

## Related

- Closes #254 — the tracking issue enumerating all three findings.
- Follow-up from PR #248 (`feat/claude-ops-plugins-skill`), the origin
review cycle where these three were verified VALID and deliberately
deferred.
- #797 — follow-up: `FLEET_STATE_HOOK_UTILS`'s test-only override isn't
gated to test contexts at runtime. Pre-existing (unchanged by this PR),
out of scope here — filed post-green per the lane's review-deferral
rule.

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant