feat(source-control): landed-vs-stranded worktree detection

[kyle-sexton]

Summary

A worktree that is old, quiet, and clean looks exactly like a worktree holding four unpushed commits that exist nowhere else. Nothing in this repo could tell those apart, and /worktree cleanup was happy to remove either — then emit a git branch -D that finished the job. This PR is the detection that separates them, the guards that consume it, and the hook that stops the placement problem at its source.

The engine — plugins/source-control/scripts/landed-work.sh, read-only, no network. One TSV row per registered worktree: unpushed, landed, the method and base SHA the verdict was reached with, the in-progress sequencer operation, four independent working-tree counts, peer worktrees, a risk class, and a reason.

Only affirmative proof yields landed=yes. Every failed command, empty result set, unresolvable base, and ambiguity yields ?, and every consumer treats ? exactly as no. A false no costs a confirmation prompt; a false yes destroys work.

Each method choice is a measured rejection of an obvious alternative:

• Unpushed set is HEAD --not --remotes. --branches reports every other branch in the repository and nothing about a detached worktree's own commits — the one case where removal is immediately terminal. @{upstream}..HEAD returns nothing at all for a locally created branch, which described all 17 worktrees this work started from.
• Landedness is decided by RANGE patch-id first. A squash-merge collapses N commits into one patch, so no per-commit primitive — git cherry included — can ever match it, while the branch's range id equals the squash commit's and stays matched as the base advances.
• Patch ids are computed --verbatim. The default and --stable hash the patch after stripping whitespace, so a b and ab produce one id (git 2.54: both 7ad14294…). The cost is that an EOL-renormalized branch no longer matches and reports no — a confirmation prompt in exchange for a silent deletion.
• No verdict from an incomplete patch-id set. A commit producing no patch (an empty commit among them) is invisible to patch-id, so the id count must equal the non-merge commit count first.
• The two-dot fallback carries no direction test. git diff base..HEAD reports deletions both for a branch merely BEHIND the base and for a branch whose own unique work IS a deletion; the numstat rows are identical, so "additions are zero" classified a delete-only branch as landed.
• Work-tree-root-ness is probed with rev-parse --show-prefix. --is-inside-work-tree returns true for a leftover directory inside a repository and reports that repository's clean state as the directory's own.
• In-progress state goes through rev-parse --git-path, the only form that resolves the per-worktree vs common-dir split for a linked worktree.
• Every field is emitted non-empty (- for absent). Tab is IFS whitespace, so an empty field shifts every later column left for a tab-splitting read consumer — the shape this plugin's own prose tells callers to use.

cygpath is not used — the path key follows worktree-create.sh:247's recorded rejection and its remedy, extended with the drive-letter fold that path_key() in the audit plugin never needed because both of its operands come from one source.

The consumers. status gains a Work axis classified ahead of Status, so a worktree with unpushed unlanded commits is stranded rather than merely stale, and the summary names the at-risk commit total. cleanup guards both places work dies — the pre-removal site AND the git branch -D it emits one step later, which is where commits actually die since removal leaves the branch ref intact. Two pre-removal guards with a stated order, --acknowledge-stranded per worktree rather than a bare --force answering a different question, and every path offers git -C <path> push -u origin HEAD first.

The hook. worktree-create-gate covers the three creation paths that bypass /worktree create entirely — claude --worktree, a subagent with isolation: "worktree", and a background session — by delegating to the same helper. It was deferred pending two unanswered questions about the WorktreeCreate event; both were measured, so it ships. A user-scope hook does fire (settings.json under a CLAUDE_CONFIG_DIR, headless, before login was even resolved), ${CLAUDE_PROJECT_DIR} resolves to the project root the session started in rather than the worktree being created, and stdout's last non-empty line is taken as the path — a banner line before the path still succeeds, refuting the claim that any other output fails the session.

repo-fleet-hygiene:audit gains three findings computed natively rather than copied: worktree-not-a-root, worktree-root-unverifiable, and worktree-nested-in-repository.

Also fixed: the nesting invariant's as-of stamp was 2.1.220 / 2026-07-31 and its recheck trigger cited two issues that are both CLOSED (verified live: #29599 duplicate/COMPLETED, #23565 NOT_PLANNED). It now names #16600 (OPEN) and states the gap that leaves — #16600 concerns memory files, which 2.1.224 already handles correctly, so the surface still leaking has no open upstream issue.

Independent review

Routed to four independent reviewers — three fresh-context, one cross-vendor (Codex) — over two rounds. Between them they returned eighteen findings; every real one is fixed in this PR rather than filed. The Codex pass alone found eight, all in the direction that loses work:

Defect	Effect before the fix
Direction test on the two-dot fallback	A delete-only branch classified landed
patch-id --stable strips whitespace	a b and ab hashed alike → false landed
--name-only and --numstat disagreed on core.quotePath	A non-ASCII path joined against nothing → matched=0 → false landed
Patch-id set only checked non-empty	An empty commit made the set under-represent the branch
comm exit status unchecked	A failed comm emits empty stdout — the "all landed" shape
awk result read into an unvalidated variable	Empty compared numerically as zero → false landed
git status exit status unchecked	An unreadable index reported as a clean tree
Enumeration streamed through a process substitution	A truncated list that every downstream count agreed with
Ambiguous base ref / criss-cross merge base	Silently disambiguated to one, possibly the wrong history

Two further findings were reachable only by exporting shell-function overrides into the script's environment — not a defensible threat model — but the underlying unchecked exit statuses were real on their own terms and are among the fixes above.

The second round, run against the already-fixed code, found more:

Defect	Effect before the fix
The quoting fix was incomplete — ", \, and control characters are escaped regardless of core.quotePath	The same vacuous empty-join → false landed, on a narrower character class. Live on Linux/macOS; not reproducible on Windows, where core.protectNTFS refuses such paths
cleanup.md's guard covered 4 of 9 risk values; status.md's table covered 7 of 9	An agent had no instruction for in-progress or dirty and could read the silence either way
The - placeholder was undocumented	An agent told to "present the base stamp" would show a literal - as the base
A bare hub's --show-toplevel fails by design	The placement check silently never ran for any worktree under it, with no finding to say so
worktree-root-unverifiable had no test	The collector's probe-failure branch was dead code as far as the suite was concerned
One handoff row claimed worktree-root-unverifiable proves what only worktree-not-a-root proves	Overclaimed evidence; the recommended action was already correct
Base-side patch-id completeness had no count check	Inert — an under-complete base set only biases toward no — but asymmetric with the branch side

The first of those retired the text-matching approach entirely: the touched paths are now handed back to git as :(literal) pathspecs so git does its own matching, which removes the whole escaping-mismatch class rather than its current member.

Test plan

