T-XM0B-ensure-ready-script-emits-markers-and-cleanup

Status: closed/done · Impact: high · Complexity: small

Goal

PR #165 (marker namespacing) renamed task-ensure-ready’s terminal markers from bare READY: / NEEDS-DEFINITION: to slug-prefixed ENSURE-READY-OK: / ENSURE-READY-NEEDS-DEFINITION: to remove the collision class with task-work’s own terminal verdicts. PR #167 (state-on-main) added a NEEDS-DEFINITION cleanup path: when ensure-ready returns the fail marker, task-work tears down the worktree + branch + lease before returning its own TASK-WORK-NEEDS-DEFINITION verdict.

Both fixes are LLM-prose-driven, which makes them per-dispatch non-deterministic. Observed on 2026-05-28 during a single orchestrator tick: two parallel task-work dispatches against open/ready tasks that both surfaced a definition gap produced different outcomes. One sub-agent followed the new flow correctly (emitted TASK-WORK-NEEDS-DEFINITION slug=..., tore down its worktree+branch+lease). The other emitted the bare NEEDS-DEFINITION: <basename> marker AND skipped the teardown — leaving a stuck worktree+branch+lease that needed manual recovery. Same SKILL.md prose, same code path, different LLM interpretations.

The root cause is that the markers and the cleanup are printed/executed by the LLM running each skill, not by the underlying scripts. The SKILL.md prose tells the LLM what to print and do; the LLM “improvises” each invocation. Most of the time the prose is followed; sometimes it isn’t.

The fix: move both responsibilities into plugin/skills/task-ensure-ready/ensure_ready_mutate.py (the script that already does the file edit

commit). Make it print the deterministic marker on stdout and, behind an opt-in --cleanup-on-fail flag set by task-work’s invocation, perform the worktree+branch+lease teardown on the fail path. Then task-work’s invocation becomes one shell-out that returns either a clean PASS marker or a clean (already-cleaned-up) FAIL marker, with no LLM judgment between ensure-ready and the next step.

Today

