source-control/setup: uninstall guidance omits --keep-data (destroys babysit worktrees and lease state)

[kyle-sexton]

From a /plugin-quality:audit run (packet source-control-config/20260823T014416Z), source-control 0.54.16. 8 findings — 2 High, 3 Medium, 3 Low. claude plugin validate passes. Cross-plugin findings from the same run: #3128. Prior hardening pass: #912.

How this surfaced. The tracked .claude/source-control.md team layer in melodic-software/.github was read, audited against reference/config-resolution.md, and modified. No source-control skill was executed — this is a config-surface audit.

F3 (High) — uninstall guidance omits --keep-data

setup/SKILL.md:245-259 prescribes claude plugin uninstall … then reinstall and warns at length about losing pluginConfigs — but never mentions --keep-data. Per the docs, uninstalling from the last remaining scope also deletes the plugin's ${CLAUDE_PLUGIN_DATA} directory, where this plugin stores babysit worktrees and lane/lease state.

grep -rn 'keep-data' across the whole plugin: zero hits.

One-token fix, and the highest damage-per-character finding in the set. This was outside the audit's original dispatch scope and is included because scope was locked to all findings.

Caveat carried with it: F4 below undercuts the premise this path rests on.

F1 (High) — overlay-ignore guard reachability → tracked in #3128 (S2)

The trigger-condition defect is shared with work-items and is filed once in #3128 to avoid sending the same fix request twice. Summary only: the guard is conditioned on the overlay already existing (setup/SKILL.md:77-78, Absent → INFO, which is the common case), so it cannot fire in the window that creates the exposure. source-control's probe itself is correct — it is the two-probe form work-items lacks. Cheapest fix, per #3128, is decoupling the rule probe from the file probe.