• landed-work.test.sh — 52 cases, exit 0. The discriminating ones: a multi-commit squash classifies landed and stays landed after the base advances; a genuinely unmerged branch does not; a delete-only branch does not; a whitespace-only difference does not; an empty commit yields ?; a criss-cross history never yields an affirmative verdict; a directory inside a repository is notgit; a bare hub is bare; a non-ASCII filename, one containing a glob metacharacter, and one beginning with : all classify STRANDED; and a tab-splitting read consumer lands on the columns it names. Fixtures pin core.autocrlf=false, because the Windows default normalizes CRLF into the object store and the EOL case would otherwise have passed for a reason unrelated to the classifier.
• worktree-create-gate.test.sh — 20 cases, exit 0.
• audit-fleet.test.sh — 86 cases, exit 0. Its tier-table drift gate caught the new worktree-placement-unverifiable kind before it could ship undocumented.
• Every other source-control suite — pr-body-linkage-gate, pr-linkage-mcp-gate, babysit-wrapper-help, babysit-readiness-gate, fetch-all-pr-comments, worktree-create — all exit 0.
• Gates green: check-shell-portability.sh --paths, check-silent-skips.sh, check-hook-userconfig-argv.sh, check-orphaned-fixtures.sh --check, check-cross-plugin-source-drift.sh, check-contract-slice-prune.sh --check, check-skill-leaf-names.sh, check-skill-portability.sh --paths, check-changed-skills.sh origin/main (both skills PASS, 0 errors), check-changelog-parity.sh --check and --check-bump origin/main, validate-plugins.sh, validate-plugin-contracts.mjs, markdownlint-cli2, shellcheck --rcfile .shellcheckrc -x, shfmt -d.
• The engine was run across the whole machine: 14 worktree rows over 13 checkouts, 0 not-ok.
• Full CI green on this branch, plugin-gate (the whole plugin suite) included.

Three pre-existing failures on clean main, none from this branch — claude-ops/.../fleet-state.test.sh, ruff-format/hooks/ruff-format.test.sh, and babysit-prs/scripts/engine.test.sh (597 unit tests OK; the exit code comes from its ruff lint pass). git diff --name-only origin/main...HEAD confirms this branch touches none of those files, and each was reproduced in isolation. Filed as #1972 rather than folded in here.

Related

Closes #1977.

Implements every phase of the stranded-work plan: the detection engine, the status and cleanup guards, placement drift, the surface pass, the corrected nesting-invariant citations, and the WorktreeCreate hook the plan had deferred as blocked.

The dotfiles half — granting the worktree root, retiring ~/.claude-loop-worktrees, the Codex pointer, the Cursor density capture — is melodic-software/dotfiles#410 and needs a human-run deploy step.

Filed alongside: melodic-software/standards#334 (a git restore <path> deny asymmetry), #1971 (whether the path-scoped rule leak deserves an upstream issue), #1972 (the three red suites on main), #1976 (the 52 stranded/* quarantine refs — 50 of which hold content that exists nowhere else).

[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 6m 26s —— View job

---

Code review

Reviewed the diff against origin/main (16 files, +1404/-19), per this org's REVIEW.md code-review-lane scope (correctness, design, error handling, tests, docs — no security lane exists in this repo, so I checked the security-adjacent items too and found nothing to flag there).

The architecture is sound: fail-closed by construction (? treated as no everywhere), the method choices in landed-work.sh are each backed by a stated rejection of an obvious alternative, and the two-guard-site design in cleanup.md correctly identifies that git branch -D — not git worktree remove — is where a stranded commit actually dies. Test coverage is substantial (40 + 84 cases) and the four discriminating cases called out in the PR body are real discriminators, not padding.

🔴 Important — core.quotepath mismatch can misclassify stranded work as landed=yes

plugins/source-control/scripts/landed-work.sh#L416-L434

In the two-dot fallback inside classify_landed(), the touched set and the numstat set are computed with different core.quotepath settings:

if ! git -C "$p" diff --name-only --no-renames "$mb..HEAD" 2>/dev/null | sort -u >"$touched"; then
...
if ! git -C "$p" -c core.quotepath=false diff --numstat --no-renames "$base_sha..HEAD" >"$numstat" 2>/dev/null; then

touched uses git's default core.quotePath=true (paths containing bytes ≥ 0x80 are quoted/octal-escaped), while numstat explicitly overrides it to false (raw UTF‑8, unquoted). A file whose name contains non-ASCII bytes therefore appears in $touched in quoted form and in $numstat's $3 column in unquoted form — they never match in the awk join at line 430-434. That makes matched undercount, and in the case where the only files reaching this fallback have non-ASCII names, matched hits exactly 0:

if [[ "$matched" -eq 0 ]]; then
  L_STATE="yes"
  L_METHOD="two-dot-empty"
  return 0
fi

— i.e. a branch whose changes genuinely aren't on the base gets reported landed=yes purely because its touched file has a non-ASCII name. This is the exact failure direction the file's own docstring calls out as the one that must never happen ("a false yes destroys work," lines 17-21). Every other place in this script that computes two related path sets from git (e.g. wt_path/canonical_top in the sibling audit-fleet.sh change) takes care that both operands come from the same invocation shape; this is the one place that doesn't.

This path is only reachable when patch-id hasn't already resolved landedness (patch-id itself is unaffected, since both its base- and branch-side git log -p calls share the same default quotepath and stay internally consistent) — so it's a real but narrower window: a genuinely-unlanded branch whose only touched files have non-ASCII names, or a mixed branch where the non-ASCII file is what would have made adds non-zero.

Fix: add -c core.quotepath=false to the touched-paths git diff call too (line 416), so both sides quote identically. No existing fixture in landed-work.test.sh uses a non-ASCII filename, so this gap isn't caught by the 40-case suite — worth a regression case alongside the EOL-renormalization one, since it's testing the same fallback path.

Fix this →

(Note: I couldn't execute a live repro in this sandboxed review environment — git init/script execution on scratch paths was blocked — so this is reasoned from git's documented core.quotePath default and the visible asymmetry in the two invocations, not an executed reproduction. Worth a quick manual check before treating it as confirmed.)

🟡 Nit — silent skip on canonical_top resolution failure

plugins/repo-fleet-hygiene/skills/audit/scripts/audit-fleet.sh#L1022-L1023

local canonical_top
canonical_top="$(run_git_probe -C "$canonical" rev-parse --show-toplevel 2>/dev/null | tr -d '\r')"

Unlike the sibling probes in this function (expected_common at line 1011-1016, wt_prefix at line 1094-1099), a failed rev-parse --show-toplevel here isn't reported — canonical_top just ends up empty, and the worktree-nested-in-repository check at line 1110 silently no-ops (-n "$canonical_top" guards it out) instead of emitting an UNKNOWN finding the way every other degradation path in this file does. In practice the window is narrow: canonical already passed an identical --show-toplevel probe in add_target() and a --git-common-dir probe earlier in this same function, so this would only fire on a repo that became unreachable mid-run. Given how deliberately this PR fails closed everywhere else (and that check-silent-skips.sh is part of this repo's own gate), it's worth either emitting an UNKNOWN/worktree-placement-unverifiable-style finding here too, or a one-line comment noting why this specific probe is exempt.

Everything else

• landed-work.sh's memoization key ($mb.$base_sha), the row-count assertion (both mid-run and at emission), the bare-hub short-circuit, and the peer detection's ancestry direction all check out against their stated rationale.
• The audit-fleet.sh worktree-not-a-root / worktree-nested-in-repository ordering is correct — a not-a-root path continues before reaching the placement test, so a husk directory can't also be reported as nested, and audit-fleet.test.sh's assert_kind_targets helper correctly proves that with a scoped (not whole-file) match.
• Docs (SKILL.md, context/status.md, context/cleanup.md, context/audit.md) are consistent with the engine's actual risk vocabulary (landed/STRANDED/UNKNOWN/superseded/notgit/bare) and with each other.
• CHANGELOG and version bumps (0.46.2→0.47.0, 0.8.1→0.9.0) match the added surface.

The PR body's own review note flags that this branch hasn't had independent review yet — this pass is that independent read. Recommend fixing the quotepath issue before merge given the stated fail-closed design goal; the audit-fleet.sh nit is optional.

[claude]

Claude finished @kyle-sexton's task in 2m 17s —— View job

---

Security review — no findings

Todo list

• Gather context: fetch PR diff, read changed files
• Review landed-work.sh (new 729-line engine) for command injection / path handling issues
• Review landed-work.test.sh for unsafe patterns
• Review audit-fleet.sh changes (new findings)
• Review skill/context markdown changes for anything security-relevant
• Review CHANGELOG/plugin.json bumps
• Synthesize findings and post final review

This PR was scoped to the 16 files changed vs origin/main, focused on the two shell scripts (plugins/source-control/scripts/landed-work.sh — new, 729 lines — and the additions to plugins/repo-fleet-hygiene/skills/audit/scripts/audit-fleet.sh), their test files, and the skill/context markdown. No GitHub Actions workflow files are touched, so no zizmor-covered surface applies here.

No security issues found.

Notes from the review (informational, not findings):

