…upe defense + fail-closed resume guard (#44)

Compounds on PR #42 (v1.40.2.0). PR #42 fixed the main bug (slice-tail
merge mis-aligning state.phases when feature-review inserts mid-array)
but left three adversarial-review concerns unaddressed:

1. Parser does NOT dedupe phase numbers. PR #42's reconciler comment
   claimed "both are unique within a plan because the parser rejects
   duplicate headings" — wishful thinking. If an LLM verdict emits
   `### Phase 1.review-1` twice in additionalPhasesMd, the by-number
   Map aliases the same PhaseState into two array slots and a status
   write on one silently mutates both. Same risk if state.phases on
   disk has a duplicate from a pre-fix gstack version's slice merge.

2. No resume-time alignment guard. cli.ts after loadState ran
   backfillKindFromPlan and proceeded. If state.phases is desynced on
   disk (pre-fix corruption, hand-edited plan), the build would run
   against stale slot mapping. PR #42's reconciler only fires on the
   in-run FEATURE_NEEDS_PHASES path.

3. Feature-only drift (user renames `## Feature N:` between runs)
   slipped past silently — phase count + numbers could match while
   state.features had stale metadata that downstream gates would read.

Fixes:

state.ts:
- reconcileStatePhasesAfterReparse throws on duplicate phase numbers
  in EITHER the parser side (LLM verdict bug, hand-edited plan) or
  state side (pre-fix on-disk corruption). Defense fires before any
  mutation, so a thrown reconciler leaves state.phases/.features
  untouched and the cli.ts catch block can surface via
  BLOCKED-feature-N.md.
- New arePhasesAligned(state, reparsed) predicate. Checks both
  phase-level (length + per-index .number) and feature-level
  alignment. Used by the new cli.ts resume guard.

cli.ts:
- Resume path: after backfillKindFromPlan, call arePhasesAligned.
  On disagreement, print a remediation block naming all four counts
  (state vs parsed, phases and features) and three recovery paths
  (--no-resume, delete state file, manual realign), then exit code 2
  via the existing setupFailed flow. Does NOT auto-merge on resume:
  runtime artifacts (gemini outputs, codexReview, committedAt) on
  shifted slots belong to the work that physically ran there, not
  to the phase whose `.number` was stored. By-number merge would
  silently re-attribute them — same silent-corruption shape as the
  bug PR #42 fixed.
- Import statePath for the remediation message.

tests:
- New build/orchestrator/__tests__/state-reconcile-hardening.test.ts.
- 9 tests / 14 assertions covering:
  * Parser-side duplicate-number rejection (LLM emits same heading twice).
  * Pre-mutation guarantee: state.phases / .features pristine on throw.
  * State-side duplicate (pre-fix on-disk corruption).
  * arePhasesAligned: aligned, phase-count drift, per-index number drift,
    feature-count drift, feature-number drift at same index, undefined
    state.features tolerance.

