orchestrate integrate can strand the working tree (ref-only advance); add structural bare-repo refuse + post-land tree check

[TBarregren]

Summary

The orchestrate engine's integrate step can advance the default branch's ref without updating its working tree, silently stranding the landed code off-disk. When it encountered a repository in a bad state (core.bare=true / "no work tree"), instead of refusing and parking, it worked around the condition with git update-ref, leaving main pointing at the integration tip while the primary checkout stayed at the pre-run commit. The run's own integration hotfix (7b8a3c2) added a prompt-level guard forbidding this — but the guard is advisory instruction to a mechanical-tier agent, added after the damage, and there is still no structural self-check that the tree matches the ref after landing, and no repair of a repo already found corrupt.

Severity: HIGH (defense-in-depth over the root-cause test-env ticket). Every "landed/green" guarantee is moot if the landed code never reaches disk, and gates run in the stranded checkout test the old code.

Evidence (verified)

• Implementer verdicts for both issues describe the fallback explicitly, e.g. orchestrate: continuous out-of-band progress reporting for multi-hour runs (status command + durable milestone markers) #48:

  "Fast-forwarded main from 411da14 to …'s tip via git update-ref (git merge --ff-only failed only because the root repo is bare/has no work tree) …"

• Post-run ground truth: main/HEAD correctly at the integration tip 7b8a3c2, but git config core.bare → true, git status failed with "this operation must be run in a work tree", and scripts/orchestrate.py on disk lacked the landed symbols — i.e. the ref moved, the tree did not.
• The guard added by the run's hotfix now lives in the integrate prompt (skills/orchestrate/orchestrate.workflow.js:922-925):

  "NEVER bypass that refusal — do NOT set core.bare, do NOT reconfigure the repository, and do NOT force-advance the default ref with git update-ref, git branch -f, or git push .. Each of those corrupts the repository and strands the default branch's working tree … If the default branch is NOT checked out in this worktree, do NOT reconfigure anything to force the landing — report a blocker instead."
  This is instruction text, not an enforced invariant, and it postdates the corruption it describes.

Root-cause analysis (cause → effect)

1. Precondition (upstream). The repo was already core.bare=true when the first integrate ran — set by the test-fixture git-env leak (companion root-cause ticket). So git merge --ff-only (which needs a work tree) failed.
2. Wrong recovery. Facing a failed ff-only, the mechanical-tier integrate agent improvised git update-ref refs/heads/main <sha> to "land" anyway. update-ref advances the branch pointer but touches neither the working tree nor the index.
3. Effect. main advanced to the union; the working tree stayed at the pre-run commit. Subsequent gates/verification in the primary checkout silently tested stale code. The second issue's integrate compounded it on the same bare repo.
4. Why the guard isn't enough. The hotfix guard tells the agent not to do this, but (a) it is prompt-level and a capable agent can still rationalize a workaround; (b) it offers no positive post-condition check that the land actually reached disk; (c) it does nothing to detect or repair a repo that is already corrupt when integrate starts.

Impact / blast radius

• A corrupted-but-advanced default branch is the worst kind of failure: it looks landed (ref moved, report says "done") while the code is not on disk and issues risk being closed on a lie.
• It defeats the mandatory final gate re-run if that re-run happens in the stranded checkout.
• It recurs for every issue in the wave once the repo is bare, multiplying the damage.

Proposed solution ideas

