Skip to content

claude-ops:plugins sync — 2 IMPORTANT + 3 SUGGESTION findings (marketplace refresh CLI bug, missing version-capture, TOCTOU catalog race, PROJECT_ROOT fallback, progressive-disclosure gap) #1764

Description

@kyle-sexton

Source: plugin-quality audit of claude-ops:plugins skill (sync action), triggered after a
real /claude-ops:plugins sync run. Full evidence packet, contract, and independent review pass
retained locally; summarized here.

Static gates already pass: claude plugin validate . PASS; skill-quality:check-skill.sh plugins
PASS, 0 errors/warnings. Findings below are design/doc-completeness gaps found on top of that.

F1 (IMPORTANT) — Step 1 marketplace refresh claims "self-heals"; the CLI call it issues has a known, open, unresolved upstream bug, with zero documented failure/fallback behavior

  • context/sync.md:25-26: "Self-heals a stale or corrupt local clone by re-fetching from the
    marketplace's registered source (per Brief Decision 4 — no manual re-clone or cache surgery)."
  • Observed: claude plugin marketplace update melodic-software failed on Windows with:
    Failed to clone marketplace repository: fatal: destination path '...\plugins\marketplaces\melodic-software' already exists and is not an empty directory.
  • This exact error string matches a known, currently open upstream bug:
    anthropics/claude-code#76129 — its
    diagnosis (paraphrased, not verbatim): the refresh flow appears to always attempt a fresh
    git clone into an existing marketplace directory, rather than fetching/pulling the existing
    clone. Reported on macOS there, reproduced here on Windows — cross-platform, not
    Windows-specific.
  • Ruled out the separate, closed-not-planned Windows EPERM .bak-rename-loop variant
    (#46830) — no .bak/staging leftover
    found, so this was a transient, recoverable failure, not a permanent wedge.
  • Per the official docs (plugin-marketplaces,
    "Background auto-updates"), the documented design is pull-first, reclone-only-as-fallback — for
    both background and manual paths. Our manual invocation instead hit a raw clone failure against
    a still-present, non-empty directory, meaning the fallback's own delete-before-reclone step
    didn't complete (or didn't run) before the clone attempt.
  • context/sync.md has explicit inline-failure handling for all-mode per-marketplace failures
    (lines 27-28) and Step 3 per-plugin update failures (lines 72-73) — but Step 1 in
    single/default-marketplace mode has no stated behavior on non-zero exit at all
    . Nothing tells
    a session whether a failed refresh is fatal-and-stop or a silent no-op.
  • This is recurring, not one-off: every sync invocation on an affected machine without an
    explicit all argument hits the same undocumented path.

Candidate remediation (cheapest → most ambitious):

  1. Add explicit failure-handling prose to sync.md Step 1 mirroring Step 3's and all mode's
    existing pattern: on non-zero exit, report inline under "Action needed" rather than treating it
    as fatal-and-stop or silently absorbed. Cite #76129; note a git fetch + compare inside the
    marketplace's installLocation is a safe, read-only diagnostic (not a sanctioned mutation
    workaround) when this hits.
  2. Have fleet-state.sh independently surface staleness (e.g. git rev-parse HEAD vs a cheap
    git ls-remote of the registered source) so sync's report can say "marketplace N commits
    behind" even when the mutating refresh CLI path is broken.
  3. Track the upstream issue and gate any "self-heals" language on a fixed Claude Code version;
    until then, downgrade to "attempts to refresh; known to fail on some checkouts, see #76129."