Suite: 1339 pass / 2 skip / 0 fail across 54 files (4135 assertions).
PR #42's reconciler tests (17 pass, including 8 in state-reconcile.test.ts
+ append-and-reconcile-integration.test.ts) unchanged.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…-flake timeout tests (#46)

* feat(build/orchestrator): list existing phase numbers in FEATURE_NEEDS_PHASES prompt

The feature-reviewer picks `K` for `### Phase N.review-K` headings with no
feedback loop across review cycles. Without an explicit collision-set, the
LLM can re-emit the same K. The reconciler now (v1.40.3.0 hardening) fails
closed on duplicate phase numbers — better to prevent the collision
upstream than to recover from it.

Add a constraint sentence to the `## Additional phases` block listing every
phase number currently under the feature, prefixed with K MUST NOT collide
guidance. Defensive `(none)` rendering for the unreachable case of a
feature with zero phases.

Two new tests in feature-review.test.ts cover the K-history block (existing
phases including `1.review-1` listed in backticks) and the `(none)`
fallback. All 35 prompt-builder tests pass.

Adversarial review finding #7 from PR #42's /ship session; resolves the
upstream contributor to findings #1 and #4 (now handled at the reconciler
by PR #44).

* fix(build/orchestrator): de-flake retry/no-retry timeout tests (500ms→2000ms)

The two timeout-budget tests in sub-agents.test.ts ("retries on timeout by
default" and "skips retry when retryOnTimeout: false") spawn a fake kimi
shell script that appends to a counter file before sleeping, then assert
the file ends with N entries (2 for retry, 1 for skip).

The 500ms / 800ms timeouts gave the spawned shell insufficient budget to
flush its `echo call >> $callsPath` write under parallel test-suite load.
SIGTERM landed before the append was observable on disk, so the counter
came back with one fewer entry than expected. The retry/skip logic itself
was correct — only the assertion mechanism was brittle.

Measured at 500ms: 4/5 fail in `bun test build/orchestrator/__tests__/`,
3/3 pass in `bun test sub-agents.test.ts` alone. Race only fires under
concurrency. Bumping to 2000ms gives a 4x margin on the spawn+flush
window. Wall-clock for the retry test goes from ~1s to ~4s (acceptable
given a single test of 199 in the file).

Acceptance gate: orchestrator suite ran 5x in a row with sub-agents tests
green every run. Pre-existing skill-md.test.ts failures unchanged
(unrelated fork-skill drift).

* chore: bump version and changelog (v1.40.4.0)

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* docs: note v1.40.4.0 upstream collision prevention in state-machine doc

Section 6.5 (Phase-array reconciler) already describes the v1.40.3.0
fail-closed dedup. v1.40.4.0 added an upstream layer: the
FEATURE_NEEDS_PHASES prompt now lists existing phase numbers under the
feature so the reviewer model can pick a fresh K, instead of relying on
the reconciler to reject duplicates after the fact. Document it next to
the existing reconciler write-up so the prevention/recovery pairing is
discoverable.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>

…ark-phase-committed dirty-tree recovery (#48)

* fix(build/orchestrator): un-flip plan checkboxes when restartFeatureFromOriginIssues rewinds a committed phase

Root cause of the 2026-05-18 mitosis PREMATURE_COMPLETION fault class.

When origin verification fails after a feature reaches phases_done,
restartFeatureFromOriginIssues (cli.ts:3385) rewinds the last
committed phase back to `tests_green` and re-points currentPhaseIndex
at it so the review/QA loop re-runs. The status rewind is intentional
and load-bearing — the re-run is how origin-plan gaps get patched.

But the plan markdown checkboxes (flipped by markCommitted via
flipPhaseCheckboxes at cli.ts:5835) were NOT un-flipped. If the re-run
then fails — any of the 22 `phaseState.status = "failed"` sites in
cli.ts fires — the phase ends up at status=failed with checkboxes
still [x][x][x]. That's the exact PREMATURE_COMPLETION invariant the
fault detector fires on (skill-fault-detector.ts:429).

The fault reports from 2026-05-18 (agnt2-prototype, mitosis-control-plane,
mitosis-prototype-v3.1) claimed the mechanism was per-sub-step incremental
checkbox flipping. That hypothesis was wrong — there are only 5 flip sites
in cli.ts and all are gated. The actual mechanism is the rewind-without-
un-flip race in the origin-verification restart path.

Fix:
- New unflipPhaseCheckboxes(planFile, phase) in plan-mutator.ts: the
  symmetric counterpart of reconcilePhaseCheckboxes. Flips [x] → [ ]
  for test-spec, implementation, and review checkboxes using the
  existing kind-specific markers (IMPL_MARKER_BY_KIND, etc).
- restartFeatureFromOriginIssues accepts optional `phases: Phase[]`
  arg. When the rewound phase was `committed` AND phases is passed,
  un-flips that phase's checkboxes. Older callers that omit phases
  (existing tests) get the pre-fix behavior — no breaking change.
- Both production call sites (cli.ts:9875, cli.ts:10018) updated to
  pass phases (already in scope at both call sites).

Markdown must follow state backward as faithfully as it follows forward.

Tests:
- plan-mutator.test.ts: 5 new tests for unflipPhaseCheckboxes covering
  all-three-flip, skip-test-spec, idempotent, kind-specific markers
  (writing), and the round-trip symmetry with reconcilePhaseCheckboxes.
- cli.test.ts: 2 new tests for restartFeatureFromOriginIssues — one
  proves the un-flip fires on a real temp plan file when phases is
  passed (and that non-rewound phases are NOT touched), one proves
  omitting phases preserves pre-fix behavior for legacy callers.

All 52 plan-mutator tests pass. All 4 restartFeatureFromOriginIssues
tests pass.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat(build/orchestrator): --mark-phase-committed dirty-tree guard with --commit-dirty / --force-dirty

Closes the recovery anti-pattern that all four 2026-05-18 mitosis
PREMATURE_COMPLETION faults triggered: --mark-phase-committed
silently force-marked over a dirty worktree, leaving the next phase
to start on inconsistent state.

The mitosis-prototype-v3.1 fault report's "Recovery Performed" section
shows the failure shape clearly: --mark-phase-committed 2.1 advanced
the state machine but left 5+ files Codex had written during QA
uncommitted. The next feature's worktree-clean check would see those
files but the state would say the previous feature was committed —
ambiguous on whose work it was.

This is orthogonal to PR #41's auto-split path. Auto-split runs inside
the gate flow when the agent exits 0; --mark-phase-committed is the
operator's emergency recovery tool used AFTER the gate has already
failed. Until now it had no dirty-tree opinion at all.

New behavior:
- Dirty worktree + no flag → REFUSE with exit 2, list dirty files,
  print the two recovery options.
- --commit-dirty → `git add . && git commit -m "fix(recovery): <phase>
  auto-commit..."` then mark. Pre-commit hooks still run (we do NOT
  pass --no-verify; if a hook fails, the operator sees the hook output
  and can fall back to --force-dirty).
- --force-dirty → emit WARN listing dirty files, mark anyway, leave
  the dirty state on disk. For when the operator has reviewed the
  dirty files and wants them preserved (e.g. WIP work they'll commit
  manually after the mark).
- The two flags are mutually exclusive (clear error on conflict).
- Clean tree → no-op, marks as before.
- dryRun → skips the guard entirely (preview must not inspect git).
- cwd omitted → skips the guard (preserves backwards compat for
  legacy callers / unit tests that exercise the state-only transition
  without a real git fixture).

Implementation:
- New optional args on markPhaseCommittedAfterManualRecovery: cwd,
  forceDirty, commitDirty. Refused-with-dirty-files response includes
  the structured dirtyFiles list so callers can render it.
- CLI argv: new --force-dirty and --commit-dirty flags wired through.
- --help text updated.
- Production call site (cli.ts:9020) passes projectRoot as cwd and
  forwards both flags.

Tests:
- 7 new tests in the "markPhaseCommittedAfterManualRecovery > dirty-
  tree guard" describe block. Each test stands up a real git repo
  (`git init` + seed commit) so captureGitSnapshot has something real
  to inspect. Coverage:
  - refuses without a flag (verifies dirtyFiles in response + error
    message lists both recovery options + state is NOT mutated).
  - --force-dirty marks anyway (dirty file still on disk + uncommitted).
  - --commit-dirty stages + commits (dirty file in HEAD + recovery
    message present + dirty.txt no longer in `git status --porcelain`).
  - mutual exclusivity (clear error).
  - clean tree no-op (no flag needed).
  - cwd omitted skips guard (legacy callers).
  - dryRun skips guard (preview mode).

All 259 cli.test.ts tests pass. All 1362 orchestrator tests pass.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs(inbox): revised skill-faults fix plan with corrected findings

The original plan I drafted earlier today was Explore-agent-summarized
analysis. When I re-read the orchestrator code directly to start
implementation, I found that:

- All four 2026-05-18 faults pre-date PR #41 (squash-merged at
  2026-05-18T13:37Z, last fault detected 12:26Z).
- PR #41 was already a deliberate four-failures response to a different
  mitosis-oasis batch the same day, shipping most of what the original
  draft proposed (auto-split mixed test+prod diffs, producer-side
  layer-purity rule).
- The original draft's Fix 1 premise was factually wrong: the fault
  reports asserted per-sub-step incremental checkbox flipping; the
  orchestrator has 5 flip sites total, all gated. That mechanism does
  not exist in shipped code.

The actual mechanism: restartFeatureFromOriginIssues rewinds a
committed phase to tests_green without un-flipping plan checkboxes.
A subsequent re-run failure produces the PREMATURE_COMPLETION
[x][x][x] + status=failed signature.

Revised plan documents:
- What's already shipped by PR #41 / PR #42 / PR #44 (don't
  re-implement).
- Two narrow gaps that were real, both fixed on this branch:
  - Fix 1 (un-flip on rewind) — commit 381cf84.
  - Fix 4 (dirty-tree guard on --mark-phase-committed) — commit
    bfdd569.
- Why the original draft's Fixes 2, 3, 5, 6 were descoped (better
  approaches already shipped, or speculative upstream issue).