Location	Role today
`plugin/skills/task-ensure-ready/ensure_ready_mutate.py`	Does the file mutation + frontmatter validation + commit. Returns exit 0 on success, non-zero on failure. Does not print any terminal markers — the `READY:` / `NEEDS-DEFINITION:` lines are LLM-improvised from the SKILL.md prose’s “Report on stdout” sub-step.
`plugin/skills/task-ensure-ready/SKILL.md`	Step 4’s “Report on stdout exactly: ENSURE-READY-OK: ” and Step 5’s equivalent for NEEDS-DEFINITION instruct the LLM to print the marker after the mutator returns. The marker shape is documented in prose; nothing in the script enforces it.
`plugin/skills/task-work/SKILL.md`	Step 5a’s NEEDS-DEFINITION-cleanup paragraph (added in PR #167) tells the outer task-work LLM to tear down the worktree+branch+lease when ensure-ready returns the fail marker. The teardown is three shell commands the LLM emits — none of them are script-encapsulated.
`plugin/scripts/sdlc_lease.py`	Provides `release <ref>` for lease teardown; ensure-ready could shell out to this for the cleanup path.
`plugin/skills/task-ensure-ready/test_ensure_ready_mutate.py` (if it exists, else sibling test_*.py)	Existing tests cover the file-mutation + commit-on-target-branch paths but not the marker-emission shape (because the script doesn’t emit markers today).

Proposed

ensure_ready_mutate.py:

On PASS (--mode pass success): print the marker on stdout exactly as ENSURE-READY-OK: <basename> followed by readiness_verified_at: <iso>. The shape matches what task-ensure-ready/SKILL.md currently documents.
On FAIL (--mode fail success): print the marker on stdout exactly as ENSURE-READY-NEEDS-DEFINITION: <basename> followed by definition_gap: <one-line summary>. Same shape as documented.
Add a new --cleanup-on-fail flag. When set AND the run takes the fail path AND the downshift commit lands successfully on main:
- Resolve the worktree path: <main-repo>/.sdlc/worktrees/<basename>. If it exists, run git worktree remove --force <worktree-path>.
- Resolve the task branch: task/<basename>. If it exists locally, run git branch -D <branch>. If a stale feat/<basename> legacy branch exists (transition window — already squashed in #168 but the code can be defensive), don’t bother — task/<basename> is the only shape going forward.
- Release the lease: plugin/scripts/sdlc_lease.py release refs/sdlc/tasks/<basename>. Tolerate REF-NOT-FOUND (treat as already-released, no error).
- After cleanup, the script prints an additional marker line: cleaned-up: worktree=<removed|absent> branch=<deleted|absent> lease=<released|absent>.
The --cleanup-on-fail flag is opt-in. Standalone /sdlc:task-ensure-ready invocations (outside task-work) do NOT set it; they want to leave the worktree as-is for the operator. Only task-work’s invocation sets it.
Exit codes stay the same: 0 on success (both PASS and FAIL paths emit their respective markers and exit 0); non-zero on error (validator rejection, lease release library failure, etc.).

task-ensure-ready/SKILL.md:

Step 4 / Step 5 prose updated: the script now prints the marker. The LLM running the skill does NOT re-print it on top — it relays whatever the script wrote.
Add a “When called by /sdlc:task-work” sub-section documenting the --cleanup-on-fail flag’s contract.

task-work/SKILL.md:

Step 5a’s invocation of /sdlc:task-ensure-ready now passes --cleanup-on-fail through the --commit-on main invocation.
The NEEDS-DEFINITION cleanup paragraph (the three-shell-command teardown the LLM was supposed to do) is replaced with a single sentence: “ensure-ready handled the teardown via --cleanup-on-fail; just emit TASK-WORK-NEEDS-DEFINITION slug=<basename> and exit.”
task-work’s own terminal verdict on the fail path remains TASK-WORK-NEEDS-DEFINITION slug=<basename> — that’s the documented contract and stays LLM-emitted at the task-work level (the task-work skill itself is what emits this; if we want to harden this too, that’s a separate task).

Approach

Add marker emission to ensure_ready_mutate.py’s PASS path. After _commit_in returns 0, print the two-line marker block:
```
ENSURE-READY-OK: <basename>
readiness_verified_at: <iso>
```
The <iso> value comes from the now variable already in scope (line ~398).

Add marker emission to the FAIL path. Same shape:

ENSURE-READY-NEEDS-DEFINITION: <basename>
definition_gap: <one-line summary>

Add the --cleanup-on-fail flag to argparse. Default false. Only meaningful when --mode fail is set.
Implement the cleanup-on-fail branch. After the FAIL marker prints, if the flag is set:
- Resolve <main-repo> via _resolve_main_checkout (helper already exists).
- Compute <worktree-path> = <main-repo>/.sdlc/worktrees/<basename>.
- Run the three teardown commands, tolerating already-clean states (worktree-not-present, branch-not-present, REF-NOT-FOUND for the lease).
- Print cleaned-up: worktree=<state> branch=<state> lease=<state>.
Update tests. Add cases for the marker-emission shape (PASS and FAIL), and for the cleanup-on-fail teardown behaviour. The teardown test creates a worktree+branch+lease via the lease CLI, runs the fail path with --cleanup-on-fail, asserts all three are gone after.
Update SKILL.md prose in both task-ensure-ready and task-work per Proposed.
Run quality checks with the baseline-gated invocation. The lint_task_state_commit_origin linter should not flag anything (all changes are implementation diff, no task-state frontmatter on the task branch).

Files to touch

Location	Kind	Change
`plugin/skills/task-ensure-ready/ensure_ready_mutate.py`	modify	Add marker emission on PASS + FAIL paths; add `--cleanup-on-fail` flag; implement worktree+branch+lease teardown when flag is set on the fail path.
`plugin/skills/task-ensure-ready/SKILL.md`	modify	Step 4 + Step 5 prose: script emits the marker; LLM relays it. Document `--cleanup-on-fail` contract under a “When called by task-work” sub-section.
`plugin/skills/task-work/SKILL.md`	modify	Step 5a’s invocation passes `--cleanup-on-fail`. NEEDS-DEFINITION-cleanup paragraph replaced with “ensure-ready handled teardown; emit TASK-WORK-NEEDS-DEFINITION slug= and exit.”
`plugin/skills/task-ensure-ready/test_ensure_ready_mutate.py` (or sibling test_*.py if naming convention differs)	modify	Add marker-emission shape tests for PASS + FAIL; add cleanup-on-fail teardown test (worktree + branch + lease all gone after fail run with the flag).

Acceptance criteria

AC-1: plugin/skills/task-ensure-ready/ensure_ready_mutate.py --mode pass --commit-on main ... (against a fixture task) writes the marker on stdout exactly as ENSURE-READY-OK: <basename> + readiness_verified_at: <iso>. Verified by a unit test that captures stdout and asserts the literal line shape.
AC-2: Same script --mode fail --gap "..." --commit-on main ... writes ENSURE-READY-NEEDS-DEFINITION: <basename> + definition_gap: <gap> on stdout. Verified by a unit test.
AC-3: With --cleanup-on-fail added to the fail-path invocation, the worktree at .sdlc/worktrees/<basename>, the branch task/<basename>, and the lease refs/sdlc/tasks/<basename> are all removed after the script exits 0. Verified by a unit test that sets up all three in a fixture and asserts each is gone post-run.
AC-4: A live /sdlc:task-work run against a task with a known definition gap produces: (a) the downshift commit on main, (b) the worktree+branch+lease all gone, (c) the outer terminal verdict TASK-WORK-NEEDS-DEFINITION slug=<basename>. The bare NEEDS-DEFINITION: <basename> form must not appear in the verdict — the LLM relays the script’s slug-prefixed marker, not the documented prose shape. Validated by a manual dispatch against a task with a stale Today row.

AC-5:

/Users/sksizer2/.claude/plugins/sdlc/scripts/run_quality_checks.py --config /Users/sksizer2/Developer/dev/sdlc.yaml --diff-against-baseline <origin-main-sha> --baseline-dir /Users/sksizer2/Developer/dev/.sdlc/quality-baselines --line

reports zero new drift.

Out of scope

Hardening task-work’s OWN terminal verdict emission. task-work currently has the outer LLM emit TASK-WORK-DONE pr=#<N> / TASK-WORK-BLOCKED reason= / TASK-WORK-NEEDS-DEFINITION slug= based on SKILL.md prose. The same LLM-nondeterminism risk applies, but it’s a larger refactor (task-work is the orchestrator, not a script that can be shelled out to). If marker drift becomes an observed issue at the task-work level, that’s a follow-up task.
task-define’s marker emission (TASK-DEFINE-DEFINED: / TASK-DEFINE-NO-CHANGES: / TASK-DEFINE-ALREADY-READY:). Same pattern; deferred to a follow-up.
spawn-task-pr’s marker emission (SPAWN-TASK-PR-DONE pr=...). Same pattern; deferred.
Refactoring the lease release call into a Python import rather than a shell-out. Keeping the shell-out boundary so the script remains independently runnable from a shell session for ops/debugging.

Dependencies

none — sits on top of the post-#167 + post-#165 + post-#168 codebase.

Discovery context

Surfaced 2026-05-28 during the first batch of two parallel post-#167 task-work dispatches. Both tasks were open/ready with stale Today rows (drift the validator catches). Both sub-agents took the FAIL path inside ensure-ready and produced a downshift commit on main correctly. One sub-agent (plugin-scripts-self-discover-project-root) then followed the SKILL.md prose perfectly: emitted TASK-WORK-NEEDS-DEFINITION slug=... as task-work’s verdict AND tore down its worktree+branch+lease. The other (python-runtime-doc-additions) emitted the bare NEEDS-DEFINITION: <basename> form (ensure-ready’s intermediate marker, leaked) AND skipped the teardown — same SKILL.md, same code path, different LLM-prose interpretation. The split-outcome pattern names the abstraction failure: LLM-improvised markers + LLM-improvised cleanup is fragile. The fix moves both responsibilities into the deterministic script tier.

Post-mortem

Captured by /sdlc:task-work on 2026-05-28. PR: pending.

Acceptance criteria coverage

AC-1: auto — plugin/skills/task-ensure-ready/tests/run_evals.py case pass-marker-shape asserts the literal two-line ENSURE-READY-OK: <basename> + readiness_verified_at: <iso> shape captured from stdout against a fixture repo on --commit-on main.
AC-2: auto — same suite’s fail-marker-shape case asserts ENSURE-READY-NEEDS-DEFINITION: <basename> + definition_gap: <gap> from stdout on the --mode fail --commit-on main path.
AC-3: auto — cleanup-on-fail-tears-down-worktree-and-branch builds a fixture repo with git worktree add + task/<basename> branch and asserts both are gone after --cleanup-on-fail; the lease verb is exercised via the deterministic “no sdlc.yaml ⇒ lease=absent” path (hermetic, no external authority needed). Companion case cleanup-on-fail-tolerates-absent-targets covers the idempotent “everything already gone” run.
AC-4: deferred-user — a live /sdlc:task-work dispatch against a task with a known definition gap requires the merged PR to propagate to ${CLAUDE_PLUGIN_ROOT} first (the runtime SKILL.md the operator’s harness loads is the global plugin path, not the worktree). Please verify after merge by running /sdlc:task-work <slug> against a planning/proposed task with a stale Today row and confirming (a) the downshift commit landed on main, (b) the worktree+branch+lease are all gone, (c) the outer verdict is TASK-WORK-NEEDS-DEFINITION slug=<basename> (NOT the bare NEEDS-DEFINITION: <basename> form).
AC-5: auto — run_quality_checks.py --diff-against-baseline 079f7957d... --line reported OK 14/14 (baseline-gated; pre-existing findings ignored) — zero new drift.

What worked

The existing ensure_ready_mutate.py already had a clean separation between argparse, mutation helpers, and the commit pipeline, so the marker emission slotted in with one helper per shape and a small main() change.
The existing eval suite’s _build_marker_fixture_repo pattern (init bare repo + seed task fixture) was easy to reuse for the 7 new behavioural tests.
run_quality_checks.py --diff-against-baseline made it cheap to confirm zero new drift even though the baseline already had ~243 pre-existing findings.

Friction and automation gaps

A pre-existing grep alias in the user’s shell (--no-heading not recognized) interfered with one diagnostic command; mitigated by switching to command grep. The SKILL.md already calls this out in Step 2’s relevance check, but the same hazard hit a casual diff confirmation here — task-work or the validator helpers could shell out via command grep consistently in any wrapped diagnostics they emit.
The runtime SKILL.md for /sdlc:task-ensure-ready loaded by the Skill harness is the pre-#165/#167 version (no --commit-on main, no slug-prefixed markers), so when this skill invoked it via the Skill tool the operator-facing prose disagreed with the worktree’s source. The parent task-work prose anticipates this (“the runtime version of each is loaded from ${CLAUDE_PLUGIN_ROOT}, not the worktree”), but only by manual hook output — the SKILL.md surface could carry an explicit “if these instructions conflict with the worktree’s source, the worktree wins for this task” header to make the precedence unambiguous.
The placeholder scanner flagged this task’s <basename> / <iso> mentions as disqualifiers because they live in markdown table cells and prose, even though they’re meta-discussion of marker shapes. The scanner already skips fenced code blocks; it could also skip rows that are clearly describing marker/template shapes (e.g. rows whose left column is a path ending in .py or .md and whose right column is descriptive prose mentioning the shape), or — narrower — allow <basename>, <iso>, <one-line summary>, <sha>, <...> as known marker-shape tokens when the surrounding prose contains “marker” / “shape” / “stdout” / “emits” within ±2 lines.

← Back to Tasks