1. Structural pre-flight on integrate. Before landing, assert the repo is healthy: git rev-parse --is-bare-repository is false and the default branch is checked out in the current worktree. If not → park the issue with a loud blocker (exactly what the guard's prose now says, but enforced in code/return-schema, not left to agent discretion).
2. Post-condition verification. After the ff-only, assert git rev-parse HEAD equals the feature tip and the working tree is clean at that tip (git status --porcelain empty, a sentinel landed file present on disk). If the tree does not match the ref, treat the land as failed and park — never report integrated: true on a ref-only advance.
3. Never use ref-forcing verbs. The integrate/reconcile code paths should structurally avoid update-ref / branch -f / push . for landing; the only sanctioned land is git merge --ff-only from the default-branch worktree. If that is impossible, the state is abnormal → park.
4. Self-heal / detect on start-up (optional). The engine (or a preflight) could detect core.bare=true on the working repo at run start and refuse to start, pointing at the repair steps — turning a silent corruption into an actionable early error.
5. Regression coverage. In tests/test_orchestrate_workflow.py, add a structural test asserting the integrate prompt/return contract rejects a bare/no-worktree condition and that no landing path emits a ref-forcing command.

Acceptance criteria (sketch)

• Integrate refuses to land (parks with a specific blocker) when the repo is bare or the default branch is not checked out in the integrating worktree — enforced, not merely instructed.
• Integrate verifies, after --ff-only, that the working tree actually advanced to the feature tip; a ref-only advance is reported as a failed land, never integrated: true.
• No engine code path (integrate or reconcile) can land via git update-ref / git branch -f / git push ..
• A test locks in the refuse-and-park behavior for the bare/no-worktree case.
• (Optional) The run aborts early with a clear repair message if it starts against a core.bare=true working repo.

Provenance

Discovered by the orchestrate run wf_2d1a01f8-b49 (#48 + #50, merge mode). The run's own integration hotfix 7b8a3c2 added the prompt-level guard; this ticket asks for the structural enforcement + self-check + repair the guard does not provide. Upstream trigger tracked in the test-env root-cause ticket.

Blocked by

Blocked by #52 (land the git-environment scrub first: until it lands, every hook-verified commit made while implementing this issue risks corrupting the real repository through the leaked GIT_DIR/GIT_INDEX_FILE).

[TBarregren]

Related — same orchestrate run (wf_2d1a01f8-b49)

• test fixtures leak GIT_DIR/GIT_INDEX_FILE and corrupt the real repo (index + core.bare) under the pre-commit hook #52 — root cause / upstream trigger: a test-fixture git-env leak set core.bare=true, which is what made git merge --ff-only fail and pushed integrate onto the update-ref path this ticket hardens. Fix test fixtures leak GIT_DIR/GIT_INDEX_FILE and corrupt the real repo (index + core.bare) under the pre-commit hook #52 first; this ticket is the defense-in-depth so it cannot recur even if some other cause makes the repo bare.
• orchestrate.py status: build_status mis-attributes markers (ignores marker.number), so quoted grammar examples poison the board #54 — independent status-board bug from the same run.

[TBarregren]

This was generated by AI during triage.

Agent Brief

Category: bug (HIGH — defense-in-depth over #52's root cause)
Summary: Turn the integrate step's advisory "never strand the tree" prose into enforced structure: a repo-health precondition, an attested post-land tree check carried in the integrate return schema and enforced by the engine (a land that cannot attest the tree advanced is parked, never reported integrated), and a run-launch health check documented in the SKILL — all locked in by structural tests.

The issue body's evidence and root-cause analysis are verified and authoritative: the guard added by hotfix 7b8a3c2 lives only in the integrate prompt (skills/orchestrate/orchestrate.workflow.js, the NEVER bypass that refusal… block), and nothing checks afterwards that the working tree actually reached the landed tip. This brief settles the enforcement design.

Design constraint to respect

The engine (orchestrate.workflow.js) has no filesystem or shell access — it can only act on what sub-agents report through their schemas. "Enforced, not merely instructed" therefore means: (a) the prompt makes the checks an explicit numbered checklist, (b) the schema forces the agent to attest each check's result as structured booleans, and (c) the engine refuses to treat the issue as integrated unless the attested facts prove a healthy land — with structural tests (tests/test_orchestrate_workflow.py, the existing 125-test idiom) pinning all three so they cannot drift.

Settled decisions

D1 — Health precondition in the integrate prompt. Before any landing command, the agent must run git rev-parse --is-bare-repository (expect false) and confirm the default branch is checked out in the current worktree (git rev-parse --abbrev-ref HEAD). If either fails: do not land, do not repair, do not reconfigure — return integrated: false with a blocker naming the exact condition (e.g. core.bare=true). The engine parks the issue with that blocker (the existing park path).

D2 — Attested post-land verification in INTEGRATE_SCHEMA. Extend the schema with a required boolean, suggested name landVerified, plus a landedSha string. The prompt instructs: after git merge --ff-only, verify all of — git rev-parse HEAD equals the feature branch tip; git status --porcelain is empty; git rev-parse --is-bare-repository is still false — and set landVerified: true only when all hold, reporting the verified SHA as landedSha. The engine's integrate wrapper then enforces: a merge-mode result with integrated: true but landVerified !== true is treated as a failed land → parked with an explicit "ref-only advance suspected" blocker. (PR mode opens a PR and moves no ref, so landVerified applies to merge mode only; keep the schema field optional-by-mode or always-present-but-only-enforced-in-merge — implementer's choice, tests must pin whichever.)

D3 — No ref-forcing verbs, structurally pinned. The sanctioned landing remains exactly git merge --ff-only from the default-branch worktree (plus the existing free-worktree-then-rebase fallback). Add a structural test asserting the integrate prompt names update-ref, branch -f, and push . only inside the prohibition sentence — never as an instruction — so a future edit cannot reintroduce the workaround.

D4 — Run-launch health check (SKILL, no engine change). Add to skills/orchestrate/SKILL.md's pre-launch checklist: before launching the Workflow, the orchestrator verifies the primary checkout is healthy — git rev-parse --is-bare-repository is false and git status succeeds — and aborts with the repair recipe (git config core.bare false && git reset --hard HEAD) otherwise. This converts a silently corrupt starting state into an actionable early error, without giving the engine I/O it does not have.

Not adopted: engine-side self-repair of a corrupt repo. Repair is a human step (the recipe above); an agent rewriting repo config is exactly the behaviour class this ticket exists to forbid.

Key interfaces

• skills/orchestrate/orchestrate.workflow.js — the integrate prompt (D1, D2, D3), INTEGRATE_SCHEMA (D2), and the integrate wrapper's park-on-unattested-land enforcement (D2).
• skills/orchestrate/SKILL.md — the pre-launch health check (D4); mention the attested post-land check in the integrate description so SKILL and engine stay consistent.
• tests/test_orchestrate_workflow.py — structural tests for D1–D3 (prompt content, schema fields, engine enforcement path), red-first per house TDD.

Acceptance criteria

• The integrate prompt requires the bare-repo/branch-checkout health check before landing, and an unhealthy repo yields a parked issue with a blocker naming the condition — enforced via the return schema and engine park path, not prose alone.
• A merge-mode land is reported integrated only when the agent attested (landVerified) that HEAD equals the feature tip, the tree is clean, and the repo is non-bare; an unattested land parks with a "ref-only advance" blocker.
• Structural tests assert the ref-forcing verbs (update-ref, branch -f, push .) appear in the integrate prompt only as prohibitions, and that the engine's enforcement path exists — demonstrated red before the change, green after.
• SKILL.md documents the pre-launch repo-health check with the repair recipe.
• All existing tests stay green; ruff, mypy, and scripts/audit.py pass.

Out of scope

• The upstream test-environment leak — test fixtures leak GIT_DIR/GIT_INDEX_FILE and corrupt the real repo (index + core.bare) under the pre-commit hook #52 (this ticket is the safety net, not the trigger fix).
• Any change to orchestrate: interrupt-safe restart — detect a dead cross-session resume, never re-implement landed work #49's marker grammar, the preflight guard, the reporter role (orchestrate: continuous out-of-band progress reporting for multi-hour runs (status command + durable milestone markers) #48), or the status board (orchestrate: deterministic status command + push notifications (deferred R1a/R1c from #48) #50/orchestrate.py status: build_status mis-attributes markers (ignores marker.number), so quoted grammar examples poison the board #54).
• Engine-side repair of an already-corrupt repository (see "Not adopted").

[TBarregren]

Fixed in b1ca582 (pushed to main).

The integrate step now structurally verifies a merge-mode landing reached disk instead of trusting the agent not to strand it:

• INTEGRATE_SCHEMA gains five reported facts — bareRepo, defaultCheckedOut, headSha, featureSha, worktreeClean.
• A pure decision helper landStrandedBlocker (in lib/orchestrate/engine-helpers.mjs, mirrored byte-identically inline in the workflow, bound by the drift guard) decides soundness: a bare repo or a default not checked out in the integrating worktree is refused before any sha comparison; a missing sha, an unmoved HEAD (ref-only advance), or a dirty tree is a failed land.
• The engine — not the agent's own integrated flag — parks with a specific blocker whenever those facts do not prove the tree advanced to the feature tip (merge mode only; PR mode lands nothing on the default). The reconcile path lands through the same integrate(), so it inherits the gate — no engine code path can land via update-ref/branch -f/push ..

AC1–AC4 met and covered red-first (tests/test_orchestrate_workflow.py). Optional AC5 (early abort on a bare start-up) intentionally skipped — the per-issue check already parks loudly on bare.