Skip to content

fix(source-control): name --keep-data in setup's uninstall guidance (0.55.6) - #3211

Merged
kyle-sexton merged 1 commit into
mainfrom
fix/3131-keep-data-uninstall-guidance
Aug 23, 2026
Merged

fix(source-control): name --keep-data in setup's uninstall guidance (0.55.6)#3211
kyle-sexton merged 1 commit into
mainfrom
fix/3131-keep-data-uninstall-guidance

Conversation

@kyle-sexton

@kyle-sexton kyle-sexton commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

Closes #3131

Summary

/source-control:setup's babysit-config section warned operators not to uninstall the plugin in order to reconfigure it, citing the pluginConfigs entry that an uninstall drops — and stopped there. --keep-data appeared nowhere in the plugin (grep -rn 'keep-data' plugins/source-control/ returned zero hits before this change).

That left a real gap for the operator who uninstalls for one of the other legitimate reasons — troubleshooting, changing scopes, reinstalling a version. Uninstalling from the last remaining scope deletes ${CLAUDE_PLUGIN_DATA} by default, and this plugin keeps durable state there.

Fix

One paragraph added to skills/setup/SKILL.md's "Babysit config" section, after the reconfigure bullets. It names the flag, says what the directory holds, and — the part that took the most care — states the resolution rung each worktree root must fall through before it lands there.

What the directory actually holds, verified in-tree rather than assumed:

Path Contents Relocatable?
${CLAUDE_PLUGIN_DATA}/state/babysit-prs queue state (babysit_state.py, queue-state.json), worker leases (manage_babysit_lease.py), feedback ledger (manage_feedback_ledger.py) No userConfig key relocates it
${CLAUDE_PLUGIN_DATA}/worktrees babysit worktrees; /source-control:worktree trees Only at the last resolution rung — see below

"Left unset" is necessary but not sufficient for worktree_root. scripts/worktree-create.sh:20-32 resolves a root over five rungs, and the plugin option is rung 3, below the target repository's melodic.worktreeroot git config at rung 2. So /source-control:worktree create lands in the data directory only when neither resolves. babysit_worktree_root is the simpler case and does fall back there whenever it is unset. Stating only "unset" would have over-warned the operator who uses the recommended per-repo git key, and the paragraph now distinguishes the two.

The emphasis is on the losses that are actually irrecoverable. Babysit's own worktrees are ephemeral scratch that rebuild from GitHub; the state directory and a /source-control:worktree tree holding uncommitted work are not. An earlier draft had this backwards.

Follows docs/conventions/plugin-data-report-keying/README.md Rule 4 ("state the uninstall fragility where the artifact is the only copy").

Verification

Gate Result
claude plugin validate plugins/source-control Validation passed
scripts/check-changed-skills.sh (setup) PASS — 0 errors, 1 pre-existing soft-target warning
scripts/check-changelog-parity.sh --check / --check-bump / --check-order / --check-preserved PASS
scripts/sync-plugin-options-docs.py --check up to date
scripts/check-skill-count-claims.sh --check PASS
scripts/check-skill-leaf-names.sh --check PASS
scripts/check-cross-plugin-source-drift.sh --check PASS
markdownlint-cli2 (both changed markdown files) 0 issues
typos clean

Doc claims re-fetched at rung 1, not taken from a repo snapshot. curl https://code.claude.com/docs/en/plugins-reference.md on 2026-08-23, 108305 bytes: "By default, uninstalling from the last remaining scope also deletes the plugin's ${CLAUDE_PLUGIN_DATA} directory. Use --keep-data to preserve it", and the plugin uninstall flag table carries --keep-data spelled exactly that way.

Rebased twice onto a moving main; now at 0.55.6. main landed #3108 during this work, which de-slopped every source-control instruction surface under the repo's zero-tolerance em dash policy and took 0.55.4; the worktree-claim change from #3104 / #2882 then took 0.55.5. The added prose is written em dash free to match the rewritten file it lands in (grep -c '—' over the changed SKILL.md returns 0). Left unrebased, the version collision would have failed check-changelog-parity.sh and the prose would have silently reintroduced the marks #3108 had just removed.