F3 (IMPORTANT) — SKILL.md's report format requires <old> → <new> per updated plugin; sync.md's algorithm never instructs capturing either value

  • SKILL.md:97 mandates: Updated: <N> plugin(s) — <id>@<marketplace>: <old> → <new> (only when N > 0).
  • sync.md Steps 2, 3, and 6 — the steps that actually run claude plugin update <id> — never
    instruct capturing that id's pre-update version, nor re-reading its post-update version.
    fleet-state.sh's installed[].version field carries exactly the pre-update value and is
    already re-read "immediately before each mutating step" per sync.md's own Concurrency section
    (lines 8-14) — but only framed as a race-safety measure, never as the source for the report's
    <old> value, and nothing instructs re-reading afterward for <new>.
  • Note: sync.md Steps 2-3 are explicit and correct that no separate drift-detection signal is
    needed (unconditional claude plugin update <id> everywhere, CLI reports the no-op) — that part
    is a deliberate, reasoned design choice, not a gap. The gap is specifically the missing
    old/new-version capture needed to honestly fill the report line, not drift detection itself.
  • Propagates to audit too (SKILL.md:81-84): a prediction still needs a "would go from X to Y"
    pair, and the same gap applies there, unverified independently.

Candidate remediation (cheapest → most ambitious):

  1. Unverified, flag to confirm empirically first: check whether claude plugin update <id>'s
    own stdout already reports the old/new version pair — if so, sync.md Steps 2/3 just need one
    sentence instructing the runner to capture and use that line.
  2. Re-run fleet-state.sh once after the full Step 2+3 sweep and diff installed[].version per id
    against the pre-sweep snapshot the Concurrency section already has the runner capture for race
    detection — reuses data already being read.
  3. Extend fleet-state.sh to optionally accept a "before" snapshot argument and emit a
    version-diff summary itself, making report-filling deterministic and script-owned.

F2 (SUGGESTION) — Catalog-content TOCTOU race between two separate reads; not a fleet-state.sh enumeration bug

  • A 60-vs-61 plugin count discrepancy between two reads of the marketplace catalog during one
    session traced to a genuine TOCTOU race on catalog content (a refresh landed between reads —
    known_marketplaces.json's lastUpdated moved forward, matching the marketplace.json file
    mtime), not a bug in fleet-state.sh's enumeration (catalog=$(jq -c '[.plugins[]?.name // empty] | unique' ...)
    is a faithful 1:1 map, re-verified live with zero duplicate names).
  • This is the same class of race context/gotchas.md:42-49 ("Concurrency / TOCTOU") already
    documents for installed/enabled state, just not currently scoped to catalog content too.
  • Root mechanism of the mid-session refresh (background autoUpdate vs. a concurrent session's
    manual update) is unresolved — plausibly autoUpdate, not confirmed.
  • Latent sub-finding, same root cause investigation: fleet-state.sh's jq ... | unique would
    silently collapse two catalog entries sharing an identical .name with no warning. Not observed
    in practice (zero duplicates today), but worth a guard.

Candidate remediation (cheapest → most ambitious):

  1. Extend gotchas.md's "Concurrency / TOCTOU" section to state explicitly that catalog content
    itself (not just installed/enabled state) can shift between separate reads within one session,
    and that diffing fleet-state.sh's catalog count against a separately-read raw
    marketplace.json is not a valid staleness check for that reason.
  2. Have fleet-state.sh stamp its output with the catalog file's mtime or a content hash so a
    later re-read can detect "the catalog changed since I looked" deterministically.
  3. Have the unique step warn (not silently collapse) on an actual duplicate .name collision.

F4 (SUGGESTION, latent) — fleet-state.sh's PROJECT_ROOT self-aliases to bare $PWD outside a git repo when CLAUDE_PROJECT_DIR is unset

PROJECT_ROOT="${CLAUDE_PROJECT_DIR:-}"
if [[ -z "$PROJECT_ROOT" ]]; then
  PROJECT_ROOT=$(git rev-parse --show-toplevel 2>/dev/null | tr -d '\r')
fi
[[ -n "$PROJECT_ROOT" ]] || PROJECT_ROOT="$PWD"

