diff --git a/plugins/source-control/.claude-plugin/plugin.json b/plugins/source-control/.claude-plugin/plugin.json index eb9075b627..963b26a1ae 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.33.1", + "version": "0.33.2", "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 standing merge-rung raises binding from the team-tracked layer only — with one named exception, where an invocation line explicitly typing both the autopilot tier keyword and the dedicated raise argument --merge c3-this-run widens that single invocation's merge authority up to C3 behind a fresh independent frontier-tier resolver, while C4-structural and C5-untrusted-provenance stay unconditionally human-merge), /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 aa8578e7da..e748ef0dfd 100644 --- a/plugins/source-control/CHANGELOG.md +++ b/plugins/source-control/CHANGELOG.md @@ -3,6 +3,68 @@ 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.33.2] + +### Fixed + +- **Every git-bearing skill in this plugin was uninvocable from a worktree-isolated agent + (melodic-software/claude-code-plugins#1619).** The harness composes an entire + `## Pre-computed context` block into ONE shell invocation, and the worktree-isolation Bash guard + refuses a git-bearing compound command it cannot statically verify — so `commit`, `pull-request`, + `worktree`, `resolve-conflicts`, and `babysit-prs` all failed at load with `this command is too + complex to verify that it stays inside the worktree`. `worktree` is the sharpest case: the skill + for managing worktrees could not be invoked from inside one. + - The git lines are removed from each skill's pre-compute block and re-acquired in the skill body + as **individual** Bash calls, one command per call. Non-git pre-compute lines are untouched — + `commit` keeps its exec-bit and user-global config probes, `babysit-prs` keeps both `gh` lines. + - `commit`'s two repo-scoped config-layer probes were themselves compound one-liners that + re-derived the repository root inline. They are rebuilt on git's repo-root-relative magic + pathspec `:/` rather than on a substituted root — `git ls-files --error-unmatch --` and + `git ls-files --cached --others --`, each given `":/.claude/source-control.md"` (or the + `.local.md` overlay). Nothing is substituted, so a repository root containing a space, `$(…)`, + a backtick, or a double quote can neither break the command nor inject into it; double-quoting + a substituted root does *not* neutralize a command substitution, which is why quoting was the + wrong fix. Verified from a subdirectory: `:/` resolves against the working-tree root regardless + of the session's cwd, and the same existence probe replaces the personal overlay's old + `test -f "/…"`. + - `commit`'s team layer keeps all three of its states — `present (tracked)`, + `present but UNTRACKED`, `absent` — which a single `--error-unmatch` call cannot express, since + it exits nonzero for both of the last two. The `git ls-files --cached --others` existence probe + separates them (`--exclude-standard` deliberately omitted so a gitignored file is still seen), + and the generic unknown-value rule is narrowed so it no longer swallows the distinction: a + nonzero `--error-unmatch` exit is a *result*, and only a probe that could not run at all (git + unavailable, not a repository) is an unknown value. + - `babysit-prs` is held at exactly 499 lines — the change is net-zero on line count, so it does + not consume the one line it has left under the 500-line hard cap (see #1626). + - The pre-compute lines carried `2>/dev/null || echo "unknown"` fallbacks **and** output caps + (`git status --short | head -20`, `git diff --cached --stat | tail -1`, + `git worktree list | head -30`, `git status | head -4`). The fallbacks are restated as a reading + rule — a failed command means "unknown, carry on". The caps are **kept as pipes** on the body + commands in `commit`, `worktree`, and `resolve-conflicts`. An earlier revision of this change + restated them as read-time prose ("read at most the first 20 entries"); that bounded nothing, + because the Bash tool returns a command's complete output into context before there is anything + to decide about. A real bound beats a fictional one. + - What was verified, precisely: from an `Agent` with `isolation: "worktree"`, the **unfixed** + skills were observed to be refused, plain git commands were observed to succeed as individual + Bash calls, and a multi-line non-git pre-compute block was observed to load. The restored pipes + (`git status --short | head -20`, `git diff --cached --stat | tail -1`) were observed to pass as + ordinary body Bash calls in a **non-isolated** session; whether a pipe also clears the isolation + guard as a body call is not verified here. The **edited** skills have not been invoked from an + isolated agent — skills load from the version-keyed plugin cache, so `0.33.2` does not exist + there until this ships. Confirm then; CI cannot prove it. + - `shell: bash` is deliberately left in place on every affected skill, including the three that + now have no `!` lines at all. The key is inert without pre-compute lines, and removing it is a + frontmatter-contract change with no behavioral benefit. + +### Changed + +- **Two reference spokes described the moved commands as pre-computed and are corrected.** + `commit/reference/exec-bit.md` no longer calls the config-layer probes pre-computed, and + `pull-request/reference/create.md`'s `--pushed` section is regrounded: it still says to ignore the + session-cwd context for an out-of-tree orchestrator, but its stated reason — that a + `!`-substituted line cannot be `git -C`-redirected — stopped being true once those became ordinary + Bash calls. The instruction to re-resolve explicitly from the target worktree is unchanged. + ## [0.33.1] ### Fixed diff --git a/plugins/source-control/skills/babysit-prs/SKILL.md b/plugins/source-control/skills/babysit-prs/SKILL.md index c99d387cc9..7725bcda41 100644 --- a/plugins/source-control/skills/babysit-prs/SKILL.md +++ b/plugins/source-control/skills/babysit-prs/SKILL.md @@ -9,11 +9,11 @@ shell: bash ## Pre-computed context -Current branch: !`git branch --show-current 2>/dev/null || echo "unknown"` -Working tree status: !`git status --porcelain 2>/dev/null || echo "clean"` Current login: !`gh api user --jq .login 2>/dev/null || echo "unknown"` Own open PRs here: !`gh pr list --state open --author "@me" --limit 200 --json number --jq 'length' 2>/dev/null || echo "unknown"` +Branch and working tree: gather with two separate Bash calls, `git branch --show-current` then `git status --porcelain`; treat a failure as an unknown value and carry on. They moved out of pre-compute in #1619 — the harness composes the block into one shell invocation and a worktree-isolated agent refuses a git-bearing compound command — so run them individually and do not fold them back. + ## Purpose Keep pull requests moving without taking unsafe GitHub actions. Guarantees are enforced in diff --git a/plugins/source-control/skills/commit/SKILL.md b/plugins/source-control/skills/commit/SKILL.md index 078d80281b..b347b08846 100644 --- a/plugins/source-control/skills/commit/SKILL.md +++ b/plugins/source-control/skills/commit/SKILL.md @@ -9,28 +9,82 @@ shell: bash ## Pre-computed context -Current branch: !`git branch --show-current 2>/dev/null || echo "unknown"` -Staged: !`git diff --cached --stat 2>/dev/null | tail -1 || echo "nothing staged"` -Unstaged: !`git status --short 2>/dev/null | head -20 || echo "clean"` -Recent commits: !`git log --oneline -5 2>/dev/null || echo "no commits"` Exec-bit backstop: !`bash "${CLAUDE_PLUGIN_ROOT}/skills/commit/scripts/exec-bit-check.sh" --probe 2>/dev/null || echo "unavailable — run the check manually"` Config layer (user-global): !`test -f "$HOME/.claude/source-control.md" && echo present || echo absent` -Config layer (tracked team): !`R="$(git rev-parse --show-toplevel 2>/dev/null)" && git -C "$R" ls-files --error-unmatch .claude/source-control.md >/dev/null 2>&1 && echo "present (tracked)" || { test -n "${R:-}" && test -f "$R/.claude/source-control.md" && echo "present but UNTRACKED — not a config layer" || echo absent; }` -Config layer (personal overlay): !`R="$(git rev-parse --show-toplevel 2>/dev/null)" && test -n "$R" && test -f "$R/.claude/source-control.local.md" && echo present || echo absent` -**Both probes are snapshots taken at invocation, not substitutes for the checks.** The exec-bit +## Repository context — gather first + +Collect these with **individual** Bash calls, one command per call, never combined into a single +invocation: + +- Current branch — `git branch --show-current` +- Staged — `git diff --cached --stat | tail -1` +- Unstaged — `git status --short | head -20` +- Recent commits — `git log --oneline -5` + +**The two pipes are the bound and belong in the command.** They were briefly restated as read-time +prose ("read only the summary line", "read at most the first 20 entries"); that bounds nothing. The +Bash tool returns a command's complete output into context before there is anything to decide about, +so a read-time cap is advisory after the fact — `head` / `tail` truncate before the result exists. +These are ordinary body Bash calls, not pre-compute: the shape #1619 is about is the harness +composing the whole `## Pre-computed context` block into one shell invocation, which does not apply +to a call the body tells you to make. + +Then resolve the two repo-scoped config layers as separate calls. Each uses git's repo-root-relative +magic pathspec `:/`, which resolves against the top of the working tree, so these are correct from +any subdirectory and never need the repository root substituted into them. **Quote the pathspec** — +the leading `:` is pathspec magic, and quoting keeps the shell from reinterpreting it. + +1. Team layer, trackedness — + `git ls-files --error-unmatch -- ":/.claude/source-control.md"` +2. Team layer, existence — + `git ls-files --cached --others -- ":/.claude/source-control.md"` +3. Personal overlay, existence — + `git ls-files --cached --others -- ":/.claude/source-control.local.md"` + +Probes 2 and 3 deliberately omit `--exclude-standard`, so a gitignored file is still listed: the +personal overlay is expected to be gitignored, and an untracked team file has to be *seen* before it +can be reported as untracked. + +**Combine probes 1 and 2 in that order** — trackedness first, existence consulted only when +trackedness comes back nonzero. They are not independent readings: + +| Trackedness (1) | Existence (2) | Team-layer state | +|---|---|---| +| exit 0 | not consulted | `present (tracked)` — this is a config layer | +| nonzero | non-empty output | `present but UNTRACKED` — deliberately **not** a config layer | +| nonzero | empty output | `absent` | + +A single `--error-unmatch` call cannot separate the last two rows; it exits nonzero for both. That +is the whole reason probe 2 exists. + +**A nonzero exit from probe 1 is a result, not a failure** — it answers the question, and probe 2 +resolves which answer it is. The unknown-value rule is narrower than that: only a probe that could +not run at all (git unavailable, not a repository) yields an unknown value to carry on past. Never +fold `present but UNTRACKED` into "unknown" — they mean opposite things, and treating the untracked +case as unknown is exactly what readmits a file the resolution rules exclude. + +The git lines above moved out of pre-compute in #1619 — the harness composes that block into one +shell invocation and a worktree-isolated agent refuses a git-bearing compound command it cannot +statically verify. Do not fold them back into `## Pre-computed context`. + +**These are snapshots taken when they run, not substitutes for the checks.** The exec-bit line only sees what was already staged when the skill loaded; anything staged in step 2 below is -invisible to it, so step 4 re-runs the script for real. The config-layer lines report **presence**, +invisible to it, so step 4 re-runs the script for real. The config-layer probes report **presence**, not merged content — read the layers that are present and merge them per key. A tracked-team layer reported `present but UNTRACKED` is deliberately not a layer: resolution requires the team file to be git-tracked, so an untracked or gitignored file at that well-known path must not drive the convention. -**Every probe anchors at the repository root** (`git rev-parse --show-toplevel`), never at the -session's current directory. A session started in a subdirectory would otherwise look for +**Every repo-scoped probe anchors at the repository root**, via the `:/` magic pathspec, never at +the session's current directory. A session started in a subdirectory would otherwise look for `/.claude/` and report both repo-scoped layers absent — silently dropping the team convention -and `trailer_policy`, and producing a commit with the wrong subject shape or attribution. This -matches the root-resolution requirement +and `trailer_policy`, and producing a commit with the wrong subject shape or attribution. The +pathspec form also retires the older two-step (`git rev-parse --show-toplevel`, then substitute the +literal path it printed into the probe): a repository root containing `$(…)`, a backtick, or a +double quote is *not* made safe by wrapping the substituted text in double quotes — the substitution +still evaluates, or the quote terminates the argument — and `:/` never substitutes the root at all. +This matches the root-resolution requirement [`${CLAUDE_PLUGIN_ROOT}/reference/config-resolution.md`](../../reference/config-resolution.md) already states for resolution itself; the probes must not disagree with it. `exec-bit-check.sh` anchors itself the same way. diff --git a/plugins/source-control/skills/commit/reference/exec-bit.md b/plugins/source-control/skills/commit/reference/exec-bit.md index b85e9fc28d..7def5729ed 100644 --- a/plugins/source-control/skills/commit/reference/exec-bit.md +++ b/plugins/source-control/skills/commit/reference/exec-bit.md @@ -76,7 +76,7 @@ Caller pathspecs are relative to the caller's cwd, so they are re-anchored via subdirectory would silently match nothing. Absolute paths and `:`-prefixed magic pathspecs are already unambiguous and are left alone. -The skill's pre-computed config-layer probes anchor the same way, for the same reason: a session +The skill's config-layer probes anchor the same way, for the same reason: a session started in a subdirectory would otherwise report both repo-scoped config layers absent and silently drop the team convention and `trailer_policy`. diff --git a/plugins/source-control/skills/pull-request/SKILL.md b/plugins/source-control/skills/pull-request/SKILL.md index affca6941c..d3770a578c 100644 --- a/plugins/source-control/skills/pull-request/SKILL.md +++ b/plugins/source-control/skills/pull-request/SKILL.md @@ -7,12 +7,19 @@ argument-hint: " [args] (e.g., /pull-request prep, /pull-request create, shell: bash --- -## Pre-computed context +## Repository context — gather first -Current branch: !`git branch --show-current 2>/dev/null || echo "unknown"` -Recent commits: !`git log --oneline -5 2>/dev/null || echo "no commits"` -Working tree status: !`git status --porcelain 2>/dev/null || echo "clean"` -Changed files (staged+unstaged): !`git diff --name-only HEAD 2>/dev/null || echo "none"` +Collect these with **individual** Bash calls, one command per call, never combined into a single +invocation: + +- Current branch — `git branch --show-current` +- Recent commits — `git log --oneline -5` +- Working tree status — `git status --porcelain` +- Changed files (staged+unstaged) — `git diff --name-only HEAD` + +Treat a failure (not a repository, git unavailable) as an unknown value and carry on. These moved +out of pre-compute in #1619 — the harness composes the block into one shell invocation and a +worktree-isolated agent refuses a git-bearing compound command; do not fold them back. ## Purpose diff --git a/plugins/source-control/skills/pull-request/reference/create.md b/plugins/source-control/skills/pull-request/reference/create.md index 7db02b15bb..890de915d0 100644 --- a/plugins/source-control/skills/pull-request/reference/create.md +++ b/plugins/source-control/skills/pull-request/reference/create.md @@ -510,7 +510,7 @@ Report the PR URL, captured ``, and recorded list of expected CI work `create --pushed --worktree ` opens the PR when the branch is **already committed and pushed** — the orchestrated case where a dispatched worker did the edits, commit, and push inside its own out-of-tree worktree and returned that worktree's path (`/work-items:work`, `#572`). The invoking orchestrator is typically **out-of-tree** (its session sits on the default branch or elsewhere), so this mode runs neither the commit/push half of the normal `create` path nor trusts the session cwd. -**Ignore the pre-computed context.** [SKILL.md](../SKILL.md)'s `!`-substituted frontmatter (`git branch --show-current`, `git diff --name-only HEAD`, working-tree status) reflects the **session cwd**, which for an out-of-tree orchestrator is the wrong branch and diff — and a `!`-substituted line cannot be `git -C`-redirected. Under `--pushed`, re-resolve everything from the target worktree instead: +**Ignore the gathered repository context.** [SKILL.md](../SKILL.md)'s gather step (`git branch --show-current`, `git diff --name-only HEAD`, working-tree status) reports the **session cwd**, which for an out-of-tree orchestrator is the wrong branch and diff. Since #1619 those are ordinary Bash calls rather than `!`-substituted lines, so unlike before they *can* be `git -C`-redirected — but do not redirect them ad hoc. Under `--pushed`, re-resolve everything from the target worktree explicitly: ```bash WT="" # from --worktree diff --git a/plugins/source-control/skills/resolve-conflicts/SKILL.md b/plugins/source-control/skills/resolve-conflicts/SKILL.md index f8caf76be2..65e409573a 100644 --- a/plugins/source-control/skills/resolve-conflicts/SKILL.md +++ b/plugins/source-control/skills/resolve-conflicts/SKILL.md @@ -7,11 +7,24 @@ disable-model-invocation: false shell: bash --- -## Pre-computed context +## Repository context — gather first -Operation state: !`git status 2>/dev/null | head -4 || echo "not a git repo"` -Conflicted paths: !`git diff --name-only --diff-filter=U 2>/dev/null || echo "none"` -Current branch: !`git branch --show-current 2>/dev/null || echo "detached/unknown"` +Collect these with **individual** Bash calls, one command per call, never combined into a single +invocation: + +- Operation state — `git status | head -4` (that is where the in-progress + merge/rebase/cherry-pick banner sits) +- Conflicted paths — `git diff --name-only --diff-filter=U` +- Current branch — `git branch --show-current` + +The pipe is the bound and belongs in the command. A read-time cap ("read only the first 4 lines") +bounds nothing: the Bash tool returns the command's complete output into context before there is +anything to decide about. These are ordinary body Bash calls, not pre-compute — the shape #1619 is +about is the harness composing the whole pre-compute block into one shell invocation. + +Treat a failure (not a repository, git unavailable) as an unknown value and carry on. These moved +out of pre-compute in #1619 — the harness composes the block into one shell invocation and a +worktree-isolated agent refuses a git-bearing compound command; do not fold them back. ## Purpose diff --git a/plugins/source-control/skills/worktree/SKILL.md b/plugins/source-control/skills/worktree/SKILL.md index 2829f20ae0..0edeaad3d3 100644 --- a/plugins/source-control/skills/worktree/SKILL.md +++ b/plugins/source-control/skills/worktree/SKILL.md @@ -7,12 +7,26 @@ argument-hint: " [args] (e.g., /worktree create feat/my-feature, /worktr shell: bash --- -## Pre-computed context - -Current branch: !`git branch --show-current 2>/dev/null || echo "unknown"` -Worktree inventory: !`git worktree list 2>/dev/null | head -30 || echo "not a git repo"` -Git dir: !`git rev-parse --git-dir 2>/dev/null || echo "none"` -Git common dir (differs from git dir when in a linked worktree): !`git rev-parse --git-common-dir 2>/dev/null || echo "none"` +## Repository context — gather first + +Collect these with **individual** Bash calls, one command per call, never combined into a single +invocation: + +- Current branch — `git branch --show-current` +- Worktree inventory — `git worktree list | head -30` +- Git dir — `git rev-parse --git-dir` +- Git common dir (differs from the git dir when in a linked worktree) — + `git rev-parse --git-common-dir` + +The pipe is the bound and belongs in the command. A read-time cap ("read only the first 30 +entries") bounds nothing: the Bash tool returns the command's complete output into context before +there is anything to decide about. These are ordinary body Bash calls, not pre-compute — the shape +that #1619 is about is the harness composing the whole pre-compute block into one shell invocation. + +Treat a failure (not a repository, git unavailable) as an unknown value and carry on. These moved +out of pre-compute in #1619 — the harness composes the block into one shell invocation and a +worktree-isolated agent refuses a git-bearing compound command, which made the worktree skill itself +uninvocable from inside a worktree; do not fold them back. ## Purpose