fix(overengineering): never give a detached checkout a branch identity (0.3.0)

[kyle-sexton]

Summary

overengineering:audit and overengineering:realign precomputed the current branch with
git rev-parse --abbrev-ref HEAD, which answers the literal string HEAD when HEAD is detached —
the ordinary shape for a scheduled CI runner. HEAD looks like a branch name, is the same string
for every ref, and compares equal to itself, so three consequences followed and all of them were
silent:

• audit wrote branch: HEAD into the findings artifact, giving it an identity that is not a
  branch.
• realign's branch-match refusal compared HEAD to HEAD, passed by construction, and could
  accept another ref's artifact as this ref's own — in the only skill in this plugin that mutates
  anything, where per-item human consent is spent down a rollback ladder as far as deletion.
• The <branch-slug> memory-tier home key resolved every detached ref to one directory, so
  unrelated runs shared a single artifact and merged each other's carried-forward operator
  judgments.

The delta lane already resolved identity correctly with git symbolic-ref and could not fix any
of these from where it sits, which is why this was filed separately from #3146.

Fix

Both precomputes now use git symbolic-ref --quiet --short HEAD, which fails rather than inventing
a name, and emit the same sentinel string delta already used. All three skills now agree on one
resolution contract: prefer a logical ref where the environment supplies one naming a branch — with
no CI vendor's variables named or assumed — and otherwise treat the identity as unresolved.

On an unresolved identity:

• audit persists no findings artifact at all. Not the file with branch: omitted, not a
  placeholder value, not a file at a substitute home. The walk still runs and the inline summary is
  still emitted, so the operator still gets the findings; only the persisted write is declined, and
  the read-only disclosure line says that instead of naming a path.
• realign refuses, in both unresolved states: when its own checkout has no identity (it stops
  before reading the artifact at all) and when the artifact it finds carries branch: absent,
  empty, or HEAD. Neither path reaches the comparison. The refusal is a positive precondition
  rather than a consequence of the file being missing, which also renders any stale pre-fix
  artifact already on disk unreachable without a cleanup step.

Identity resolution is a stated body step, not only a precompute. A worktree-isolated or
dispatched executor may decline to inject the precomputed context block entirely — and that is the
same unattended context in which a detached checkout is most likely — so both skills state the
resolution command in the step that uses it and treat the precompute as a convenience that may be
absent. Confined to the precompute line, the fix would have verified green on an attached local
checkout and done nothing in the environment the defect actually lives in.

Two contract documents now state the case rather than leaving it to be inferred.
context/findings-artifact.md documents branch: as resolved with git symbolic-ref and never the
literal HEAD, adds a "No branch identity, no artifact" section, and carries a per-skill
obligations row. reference/topic-docs.md records that an unresolved identity keys no home and does
not run the rung order, rejecting each substitute on its own terms: HEAD is one directory for
every ref, a commit sha is a fresh home every commit that never resumes, and a fixed literal such as
detached is HEAD under another name.

Three adjacent surfaces were made inconsistent by the change and are corrected in the same PR.
skills/audit/context/report-template.md, which owns the read-only disclosure line, still declared
the artifact written "always" and hardcoded the path-naming form. skills/delta/SKILL.md closed its
own detached section with "the audit still runs, exactly as it otherwise would" — true when written,
false once the audit began declining its write — and its step 4 would have looked for a post-run
artifact that no longer exists; delta's behavior is unchanged, only its account of the audit's.
And skills/delta/context/recurring-wiring.md, the wiring document read by exactly the affected
population, now names the detached case and the logical-ref remedy.

The branch: decision was made by an independent panel, not by the implementing agent, and is
recorded in full on the issue with its reasoning, its rejected alternatives, and its caveats:
#3149 (comment).
Two fresh-context agents on different models, spawned separately with the implementing agent's
rationale withheld, independently reached the same answer as that agent's own prior read.

Verification