- What's still not addressed: defensive markFailed() guard against
  the broader committed→failed overwrite class (22 status="failed"
  sites in cli.ts, only one confirmed bridge).
- Lessons: read code before designing fixes, fault reports are
  failure-shape artifacts not code-truth, open questions often have
  answers in types.ts.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* test(build/orchestrator): close coverage gaps surfaced by ship audit

The ship-time coverage audit on this branch flagged ~65% coverage
(above 60% min, below 80% target). The two material gaps:

1. parseArgs flag→arg mapping for --commit-dirty / --force-dirty was
   uncovered (only the downstream behavior was tested).
2. unflipPhaseCheckboxes error-accumulation path was uncovered (only
   happy paths were tested).

This commit adds:

- 5 tests in cli.test.ts for the new flag wiring, mirroring the
  pattern from --skip-ship / --single-branch / --ship-on-plan-complete
  describe blocks: defaults both false, --force-dirty sets one,
  --commit-dirty sets the other, both can be set at parser level
  (the mutex check lives in markPhaseCommittedAfterManualRecovery),
  --help mentions both flags.

- 1 test in plan-mutator.test.ts for unflipPhaseCheckboxes when
  setCheckboxState rejects a line (wrong expectedMarker, simulating
  hand-edited plan between parse and rewind). Verifies the function
  collects the error but continues un-flipping the remaining
  checkboxes, returning a partial result.