(fleet-state.sh:201-205)

  • When cwd isn't a git repo and the env var is unset, PROJECT_ROOT falls through to bare $PWD
    unconditionally, for any non-repo cwd — not just $HOME.
  • Harmless today: the "project" settings read this produces is byte-identical to the user-scope
    settings file in that case, so the effective merged map is idempotent and no plugin has an
    actual project/local-scope install record with projectPath equal to that path, so no phantom
    currentProject: true gets manufactured either.
  • Becomes actively misleading once converge.md's own documented "V1 scope" gap (lines 20-25,
    exposing each scope's raw per-scope map) is fixed — a "project" raw map that's silently the user
    map for every non-repo invocation stops being inert once something reads it as real project
    signal.

Candidate remediation (cheapest → most ambitious):

  1. Only treat PROJECT_ROOT as real project context when it resolved from CLAUDE_PROJECT_DIR or
    a genuine git toplevel; otherwise keep an explicit "no project" state instead of falling
    through to bare $PWD.
  2. Add an explicit has_project_context boolean alongside PROJECT_ROOT so downstream logic
    (including future converge raw-per-scope-map work) can distinguish "no real project" from
    "project == cwd."

F5 (SUGGESTION) — SKILL.md's router-table summary is detailed enough that a session can skip opening context/sync.md entirely

  • Directly observed this session: the sync run was executed by inferring steps from SKILL.md's
    action-router row rather than opening context/sync.md.
  • SKILL.md's router-table Description column (line 47) already spells out "Marketplace refresh →
    in-repo update → user-scope update sweep → install new per policy → enabledPlugins completeness
    → report" — detailed enough to read as a complete algorithm summary rather than a load-when
    pointer, per the skill-audit lens's progressive-disclosure check.
  • This is why F1's Step-1 failure-handling gap and F3's report-format gap went unnoticed live —
    the executing session never read sync.md closely enough to hit either gap; both only surfaced
    under this audit's side-by-side read.

Candidate remediation (cheapest → most ambitious):

  1. Reword the router table's Description column to name categories only (e.g. "marketplace,
    install, and enable-state maintenance — see linked detail"), removing the six-clause chain that
    currently reads as algorithm-complete.
  2. Add one imperative sentence above the table: "Read the linked detail file before executing any
    action — this table is an index, not a substitute."

Blindspots / unverified claims (carried forward honestly)

  • Step 2 (project/local-scope update sweep, "the primary value path" per sync.md:29) got zero
    empirical exercise in the triggering session — cwd was non-repo, so every installed[] record's
    currentProject was null. Every claim about Step 2's real-world correctness is doc-only.
  • audit shares sync.md's algorithm verbatim (SKILL.md:81-84) — F1 and F3 both propagate to it
    identically, never independently exercised.
  • converge was not exercised at all this session — its Step 2 strategy-selection, Step 3
    confirm-gating, and Step 5 diff-surfacing remain doc-only claims from a prior audit pass.
  • The eval suite's 6 cases cover triggering/routing/policy behavior but exercise neither F1 (a
    Step 1 CLI failure) nor F3 (the report's old/new-version capture requirement) — regressions or
    fixes to either would go uncaught.
  • F2's root-cause mechanism (which refresh path landed mid-session) is unresolved — background
    autoUpdate vs. a concurrent session's manual update both remain plausible.
  • The official docs' "Background auto-updates" section describes background refresh's
    pull-then-reclone-fallback explicitly and says manual updates "still pull with your
    credentials," without fully specifying whether the manual path shares the exact same
    delete-then-reclone fallback implementation — parity is inferred from phrasing and from the
    observed symptom matching #76129, not a direct doc statement.
  • F3's remediation option 1 (whether claude plugin update's stdout already reports an old/new
    version pair) is unverified — flagged as a candidate to confirm, not asserted fact.
  • Whether other claude-ops (or third-party) skills share fleet-state.sh-style marketplace
    refresh calls and would inherit F1's gap identically was not checked.

Filed via plugin-quality:audit after independent adversarial review (all 5 findings PASS, 3
citation/attribution corrections applied pre-filing). Full evidence packet, locked contract, and
review transcript retained locally on the reporting machine, available on request.

Metadata

Metadata

Assignees

Labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions