test(acceptance): cover detached HEAD and branch flow-through (#2909)

[swayamg20]

Summary

• Retains focused real-git coverage for the detached-HEAD sentinel.
• Drives detached and normal branch cases through the root spec-kitty accept --diagnose --json CLI boundary.
• Replaces the private acceptance-context spy with assertions on operator-visible diagnostics.

Why Now

PR #2906 introduced these two low-risk branches, and #2909 tracks the remaining coverage gaps. Maintainer review requested proof through the active public acceptance path rather than helper-level coverage alone.

What This PR Does

• Keeps the focused _git_head_of sentinel regression requested by Follow-up: test-coverage nits from PR #2906 (detached-HEAD sentinel + accept-gate branch flow-through e2e) #2909.
• Verifies detached HEAD is normalized to branch=null, blocked by mission_branch, and prevented from reaching matrix judgment.
• Verifies a normal mission branch reaches matrix evaluation and exposes the seeded failing verdict.
• Proves both paths by mutation: dropping branch forwarding or preserving literal detached HEAD makes the corresponding CLI test fail.

Effect on Existing Projects

• Runtime / compatibility: None; this is test-only.
• Upgrade / migration: None.
• Reviewer impact: Regressions in detached normalization or branch forwarding now fail at the public operator boundary.

Validation

• uv run pytest -q tests/acceptance/test_gate_execution_context.py — 28 passed
• uv run pytest -q tests/acceptance/ — 61 passed
• uv run ruff check tests/acceptance/test_gate_execution_context.py
• uv run mypy --strict tests/acceptance/test_gate_execution_context.py
• Mutation checks for detached normalization and normal-branch forwarding
• git diff --check

Tickets / Contracts

Ticket	Relationship
#2909	Implements both requested coverage cases

Mission Artifacts

• Spec: N/A - focused issue follow-up
• Plan: N/A
• Tasks: N/A
• Checklist: N/A

Follow-ups

• None.

AI Assistance Disclosure

AI assistance was used to inspect the repository, draft the tests, and run validation.

Closes #2909

---

^{Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.}

[robertDouglass]

Adversarial squad review + maintenance reroll

Three independently profile-loaded lenses reviewed the test-only aggregate diff: reviewer-renata, debugger-debbie, and paula-patterns.

Verdict: BLOCK_MATERIAL against closing #2909 as written. Both tests are genuine and load-bearing, but they stop below the operator boundary explicitly requested by the issue. Per queue policy, that material coverage gap was not fixed.

• High — tests/acceptance/test_gate_execution_context.py:589: branch-flow test enters at collect_feature_summary(); Follow-up: test-coverage nits from PR #2906 (detached-HEAD sentinel + accept-gate branch flow-through e2e) #2909 explicitly requires the full operator validation chain through validate_acceptance. Current code no longer contains that named symbol, so the canonical replacement boundary must be clarified, then tested through the actual public/CLI acceptance entry.
• High — tests/acceptance/test_gate_execution_context.py:251: detached test calls private _git_head_of() directly. It does not prove detached-state normalization/wiring through the accept command and execution-context gate.
• Medium — tests/acceptance/test_gate_execution_context.py:571: private _acceptance_gate_context spy couples the flow test to call shape. Observable fail-vs-ref-mismatch behavior can prove forwarding without patching private construction.

Evidence: both added tests pass on current main rebase (2 passed). A branch-dropping mutant makes the forwarding test fail, so this is not a false-green complaint; the blocker is contract boundary/traceability.

Maintenance only: rebased conflict-free onto current main, removed one pre-existing stale type: ignore in the touched file, and rerolled dbc52d21b → 09be9dbbd. Verification: targeted ruff clean, strict mypy clean, 2 passed, diff check clean. No material finding was modified.

[robertDouglass]

Automated review — corrected and condensed after validation. No code was modified.

Verdict: approve.

This is a focused red-first acceptance-test change that exercises the operator-visible accept --diagnose --json boundary.

The test's strict json.loads(result.output) is desirable: --json should emit clean JSON and fail if warnings or other text pollute stdout. The previous suggestion to scan from the first { is withdrawn because it would hide a contract regression rather than make the test more robust.

No blocking findings.

[devin-ai-integration]

Adversarial review of test/issue-2909-accept-gate-coverage @ f1ea2221. Review-only; nothing in this branch was modified. I did not take the "test-only, therefore low risk" shortcut — I mutation-tested the new assertions against the production code they claim to guard.

Verdict: approve. One of the two new tests is a genuine, mutation-proven regression guard; the other pins behaviour that is weaker than its name suggests. Nothing here is blocking.

Evidence

Ran the whole file on this head: tests/acceptance/test_gate_execution_context.py → 28 passed (76 s, real git). mypy and ruff on the changed file: clean (so removing # type: ignore[misc] from ctx.surface = Path("/elsewhere") is correct, not a silenced regression).

Mutation test of the flow-through claim — injected branch = None at the top of _acceptance_gate_context (src/specify_cli/acceptance/gates_core.py) and re-ran:

FAILED test_accept_command_forwards_normal_branch_to_matrix_gate
FAILED test_gec2_primary_ref_drift_gate_refuses_instead_of_passing
FAILED test_gec2_real_cross_checkout_branch_drift_gate_refuses
3 failed, 25 passed

So test_accept_command_forwards_normal_branch_to_matrix_gate really does die when a production forwarding hop drops the invocation branch, and it does so through the operator-visible spec-kitty accept --diagnose --json boundary rather than a spy. That is exactly the coverage #2909 asked for, and it is the strongest part of this PR.

Should fix (non-blocking)

1. The detached-HEAD test does not guard the sentinel it is named after. Its own docstring concedes the mechanism: a detached invocation becomes branch=None and is "rejected before an acceptance-matrix GateExecutionContext can be built". So test_accept_command_normalizes_detached_head_before_matrix_gate asserts the branch gate's refusal, not that _DETACHED_HEAD_SENTINEL makes "no expectation on either side compare equal" in assert_at_ref. The only coverage of that behaviour is test_git_head_of_detached_checkout_returns_head_sentinel, which asserts _git_head_of(...) == "HEAD" — a private symbol, imported as such. That directly contradicts the PR body's "Replaces the private acceptance-context spy with assertions on operator-visible diagnostics": the private assertion was narrowed, not removed. Either say so in the body, or add a case that reaches assert_at_ref with a HEAD-sentinel ref on both sides.

2. Both new tests run --lenient. _run_public_accept_diagnosis passes --lenient, i.e. strict_metadata=False — not the default operator path. Also assert result.exit_code == 0 is doing no work for --diagnose, which reports through the payload rather than the exit code. Consider dropping --lenient (or covering the strict path too) so the test matches the invocation an operator actually types.

3. Two assertions are coupled to prose. "detached HEAD" in issue and "verdict is 'fail'" in issue will break on a wording change that is not a behavioural regression; the payload already carries structured blocked_checks / skipped_checks entries you assert on elsewhere in the same test. Prefer the structured fields where they exist.

4. No CI evidence, and the branch is stale. Authoritative state: state=BLOCKED, checks {'SKIPPED': 1} — nothing has actually run on this head — and it is 170 commits behind origin/main. A test-only diff is still worth one real run, particularly because these cases shell out to real git and take ~76 s for one file, which is a measurable addition to whichever shard they land in.

Verified sound

• The new cases go through the public CLI (CliRunner on specify_cli.app) with catch_exceptions=False, so a production traceback surfaces instead of being swallowed into a non-zero exit assertion.
• The flow-through test makes the mission branch deliberately differ from main, which is what gives the "every hop preserves the branch" claim its teeth — a same-branch fixture would have passed under the mutation above.
• Assertions are negative as well as positive (not any("verdict is" ...) on the detached path, not any("GATE_SURFACE_REF_MISMATCH" ...) on the normal path), so the tests distinguish "refused for the right reason" from "refused at all".
• Real detached state is created with git checkout --detach and verified via git branch --show-current before the assertion, rather than being faked.
• Diff is a single test file, +97/-1, no production code and no fixture surgery — nothing here can weaken another gate.