Coverage now ~80% (target). All 11 new tests pass.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(build/orchestrator): dirty-tree guard fails closed when git status errors

Adversarial review finding (confidence 9/10):
markPhaseCommittedAfterManualRecovery's dirty-tree guard was
silently treating `git status` failures as "clean tree." When
captureGitSnapshot can't read status (stale .git/index.lock,
non-git directory, permissions issue), it encodes the failure
as a single `<git error: ...>` line in snapshot.status. The
guard filtered those out before counting dirty files, so the
dirty list was empty → guard bypassed → silent force-mark over
unknown worktree state. This recreates a narrower version of
the exact silent-mark-over-dirty bug the guard was added to fix.

Now: detect the `<git error: ...>` sentinel before filtering and
return `ok: false` with a clear message naming the git error and
the retry options. The check is bypassed only by --force-dirty,
which already opts the operator into "I know the tree state may
be unknown — mark anyway."

Tests:
- "fails closed when captureGitSnapshot reports a git error":
  points cwd at a non-git directory, verifies refusal with
  --force-dirty in the error message and state NOT advanced.
- "--force-dirty bypasses git-error fail-closed": same fixture
  + --force-dirty, verifies the phase commits (operator
  explicitly accepted the unknown state).

All 12 dirty-tree guard tests pass. All 319 cli + plan-mutator
tests pass.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(build/orchestrator): atomic unflipPhaseCheckboxes + fail-closed restartFeatureFromOriginIssues

Codex adversarial review (BLOCK verdict) caught two real holes in
the rewind path that would have recreated the PREMATURE_COMPLETION
bug class this branch claims to fix.

Hole 1 (plan-mutator.ts): unflipPhaseCheckboxes did three SEPARATE
atomic writes. If the test-spec flip succeeded, the impl flip failed
marker validation, and the review flip succeeded, the plan was left
half-rewound — test-spec back to [ ], impl still [x], review back
to [ ]. That's silent corruption under exactly the hand-edit race
the function exists to handle.

