fix(source-control): resolve babysit worktree pruner main-checkout without hard ghq dependency - #741
Conversation
…thout hard ghq dependency The engine-backed pruner resolved a linked worktree's main checkout by shelling out to `ghq` (the plugin author's personal repo-layout tool) and raised a hard RuntimeError for any consumer without it — an undeclared prerequisite absent from the README's "runs on git, gh, jq" contract and a violation of the repo's plugin-agnosticism rules. Resolve the main checkout natively from the worktree's own gitdir/commondir pointer via `git rev-parse --git-common-dir`, and drop `ghq` from the executable allowlist entirely. Native resolution is strictly more correct than ghq's guess from a configured root plus an assumed <root>/github.com/owner/repo layout, so no optional ghq path is retained. Adds a hermetic regression test exercising resolution and removal against a real linked worktree with no ghq on PATH. Closes #438 Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
|
Claude finished @kyle-sexton's task in 2m 37s —— View job PR Review: fix(source-control): resolve babysit worktree pruner main-checkout without hard ghq dependencyTasks
SummaryThis is a clean, correct fix that eliminates a hard Core Logic:
|
|
Claude finished @kyle-sexton's task in 2m 21s —— View job PR Review: fix(source-control): resolve babysit worktree pruner main-checkout without hard ghq dependencyTasks
SummaryThis is a clean, correct fix. The new Core Logic:
|
|
Classification of the two nits from the CI review (babysit worker pass; both bot review runs are duplicate re-runs of the same findings):
Destructive-correctness re-verified independently: the removal target is |
The `repo_path` else-branch (`return common` when the git-common-dir is not named `.git`) resolves a bare-clone hub to the bare repo directory itself. That path had no CI coverage — only the standard-clone branch did. Add a test that clones a `--bare` hub, adds a linked worktree, and asserts `repo_path` returns the bare repo directory (not its parent). Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Claude finished @kyle-sexton's task in 3m 6s —— View job Security Review: fix(source-control): resolve babysit worktree pruner main-checkout without hard ghq dependencyTasks
Overall AssessmentThis PR reduces the attack surface rather than expanding it. Removing SUGGESTION —
|
| Area | Before | After |
|---|---|---|
| Executable allowlist | ("git", "gh", "ghq") |
("git", "gh") — surface reduced |
repo_path arguments |
(owner, repo) — strings extracted from directory names, passed into ghq list -p owner/repo |
(worktree_path: Path) — no user-derived strings in subprocess args |
| Resolution strategy | External tool with assumed layout | Git's own metadata — authoritative, no layout assumption |
| Failure mode | RuntimeError ("install ghq") — hard crash for all consumers |
run(check=True) raises on unexpected git state rather than silently misbehaving |
| Test hermeticity | No regression guard | ALLOWED_EXECUTABLES in test process; any ghq call raises immediately |
The path-containment guard in remove_worktree (if allowed_root not in resolved.parents) is unchanged from before this PR and correctly blocks symlink attacks (.resolve() follows symlinks before the check).
No GitHub Actions workflows, permission configurations, secret handling, or supply-chain dependencies changed in this PR.
|
Claude finished @kyle-sexton's task in 1m 35s —— View job PR Review: fix(source-control): resolve babysit worktree pruner main-checkout without hard ghq dependencyTasks
SummaryThe sync commit ( New test:
|
| # | Finding | Resolution |
|---|---|---|
| Bare-clone else-branch has no automated test | ✅ Fixed — test_bare_hub_worktree_resolves_to_the_bare_repo_itself |
|
.git-name heuristic misfires on a bare repo literally named .git |
✅ Deferred — pathological case (standard bare repos are <name>.git); docstring documents the assumption; is-bare-repository probe would add subprocess latency for a case unreachable in the babysit topology |
Both dispositions are correct.
No new findings
The PR is ready to merge.
|
Follow-up: bare-clone hub test committed in |
#752) ## Summary Gates the `🤖 Generated with [Claude Code]…` PR-body line behind config, matching the seam `/commit` already has for the commit trailer (`trailer_policy`). ## Fix Resolves a new `pr_body_attribution` key across the same three `source-control.md` config layers `config-resolution.md` documents: absent → the default line (unchanged current behavior — no existing consumer sees a change), `none` → omit the line, any other value → that literal line. Chosen as a **sibling** key rather than reusing `trailer_policy` so opting out of the commit trailer doesn't silently strip the PR-body line for existing consumers who only wanted one or the other gated. `create.md` §2.4.1 resolves the value at the model level and splices it in as literal text alongside the existing `CLOSES_LINE` pattern — preserving the same shell-injection-safety property (parameter expansion of `"${VAR}"` does not re-evaluate the value; a `$(...)`-bearing custom attribution string stays inert). `setup`'s interview, config template, and `check` action render the new key; `SKILL.md` and `config-resolution.md` document it. New evals pin both the default-present and `none` opt-out paths. **Known same-plugin version collision (sanctioned, not an error):** PR #741 (issue #438, `fix/438-babysit-pruner-ghq-dependency`) is still open and also bumps `source-control` to `0.15.2` from the same `0.15.1` base. Per this repo's same-plugin serialization convention, this PR is opened as DRAFT + `do-not-merge` and held until #741 merges, at which point it needs a rebase + re-bump to whatever version `main` is at by then. ## Verification - `jq empty` on `evals.json` and `plugin.json`: both valid JSON. - `bash scripts/validate-plugins.sh`: full marketplace + all plugin manifests validate, including `source-control`. - New evals added (`evals/evals.json`) covering the default-present and `none` opt-out paths — model-graded, will run under this repo's eval pipeline. - No existing test file exists for `create.md` itself (it's a reference doc with bash snippets, not an executable script) — verification here is manifest/JSON validity plus the evals; CI's full gate suite (`plugin-gate`, `skill-quality-gate`, `changelog-parity-gate`, etc.) is the remaining check. Closes #439 ## Related - #438 / PR #741 — the sibling same-plugin PR this one is serialized behind (version collision, see above) --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ne 4)
Lane 4 of the pocock-skills-v12-sync plan: Q10 resolved by the lane
interview; the user locked PORT, hardened, as a new single-capability
plugin.
- New plugin `wizard` 0.1.0, one skill `generate` (/wizard:generate,
leaf named via naming tournament; grammar-clean, no naming-exception
entry). Model-invoked with upstream's non-trigger fence kept ("Don't
invoke this for steps the agent can perform itself").
- Hardened template.sh (the gating outcome of a completed security
review, all conditions shipped): mandatory human read-and-approve of
the full STAGES block before chmod +x; https-only open_url with the
URL printed before dispatch (closes a Windows UNC/NTLM leak via
explorer.exe); /dev/tty fail-closed prompts retiring a verified
multi-line-paste confirm bypass and pause's EOF fail-open; quoted
0600 .env writes + gitignore pre-flight assert + trap-cleaned atomic
temp; repo-resolved/confirmed --repo-explicit gh writes with stderr
surfaced into SKIPPED and empty values refused; key-name validation
in every helper; readline on non-secret asks (upstream #741 fixed
where safe); set_var via --body-file - (stdin, never argv).
- Scoping honesty fix: step 1 reads .env.example/README/workflows fully
but takes key NAMES only from a live .env, never values; the skill
states the secrets-and-context property honestly.
- Fresh-context static trace delegated in the skill's verify step per
the fresh-eyes rules; bash -n/shellcheck stay deterministic gates.
- Records: SSOT attribution row + open-evaluations update, map row 18,
PLAN.md Q10/lane-4 closure, MIGRATION-PLAYBOOK ACCEPT record for the
model-generated-executable surface (deliberately breaks the
statusline-shim no-templating precedent, mitigations recorded).
- Marketplace entry + regenerated CATALOG and skill cheat sheet.
- Gates: bash -n + shellcheck clean; skill-quality check PASS (0 warn);
portability gate clean (gh sites declared portability-ok); typos and
markdownlint clean; claude plugin validate + --strict catalog pass;
functional smoke of the library helpers; fresh-context verifier
subagent returned 25/25 PASS (its two non-blocking findings fixed:
explorer.exe spurious warning, gitignore check now pre-flight).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Summary
The
babysit-prsengine-backed worktree pruner (prune_babysit_worktrees.py) hard-depended onghq— the plugin author's personal repo-layout tool — to resolve a linked worktree's main checkout. A consumer withoutghqhit a hardRuntimeError("install ghq or set ghq.root") on every--applyremoval, and even theghq.rootfallback baked in ghq's<root>/github.com/owner/repolayout assumption.ghqis an undeclared prerequisite: it appears nowhere in the README's "runs ongit,gh,jq" self-contained claim, violating the repo'sdocs/PLUGIN-PHILOSOPHY.md"declare every required CLI at the point of use and in the README" and graceful-degrade rules. This bit engine-backed (Python) worker/autopilot runs.Fix
repo_path()now resolves the main checkout natively from the linked worktree's own gitdir/commondir pointer viagit -C <worktree> rev-parse --git-common-dir:.git, so its parent is the checkoutgit worktreecommands run from;ghqis removed fromALLOWED_EXECUTABLESentirely, not retained as an optional path. Design call (per the issue's open question): native git-metadata resolution reads the actual checkout the worktree belongs to, which is strictly more correct than ghq's guess from a configured root plus an assumed layout — so a presence-gated ghq enhancement would add config surface with no correctness benefit. No README change is needed:ghqappeared only in this script (never in README/docs), so removing it makes the existing "runs ongit,gh,jq" claim true rather than requiring a new prerequisite declaration.Both resolution branches (standard and bare) were verified empirically against real linked worktrees before implementation.
Verification
Full stdlib-unittest suite + ruff, run from
plugins/source-control/skills/babysit-prs/scripts:New hermetic regression test
tests/test_prune_babysit_worktrees.py(nogh, noghqonPATH— any strayghqcall now raises "not in the caller's allowlist"):RepoPathResolvesFromGitMetadata.test_linked_worktree_resolves_its_main_checkout—repo_pathresolves a linked worktree to its real main checkout from git metadata alone.RemoveWorktreeIsHermetic.test_removes_a_clean_worktree_under_root_without_ghq— end-to-end removal viagit worktree removewith onlygitpresent.RemoveWorktreeIsHermetic.test_refuses_to_remove_a_worktree_outside_the_babysit_root— the path-containment safety guard still fires for a real worktree outside the declared root.markdownlint-cli2 on the CHANGELOG: 0 errors.
plugin.jsonvalid, version bumped0.15.1→0.15.2(patch; internal bug fix, no config surface change).Closes #438
Related
Co-authored-by: Claude Opus 4.8 noreply@anthropic.com