diff --git a/plugins/source-control/.claude-plugin/plugin.json b/plugins/source-control/.claude-plugin/plugin.json index 8c485fc94..4cc220f23 100644 --- a/plugins/source-control/.claude-plugin/plugin.json +++ b/plugins/source-control/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "$schema": "https://json.schemastore.org/claude-code-plugin-manifest.json", "name": "source-control", - "version": "0.26.5", + "version": "0.26.6", "description": "Git and GitHub delivery workflow: /commit (Conventional Commits + Co-Authored-By trailer via safe heredoc mechanics), /pull-request (prep, create, CI monitoring, review-comment triage, merge, CI-log fetch), /babysit-prs (self-pacing fleet loop — safe by default; opt-in worker/autopilot tiers add gate-checked merge and thread resolution behind a deterministic Python engine), /babysit-loop (the loop-lane merge lane: a standing or drain loop that invokes babysit-prs per cycle, configured through repo-scoped babysit_loop_* keys on the layered source-control.md seam, with merge authority human-only until the target repo's tracked config adopts the lane, a gate-proven C2-mechanical baseline once adopted, and merge-rung raises binding from the team-tracked layer only), /worktree (create, status, cleanup, audit for parallel-session isolation), /setup (check the effective commit-subject / PR-title convention merged across its config layers and the babysit-prs config, or apply — interview the repo and write the convention config to a chosen layer), and /resolve-conflicts (intent-first merge/rebase conflict resolution with a semantic-conflict sweep — never --abort). The commit-subject / PR-title convention is configurable via a source-control.md config written by a re-runnable setup skill, layered across a ~/.claude user-global file, the tracked team file, and a gitignored .claude/source-control.local.md personal overlay merged per key; Conventional Commits is the default when no convention is declared.", "author": { "name": "Melodic Software", diff --git a/plugins/source-control/CHANGELOG.md b/plugins/source-control/CHANGELOG.md index 688d32cba..c6917012c 100644 --- a/plugins/source-control/CHANGELOG.md +++ b/plugins/source-control/CHANGELOG.md @@ -3,6 +3,33 @@ All notable changes to the `source-control` plugin are documented here. Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); this plugin uses semantic versioning. +## [0.26.6] + +### Fixed + +- **`scripts/worktree-create.sh` now refuses a git-illegal `--name` with the documented usage exit 2 + instead of environment exit 4 (`#1016`).** The up-front character class (letters, digits, dots, + underscores, dashes per `/`-separated segment) is not a subset of git's ref grammar, so names like + `feat/foo..bar`, `foo.lock`, `.foo`, `HEAD`, and `-lead` passed validation, reached + `git worktree add`, and failed there as exit 4 — the code the helper reserves for environment + faults. A caller's correction flow keys on exit 2, so an invalid name was indistinguishable from a + broken environment. The schema check is now followed by `git check-ref-format --branch`, whose + output is discarded on both streams: on success `--branch` echoes the name to stdout, which would + break the helper's "created path is the sole stdout line" output contract. Verified across every + name the character class admits, `check-ref-format` and `git worktree add -b` agree exactly, so no + previously-creatable name is newly refused. `skills/worktree/context/create.md` gains the matching + constraint bullet, and the header comment that claimed the character class was "a strict subset of + what git refs allow" is corrected. + + The grammar check runs **after** the repository is resolved and is scoped with `-C "$toplevel"`: + `--branch` takes a branchname-shorthand and so performs repository discovery, which dies outright + when the process's CWD is a stale checkout (a `.git` file naming a gitdir that no longer exists — + what this plugin's own worktree cleanup handles). Run unscoped, that turned a valid name into a + false exit 2 from such a directory, and the documented invocation omits `--repo-dir`, so the CWD is + the default. Consequence: exits 3 (root unconfigured) and 4 (not a repository) can now precede the + grammar refusal, matching how the pre-existing `--base-ref` and empty-slug exit-2 checks already + sit after them. The character-class and length checks still run first, before any git call. + ## [0.26.5] ### Fixed diff --git a/plugins/source-control/scripts/worktree-create.sh b/plugins/source-control/scripts/worktree-create.sh index a2d80971c..36dc9a152 100755 --- a/plugins/source-control/scripts/worktree-create.sh +++ b/plugins/source-control/scripts/worktree-create.sh @@ -26,7 +26,7 @@ # # Exit codes: # 0 success — worktree created; path on stdout -# 2 usage error — unknown/missing flag +# 2 usage error — unknown/missing flag, or a --name git rejects as a branch # 3 refuse — external root unconfigured (guidance on stderr); nothing created # 4 environment error — not a git repo, or `git worktree add` failed @@ -43,6 +43,13 @@ Usage: Options: --name Branch/worktree name (e.g. feat/my-feature). Required. + Max 64 chars; each /-separated segment holds only letters, + digits, dots, underscores, and dashes. Also checked against + git's own branch-name grammar, so a name that satisfies the + character rules but is an illegal ref (feat/foo..bar, + foo.lock, HEAD) is refused (exit 2), not left to git later. + That grammar check runs after the repository is resolved, + so exits 3 and 4 can precede it. --root External worktree root. Required and must be configured; an empty value or an unexpanded \${user_config.*} token makes the helper refuse (exit 3) rather than fall back to @@ -95,10 +102,9 @@ fi # Validate the name up front against the EnterWorktree schema: max 64 chars, and # each '/'-separated segment contains only letters, digits, dots, underscores, -# and dashes. This is a strict subset of what git refs allow, so a validated name -# is always a creatable branch — reject invalid names loudly (exit 2) here rather -# than let `git worktree add` fail opaquely downstream. The branch is used -# verbatim; only the directory slug transforms it. +# and dashes. Reject invalid names loudly (exit 2) here rather than let +# `git worktree add` fail opaquely downstream. The branch is used verbatim; only +# the directory slug transforms it. if (( ${#name} > 64 )); then printf '%s: --name %q exceeds 64 characters\n' "$PROG" "$name" >&2 exit 2 @@ -108,6 +114,9 @@ if [[ ! "$name" =~ ^[A-Za-z0-9._-]+(/[A-Za-z0-9._-]+)*$ ]]; then exit 2 fi +# The remaining name check — git's own ref grammar — needs a healthy repository +# to run in, so it waits until $toplevel is resolved below. + # Refuse-with-guidance: unconfigured root. Treat an empty value or an unexpanded # ${user_config.*} token (what Claude Code leaves when the key is unset) as unset. # SC2016: the single-quoted ${user_config token is matched literally on purpose — @@ -152,6 +161,31 @@ if ! toplevel=$(git -C "$repo_dir" rev-parse --show-toplevel 2>/dev/null); then exit 4 fi +# Second half of name validation: the character class checked above is NOT a +# subset of git's ref grammar, so it alone cannot uphold the exit-2-for-an- +# invalid-name contract. `feat/foo..bar` (`..`), `.foo` and `feat/.` (component +# starting/ending with `.`), `foo.lock` (reserved suffix), `HEAD`, and `-lead` +# all sit inside the class yet are illegal refs, and would otherwise fail inside +# `git worktree add` as environment exit 4 — which a caller cannot tell from a +# genuinely broken environment. Ask git rather than reimplementing its rules. +# +# Runs HERE, not beside the character check, and scoped with `-C "$toplevel"`: +# `--branch` takes a branchname-shorthand and so performs repository discovery, +# which fails outright when the process's CWD is a stale checkout (a `.git` file +# naming a gitdir that no longer exists — precisely what this plugin's own +# worktree cleanup deals with). Inheriting that CWD turned a perfectly valid name +# into a false exit 2, and the documented invocation omits `--repo-dir`, so the +# CWD is the default. Deferring past the exit-4 repository probe guarantees a +# healthy repo to run in and makes a non-zero exit mean the NAME, nothing else. +# Both streams are discarded: on success `--branch` echoes the name to stdout, +# which would corrupt the sole-stdout-line output contract, and on failure git's +# message is superseded by ours. An option-shaped name is read as a ref because +# git parses no further options after `--branch`. +if ! git -C "$toplevel" check-ref-format --branch "$name" >/dev/null 2>&1; then + printf '%s: --name %q is not a valid git branch name (see: git help check-ref-format)\n' "$PROG" "$name" >&2 + exit 2 +fi + # parse_owner_repo — derive an `ownerrepo` pair from a remote URL for # the directory name. Sets globals `owner` and `repo`. Handles the shapes we see: # git@host:owner/repo.git · https://host/owner/repo.git · ssh://git@host/owner/repo diff --git a/plugins/source-control/scripts/worktree-create.test.sh b/plugins/source-control/scripts/worktree-create.test.sh index 799341881..3ba6014c9 100755 --- a/plugins/source-control/scripts/worktree-create.test.sh +++ b/plugins/source-control/scripts/worktree-create.test.sh @@ -273,6 +273,36 @@ assert_contains "unsafe-char refusal explains the rule" "$err" "not a valid work # An over-long name (>64 chars) is likewise refused. bash "$HELPER" --name "feat/$(printf 'a%.0s' {1..70})" --root "$TEST_TMPDIR/wtroot10" --repo-dir "$repo" >/dev/null 2>&1 assert_exit "over-64-char name refused (exit 2)" 2 "$?" +# A name whose characters are all in the allowed class but which git rejects as a +# branch is refused here (exit 2) rather than reaching `git worktree add` and +# surfacing as environment exit 4 — the correction flow keys on the usage error. +for badref in 'feat/foo..bar' 'feat/.' 'foo.lock' 'feat/x.lock' '.foo' 'foo.' 'HEAD' '-lead'; do + code=0 + err=$(bash "$HELPER" --name "$badref" --root "$TEST_TMPDIR/wtroot10c" --repo-dir "$repo" 2>&1 >/dev/null) || code=$? + assert_exit "git-invalid ref name $badref refused (exit 2, not 4)" 2 "$code" + assert_contains "git-invalid $badref refusal names the branch grammar" "$err" "not a valid git branch name" +done +# The ref check must not eat stdout: `git check-ref-format --branch` echoes the +# name on success, which would corrupt the sole-stdout-line path contract. +root="$TEST_TMPDIR/wtroot10d" +out=$(bash "$HELPER" --name "feat/refok" --root "$root" --repo-dir "$repo" 2>/dev/null) +assert_exit "ref-valid name still creates (exit 0)" 0 "$?" +assert_eq "ref check leaves stdout as the sole path line" "$root/acme-widget-feat-refok" "$out" +# `check-ref-format --branch` takes a branchname-shorthand, so it does repository +# discovery and dies when the process CWD is a stale checkout (a .git file naming +# a gitdir that no longer exists). Scoping it to $toplevel keeps a VALID name from +# being rejected because of where the caller happened to stand — the documented +# invocation omits --repo-dir, so the CWD is the default. +stale="$(mktemp -d "$TEST_TMPDIR/staleXXXXXX")" +printf 'gitdir: %s/definitely-not-here\n' "$TEST_TMPDIR" > "$stale/.git" +root="$TEST_TMPDIR/wtroot10e" +out=$(cd "$stale" && bash "$HELPER" --name "feat/cwdok" --root "$root" --repo-dir "$repo" 2>/dev/null) +assert_exit "valid name unaffected by a stale .git in the CWD (exit 0)" 0 "$?" +assert_eq "stale-CWD run still prints the worktree path" "$root/acme-widget-feat-cwdok" "$out" +# ...and an invalid name is still caught from that same stale CWD. +code=0 +(cd "$stale" && bash "$HELPER" --name 'feat/foo..bar' --root "$TEST_TMPDIR/wtroot10f" --repo-dir "$repo") >/dev/null 2>&1 || code=$? +assert_exit "stale CWD still rejects a git-invalid name (exit 2)" 2 "$code" # A valid multi-segment name keeps the branch verbatim but transforms the slug. root="$TEST_TMPDIR/wtroot10b" out=$(bash "$HELPER" --name "feat/scope.v2_final-1" --root "$root" --repo-dir "$repo" 2>/dev/null) diff --git a/plugins/source-control/skills/worktree/context/create.md b/plugins/source-control/skills/worktree/context/create.md index 0c5865ff6..b79f76a14 100644 --- a/plugins/source-control/skills/worktree/context/create.md +++ b/plugins/source-control/skills/worktree/context/create.md @@ -21,6 +21,7 @@ The name (branch and, via the helper's slug, directory) has these constraints (t - Each `/`-separated segment may contain only **letters, digits, dots, underscores, and dashes** - Max **64 characters** total - `/` is a valid segment separator (enables `feat/my-feature` format) +- The name must also be a **legal git branch name** (`git check-ref-format --branch`). The character rule above does not imply this — `feat/foo..bar`, `foo.lock`, `.foo`, `HEAD`, and `-lead` all satisfy it yet git rejects them as refs. The helper checks this after resolving the repository, so exits 3 and 4 can precede an invalid-name exit 2. Validate the name against these rules. If invalid, explain what's wrong and ask for correction. The helper re-validates defensively and **refuses** a name that violates them (exit 2) rather than let `git worktree add` fail opaquely. The branch keeps the name verbatim; the helper derives the **directory slug** from it (each `/` → `-`).