• landed-work.sh is read-only by construction — no fetch, checkout, or ref writes. All temp state lives under a mktemp -d private working directory (landed-work.sh#L528); nothing is written outside it.
• --merged-refs-file matches are done with grep -Fxq (fixed-string, whole-line) against caller-supplied branch names (landed-work.sh#L550) — no regex/glob injection surface.
• All git -C "$p" ... invocations pass $base/$p as literal argument values (never shell-interpolated into a command string), so there's no command-injection path even for unusual ref/path values.
• The audit-fleet.sh change extends the existing git_probe_allowed allowlist (audit-fleet.sh#L124-L131) with --show-prefix, which is read-only and operand-free like the existing --show-toplevel entry — consistent with the fail-closed allowlist pattern already in place, and no widening of what's permitted beyond that.
• The new worktree-nested-in-repository containment check (audit-fleet.sh#L1108-L1114) uses a bash [[ ... == prefix/* ]] glob match on path_key-normalized strings, not eval or a constructed regex — no injection surface there either.
• The markdown/skill-context changes (cleanup.md, status.md, SKILL.md, audit.md, create.md, evals.json) are prose instructions and eval fixtures for the orchestrating agent; they don't introduce new code execution paths.
• Fail-closed semantics are consistently applied: every ambiguous/failed probe resolves to ?/unknown, and the doc changes correctly state that consumers must treat unknown exactly as stranded/unsafe — this is a safety property, not something I could find a bypass for within this diff.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2770bcf681

ℹ️ 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".

[github-actions]

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

[claude]

Claude finished @kyle-sexton's task in 3m 9s —— View job

---

Security review

Scope: files changed in this PR vs origin/main (19 files, +1999/-19), re-reviewed at HEAD 2320ab7. Two earlier automated passes reviewed this PR at 2770bcf; this pass focuses on what changed since (the WorktreeCreate hook, and the "eight false-landed paths" rework of landed-work.sh) plus independent verification of what's still open.

• Gather context: git diff 2770bcf..2320ab7 and git diff origin/main...HEAD
• Review worktree-create-gate.sh (new hook) for injection / trust-boundary issues
• Review landed-work.sh's delta since the last review (quotepath fix, patch-id completeness, merge-base ambiguity, superseded classification)
• Review worktree-create.sh (unchanged, but newly reachable via the hook) for the --name validation path
• Confirm audit-fleet.sh / hook-utils.sh are untouched by the latest commit
• Synthesize findings

🔴 IMPORTANT — superseded is a name-only match with no identity binding to the merged PR

plugins/source-control/scripts/landed-work.sh#L654-L658 and #L815-L819

merged_ref() grants risk=superseded on a bare branch-name match against --merged-refs-file — no check that the file's headRefName entry corresponds to the commits currently on this branch. status.md#L32-L35 shows the file is populated from gh pr list --json headRefName filtered to merged PRs — again, name only, no head SHA.

superseded is then treated as fully safe by the consumer: cleanup.md#L78 says "risk=landed, ok, bare, or superseded → proceed" and cleanup.md#L113 authorizes the branch-deleting git branch -D for the same three risk classes, with no extra confirmation — the same treatment as a proven landed=yes.

Failure scenario: a branch name gets reused — an everyday git workflow (GitHub's "delete head branch on merge" followed by someone later creating a new branch called fix/lint or chore/deps for unrelated work). The old, merged PR's headRefName is still in the merged-refs file. The new branch's genuinely unpushed, unlanded commits classify landed=no (correctly — they aren't on base), then merged_ref("fix/lint") matches by name alone and the row is reported superseded. Cleanup then removes the worktree and runs git branch -D fix/lint, destroying commits that exist nowhere else — silently, with no confirmation prompt, exactly the "false yes destroys work" failure this entire engine was built to prevent (per the PR body's own stated invariant).

This was already flagged independently by the Codex review on this PR ("Require current-head evidence before marking work superseded") and is not addressed by the latest commit (2770bcf..2320ab7 does not touch merged_ref() or the superseded branch of the risk classifier) — confirmed by diff, not assumed.

Fix direction: bind the merged-refs evidence to a revision, not just a name — e.g. collect headRefName<TAB>headRefOid (or mergeCommit SHA) instead of bare names, and require the branch's current tip (or its merge-base-derived content) to actually be an ancestor of / match the recorded merged commit before granting superseded. A name match alone proves the name was once merged, not that this branch's current commits were.

Confidence: High — verified directly from the current script and docs at HEAD, not inferred.

Everything else checked — no new findings

• worktree-create-gate.sh (new): name/cwd are read from the WorktreeCreate JSON payload via jq (with a bounded sed fallback) and passed to worktree-create.sh as discrete argv elements, never shell-interpolated — no command-injection surface. worktree-create.sh's existing --name validation (regex character-class check, 64-char cap, then git check-ref-format --branch before any path use) rejects ..-bearing or otherwise malformed names before they reach path construction; this file is unchanged by the PR but is newly reachable through the hook, and the validation holds under that new entry point.
• The --root/--data-root-file handoff never substitutes ${user_config.*} into shell source (uses a mktemp file channel instead, per the script's own stated rationale for avoiding exactly that class of bug); the temp file is cleaned up via trap ... EXIT.
• The core.quotepath mismatch flagged in the prior review round (two path sets computed under different quoting, letting a non-ASCII filename undercount to a false landed=yes) is now fixed consistently — both the touched and numstat diffs pin -c core.quotepath=false (landed-work.sh#L505, #L514).
• The Codex P1 finding about zero-additions two-dot fallback misclassifying a delete-only unlanded branch as landed=yes is fixed by removing two-dot-direction's additions-based verdict entirely — the fallback now only ever answers "identical to base" (two-dot-empty) or "differs" (two-dot), never infers landedness from addition count.
• New completeness/ambiguity guards (patchid-set-incomplete-*, ambiguous-merge-base-*-candidates, base-ref-ambiguous, two-dot-count-unreadable, status_counts now returning failure instead of silently reporting zero counts) are all fail-closed — each new failure mode routes to ?/refusal rather than a default that could read as safe.
• audit-fleet.sh and hook-utils.sh are untouched by the latest commit (verified via git diff 2770bcf..2320ab7); the security posture already reviewed for those files at the prior commit stands unchanged.
• plugin.json's only change is the new worktree_create_gate_enabled boolean userConfig option — no permission widening, no new egress, no .mcp.json/trust changes.
• No GitHub Actions workflow files are touched; zizmor-covered surface (unpinned actions, dangerous triggers, permission widening, template injection) doesn't apply here.

Recommend fixing the superseded name-only match before merge — the failure mode is exactly the class of data loss this PR exists to prevent, and it's reachable through an ordinary branch-naming workflow rather than an edge case.

[claude]

Claude finished @kyle-sexton's task in 4m 59s —— View job

---

Code review

• Gather context (diff, CLAUDE.md, prior review comments)
• Review new commits since last Claude review (2770bcf → 2320ab7)
• Check whether flagged issues (quotepath mismatch, Codex P1s) were addressed
• Review remaining changed files for correctness, error handling, tests
• Post final review

Scoped to what changed since the last full review (commit 2770bcf): commit 2320ab7 ("close eight false-landed paths, and add the WorktreeCreate gate"). audit-fleet.sh and the other files already reviewed are unchanged since then, so I didn't re-review them.

The eight fixes in 2320ab7 are good and each closes a real hole: the quotepath mismatch I flagged in the prior review is fixed (both diffs now pinned to core.quotepath=false with -z, landed-work.sh#L495-L502), and Codex's zero-additions P1 is fixed by removing the direction test entirely rather than patching it (landed-work.sh#L474-L487) — the right call, since the case it existed for is already covered by range patch-id. --verbatim patch-ids, the completeness check against --no-merges count, and checking comm/awk/git status exit codes explicitly are all real fail-closed improvements with fixtures behind them.

🔴 Not fixed — superseded still matches on branch name alone, not a specific revision

landed-work.sh#L654-L658

merged_ref() {
  local name="$1"
  [[ -n "$MERGED_REFS_FILE" && -n "$name" && "$name" != "(detached)" ]] || return 1
  grep -Fxq "$name" "$MERGED_REFS_FILE"
}

This is the second P1 Codex flagged on this PR and it's still open — 2320ab7's summary says all eight review findings were fixed, but this was a separate review-comment thread, not one of the eight. MERGED_REFS_FILE is populated from gh pr list --json headRefName (status.md#L32) — branch names, not commit SHAs — so merged_ref proves only "a PR with this head ref name was merged at some point," never "the commits currently on this branch are the ones that PR merged."

Concretely: branch fix-bug gets merged and closed. Weeks later someone reuses the local name fix-bug for unrelated work, with unpushed commits whose content is genuinely not on base (classify_landed correctly returns landed=no). merged_ref("fix-bug") still matches, so the row is classified risk=superseded instead of STRANDED. cleanup.md treats superseded as safe at both guard sites this PR added — the pre-removal check (cleanup.md#L78) and the git branch -D emission (cleanup.md#L113) — so this is the one classification path in the whole engine where genuinely unlanded, unpushed commits reach git branch -D with no prompt. That's the exact failure direction the file's own docstring says must never happen.

No fixture in landed-work.test.sh exercises branch-name reuse (grep -n superseded landed-work.test.sh shows one case, and it doesn't touch this). A minimal fix: have the caller pass headRefName<TAB>headRefOid instead of bare names, and check the branch's merge-base or current tip against that OID rather than matching on name alone. If that's out of scope for this PR, it should at least be called out as a known limitation in the --merged-refs-file help text and status.md/cleanup.md, since right now it reads as a solved case.

Fix this →

🟡 Needs verification — WorktreeCreate payload field name

worktree-create-gate.sh#L88 reads the worktree name as .name, and the docstring above it (lines 13-16) states this contract was "measured on 2.1.224, not inferred." I tried to independently verify the current field name against https://code.claude.com/docs/en/hooks per CLAUDE.md's fresh-docs mandate (this is a hook-contract change, squarely in scope), but WebFetch gave me inconsistent results across three attempts on this page — one response fabricated a full worktree_name/worktree_reason/base_branch schema that a follow-up literal-string search against the same page could not find at all — so I can't confirm the docs either way from this environment, and I'm not treating the fabricated result as evidence. I can't execute the test suite here either (sandboxed) to check it against a real harness payload.

I'm flagging this only because the failure mode is severe if the field name is wrong: json_field name would always return empty, hitting the -z "$name" refusal at line 91, which fails creation outright (fail-closed by design) for every claude --worktree, isolation: "worktree", and background-session worktree creation — not a degraded fallback, a hard break of those three paths. Given the PR's own claim is "measured," this is likely just a case I couldn't corroborate in this sandbox rather than a real bug — worth a final sanity check against a live payload capture (or the docs page directly) before merge, given the blast radius.

Everything else

• status_counts, the comm/awk failure handling, the ambiguous-base and criss-cross merge-base checks, and the -z-delimited worktree-list enumeration all check out against their stated rationale, and each has a fixture that fails against the prior implementation per the PR body.
• worktree-create-gate.sh's fail-closed structure (disabled → exit 0 silently by construction since stdout is the path channel; every other failure → exit 1) is correctly reasoned given the constraint that this event has no non-corrupting notice channel.
• worktree-create-gate.test.sh is genuinely black-box and covers the refusal paths well (no root + no data dir, illegal branch name, empty .name, field-order independence). It does encode the same .name assumption as the hook itself, so it can't catch a field-name mismatch against the real harness — see above.
• Version bump (0.46.2 → 0.47.0) and CHANGELOG match the added surface.

From the previous review, still open and not touched by this commit: the canonical_top silent-skip nit in audit-fleet.sh (optional, unchanged since last review).

[claude]

Claude finished @kyle-sexton's task in 3m 28s —— View job

---

Code review

• Gather context (diff, prior review threads, CLAUDE.md/REVIEW.md)
• Identify what changed since the last reviewed commit (2320ab7 → 8fca7cd)
• Check whether the open superseded name-only-match finding was addressed
• Check whether the canonical_top silent-skip nit was addressed
• Review the new commit's own diff for correctness/tests
• Post final review

Scoped to what changed since the last full review round: commit 8fca7cd ("never emit an empty TSV field"), which touches only landed-work.sh and landed-work.test.sh. Per REVIEW.md's lane split, security-adjacent items are left to the dedicated security lane.

The stated goal of 8fca7cd is good and mostly delivered: 12 of 15 TSV fields now fall back to - when empty, closing the "empty field shifts every later column left under IFS=$'\t' read" failure this file's own docstring warns about, and the new test (landed-work.test.sh ~L478-486) correctly proves it by reading a row the natural way and asserting no field is empty.

🔴 One field was missed — head can still be emitted empty, undermining the fix it ships next to

landed-work.sh#L843-L844

printf '%s\t%s\t%s\t%s\t%s\t%s\t%s\t%s\t%s\t%s\t%s\t%s\t%s\t%s\t%s\n' \
  "${T_PATH[$idx]:--}" "${T_BRANCH[$idx]:--}" "${T_HEAD[$idx]:0:12}" "${R_UNPUSHED[$idx]:--}" \

Every field got a :-- fallback except head (field 3), which uses substring expansion (:0:12) with no fallback. Bash's parameter expansion can't combine a default value and a substring in one ${...} — so this field is silently exempt from the invariant the rest of the commit establishes. Substring-of-empty is empty (${v:0:12} on an unset/empty v yields ""), so whenever T_HEAD[$idx] is empty at print time, this field prints as nothing — the exact "empty field shifts every later column left" bug the commit's own comment (L837-842) describes, just relocated to column 3 instead of fixed everywhere.

Two concrete, reachable paths hit this — not a hypothetical:

1. The bare-hub row, a normal, tested, first-class case (not an edge case): the script's own comment says a bare hub "carries no HEAD" (landed-work.sh#L691), and collect_targets initializes head="" and never gets a "HEAD " porcelain line for that entry, so T_HEAD[idx] stays "" all the way to the final printf (the bare branch continues at L707 before the L724-726 rev-parse HEAD fill-in ever runs).
2. A --worktree <path> explicit target that isn't a worktree root (the notgit path, L668-686): explicit targets start with T_HEAD+=("") (L565), and the notgit branch continues before the fill-in too.

Neither case is caught by the new regression test, which reads a healthy worktree's own row, and neither is caught by the existing bare-hub/notgit tests (landed-work.test.sh L377-386), which assert risk/landed/reason but never the head column — there's no C_HEAD constant in the test file at all.

Confidence: high on the bash semantics (well-defined: substring of an empty/unset variable is empty) and on both trigger paths, reasoned from the code and the script's own comments; I could not execute the script in this sandboxed review environment to produce a literal repro (same limitation earlier review rounds on this PR noted), so treat the exact output bytes as reasoned-not-executed if you want a final sanity check.

Fix: compute the truncated value into a local before the printf call so it can get the same :- fallback as everything else, e.g.:

head_short="${T_HEAD[$idx]:0:12}"
printf '...' ... "${head_short:--}" ...

and add a regression case (bare hub or notgit-via---worktree) that asserts the head column specifically, since that's the gap in the current suite.

Fix this →

Still open from earlier review rounds (unchanged by this commit)

• 🔴 merged_ref() still matches on branch name alone, no revision binding to the merged PR (landed-work.sh#L658-L662) — flagged by both Codex and a prior Claude review round; a reused branch name can get genuinely unlanded, unpushed commits classified superseded, which cleanup.md treats as safe for both worktree removal and git branch -D. Confirmed unchanged: this file's diff since 2320ab7 touches only the printf/fallback logic reviewed above.
• 🟡 canonical_top silent-skip nit in audit-fleet.sh (optional) — also unchanged, since audit-fleet.sh isn't part of this commit's diff at all.

Everything else in this commit

• The 12-field fallback and its test are correct and well-targeted; assert_not_contains ... "\t\t" is a reasonable belt-and-suspenders check on top of the column-read assertion.
• No test regressions introduced; the new assertions are additive to the existing worktree fixture rather than a new one, keeping the row-count invariant intact.

[claude]

Claude finished @kyle-sexton's task in 1m 0s —— View job

---

Security review

Scope: files changed in this PR vs origin/main, re-reviewed at HEAD 8fca7cd. Two commits have landed since the last security pass (at 2320ab7): 2320ab7..8fca7cd is "never emit an empty TSV field," touching only landed-work.sh and landed-work.test.sh.

• Diff 2320ab7..8fca7cd to see what actually changed
• Check whether the superseded name-only-match finding (flagged twice: Codex P1 + prior Claude security review) was fixed
• Review the TSV-field fix itself for any new injection/parsing surface
• Confirm no other files changed since last full review
• Post final findings

🔴 IMPORTANT — superseded is still a name-only match with no identity binding to the merged PR (unaddressed)

landed-work.sh#L658-L661

merged_ref() {
  local name="$1"
  [[ -n "$MERGED_REFS_FILE" && -n "$name" && "$name" != "(detached)" ]] || return 1
  grep -Fxq "$name" "$MERGED_REFS_FILE"
}

Verified against the diff: 2320ab7..8fca7cd touches only landed-work.sh's TSV-emission tail and its test file — it does not touch merged_ref() or the superseded branch of the risk classifier (landed-work.sh#L819-L823). This is the same P1 both Codex and the prior Claude security review (at 2320ab7) raised, and it remains open at HEAD.

MERGED_REFS_FILE is populated from gh pr list --json headRefName (status.md#L32) — branch names, not commit SHAs — so merged_ref proves only "a PR with this head ref name was merged at some point," never "the commits currently on this branch are the ones that PR merged." cleanup.md treats superseded as fully safe at both guard sites the PR adds — the pre-removal check (cleanup.md#L78) and the git branch -D emission (cleanup.md#L113) — with no extra confirmation, the same treatment as a proven landed=yes.

Failure scenario (unchanged from prior review): branch fix-bug gets merged and its worktree cleaned up. Weeks later someone reuses the local branch name fix-bug for unrelated work with genuinely unpushed, unlanded commits (classify_landed correctly returns landed=no). merged_ref("fix-bug") still matches by name alone, so the row is classified risk=superseded. Cleanup then removes the worktree and runs git branch -D fix-bug with no confirmation prompt, destroying commits that exist nowhere else — the exact "false yes destroys work" failure this engine exists to prevent, reachable through an ordinary branch-naming workflow rather than an edge case.

Fix direction (unchanged): bind the merged-refs evidence to a revision, not just a name — have the caller pass headRefName<TAB>headRefOid instead of bare names, and have merged_ref verify the branch's current tip or merge-base against that OID before granting superseded.

Confidence: High — verified directly against the code at HEAD 8fca7cd, not inferred from the earlier review.

The TSV-field fix (8fca7cd) itself — no security issue

The change adds :-- bash parameter-expansion defaults ("${T_PATH[$idx]:--}", etc.) so every emitted TSV field is - rather than empty, preventing column-shift misreads by while IFS=$'\t' read consumers. This is output-correctness hardening, not a security-relevant change: no new external input is parsed, no new shell interpolation is introduced (all :-- expansions are plain bash parameter substitution on already-computed internal variables, not command substitution), and the values being defaulted are the script's own internal classification state, not attacker-controlled data. Confirmed no new injection or trust-boundary surface.

Everything else

No other files changed since the last full security review at 2320ab7 (confirmed via git diff 2320ab7..8fca7cd --stat — only the two files above). The findings from that review — worktree-create-gate.sh's argv-based (non-interpolated) invocation, --name validation preserved through the new hook entry point, the fixed core.quotepath mismatch, and the fixed zero-additions two-dot fallback — all stand unchanged.

Recommend fixing the superseded name-only match before merge — it's now been raised three times across two independent reviewers and remains the one classification path where genuinely unlanded, unpushed commits can reach git branch -D with no prompt.

[claude]

Claude finished @kyle-sexton's task in 2m 31s —— View job

---

Code review

• Gather context: diff since last reviewed commit (8fca7cd → 38bedda)
• Check whether the open superseded name-only-match finding was addressed
• Check whether the head TSV-field-empty finding was addressed
• Check whether the canonical_top silent-skip nit was addressed
• Review remainder of the new commit's diff for correctness/tests
• Post final review

Scoped to what changed since the last full review round: commit 38bedda ("fix: close the remaining review findings across both plugins"), which touches landed-work.sh/.test.sh, audit-fleet.sh/.test.sh, SKILL.md, confidence-model.md, cleanup.md, status.md, and both CHANGELOGs. Per this org's REVIEW.md lane split, security findings are left to the dedicated security lane, but I still note where a previously-security-flagged item remains open since it bears on this lane's error-handling/test-coverage scope too.

This commit is good work and closes real gaps: the quotepath fix's remaining escaping gap (", \, control chars) is closed properly by switching to :(literal) pathspecs handed back to git rather than chasing byte classes in two independently-formatted diffs — the right fix, not a patch. The cleanup.md/status.md risk-value dichotomy gap (in-progress/dirty falling through the stop/proceed split, and the new "any unlisted value → STRANDED/unknown" closed-list default) is a real correctness fix with good judgment on the fail-closed default. The bare-hub placement-check skip in audit-fleet.sh is fixed correctly — --is-bare-repository distinguishes a legitimate skip from worktree-placement-unverifiable, and the worktree-root-unverifiable dead-code test-coverage gap is closed with a real failing-probe fixture. The canonical_top silent-skip nit from an earlier review round is also resolved by this same change.

🔴 Still open — merged_ref() remains a name-only match (raised 3 times, not part of this commit)

landed-work.sh#L679-L683 / #L840

Verified against the diff: 8fca7cd..38bedda does not touch merged_ref() or the superseded branch of the risk classifier. This is the Codex P1 ("Require current-head evidence before marking work superseded") and the finding raised in each of the last three review rounds — it's still a bare-name grep -Fxq against --merged-refs-file, with no binding to the revision the merged PR actually contained. cleanup.md#L78 and #L118 still treat superseded as fully safe for both worktree removal and the git branch -D that follows, no confirmation. The commit message states "every finding is acted on here; none is deferred," attributing the four fixes to "engine review," "prose review," and "audit review" — this finding (raised by the security lane and by Codex, a general-purpose bot) doesn't appear to have been in scope for this round, but from a correctness/test-coverage standpoint it's worth flagging explicitly as still outstanding rather than implicitly resolved by this commit's summary language. landed-work.test.sh still has no branch-name-reuse fixture (grep -n superseded shows the one existing case, unchanged, and it doesn't exercise reuse).

🔴 Still open — the head TSV field can still print empty (flagged last round, not touched by this commit)

landed-work.sh#L865

"${T_PATH[$idx]:--}" "${T_BRANCH[$idx]:--}" "${T_HEAD[$idx]:0:12}" "${R_UNPUSHED[$idx]:--}" \

Every other field in this printf got a :-- fallback in 8fca7cd; head (column 3) still has none, because substring expansion can't be combined with a default in one ${...} — confirmed unchanged by this commit's diff. T_HEAD[$idx] is empty at print time for the bare-hub row (R_BARE=yes, #L710-730, which continues before any HEAD fill-in) and for a --worktree <path> target that fails is_worktree_root (R_NOTGIT=yes, #L689-708). Both are ordinary, tested rows — not edge cases — and neither the existing bare-hub/notgit fixtures nor the new 8fca7cd regression test assert the head column, so this reintroduces the exact "empty field shifts every later column left under IFS=$'\t' read" bug that commit was written to close, just for one column. Same fix as before: compute head_short="${T_HEAD[$idx]:0:12}" first, then "${head_short:--}" in the printf.

🟡 Nit — commit/CHANGELOG claim a fixture that isn't in the diff

landed-work.test.sh#L258-269

Both the commit message ("a non-ASCII filename, a filename containing a glob metacharacter, and one beginning with :") and the CHANGELOG entry describe three path-spelling fixtures the :(literal) fix needed to handle. The actual diff only adds two: café.txt (non-ASCII) and star[1].txt (glob metacharacter). No fixture file begins with :, even though that's the case the code comment at #L529-L531 specifically calls out as a reason for :(literal) (":foo" would otherwise be read as pathspec magic rather than a literal filename). Worth adding the third fixture (e.g. :colon.txt) so the case the fix explicitly names is the case the suite actually exercises — right now it's asserted about in prose but not proven in code.

Fix this →

Everything else in this commit

• The :(literal) pathspec rewrite in classify_landed()'s two-dot fallback is a sound fix: chunking at 200 avoids the command-line-length failure mode, and the --pathspec-from-file rejection (git 2.54 exit 129) documented in the commit message matches the array-based fallback actually shipped.
• The base-side patch-id completeness check (base_count -ne base_expected) is a correct symmetry fix, and the commit is honest that it's inert today (matches earlier reviewer's confirmation that an under-complete base set can only reduce matches, never cause a false yes).
• cleanup.md/status.md's new in-progress/dirty rows and closed-list "unlisted → unsafe" default read correctly against the engine's actual risk vocabulary, and the - placeholder documentation closes a real doc gap (a base of - previously looked like broken output rather than "no base resolved").
• audit-fleet.sh's --is-bare-repository addition to git_probe_allowed is read-only/operand-free, consistent with the existing allowlist shape; worktree-placement-unverifiable is documented consistently across SKILL.md, confidence-model.md, and the CHANGELOG.
• audit-fleet.test.sh's new prefix-fail fixture correctly exercises the previously-dead probe-failure branch, and the assertions check both the finding kind and its "unproven rather than disproven" reasoning text.
• CHANGELOG entries for both plugins are accurate to the diff (aside from the fixture-count nit above) and correctly filed under the still-unreleased [0.47.0]/[0.9.0] sections rather than as new version bumps.

I could not execute the test suites in this sandboxed review environment (same limitation noted in earlier review rounds on this PR), so the head-field and superseded findings above are reasoned from the code rather than an executed repro — both follow directly from unchanged code paths already identified in prior rounds, so confidence is high, but worth a final sanity check against a real run before merge.

Recommend fixing the head field regression before merge (small, mechanical, same shape as the fix already applied to the other 12 fields) and resolving or explicitly deferring the superseded name-only match, which has now been raised in four independent review passes across two lanes without a fix or a stated deferral.

[claude]

Claude finished @kyle-sexton's task in 3m 17s —— View job

---

Security review

Scope: files changed in this PR vs origin/main (19 files), re-reviewed at HEAD 38bedda. One commit has landed since the last security pass (at 8fca7cd): 8fca7cd..38bedda ("close the remaining review findings across both plugins"), touching landed-work.sh/landed-work.test.sh, audit-fleet.sh/audit-fleet.test.sh, cleanup.md/status.md, and both CHANGELOGs.

• Diff 8fca7cd..38bedda to see what actually changed
• Check whether the superseded name-only-match finding (Codex P1, raised in 3 prior review rounds) was finally addressed
• Review the two-dot fallback rewrite (now pathspec/git diff --quiet-based instead of text-join) for new injection/parsing surface
• Confirm worktree-create-gate.sh, hooks.json, both plugin.jsons are unchanged since the last full security pass
• Post final findings

🔴 IMPORTANT — superseded is still a name-only match with no identity binding to the merged PR (still unaddressed, 4th round)

landed-work.sh#L679-L683

merged_ref() {
  local name="$1"
  [[ -n "$MERGED_REFS_FILE" && -n "$name" && "$name" != "(detached)" ]] || return 1
  grep -Fxq "$name" "$MERGED_REFS_FILE"
}

Verified by diff: 8fca7cd..38bedda does not touch merged_ref() or its call site (landed-work.sh#L840-L844). The --merged-refs-file contract is also unchanged in status.md#L35: "Write the merged headRefName values ... one per line" — names only, no headRefOid/SHA.

This is the same P1 Codex flagged on this PR and the same finding raised by two prior Claude security-review rounds (at 2320ab7 and 8fca7cd). The commit that just landed is titled "close the remaining review findings across both plugins" and does close several real issues (see below), but this specific, repeatedly-flagged one is not among them.

Failure scenario (unchanged from prior rounds): branch fix-bug gets merged and its worktree cleaned up. Weeks later someone reuses the local branch name fix-bug for unrelated work with genuinely unpushed, unlanded commits (classify_landed correctly returns landed=no). merged_ref("fix-bug") still matches by name alone, so the row is classified risk=superseded. cleanup.md treats superseded as fully safe at both guard sites this PR adds — the pre-removal check (cleanup.md#L78) and the git branch -D emission (cleanup.md#L113) — with no extra confirmation, the same treatment as a proven landed=yes. Note this row is not caught by the new "any unrecognized risk value maps to unknown" fallback this same commit adds (status.md#L54), because superseded is a recognized, explicitly-listed-safe value — that new fallback doesn't help this specific path.

Fix direction (unchanged): bind the merged-refs evidence to a revision, not just a name — have the caller pass headRefName<TAB>headRefOid instead of bare names, and have merged_ref verify the branch's current tip or merge-base against that OID before granting superseded.

Confidence: High — verified directly against the code and docs at HEAD 38bedda, not inferred from earlier rounds.

The rest of 38bedda — no new security issues

• Two-dot fallback rewrite (landed-work.sh#L507-L560): replaces the text-joined --name-only/--numstat comparison (source of the previously-fixed core.quotepath mismatch) with git diff --quiet ... -- ":(literal)$path" ... over paths read via -z/read -r -d ''. This closes the quoting-mismatch class entirely rather than patching it, and is safe against argument/pathspec injection: paths always follow a literal -- separator (so a filename starting with - can't be read as an option), and :(literal) disables pathspec-magic interpretation of characters like *, ?, [, or a leading :. Chunking at 200 paths per git diff invocation doesn't change this — chunks are still passed as discrete argv elements, never shell-interpolated.
• audit-fleet.sh: adds --is-bare-repository to the rev-parse allowlist (audit-fleet.sh#L129) — read-only, operand-free, same shape as the existing --show-toplevel/--show-prefix entries; no widening of the allowlist's trust boundary. The canonical_top silent-skip nit from an earlier review round is also fixed here (a failed non-bare --show-toplevel now emits an UNKNOWN finding instead of silently no-op'ing) — a safety improvement, not a new risk.
• worktree-create-gate.sh, hooks.json, both plugin.jsons: unchanged since the last full security review at 2320ab7/8fca7cd (confirmed via git diff 8fca7cd..38bedda — none of these files appear in it). Prior findings there (argv-based non-interpolated invocation, --name validation preserved through the hook entry point, no permission widening) stand.

Recommend fixing the superseded name-only match before merge. It has now been raised by two independent reviewers across four review passes and is the one classification path in the engine where genuinely unlanded, unpushed commits can reach git branch -D with no confirmation prompt — reachable through an ordinary branch-name-reuse workflow, not an edge case.

[claude]

Claude finished @kyle-sexton's task in 1m 17s —— View job

---

Code review

• Gather context: diff since last full review (38bedda → 47837a1)
• Check whether the open superseded name-only-match finding was addressed
• Check whether the head TSV-field-empty finding was addressed
• Check whether the fixture-count nit was addressed
• Post final review

Scoped to what changed since the last full code-review round (commit 38bedda): commit 47837a1 ("fix(source-control): spelling in a degradation reason"), a one-line, one-file change:

-    L_REASON="touched-paths-unparseable"
+    L_REASON="touched-paths-unparsable"

This is correct and harmless — it aligns landed-work.sh with the spelling this repo already uses everywhere else this word appears (babysit_lease.py, babysit_resolve_thread.py, config-resolution.md, etc. all use "unparsable"; landed-work.sh was the one outlier). No test or doc references the old string (grep -rn "touched-paths" plugins/source-control/scripts/landed-work.test.sh plugins/source-control/CHANGELOG.md plugins/source-control/skills/worktree/ returns nothing), so nothing depended on the prior spelling and nothing needed a matching update. No new issue introduced.

Still open — carried forward from prior review rounds, unchanged by this commit

• 🔴 merged_ref() still matches on branch name alone, no revision binding (landed-work.sh#L679-L683) — confirmed unchanged at HEAD. This is the Codex P1 ("Require current-head evidence before marking work superseded") and has now been raised across five review rounds (Codex + four Claude passes) without a fix or a stated deferral. A reused branch name still lets genuinely unpushed, unlanded commits classify risk=superseded, which cleanup.md treats as safe for both worktree removal and git branch -D with no confirmation — the exact "false yes destroys work" failure this engine exists to prevent.
• 🔴 head TSV field can still print empty (landed-work.sh#L865) — confirmed unchanged: "${T_HEAD[$idx]:0:12}" still has no :- fallback, unlike the other 14 fields in that printf. Substring expansion of an empty T_HEAD[$idx] (the bare-hub row and the --worktree <path>-target-fails-is_worktree_root row both hit this) still prints an empty column, reintroducing the exact column-shift bug commit 8fca7cd was written to close, for this one column. Fix: head_short="${T_HEAD[$idx]:0:12}"; head_short="${head_short:--}" before the printf.
• 🟡 Fixture-count nit (landed-work.test.sh#L258-269) — still only two of the three path-spelling fixtures the commit message/CHANGELOG describe are present (café.txt, star[1].txt); no fixture beginning with : exists, even though that's the specific case the :(literal) pathspec prefix was added to handle. Optional.

No other files changed since 38bedda, so everything reviewed in that round (the :(literal) pathspec rewrite, the cleanup.md/status.md risk-vocabulary closed-list default, the audit-fleet.sh bare-hub/canonical_top fixes) stands as previously assessed — sound.

Recommend fixing the head-field regression before merge (small, mechanical) and resolving or explicitly deferring the superseded name-only match, which remains the one classification path where genuinely unlanded, unpushed commits can reach git branch -D with no confirmation prompt.

[claude]

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

---

Security review

Scope: files changed in this PR vs origin/main, re-reviewed at HEAD 47837a1. One commit has landed since the last security pass (at 38bedda): 47837a1 ("fix(source-control): spelling in a degradation reason").

• Gather context: diff since last reviewed commit (38bedda → 47837a1)
• Review the new commit's diff for security implications
• Confirm whether the still-open superseded name-only-match finding was addressed
• Post final findings

47837a1 — no security relevance

-    L_REASON="touched-paths-unparseable"
+    L_REASON="touched-paths-unparsable"

landed-work.sh#L537 — a one-character-class spelling correction to an internal, hardcoded diagnostic string emitted in the reason TSV column. Not derived from external input, not interpolated into a command, no parsing/matching logic touched. No security implication.

Still open — merged_ref() remains a name-only match (unaddressed since first raised)

landed-work.sh#L679-L682 is unchanged by this commit (confirmed by diff — the only file touched is landed-work.sh, and the only hunk is the one shown above):

merged_ref() {
  local name="$1"
  [[ -n "$MERGED_REFS_FILE" && -n "$name" && "$name" != "(detached)" ]] || return 1
  grep -Fxq "$name" "$MERGED_REFS_FILE"
}

This is the P1 raised by both Codex and four prior Claude security-review rounds on this PR (at 2770bcf, 2320ab7, 8fca7cd, 38bedda): --merged-refs-file is populated from gh pr list --json headRefName (names only, no SHA), so a reused branch name with genuinely unpushed, unlanded new commits still classifies risk=superseded, which cleanup.md treats as safe for both worktree removal and the git branch -D that follows — no confirmation prompt. Not part of this round's change; still outstanding.

No new security issues in this round. The only change since the last security pass is the cosmetic string fix above.

Recommend resolving or explicitly deferring the superseded name-only match before merge — it remains the one classification path in the engine where genuinely unlanded, unpushed commits can reach git branch -D with no prompt, and it's now been raised across five review rounds without a fix or a stated deferral.

[claude]

Claude finished @kyle-sexton's task in 1m 14s —— View job

---

Security review

Scope: files changed in this PR vs origin/main, re-reviewed at HEAD ab4e170. One commit has landed since the last security pass (at 47837a1): ab4e170 ("fix(source-control): gate superseded worktrees exactly as stranded").

• Gather context: diff since last reviewed commit (47837a1 → ab4e170)
• Check whether the superseded name-only-match finding was addressed
• Check whether the head TSV-field-empty finding was addressed (unrelated to this commit — code-review lane, not security)
• Review remaining diff for new security-relevant changes
• Post final findings

✅ RESOLVED — superseded name-only match, closed by consumer-side gating

The finding raised by Codex and five prior Claude security-review rounds on this PR is fixed in ab4e170, verified directly against the diff.

The fix does not attempt to bind the merged-refs evidence to a revision (that data — the merged PR's head OID — is unavailable offline once the remote ref is deleted on merge, which is the normal GitHub "delete head branch on merge" flow). Instead it removes the asymmetry at the consumer layer: superseded is still emitted by merged_ref()/landed-work.sh as a distinct, informative risk label (name-only evidence, honestly documented as such — landed-work.sh#L79-86, #L841-852), but every consumer now gates it identically to STRANDED:

• Pre-removal guard (cleanup.md#L79): risk=STRANDED, UNKNOWN, or superseded → stop and require explicit per-worktree confirmation. Only landed, ok, or bare proceed unattended.
• git branch -D emission (cleanup.md#L118): emitted only for landed or ok; superseded now falls into the same "emit nothing, offer push -u instead" branch as STRANDED/UNKNOWN. This was the second, sharper guard site — the one that actually destroys commits — and it's now closed for this path.
• status.md classification (status.md#L49, #L63): superseded no longer has its own Status row; it's folded into stranded, which "outranks every row below." The prior table's dedicated superseded Status row (implicitly reading as safe/informational) is removed.
• SKILL.md's periodic health-check flags superseded alongside stranded/unknown/notgit as "not safe" (SKILL.md#L124).
• Doc language is honest about the residual limitation rather than papering over it: "a name reused after that merge makes the evidence describe different commits than the ones here, and the row is landed=no either way" (cleanup.md#L79).

Re-verified failure scenario from prior rounds: branch fix-bug is merged, worktree cleaned up, name later reused for unrelated work with genuinely unpushed, unlanded commits. landed=no, merged_ref("fix-bug") still matches by name, row is still labeled risk=superseded — but now that label carries the same weight as STRANDED at every consumption point. Cleanup stops, requires explicit per-worktree confirmation naming the actual commits, and refuses to emit git branch -D unattended. The data-loss path is closed.

Test coverage: landed-work.test.sh#L159-166 still asserts the engine emits risk=superseded for a merged-name match (correct — the classification itself is still useful information, just no longer auto-trusted), and evals.json's eval 11 was updated to match the new gating (evals.json).

Everything else in ab4e170 — no new security issues

• CHANGELOG.md update accurately describes the behavior change, no discrepancy from the actual diff.
• No other files in this commit; worktree-create-gate.sh, hooks.json, both plugin.jsons, and audit-fleet.sh are unchanged since the last full security pass (confirmed via git diff 47837a1..ab4e170 — none appear).

Summary across all review rounds on this PR

Both security findings raised on this PR are now resolved:

• The core.quotepath mismatch (false landed=yes on non-ASCII paths) — fixed in 2320ab7, then the escaping approach itself replaced with :(literal) pathspecs in 38bedda.
• The superseded name-only match (false-safe classification reachable via branch-name reuse) — fixed in ab4e170 above.

No open security findings on this PR at HEAD ab4e170.