The behavior the fix turns on, observed directly. The marketplace's own primary clone happens to
sit detached, so both forms were run against a real detached checkout and against an attached one:

Checkout	git rev-parse --abbrev-ref HEAD	git symbolic-ref --quiet --short HEAD
Detached (## HEAD (no branch))	HEAD	(no output, exit 1)
Attached (fix/3149-…)	fix/3149-…	fix/3149-…

The two forms agree exactly where a branch exists and diverge exactly where one does not, which is
the whole of the guard. git branch --show-current was checked on the same detached checkout and
returns an empty string at exit 0 — the basis for the cross-plugin finding below.

Repo gates. This plugin ships no scripts and no *.test.sh, so its coverage mechanism is the
eval sets plus the shared static gates. All were re-run after the review fixes:

• check-skill.sh — audit, realign, and delta each PASS with 0 errors. The warnings each
  reports (description length, the 200-line soft target) are pre-existing on main.
• check-evals-quality.sh — PASS, 0 warnings across both eval sets.
• check-jsonschema against plugins/skill-quality/reference/evals.schema.json — valid.
• check-skill-portability.sh — no unexcused coupling tokens.
• markdownlint-cli2 — 0 issues across all eight changed markdown files.

Eval coverage for the detached path, which existed for neither skill before. Four cases added:
audit declining to persist on a detached runner; audit preferring an environment-supplied
logical ref and naming its origin; realign stopping before it reads the artifact when its own
checkout is detached; and realign refusing an artifact carrying branch: HEAD. Each prompt
narrates the checkout state it depends on, following the convention the existing shallow-clone case
already uses, so none asserts a condition it never establishes.

Independent review. A fresh-context reviewer with the author's rationale withheld checked the
change against the issue's five acceptance criteria and returned NOT READY on the first commit. Its
blocker (the stale report-template.md) and every should-fix (the cross-plugin count, the
mutation-testing overclaim, the babysit-prs characterization, three under-specified eval
prompts, the missing wiring guidance) are addressed in the second commit. The version moved to
0.3.0 rather than a patch on its recommendation: a run that previously wrote a file now writes none.

Related

Closes #3149

• feat(overengineering): add the delta lane — recurring audits that report only what moved (0.2.0) #3146 — the delta lane PR where this was found; carries the delta-side defense this change
  converges audit and realign onto.

Do other plugins share the pattern? No other plugin exhibits the HEAD identity-collapse. The
repo-wide precompute boilerplate uses git branch --show-current, which returns an empty string on
a detached checkout rather than HEAD. The literal git rev-parse --abbrev-ref HEAD form survives
at seven occurrences across six files outside this plugin: claude-ops' two skill-usage telemetry
hooks, source-control's babysit-prs parking-branch capture and worktree current-branch
display, and three test-scaffolding sites in guardrails' stale-path-verify.test.sh and
source-control's worktree-create.test.sh. None compares the value against a stored identity to
decide whether a mutation may proceed. babysit-prs is the closest call and is still a different
defect class — its captured value parameterizes a same-session git checkout, so a detached capture
fails to restore the starting commit rather than authorizing another ref's work. All are left as-is
deliberately. This corrects the triage note, which recorded two of these sites rather than six.

One neighbor has the same shape under a different string, and is out of scope.
mutation-testing:audit documents branch: as git branch --show-current verbatim
(skills/audit/context/persist-findings.md:202) and ships no script guarding the empty result, so a
detached run there writes an empty identity that compares equal to itself. Three sibling
findings-writers (ai-slop, docs-hygiene:audit-noise, claude-config:audit-instructions) already
guard that case and exit rather than persist. Recorded rather than fixed — this issue is scoped to
one plugin — and worth its own item.

Adjacent pre-existing condition, deliberately not fixed here.
scripts/check-skill-precompute-compose.sh reports both audit/SKILL.md and realign/SKILL.md as
carrying a git command plus more than one injection line (#1619). Both files already had two
precompute lines on main, so this change neither introduces nor worsens it, and the gate is
warn-only in CI pending that remediation wave. Making resolution a body step is what keeps this fix
correct in the meantime, since a refused precompute block no longer means an unguarded identity.

[claude]

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

---

CI security review

• Gather context (PR diff, changed files)
• Apply skip gate
• Review for security-relevant surface
• Post findings

This PR is documentation/instruction changes for three overengineering skills (audit, realign, delta) plus their eval fixtures — no scripts, no CI workflow files, no code with classic injection surfaces. The change itself is a security-positive fix: it closes a real authorization gap where a detached-HEAD checkout (git rev-parse --abbrev-ref HEAD → literal "HEAD") let realign's branch-match refusal compare HEAD to HEAD and pass by construction, potentially executing one ref's findings as mutations against a different ref's surface.

One finding from reviewing the new fallback path this fix adds, posted inline on plugins/overengineering/skills/audit/SKILL.md:

IMPORTANT — The new "prefer a logical ref where the environment supplies one" fallback (skills/audit/SKILL.md:221-226, mirrored in skills/realign/SKILL.md:62-69) accepts an unvalidated string from the environment as the branch identity whenever the checkout is detached and git symbolic-ref fails. Unlike a real branch name (constrained by git's check-ref-format, which forbids .. path segments), nothing here validates the supplied value's shape before it's used to key the <branch-slug> home and populate branch:. The slug rule in reference/topic-docs.md#L91 only replaces / and non-[a-z0-9._-] characters with - — . passes through untouched, so a supplied value of .. slugs to literally .., letting the composed path escape .work/overengineering/. More importantly, since neither audit nor realign validates the "logical ref" against the actual checkout, an attacker-influenceable environment value lets both sides of realign's branch-match check be sourced from the same untrusted input — reopening the exact "one ref's findings execute against another ref's surface" class of bug this PR closes for the HEAD-collapse case, just via a different vector. Suggest constraining the accepted value to a validated ref shape (reject .. segments / disallowed characters) before it can key a home or populate branch:.

No other security-relevant issues found — no secrets handling, no permission-widening, no unpinned dependencies, no unsafe GitHub Actions patterns in this diff.
· branch

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 930c518ba4

ℹ️ 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]

Last security-reviewed head: 930c518ba4f596e16e43801cba488d1bd8803804. On the next push, the relevance gate compares only the commits since this SHA; delete this comment to force a full re-review.

[claude]

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

---

Code Review Complete

• Gather context (PR diff, changed files, CLAUDE.md)
• Check skip gate (draft/closed/trivial/already-reviewed) — none apply; this is the code-review lane (security findings, if any, belong to /review:security-review, which this repo has as a separate workflow)
• Review changed files against criteria
• Post inline findings (if any)
• Post final summary

Summary

Reviewed the diff (git diff origin/main...HEAD, 11 files) against the code-review lane's high-signal bar: correctness, maintainability, and internal consistency of the fix.

No blocking findings. The precompute swap (git rev-parse --abbrev-ref HEAD → git symbolic-ref --quiet --short HEAD) is applied consistently across audit and realign, matching the sentinel string delta already used (no branch ref (detached HEAD or no checkout)). I traced the cross-document contract end to end:

• audit's "Resolve the branch identity, then the artifact home" step correctly gates home resolution (the rung order in topic-docs.md) on having an identity in hand before running it, matching topic-docs.md's new "the rung order is not run" claim (topic-docs.md#L99-L104).
• realign's step 1/step 3 split (own-checkout refusal vs. artifact-branch: refusal) matches the "Two distinct unresolved states" section precisely, and never reaches the degenerate HEAD == HEAD comparison.
• report-template.md's read-only line variant and "next step" wording were updated consistently with the no-artifact case.
• The two new eval cases per skill match the sentinel string and behavior verbatim; ids are sequential/unique in both evals.json files.
• Verified the CHANGELOG's cross-plugin rev-parse --abbrev-ref HEAD occurrence count independently via grep: 7 occurrences across 6 files outside this plugin, matching the claim exactly.

One pre-existing item, out of scope for this lane: skills/delta/SKILL.md's step 1 ("Before anything: load the artifact") still reads "Resolve the home by running the whole rung order..." without the explicit "with an identity in hand" gate that audit and realign's step 1 now state outright — it relies on a forward reference to the "detached checkout" section instead. That wording predates this PR (introduced in a60acac, untouched by either commit here), so per the high-signal bar it's excluded as a pre-existing issue on an untouched line — flagging only as an FYI, not a blocking finding.

No inline comments were posted since no changed line warranted one under the high-signal bar.

Branch: fix/3149-detached-head-branch-identity

[github-actions]

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

[claude]

Claude finished @cursor[bot]'s task in 4m 27s —— View job

---

Code Review Complete

• Gather context (PR diff via gh pr diff 3222, prior review threads, CLAUDE.md)
• Check skip gate — none apply; this is the code-review lane (security findings belong to /review:security-review, run separately)
• Review latest commit against code-review lane's high-signal bar
• Post inline findings
• Post final summary

Summary

The prior code-review pass already covered commits 1–2 (through 930c518). This pass reviews what changed since: commit 3 (a861c922, "validate logical refs and keep inline findings uncapped"), which addresses the Codex and Claude security-review findings from the earlier rounds — normalizing (refs/heads/ stripping) and validating (git check-ref-format --branch, rejecting ./.. segments) the environment-supplied logical ref before it can key a home or populate branch:, distinguishing the no-checkout stop from the detached-in-a-repo case, and uncapping the inline findings list when no artifact is written. I traced this against plugins/overengineering/skills/audit/SKILL.md and skills/realign/SKILL.md: both now apply the same normalize-then-validate steps consistently, closing the path-escape/cross-ref gap the earlier reviews flagged.

One finding, posted inline on plugins/overengineering/CHANGELOG.md:17-19: this commit introduces a stray, truncated duplicate bullet — the same opening clause as the entry immediately below it, but cut off mid-sentence with an unclosed ** and no content — sitting between two correctly-formed entries. Verified it's isolated to this file (checked audit/SKILL.md, audit/context/report-template.md, and realign/SKILL.md for the same class of leftover-text duplication; none found).

No other issues found in this commit's changes — the normalize/validate logic is applied consistently between audit and realign, and the no-checkout-vs-detached distinction is stated once and referenced correctly from both the audit body step and the "detached checkout has no branch identity" section.

Branch: fix/3149-detached-head-branch-identity

[claude]

Malformed/duplicated changelog entry — orphaned bullet fragment.

Line 17 is a truncated duplicate of the bullet that correctly starts on line 19: same text ("audit and realign gave a detached checkout a branch identity, collapsing every ref onto one"), but cut off mid-sentence with an unclosed ** bold marker and no content, followed by a blank line, then the real entry repeats the identical opening and continues correctly.

- **`audit` and `realign` gave a detached checkout a branch identity, collapsing every ref onto one

- **`audit` and `realign` gave a detached checkout a branch identity, collapsing every ref onto one
  (#3149).** Both skills precomputed the branch with `git rev-parse --abbrev-ref HEAD`, which answers

Line 19 onward is pre-existing content from an earlier commit in this PR (unchanged); only line 17 (plus its trailing blank line 18) is newly introduced by this commit and appears to be leftover text from an edit that didn't get cleaned up. It renders as a stray, content-free bullet item with a dangling unclosed bold span directly above the real entry.

Suggest deleting lines 17–18 so the "Logical-ref fallback…" bullet (lines 10–15) is immediately followed by the "audit and realign gave a detached checkout…" bullet (line 19) with no orphaned fragment between them.

Fix this →