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

[kyle-sexton]

Fixes #2389.

Emits worktree_lock=armed|unarmed on stderr after the lock attempt. Lock failure remains best-effort (warn + exit 0) but is no longer silent to machines.

Tests: worktree-create.test.sh.

Related

• source-control: worktree-create arms the #2257 liveness lock best-effort — a lock failure warns to stderr, exits 0, and leaves the guard silently unarmed #2389 — worktree-create liveness guard lock signal

[cursor]

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

[claude]

Claude finished @kyle-sexton's task in 42s —— View job

---

Security Review

• Gather context (PR diff, changed files)
• Review changed files for security issues
• Post findings

Scope: plugins/source-control/.claude-plugin/plugin.json, plugins/source-control/CHANGELOG.md, plugins/source-control/scripts/worktree-create.sh, plugins/source-control/scripts/worktree-create.test.sh (diff vs origin/main).

Findings: none.

The functional change is minimal and low-risk:

• worktree-create.sh#L668-L671 sets worktree_lock_state to one of two hardcoded literals (armed/unarmed) based on the exit status of git worktree lock, then emits it via printf '%s: worktree_lock=%s\n' "$PROG" "$worktree_lock_state".
• The emitted value is never derived from user-controlled input (branch name, paths, hostnames, etc.), so there's no injection surface (command, format-string, or otherwise) introduced by this change — it's a closed two-value enum, not interpolated attacker data.
• No changes to trust boundaries, permissions, credential handling, or CI/CD configuration. The version bump and CHANGELOG entries are metadata-only.
• Test changes only assert on the new stderr line's content in a local, ephemeral temp-dir fixture — no security implications.

No CRITICAL / IMPORTANT / SUGGESTION items to report.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b7c14a1438

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

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 👍 / 👎.

[chatgpt-codex-connector]

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 finished @kyle-sexton's task in 3m 43s —— View job

---

Code Review: worktree_lock signal on stderr

Reviewed the 4 changed files against REVIEW.md's code-review lane scope (correctness, error handling, tests, maintainability). This repo has no claude-security-review workflow found in the changed set, but nothing here has security implications regardless — it's a stderr diagnostic line.

Findings (posted inline on the relevant lines):

1. 🔴 CHANGELOG.md L6-13 — The added [0.53.3] entry describes a fail-closed fix to fetch_pull_request_commits (source-control:babysit-prs: the signature walk inherits the PR-commits endpoint's 250-commit cap, making its own 'may only ever over-report' invariant false above 250 #2387) that does not exist in this codebase. I checked fetch_pull_request_commits in babysit_gh.py L476-502 directly — it still just returns the raw commit list with no comparison to the PR's commits count and no raise. No commit in the repo's history references source-control:babysit-prs: the signature walk inherits the PR-commits endpoint's 250-commit cap, making its own 'may only ever over-report' invariant false above 250 #2387 or that function. This looks like an unrelated/orphaned changelog entry bundled into this PR (also explains the 0.53.2 → 0.53.4 version skip in plugin.json, never actually shipping 0.53.3). Needs to be dropped, or the described fix needs to actually be implemented.
2. 🟡 worktree-create.sh L706-707 — The new worktree_lock= signal is only emitted on the final success path. If the later .worktreeinclude copy step fails and exits 4 (L699-702), the signal is silently dropped — exactly the case where a caller most needs to know the lock state, since the worktree already exists on disk. Emitting it right after the lock attempt (~L672) would make it reliable across all exit paths.
3. 🟡 worktree-create.test.sh L773-778 — Only the armed path is tested. No test exercises the unarmed (lock-failure) branch, which is the actual behavior this PR adds visibility for.

Everything else — the core change (a two-line stderr addition gated on the existing lock-attempt result), the PROG: prefix convention, and stdout ($worktree_path) being left untouched — is correct and consistent with the surrounding script's conventions.

I did not run the test suite myself; the PR description's worktree-create.test.sh claim is author-reported and unverified by me.

[claude]

🔴 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 →

[claude]

🟡 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.

[claude]

🟡 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.

[github-actions]

Claude has reviewed this PR 1 time. The lane skips further automatic reviews after 5; deleting this comment resets the count.

[kyle-sexton]

Closing as duplicate of #2394, which merged the #2389 worktree-lock failure signal (exit 5 + lock_failed=1). The armed/unarmed stderr approach here overlaps that fix.