Skip to content

fix(source-control): emit worktree_lock state when arming liveness guard #2399

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
kyle-sexton wants to merge 1 commit into from
Closed

fix(source-control): emit worktree_lock state when arming liveness guard #2399

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion plugins/source-control/.claude-plugin/plugin.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"$schema": "https://json.schemastore.org/claude-code-plugin-manifest.json",
"name": "source-control",
"version": "0.53.2",
"version": "0.53.4",
"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 \u2014 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 \u2014 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 \u2014 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 \u2014 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",
Expand Down
16 changes: 16 additions & 0 deletions plugins/source-control/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,22 @@
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.53.3]

### Fixed

- **`fetch_pull_request_commits` fails closed when the pull-commits walk is shorter than the PR's
`commits` count (#2387).** GitHub caps that endpoint at 250 commits regardless of pagination; the
helper now compares against the PR metadata and raises instead of returning a silently truncated
list.
Comment on lines +10 to +13

@chatgpt-codex-connector chatgpt-codex-connector Bot Aug 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Ship the implementation claimed by the 0.53.3 entry

The 0.53.4 artifact now advertises the #2387 fail-closed behavior, but skills/babysit-prs/scripts/babysit_gh.py::fetch_pull_request_commits still only returns the paginated /pulls/{number}/commits rows and never reads or compares the PR's commits metadata. Consequently a PR with more than GitHub's 250-commit endpoint cap is still silently truncated, so evaluate_required_signatures can omit unverified commits while users of this released version are told that case is held closed.

Useful? React with 👍 / 👎.

Comment on lines +6 to +13

@claude claude Bot Aug 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Important: This [0.53.3] entry describes a fix to fetch_pull_request_commits ("raises instead of returning a silently truncated list", #2387) that isn't in this diff and doesn't exist anywhere in this branch's history — fetch_pull_request_commits in plugins/source-control/skills/babysit-prs/scripts/babysit_gh.py (lines 476-502) still just returns out with no comparison against the PR's commits metadata and no raise. No commit in this repo's history mentions #2387 or fetch_pull_request_commits.

This entry is unrelated to the PR's stated purpose (issue #2389, worktree lock signal) and appears to be an erroneous/orphaned addition — possibly a rebase or merge artifact. It also explains why plugin.json jumps straight from 0.53.2 to 0.53.4, skipping 0.53.3 as an actual shipped version.

Either drop this block (if it doesn't belong in this PR) or, if the #2387 fix genuinely needs to land, implement it in babysit_gh.py as described. Shipping a changelog entry for a fix that was never made is misleading to anyone reading the changelog as a source of truth.

Fix this →


## [0.53.4]

### Fixed

- **`worktree-create.sh` emits `worktree_lock=armed|unarmed` on stderr (#2389).** Lock failures
still warn and exit 0, but orchestrators can see when the liveness guard did not arm.

## [0.53.2]

### Fixed
Expand Down
3 changes: 3 additions & 0 deletions plugins/source-control/scripts/worktree-create.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -665,7 +665,9 @@ fi
# absent (#2257). The owning lane (or cleanup, after explicit confirmation)
# disarms with `git worktree unlock <path>`.
lock_reason="worktree-create.sh: lane active on ${HOSTNAME:-$(hostname 2>/dev/null || printf 'unknown-host')} since $(date -u +%Y-%m-%dT%H:%M:%SZ); unlock when the owning lane is done"
worktree_lock_state=armed
if ! git -C "$toplevel" worktree lock --reason "$lock_reason" "$worktree_path" >&2; then
worktree_lock_state=unarmed
printf '%s: warning: could not lock the new worktree — cleanup sweeps will not see it as claimed\n' "$PROG" >&2
fi

Expand Down Expand Up @@ -702,4 +704,5 @@ if [[ -f "$include_file" ]]; then
fi

printf '%s: created worktree on branch %q (base %s)\n' "$PROG" "$name" "$base_ref" >&2
printf '%s: worktree_lock=%s\n' "$PROG" "$worktree_lock_state" >&2

@chatgpt-codex-connector chatgpt-codex-connector Bot Aug 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Emit the lock state before later copy failures can exit

When a matched .worktreeinclude file cannot be copied, the helper exits 4 at the copy-failure branch after the worktree and lock attempt have already occurred, so this final marker is never emitted. That leaves an orchestrator without the new worktree_lock state precisely when a partially provisioned worktree remains on disk; emit the state immediately after the lock attempt rather than only on the all-copies-succeeded path.

Useful? React with 👍 / 👎.

@claude claude Bot Aug 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Nit: The worktree_lock= signal is only emitted on the final success path. If the .worktreeinclude copy step fails (line 699-702, exit 4), the script exits before reaching this printf — so the exact case where an orchestrator most needs to know the lock state (the worktree already exists on disk, and the script is exiting non-zero) never gets it.

worktree_lock_state is already known right after the lock attempt (line 668-672). Emitting it there — or at every exit point once the worktree exists — would make the signal reliable regardless of which later step fails, rather than only on the happy path.

printf '%s\n' "$worktree_path"
3 changes: 2 additions & 1 deletion plugins/source-control/scripts/worktree-create.test.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -770,8 +770,9 @@ assert_file_absent "--root-file NUL-collapsed path never materialized" \
repo=$(mkrepo --origin "git@github.com:acme/widget.git")
root_file="$TEST_TMPDIR/rootfile-lock"
printf '%s' "$TEST_TMPDIR/wtroot18-lock" > "$root_file"
out=$(bash "$HELPER" --name feat/liveness --root-file "$root_file" --repo-dir "$repo" 2>/dev/null)
out=$(bash "$HELPER" --name feat/liveness --root-file "$root_file" --repo-dir "$repo" 2>"$TEST_TMPDIR/wt-lock-stderr")
assert_exit "lock case: creation succeeds (exit 0)" 0 "$?"
assert_contains "lock state is machine-readable on stderr" "$(cat "$TEST_TMPDIR/wt-lock-stderr")" "worktree_lock=armed"

@claude claude Bot Aug 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Nit: This test only covers the armed happy path for the new machine-readable signal. There's no test asserting worktree_lock=unarmed is emitted when git worktree lock fails (the actual case this PR's stated purpose — "no longer silent to machines" — is about, since the armed case was already visible via worktree list --porcelain). Consider adding a fixture that forces the lock to fail (e.g. pre-locking the path, or another mechanism already used elsewhere in this suite) and asserting on the unarmed stderr line and the existing warning message together.

stanza=$(git -C "$repo" worktree list --porcelain | awk -v RS= -v p="acme-widget-feat-liveness" 'index($0, p)')
assert_contains "the new worktree is locked at creation" "$stanza" "locked"
assert_contains "the lock reason names the arming helper" "$stanza" "worktree-create.sh"
Expand Down
Loading