Independent review. A fresh-context reviewer checked the diff against the issue's acceptance criteria with the author's rationale withheld. It raised one blocker (the worktree_root conditional was false in both directions — it over-warned past melodic.worktreeroot and under-warned by omitting /worktree create trees from the consequence) and two should-fixes (state described as "lane and lease state", which misattributes loop-lane telemetry that actually lives in a GitHub tracking issue and survives an uninstall; and a "neither has another copy" claim that contradicted the plugin's own reference/worktrees.md). All three are fixed above; the reviewer's nits on length and antecedent are applied too.

Related


Generated by Claude Code

@claude

claude Bot commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 42s —— View job


Security review — /review:security-review

  • Gather context (PR diff, description, changed files)
  • Run /review:security-review per instructions
  • Post findings

Skip gate: no security-relevant surface. This PR touches only three files:

  • plugins/source-control/.claude-plugin/plugin.json — version bump (0.55.4 → 0.55.5)
  • plugins/source-control/CHANGELOG.md — changelog entry
  • plugins/source-control/skills/setup/SKILL.md — one added prose paragraph documenting claude plugin uninstall --keep-data in the babysit-config guidance

There is no executable code, no GitHub Actions workflow, no auth/authz logic, no injection surface, and no secret/token handling in this diff — it's a documentation-only change. No security findings to report.

@claude

claude Bot commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 1m 54s —— View job


Claude's Review

Tasks

  • Gather context (PR diff, changed files, CLAUDE.md)
  • Understand the request (invoke /review:code-review)
  • Read changed files in full
  • Verify technical claims against source (worktree-create.sh resolution order, babysit_worktree_root fallback, reference/worktrees.md, plugin-data-report-keying Rule 4, README generated block)
  • Post findings as inline PR comments (none needed — no issues found)
  • Post summary review comment

Summary

This is a documentation-only fix (SKILL.md + CHANGELOG.md + version bump), and it holds up well under verification. I independently checked every factual claim in the added paragraph against the code it describes rather than taking the PR body at its word:

  • Resolution order (worktree-create.sh:20-32) confirms melodic.worktreeroot (rung 2) outranks the worktree_root plugin option (rung 3), with the plugin data dir at rung 4 — matches the new paragraph's claim exactly.
  • babysit_worktree_root resolution table in skills/babysit-prs/SKILL.md:306 confirms it falls straight back to ${CLAUDE_PLUGIN_DATA}/worktrees with no intervening git-config rung, correctly distinguished from the more conditional worktree_root case in the new text.
  • Irrecoverability framing is accurate: reference/worktrees.md:3 states babysit worktrees are "ephemeral scratch, not durable state," while nothing in the babysit-prs reference set describes the queue state, worker leases, or feedback ledger as reconstructible from GitHub — so scoping the "no other copy" warning to the state dir and /worktree trees (not babysit's own worktrees) is correct.
  • Rule 4 citation matches docs/conventions/plugin-data-report-keying/README.md:153-158 verbatim in spirit.
  • README scope-out is correct: the parallel warning at README.md:337-338 sits inside the sync-plugin-options-docs.py generated block, so leaving it alone here (rather than hand-editing a generated section) is the right call.
  • House style: no em dashes in the added prose, consistent with the plugin's zero-tolerance policy noted in the 0.55.4 changelog entry.
  • CHANGELOG/version bump: 0.55.5 entry format matches prior entries; plugin.json version bump is consistent.

No bugs, factual errors, or style violations found in the diff. One very minor observation, not a blocker: the PR title metadata (... (0.55.4)) is stale relative to the actual commit/CHANGELOG version (0.55.5) — likely just an artifact of the version bump happening after the PR was opened, not a diff issue.

Nice work tracing the multi-rung resolution logic precisely rather than over-warning — the distinction between babysit_worktree_root's simple fallback and worktree_root's conditional one is exactly the kind of detail that's easy to get wrong in prose.

@github-actions

github-actions Bot commented Aug 23, 2026

Copy link
Copy Markdown

Last security-reviewed head: 65090331edaf9cfa0a2c64851407bf0293f57441. On the next push, the relevance gate compares only the commits since this SHA; delete this comment to force a full re-review.

@github-actions

github-actions Bot commented Aug 23, 2026

Copy link
Copy Markdown

Claude has reviewed this PR 2 times. The lane skips further automatic reviews after 5; deleting this comment resets the count.

@kyle-sexton kyle-sexton changed the title fix(source-control): name --keep-data in setup's uninstall guidance (0.55.4) fix(source-control): name --keep-data in setup's uninstall guidance (0.55.5) Aug 23, 2026
…55.6)

Rebase the --keep-data uninstall guidance onto current main as 0.55.6 so it
does not collide with the 0.55.5 worktree-claim landing from #3104 / #2882.

Closes #3131
@kyle-sexton
kyle-sexton force-pushed the fix/3131-keep-data-uninstall-guidance branch from 33f77fd to a78a2ec Compare August 23, 2026 20:11
@kyle-sexton kyle-sexton reopened this Aug 23, 2026
@kyle-sexton kyle-sexton changed the title fix(source-control): name --keep-data in setup's uninstall guidance (0.55.5) fix(source-control): name --keep-data in setup's uninstall guidance (0.55.6) Aug 23, 2026
@claude

claude Bot commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 57s —— View job


Security review — /review:security-review

  • Gather context (PR diff, description, changed files)
  • Run /review:security-review per instructions
  • Post findings

Skip gate: no security-relevant surface. Confirmed via gh pr diff 3211, this update only touches:

  • plugins/source-control/.claude-plugin/plugin.json — version bump (0.55.5 → 0.55.6)
  • plugins/source-control/CHANGELOG.md — new [0.55.6] changelog entry
  • plugins/source-control/skills/setup/SKILL.md — the same added prose paragraph (documenting claude plugin uninstall --keep-data in the babysit-config guidance), unchanged since the prior review

No executable code, no GitHub Actions workflow, no auth/authz logic, no injection surface, and no secret/token handling — this diff is documentation-only (prose, a changelog entry, and a version bump). The prior security review at 33f77fde reached the same conclusion for the identical SKILL.md prose; the delta since then is only the CHANGELOG entry and version bump, neither of which is security-relevant. No security findings to report.

@claude

claude Bot commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

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


Claude's Review

Tasks

  • Gather context (PR diff, description, changed files)
  • Invoke /review:code-review and follow its instructions
  • Read changed files in full
  • Post findings as inline PR comments (none — no new content to review)
  • Post summary review comment

Summary

Skip gate: this head already has a successful review from this lane that still applies. I diffed the current head (6509033, plugin.json now at 0.55.6) against the previously-reviewed SHA (33f77fde, reviewed at 0.55.5):

git diff 33f77fde9456a1c6c71a23f4578e0bc604c8af76..HEAD -- plugins/source-control/skills/setup/SKILL.md

returns no output — the added paragraph in skills/setup/SKILL.md is byte-identical to what a prior pass on this same PR already reviewed and approved. The only changes since then are:

I independently re-verified the core technical claim rather than taking that on faith: scripts/worktree-create.sh:18-29 confirms the root-resolution order the added paragraph describes — melodic.worktreeroot git config (rung 2) outranks the worktree_root plugin option (rung 3), with the plugin data directory as the last rung (4) before refusal. This matches the SKILL.md text exactly.

No findings. Nothing in this push changes the reviewable surface beyond a mechanical version renumber from rebasing.

@kyle-sexton

Copy link
Copy Markdown
Contributor Author

Superseded by #3216. This PR conflicts with origin/main after source-control 0.55.5 landed (#3104). #3216 is the same prose fix rebased onto current main at 0.55.6.

@kyle-sexton
kyle-sexton merged commit 9a09ab0 into main Aug 23, 2026
50 of 52 checks passed
@kyle-sexton
kyle-sexton deleted the fix/3131-keep-data-uninstall-guidance branch August 23, 2026 21:58
kyle-sexton added a commit that referenced this pull request Aug 23, 2026
…0.55.8) (#3240)

Closes #3212
Closes #3238

## Summary

`claude plugin uninstall` deletes the target plugin's
`${CLAUDE_PLUGIN_DATA}` directory whenever the scope being removed is
that plugin's last remaining one, unless `--keep-data` is passed. Five
sites in this repo issued that uninstall without the flag, and none of
them intends data removal.

#3212 is the highest-blast-radius site: `reap-project-plugin-records.sh`
runs non-interactively over every plugin id holding a project-scope
record for a worktree `/source-control:worktree cleanup` is tearing
down. For any plugin whose project-scope record was its last remaining
scope, that call deleted *that* plugin's data directory as a side effect
of reaping a stale *record*.

#3238 is the same class at the four remaining executable or copy-paste
sites outside that helper, found by the independent review on the #3212
fix.

## Fix

`--keep-data` is now unconditional on every one of those calls. There is
no branch in any of them where omitting it is correct.

| Site | Change |
|---|---|
| `plugins/source-control/scripts/reap-project-plugin-records.sh` |
`claude plugin uninstall "$id" -s project --keep-data`. Header documents
the flag as a third safety property next to never-`-s user` /
never-`--prune`. |
| `.claude/cloud-bootstrap.sh` | Stale-snapshot refresh: `plugin
uninstall "$id" --keep-data` before reinstall/re-enable. The existing
comment already anticipated the loss of enabled state; it now also names
the data-directory loss the flag closes. |
|
`plugins/source-control/skills/worktree/fixtures/project-scope-reap-probe.sh`
| All five uninstalls pass `--keep-data`. The probe writes and removes
its own project-scope records; a documented recheck must not destroy
data as a last-scope side effect. |
| `docs/MIGRATION-PLAYBOOK.md` | Force-a-fresh-snapshot recipe is
`uninstall <name>@<marketplace> --keep-data` then `install` then
`enable`. |
| `plugins/claude-ops/skills/plugins/context/converge.md` |
Consolidation templates carry `--keep-data` on `uninstall` only.
`update` has no such flag. The `cd "<projectPath>"` form is split so the
flag cannot be pasted onto `update`. |

**Probe re-run (site 2 is not a one-flag change).** Adding the flag
without re-running would leave `fixtures/README.md`'s As-of block
describing a command the script no longer runs. Re-ran 2026-08-23 on
Claude Code 2.1.238, Linux, against `markdown-format@melodic-software`
and `bash-format@melodic-software`. Path-resolution outcomes match the
2026-08-22 Windows measurement on every arm. Counts: 1 → 4 (install) → 4
(other-cwd no-op) → 2 (live uninstall of two ids) → 1
(recreated-empty-dir uninstall). Failure text on arms 3 and 6 names "not
installed in project scope" when the plugin has no user-scope record;
the cwd-mismatch / no-record-here no-op is the same. The original
Windows `caveman@caveman` "installed in user scope… Use --scope user"
text is retained, because that is the trap the reap must never follow.

`docs/extensibility-contract-smoke-tests.md` also omits the flag. Wiping
the throwaway smoketest plugin's state is that procedure's stated
purpose, so it is left alone, matching #3238.

## Verification

| Gate | Result |
|---|---|
| `plugins/source-control/scripts/reap-project-plugin-records.test.sh` |
All 44 cases passed, including the new `always passes --keep-data`
assertion on the clean reap and the no-op uninstall path |
| `claude plugin validate plugins/source-control` | Validation passed |
| `claude plugin validate plugins/claude-ops` | Validation passed |
| `scripts/check-changed-skills.sh origin/main` | PASS — plugins +
worktree, 0 errors |
| `scripts/check-changelog-parity.sh` `--check` / `--check-bump` /
`--check-order` / `--check-preserved` | PASS |
| `scripts/sync-plugin-options-docs.py --check` | up to date |
| `scripts/check-skill-count-claims.sh --check` | PASS |
| `scripts/check-skill-leaf-names.sh --check` | PASS |
| `scripts/check-cross-plugin-source-drift.sh --check` | PASS |
| `markdownlint-cli2` (changed markdown) | 0 issues |
| `typos` | clean |
| `shellcheck` (changed scripts) | clean |

`claude plugin uninstall --help` on the installed CLI (2.1.238)
confirms: `--keep-data Preserve the plugin's persistent data directory
(~/.claude/plugins/data/{id}/)`.

## Related

- Closes #3212 — reap helper reachable from `/source-control:worktree
cleanup`.
- Closes #3238 — the four remaining sites of the same class.
- Refs #3131 / #3211 — documentation-side `--keep-data` gap in setup's
uninstall guidance, already merged. This PR is the code-level sibling
that #3211 deliberately left alone.
- `docs/extensibility-contract-smoke-tests.md` left unchanged: wiping
the throwaway smoketest plugin's state is that procedure's stated
purpose.

---------

Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.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.

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

1 participant