Fix setup.md: version-discovery fragility and unused argument-hint
Observed live: a stale plugin-cache version dir (1.0.0) coexisted with the active one (1.1.0). setup.md's 'try the hardcoded version path first' advice picked the stale one, its --install-raw-git-hook flag failed silently, and Claude went on an unscripted forensic dig (chained test&&echo, --help probing, grep/cat across both version dirs, cat'ing plugin.json, stat'ing .in_use mtimes) to figure out which version was actually active. Fix, in both plugins' commands/setup.md: - Never hardcode a version number. Always do one bounded find -maxdepth 1 -type d first; disambiguate multiple hits via the .in_use marker Claude Code itself writes, falling back to highest plugin.json version. Explicitly rule out the exploration that happened here (stat/mtime, --help, reading source, diffing versions) since setup.sh has no --help and unrecognized flags just error. - Wire up the declared but previously-unused argument-hint ([repo-name] [repo-path] / [vault-path] [mode]): if the user already supplied it on the command line, skip asking the question that would just re-collect the same info.
This commit is contained in:
+59
-34
@@ -19,9 +19,20 @@ to locate the plugin itself.
|
||||
|
||||
## 1. Ask the user — three selectable questions first, repo path last
|
||||
|
||||
**1a. First turn: four questions via the selection UI, fixed options for
|
||||
all four:**
|
||||
- Whether to register any repos at all: yes/no.
|
||||
**1a. Before asking anything, check for arguments.** This command's
|
||||
`argument-hint` is `[repo-name] [repo-path]`. If the user invoked it with
|
||||
both (e.g. `/git-manager:setup homelab-vault /home/hmk/projects/homelab`),
|
||||
that already answers "whether to register a repo" and "which repo" — use
|
||||
the given `name=path` pair directly as the `--repo` flag in step 2, and
|
||||
skip both the "register any repos" option below and step 1c entirely,
|
||||
without asking about either. If no arguments (or only one of the two) were
|
||||
given, proceed normally: nothing here is decided yet, ask 1b and, if
|
||||
needed, 1c below.
|
||||
|
||||
**1b. First turn: questions via the selection UI:**
|
||||
- Whether to register any repos at all: yes/no. **Omit this question if
|
||||
1a already determined a repo was given as an argument** — in that case
|
||||
there's nothing to ask, it's already yes.
|
||||
- Settings scope: `project` (`.claude/settings.json` in the current
|
||||
project) or `user` (`~/.claude/settings.json`). Mention: if they work
|
||||
across many separate repos/projects, `user` avoids repeating setup per
|
||||
@@ -43,27 +54,51 @@ all four:**
|
||||
`hooks/force-ask-on-raw-git.sh` for details). Independent of the
|
||||
anti-chaining hook — either, both, or neither can be installed.
|
||||
|
||||
**1b. Only if the answer to "register any repos" was yes: a second turn,
|
||||
plain chat message, no tool call.** Ask exactly: "Which repos should
|
||||
git-manager know about? Provide as `name=path` pairs." End your turn right
|
||||
after asking this, with nothing else queued up, so the user's reply is
|
||||
what actually gets collected before you continue. (Only a tool call
|
||||
reliably pauses for a reply in this environment; plain text followed
|
||||
immediately by a tool call in the same turn does not wait — that's what
|
||||
caused this question to appear skipped in an earlier version of this
|
||||
command.) If the answer to "register any repos" was no, skip this
|
||||
sub-step entirely — don't ask it at all.
|
||||
**1c. Only if the answer to "register any repos" was yes AND 1a didn't
|
||||
already supply a repo: a second turn, plain chat message, no tool call.**
|
||||
Ask exactly: "Which repos should git-manager know about? Provide as
|
||||
`name=path` pairs." End your turn right after asking this, with nothing
|
||||
else queued up, so the user's reply is what actually gets collected before
|
||||
you continue. (Only a tool call reliably pauses for a reply in this
|
||||
environment; plain text followed immediately by a tool call in the same
|
||||
turn does not wait — that's what caused this question to appear skipped in
|
||||
an earlier version of this command.) Skip this sub-step entirely — don't
|
||||
ask it at all — if the answer to "register any repos" was no, or if 1a
|
||||
already supplied a repo.
|
||||
|
||||
## 2. Run the setup script — one command
|
||||
## 2. Locate the plugin, then run the setup script — one command
|
||||
|
||||
Claude Code installs plugins added from a local marketplace at
|
||||
`~/.claude/plugins/cache/<marketplace-name>/<plugin-name>/<plugin-version>/`.
|
||||
For this repo's marketplace (`skill-repo`) and this plugin's current
|
||||
version (`1.0.0` — check this plugin's own `.claude-plugin/plugin.json` if
|
||||
this has since changed), that's:
|
||||
**Never hardcode or guess a version number** — it changes on every release
|
||||
of this plugin, and a stale directory from a previous version can still be
|
||||
sitting on disk right alongside the current one (an old version won't
|
||||
recognize newer flags like `--install-raw-git-hook`, so running it by
|
||||
mistake fails in a confusing way). Always start with exactly one bounded,
|
||||
read-only lookup instead:
|
||||
|
||||
```bash
|
||||
~/.claude/plugins/cache/skill-repo/git-manager/1.0.0/scripts/setup.sh \
|
||||
find ~/.claude/plugins/cache/skill-repo/git-manager -maxdepth 1 -type d 2>/dev/null
|
||||
```
|
||||
|
||||
- **Exactly one version directory found:** use it.
|
||||
- **More than one found:** prefer whichever one contains a `.in_use`
|
||||
marker file (Claude Code writes this into the currently-active version's
|
||||
directory). If none of them has one, or more than one does, fall back to
|
||||
whichever directory's `.claude-plugin/plugin.json` reports the highest
|
||||
`version`. Don't disambiguate any other way — no `stat`/mtime
|
||||
comparisons, no diffing scripts between versions, no probing with
|
||||
`--help` (there is no `--help`; unrecognized flags just print `Error:
|
||||
unknown argument`). That kind of exploration is exactly what this
|
||||
bounded lookup exists to avoid.
|
||||
- **None found:** the plugin likely isn't installed under this name or
|
||||
marketplace — say so and stop rather than guessing at a different path.
|
||||
|
||||
Then run, as a **single** Bash call (no `test -f ... &&` pre-check, no
|
||||
piping through `grep`/`head` to inspect it first):
|
||||
|
||||
```bash
|
||||
~/.claude/plugins/cache/skill-repo/git-manager/<version>/scripts/setup.sh \
|
||||
--settings-scope <project|user> \
|
||||
[--project-dir <path>] \
|
||||
[--repo <name>=<path> ...] \
|
||||
@@ -74,22 +109,12 @@ this has since changed), that's:
|
||||
Include `--repo name=path` once per repo from step 1, `--install-hook` and
|
||||
`--install-raw-git-hook` only for the ones the user opted into (they're
|
||||
independent flags), and `--project-dir` only if `--settings-scope
|
||||
project` and the project isn't the current working directory. Try this
|
||||
path directly first — don't `find`/`ls` preemptively.
|
||||
|
||||
If that exact path doesn't exist (installed version differs from `1.0.0`,
|
||||
differently named marketplace, or a future Claude Code cache layout
|
||||
change), fall back to exactly one bounded lookup instead of guessing
|
||||
further:
|
||||
|
||||
```bash
|
||||
find ~/.claude/plugins/cache/skill-repo/git-manager -maxdepth 2 -type d 2>/dev/null
|
||||
```
|
||||
|
||||
and construct the same `scripts/setup.sh` call using whatever version
|
||||
directory that reveals. This is expected to prompt for approval the first
|
||||
time — there's no way to pre-whitelist it before the setup that establishes
|
||||
the whitelist has run.
|
||||
project` and the project isn't the current working directory. If this one
|
||||
call errors, that means the version picked above was wrong — re-run the
|
||||
`find` above (and re-check `.in_use`/`plugin.json`) rather than falling
|
||||
back to reading source files or trying more flags. This is expected to
|
||||
prompt for approval the first time — there's no way to pre-whitelist it
|
||||
before the setup that establishes the whitelist has run.
|
||||
|
||||
## 3. Relay the result
|
||||
|
||||
|
||||
+53
-33
@@ -19,35 +19,64 @@ update) needs to locate the plugin itself.
|
||||
|
||||
## 1. Ask the user — selectable questions first, vault path last
|
||||
|
||||
**1a. First turn: two questions via the selection UI, fixed options for
|
||||
both:**
|
||||
**1a. Before asking anything, check for arguments.** This command's
|
||||
`argument-hint` is `[vault-path] [mode]`. Whatever was given this way
|
||||
already answers that part — don't re-ask it in 1b/1c below:
|
||||
- If `mode` was given and is one of `read-only`/`append`/`maintain`, skip
|
||||
the mode question in 1b.
|
||||
- If `vault-path` was given, skip the plain-chat vault question in 1c
|
||||
entirely and use that path directly (give it a short default name, e.g.
|
||||
the last path segment, unless the user's invocation also implied one).
|
||||
- Whatever wasn't given this way still gets asked normally below.
|
||||
|
||||
**1b. First turn: remaining questions via the selection UI:**
|
||||
- Mode: `read-only`, `append`, or `maintain` (as defined in SKILL.md —
|
||||
explain each briefly if unsure).
|
||||
explain each briefly if unsure). Omit if 1a already supplied a valid one.
|
||||
- Settings scope: `project` (`.claude/settings.json` in the current
|
||||
project) or `user` (`~/.claude/settings.json`).
|
||||
|
||||
**1b. Second turn, plain chat message, no tool call.** Ask exactly:
|
||||
"What's the path to your vault? Give it a short name too if you're
|
||||
registering more than one." End your turn right after asking this, with
|
||||
nothing else queued up, so the user's reply is what actually gets
|
||||
collected before you continue. (Only a tool call reliably pauses for a
|
||||
reply in this environment; plain text followed immediately by a tool call
|
||||
in the same turn does not wait — that's what caused this question to
|
||||
appear skipped in an earlier version of this command.) At least one vault
|
||||
is required — `setup.sh` errors out without one — so this question is
|
||||
always asked, unlike the optional repos question in the git-manager
|
||||
plugin's setup.
|
||||
**1c. Only if 1a didn't already supply a vault path: second turn, plain
|
||||
chat message, no tool call.** Ask exactly: "What's the path to your vault?
|
||||
Give it a short name too if you're registering more than one." End your
|
||||
turn right after asking this, with nothing else queued up, so the user's
|
||||
reply is what actually gets collected before you continue. (Only a tool
|
||||
call reliably pauses for a reply in this environment; plain text followed
|
||||
immediately by a tool call in the same turn does not wait — that's what
|
||||
caused this question to appear skipped in an earlier version of this
|
||||
command.) At least one vault is required — `setup.sh` errors out without
|
||||
one — so this question is always asked unless 1a already supplied a path,
|
||||
unlike the optional repos question in the git-manager plugin's setup.
|
||||
|
||||
## 2. Run the setup script — one command
|
||||
## 2. Locate the plugin, then run the setup script — one command
|
||||
|
||||
Claude Code installs plugins added from a local marketplace at
|
||||
`~/.claude/plugins/cache/<marketplace-name>/<plugin-name>/<plugin-version>/`.
|
||||
For this repo's marketplace (`skill-repo`) and this plugin's current
|
||||
version (`1.0.0` — check this plugin's own `.claude-plugin/plugin.json` if
|
||||
this has since changed), that's:
|
||||
**Never hardcode or guess a version number** — it changes on every release
|
||||
of this plugin, and a stale directory from a previous version can still be
|
||||
sitting on disk right alongside the current one. Always start with exactly
|
||||
one bounded, read-only lookup instead:
|
||||
|
||||
```bash
|
||||
~/.claude/plugins/cache/skill-repo/obsidian-vault-kb/1.0.0/scripts/setup.sh \
|
||||
find ~/.claude/plugins/cache/skill-repo/obsidian-vault-kb -maxdepth 1 -type d 2>/dev/null
|
||||
```
|
||||
|
||||
- **Exactly one version directory found:** use it.
|
||||
- **More than one found:** prefer whichever one contains a `.in_use`
|
||||
marker file (Claude Code writes this into the currently-active version's
|
||||
directory). If none of them has one, or more than one does, fall back to
|
||||
whichever directory's `.claude-plugin/plugin.json` reports the highest
|
||||
`version`. Don't disambiguate any other way — no `stat`/mtime
|
||||
comparisons, no diffing scripts between versions, no probing with
|
||||
`--help` (there is no `--help`; unrecognized flags just print `Error:
|
||||
unknown argument`).
|
||||
- **None found:** the plugin likely isn't installed under this name or
|
||||
marketplace — say so and stop rather than guessing at a different path.
|
||||
|
||||
Then run, as a **single** Bash call (no `test -f ... &&` pre-check, no
|
||||
piping through `grep`/`head` to inspect it first):
|
||||
|
||||
```bash
|
||||
~/.claude/plugins/cache/skill-repo/obsidian-vault-kb/<version>/scripts/setup.sh \
|
||||
--settings-scope <project|user> \
|
||||
[--project-dir <path>] \
|
||||
--mode <read-only|append|maintain> \
|
||||
@@ -56,20 +85,11 @@ this has since changed), that's:
|
||||
|
||||
The script itself validates each vault path exists and is a directory, and
|
||||
reports an error if not — no separate validation call needed beforehand.
|
||||
Try this path directly first — don't `find`/`ls` preemptively.
|
||||
|
||||
If the exact cache path doesn't exist (installed version differs from
|
||||
`1.0.0`, differently named marketplace, or a future cache layout change),
|
||||
fall back to exactly one bounded lookup instead of guessing further:
|
||||
|
||||
```bash
|
||||
find ~/.claude/plugins/cache/skill-repo/obsidian-vault-kb -maxdepth 2 -type d 2>/dev/null
|
||||
```
|
||||
|
||||
and construct the same `scripts/setup.sh` call using whatever version
|
||||
directory that reveals. This is expected to prompt for approval the first
|
||||
time — there's no way to pre-whitelist it before the setup that establishes
|
||||
the whitelist has run.
|
||||
If this one call errors, that means the version picked above was wrong —
|
||||
re-run the `find` above (and re-check `.in_use`/`plugin.json`) rather than
|
||||
falling back to reading source files or trying more flags. This is
|
||||
expected to prompt for approval the first time — there's no way to
|
||||
pre-whitelist it before the setup that establishes the whitelist has run.
|
||||
|
||||
## 3. Relay the result
|
||||
|
||||
|
||||
Reference in New Issue
Block a user