Correction on the record: an earlier framing during this audit claimed the plugin does not carry the gitignore requirement. That was wrong and is withdrawn. config-resolution.md:310 (## Consumer .gitignore, prescribing .claude/*.local.*), :321 (## Failure modes), and apply-convention.md step 6 all carry it correctly.

Remaining findings

• F5 (Medium) — the userConfig/tracked split is doc-correct (project pluginConfigs entries are ignored by Claude Code, so tracked markdown is the only carrier for repo-scoped policy), but the wrong-surface mistake fails silently in the motivated direction: a babysit_* userConfig key written as an H2 is inert with no report row. babysit_merge_method vs babysit_loop_merge is never disambiguated, though babysit_default_tier is.
• F2 (Medium) — the absent-key fall-through chain includes commit-msg hooks but not CI workflows. The consumer's pr-title.yml enforces Conventional Commits on a squash title that becomes the commit subject, while the config surface was silent on subject_pattern — correct only by coincidence of the bundled default. The contract reasons carefully about exactly this divergence for convention_source rung 2 and applies none of that care to the plain absent-key case. Cheapest surface is a third probe in setup check's existing "Neutral-SSOT drift probes" block; the plugin already names amannn/action-semantic-pull-request at pull-request/SKILL.md:41.
• F4 (Medium) — --config "ignored once installed" is undocumented (exhaustive grep -F -- '--config' over the 108305-byte page: two hits, neither says this) and unstamped, yet it is the sole premise for F3's destructive path.
• F6/F7/F8 (Low) — the density problem is ordering: the loop-lane section runs 168 lines (51% of the document) and the two rules governing both key families land at lines 250 and 271, after it. Plus unmarked eval fixtures shipped as live-looking config, and a userConfig count that has drifted — prose says twenty-nine, the manifest has 35 (independently re-verified).

Remediations, cheapest first

1. Add --keep-data to the uninstall guidance (F3).
2. Fix the userConfig count and mark the eval fixtures (F8).
3. Move the two cross-family rules ahead of the loop-lane section (F6).
4. Report a wrong-surface babysit_* key instead of silently ignoring it (F5).
5. Add a CI-workflow probe to setup check's drift block (F2).
6. Ignore-rule trigger decoupling — see Cross-plugin: hook-utils skip latch silences 9 formatters, overlay-ignore guard unreachable, plugin-quality seal premise false #3128.

Caveats

No source-control skill was executed — contract-read plus static repo state only. The auditor's scratch-repo reproduction of the F1 mechanism was blocked by the sibling guardrails PreToolUse hook and it did not route around it, so that mechanism is reasoned from source, not observed. Every check behavior cited is prose an LLM must follow, not a tested code path — temper "setup check already covers it" accordingly. Doc citations are rung-1 curl, fetched 2026-08-23, with byte counts and line numbers; slugs checked against llms.txt.

[kyle-sexton]

Follow-up from the same audit: the prescribed ignore glob collides with another convention from this same marketplace.

reference/config-resolution.md:310 (§"Consumer .gitignore") prescribes:

.claude/*.local.*

That is correct for the source-control overlay. But .claude/cloud-bootstrap.sh — synced into fleet repos by distribution/sync-manifest.yml — documents a different kind of .claude/*.local.* file:

Repo-specific work does not belong here: a repo enriches through its own .claude/cloud-bootstrap.local.sh (run below, never synced) […]

and invokes it directly at line 168:

if [[ -f .claude/cloud-bootstrap.local.sh ]]; then
  if bash .claude/cloud-bootstrap.local.sh >&2; then

cloud-bootstrap.local.sh is repo-owned and team-shared — "a repo enriches through its own" — not a per-operator overlay. But it matches .claude/*.local.* by name, so a consumer who follows the prescription literally silently gitignores a file their cloud sessions depend on. The failure mode is the bad kind: the author commits, the commit succeeds, the file never lands, and the enrichment quietly stops running on every other checkout.

Reproduced in melodic-software/.github. Applying the prescribed glob made git status stop showing cloud-bootstrap.local.sh entirely. Fixed consumer-side with an explicit re-include:

.claude/*.local.*
!.claude/cloud-bootstrap.local.sh

Verified by creating both files and reading git status --porcelain --ignored: cloud-bootstrap.local.sh reports ??, source-control.local.md reports !!. (git check-ignore's exit code is ambiguous when the last matching pattern is a negation, so it is not a reliable probe here.)

Why this belongs upstream rather than in each consumer: both conventions ship from this marketplace, so every repo that adopts both inherits the collision, and nothing warns them. Options, roughly cheapest first:

1. Narrow the prescription to the specific overlays it means — .claude/settings.local.json, .claude/source-control.local.md — rather than a name-shaped glob.
2. Keep the glob and ship the negation with it, since the marketplace knows about cloud-bootstrap.local.sh.
3. Rename one side so the two file classes are distinguishable by name (*.local.* for per-operator, something else for repo-owned extension points).

Option 1 or 2 is a documentation change; option 3 is a breaking rename and probably not worth it.

This was found after the audit issues were filed, so it is not in the packet for this issue — the collision only became visible once main brought cloud-bootstrap.sh into the consumer repo mid-session.

---

Generated by Claude Code

[kyle-sexton]

[kyle-sexton]

This was generated by AI during triage.

Verification: Confirmed by direct inspection. plugins/source-control/skills/setup/SKILL.md's babysit-config section documents two reconfigure paths and separately warns against uninstalling to reconfigure ("uninstalling drops this plugin's entire stored pluginConfigs entry, resetting every option ... to its manifest default"), but a full-file grep -rn 'keep-data' over plugins/source-control/ returns zero hits. Current official Claude Code docs (plugins-reference.md, fetched during this triage pass) confirm: by default, uninstalling a plugin from its last remaining scope also deletes its ${CLAUDE_PLUGIN_DATA} directory, and --keep-data is the documented flag that preserves it. This plugin stores babysit state under that directory (${CLAUDE_PLUGIN_DATA}/worktrees — the default babysit_worktree_root — and ${CLAUDE_PLUGIN_DATA}/state/babysit-prs, per skills/babysit-prs/SKILL.md's config table). The claim in the issue body holds.

Note: F4 from the same audit run (whether --config is honored against an already-installed plugin) is a separate, undischarged concern that this brief does not resolve — it's out of scope here and tracked independently.

Agent Brief

Type: Bug
Summary: The setup skill's uninstall/reconfigure guidance never mentions claude plugin uninstall's --keep-data flag, so an operator who follows it and later uninstalls this plugin for any reason has no warning that doing so (without --keep-data) silently destroys the plugin's persistent babysit worktrees and lane/lease state.

Current behavior:
The setup skill documents two sanctioned paths for reconfiguring babysit options (interactive /plugin configure, headless claude plugin install ... --config) and separately warns operators not to uninstall purely to reconfigure, citing loss of the stored pluginConfigs entry. Nowhere in that guidance — or anywhere else in the plugin's documentation — is --keep-data named. By default, uninstalling a plugin from its last remaining scope deletes its persistent data directory, where this plugin stores its babysit worktree root and its lease/state directory. An operator who has a legitimate reason to uninstall (troubleshooting, forced reconfiguration, changing scopes) and follows only the documented guidance has no indication that in-flight babysit worktrees and lease state will be destroyed unless they separately know about and pass --keep-data.

Desired behavior:
Wherever the setup skill's documentation discusses or could lead an operator toward uninstalling this plugin, it explicitly names --keep-data, states what the flag does, and states plainly what this plugin specifically loses if it's omitted (babysit worktrees and lane/lease state — not just a generic "data directory" mention). The existing guidance discouraging uninstall-to-reconfigure stays intact; this is an added safety caveat, not a change in the recommended workflow.

Key interfaces:

• The setup skill's uninstall/reconfigure guidance (prose) — the section warning against uninstalling to reconfigure babysit options.
• Persistent data directory semantics for this plugin — restate accurately: default uninstall from the last remaining scope deletes it; --keep-data preserves it; this plugin's babysit worktree root and state directory live there.

Acceptance criteria:

• The setup skill's documentation explicitly names --keep-data in the vicinity of any guidance that discusses or could lead to uninstalling this plugin.
• The documentation states in plain terms what this plugin specifically loses if --keep-data is omitted (babysit worktrees, lane/lease state), not a generic data-directory mention.
• The existing guidance against uninstalling purely to reconfigure remains intact and is not weakened or contradicted by the addition.
• No other command guidance in the plugin's setup documentation changes beyond what's needed to add this caveat.

Out of scope:

• Whether --config is honored against an already-installed plugin (F4 from the same audit — a separate, independently tracked concern).
• The userConfig count discrepancy, CI-workflow drift probe, or loop-lane section reordering (other findings from the same audit run — not this item).
• Changing the babysit worktree root or state directory defaults themselves.

[kyle-sexton]

Lease takeover

This issue carried a work-item lease held by instance MELO-LAP-001-triage-202608230449, acquired at 2026-08-23T15:16:23Z with a one-hour TTL. That lease expired at 2026-08-23T16:16:23Z and was never renewed. As of this comment the issue has had no activity since 2026-08-23T15:20:58Z, and no newer lease from any other instance has been posted.

Instance melo-lap-001-worker-202608231812 is taking over the lease on this issue. A fresh lease marker in the same machine-readable format follows this comment, stamped with the new instance and the same one-hour TTL. The assignee is unchanged.

This is an announced takeover of an expired lease, not a contested claim. If the prior instance is still live and intends to keep this item, it should say so on this issue and the takeover will be released.

[kyle-sexton]

[kyle-sexton]