Hole 2 (cli.ts:3465): restartFeatureFromOriginIssues only console.warn'd
on un-flip errors but still advanced state (phase status: committed →
tests_green). Result: state rewound, markdown still [x][x][x] → the
exact PREMATURE_COMPLETION invariant the fault detector fires on.

Fixes:

plan-mutator.ts — unflipPhaseCheckboxes is now ALL-OR-NOTHING:
1. Validate every target line first (no writes). Collect errors.
2. If ANY validation error → return errors, plan markdown unchanged
   byte-for-byte. No partial state on disk.
3. If all valid → ONE atomic temp+rename write applies all flips
   together. Either every checkbox flips or none do.

cli.ts:3465 — restartFeatureFromOriginIssues now FAILS CLOSED:
- Run un-flip BEFORE advancing state.
- If un-flip returns errors: pause the feature, set feature.error
  with the specific markdown drift details + manual recovery
  instructions, return `restarted: false`.
- Phase status stays `committed` — the rewind never happened,
  so the markdown's [x][x][x] is still accurate. PREMATURE_COMPLETION
  cannot fire.
- Only after the markdown is successfully rewound does state advance
  to tests_green.

Tests:

plan-mutator.test.ts — replaced the old "collects errors without
throwing" test with "ATOMIC: when any target fails validation, NO
checkbox is flipped (all-or-nothing)". Asserts:
- errors.length === 1
- unflipped === 0 (was 1 in the old broken behavior)
- plan markdown is byte-for-byte unchanged

cli.test.ts — new test "FAIL-CLOSED: pauses feature when
unflipPhaseCheckboxes errors (no state advance)". Sets up a plan
where the impl marker was hand-renamed mid-flight, calls restart,
asserts:
- restarted === false
- feature.status === "paused"
- feature.error contains "plan markdown drift" and the phase number
- state.phases[1].status STAYS "committed" (no rewind)
- plan markdown is byte-for-byte unchanged

All 53 plan-mutator tests pass. All 267 cli tests pass.

Codex adversarial review verdict: BLOCK (correctly).
After this commit: PREMATURE_COMPLETION race is closed for real.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* chore(build/skill): bump skill frontmatter version 1.23.0 → 1.24.0

Fork-local skill release per CLAUDE.md "Fork versioning rule":
no top-level VERSION or package.json bump (those track upstream
gstack at v1.40.4.0). This branch's changes are scoped entirely
to the /build skill orchestrator (plan-mutator.ts un-flip,
cli.ts dirty-tree guard + restart fail-closed), so they get
their own skill-local version bump.

MINOR bump (1.23.0 → 1.24.0) because:
- New public capability: unflipPhaseCheckboxes (atomic rewind helper)
- New public capability: --commit-dirty / --force-dirty flags
- New defensive behavior: fail-closed on restart un-flip errors
- New defensive behavior: fail-closed on git status errors during
  dirty-guard

All defensive, no breaking changes (existing callers that don't pass
the new optional args get the pre-fix behavior).

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs(build): document --commit-dirty/--force-dirty + un-flip on origin-rewind

Sync orchestrator-state-machine.md, build/orchestrator/README.md, and the
/build SKILL template to match what shipped in fix/skill-faults-2026-05-19:

- §6 hygiene gate: cross-reference the new recovery-path dirty-tree guard
- §6.6 new section: restartFeatureFromOriginIssues atomic un-flip contract,
  fail-closed semantics, why markdown must rewind in lockstep with state
- §8 markPhaseCommittedAfterManualRecovery: dirty-tree guard policy table
  (clean / no-policy / commit-dirty / force-dirty / both / git status error)
- §9 glossary: refresh stale cli.ts line numbers, add unflipPhaseCheckboxes
  and restartFeatureFromOriginIssues pointers
- build/orchestrator/README.md: three new failure-modes table rows; new
  "Manual phase recovery (--mark-phase-committed)" section explaining when
  to use --commit-dirty vs --force-dirty
- build/SKILL.md.tmpl: short paragraph surfacing the dirty-tree guard so
  agents invoking --mark-phase-committed see the new safety inline
- build/SKILL.md: regenerated from template (bun run gen:skill-docs)

No CHANGELOG / VERSION bump (fork-local rule; /build skill frontmatter
already at 1.24.0).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>