T-SSB8-split-pr-respond-and-close-out-with-lease-reacquire

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

Goal

Extract /sdlc:pr-respond as a dedicated skill (it currently lives as Steps 10/12 of pre-cutover /sdlc:task-work) and wire both /sdlc:pr-respond and /sdlc:task-close-out to re-acquire the task lifecycle lease before performing any side effect. After this task lands, every PR review cycle has three lease-aware skills (task-work → pr-respond → task-close-out), each operating on a distinct lease phase, with the lease ref as the coordination boundary between them. Closes rollout item 7 of the ADR.

The split is what makes Model B from T-TZSN-task-work-lease-integration-model-b complete. Task-work exits at PR open with the lease in awaiting-review; pr-respond re-acquires that lease when review feedback arrives and rotates ownership to the responding worker; task-close-out re-acquires when the PR merges and finalizes the lease with a CAS-DELETE (or archive-then-delete per the ADR’s Release and archive section). Each skill is independently retryable, independently testable, and never trusts the calling context to hold a valid lease — the lease ref is the source of truth.

Today

/sdlc:task-close-out exists as a standalone skill (carved out by T-RDKI-extract-task-close-out-skill under epic E0001) but is lease-unaware — it operates on git worktree list + frontmatter status: and trusts the caller to invoke it on a merged PR. No re-acquisition step exists; no CAS guard protects against two close-out workers racing on the same task. /sdlc:pr-respond does NOT yet exist as a standalone skill — the PR-feedback loop currently lives inside /sdlc:task-work (Step 12, “if review comments arrive while waiting for merge, respond and push”). The pre-cutover task-work is what handles the entire PR lifecycle; the new shape from T-TZSN-task-work-lease-integration-model-b removes that loop, so pr-respond has to land before (or at the same time as) the slice-2 cutover.

Today’s PR-feedback handling is a sub-agent invoked from inside task-work’s Step 12 prose: a Claude Code Agent call dispatched at the moment a review comment is detected. There’s no atomicity guard — if two operators happened to drive task-work for the same PR (unlikely but possible), both would attempt to push fixes against the same branch. The lease re-acquisition pattern in this task is what makes that race a CAS failure rather than a merge conflict.

Proposed

This task consumes the branch-derivation contract from PR #118 and the acquire_lease helper from PR #124. The new mechanic is reacquire_lease(task_id, target_phase) — CAS-REPLACE-with-steal-if-expired semantics for transitioning an existing lease through phases without ever doing a fresh cas_create. No env vars, no argv lease state.

Extract /sdlc:pr-respond <pr-number> as a standalone skill. Lives at plugin/skills/pr-respond/SKILL.md. Behavior: given a PR number, the skill reads the PR body, extracts the  footer (planted by T-TZSN-task-work-lease-integration-model-b), cd’s into the task’s worktree at .sdlc/worktrees/<task_id>, calls discover_lease() to read current state, validates that the discovered lease_id matches the footer (fencing check per the ADR’s Fencing and PR↔lease binding), and calls reacquire_lease(task_id, "responding") to CAS-REPLACE the lease into the responding phase. Only after the CAS succeeds does the skill apply review feedback (via an Agent sub-agent), heartbeat during work, push the response commits, and call reacquire_lease(task_id, "awaiting-review") to transition back. Exits with PR-RESPONSE-COMPLETE: pr=<n> task=<task-id>.

Teach /sdlc:task-close-out to re-acquire the lease. Existing close-out logic stays — tear down the worktree, delete the branches, write completion_note, flip status to closed/done — but is gated on a successful reacquire_lease(task_id, "closing") call. After all side effects complete, close-out calls a new release_to_archive(task_id) helper that atomically writes the final lease state to refs/sdlc/archive/tasks/<task-id> (via cas_create) and CAS-DELETE’s the active ref. If reacquire_lease fails at the start of close-out (another worker raced in), close-out exits cleanly with LEASE-CONFLICT and the operator decides whether to re-invoke.

Both skills follow the branch-derivation contract from PR #118. They derive task_id from current_task_id() once they’re cd’d into the worktree. No env vars are read; no argv lease state is consumed. Pr-respond additionally consumes the footer-derived lease_id for the fencing check (PR ↔ lease binding). If no lease ref exists for the task, both skills exit with LEASE-MISSING ref=<ref> and point the operator at task-work as the entry point.

Steal-on-expired semantics. reacquire_lease honors the ADR’s Steal an expired lease protocol: if the current lease’s expires_at <= now, the CAS-REPLACE proceeds with a rotated lease_token and the current host’s host_id as the new owner. This lets a long-stale awaiting-review lease be picked up by a fresh pr-respond or task-close-out worker without manual operator intervention.

No fallback path. Both skills fail closed if the lease library is unavailable or the lease ref’s state doesn’t match the expected phase. The pre-cutover task-close-out’s “trust the caller” path is removed; there is no opt-out flag.

Approach

Add reacquire_lease(task_id, target_phase) to the lease library. Extends plugin/lib/lease/runtime.py. Logic: discover_lease() to get current state; build a new payload with phase=target_phase, owner=current_host_id(), rotated lease_token, fresh expires_at; cas_replace(authority, ref, current_sha, new_payload). If the current lease has expires_at <= now, log a STOLEN ref=<ref> from=<previous-owner> informational marker but proceed (per ADR steal-on-expired). On CAS-REPLACE failure (raced by another worker), raise LeaseConflict(ref, actual_owner). Library’s primitives write-through to the cache so the new state is observable to subsequent shell-outs.
Add release_to_archive(task_id) to the lease library. New helper (likely in runtime.py or a new archive.py). Logic: cas_create(authority, f"refs/sdlc/archive/tasks/{task_id}", final_lease_commit); on success, cas_delete(authority, f"refs/sdlc/tasks/{task_id}", current_sha). If cas_create fails because the archive ref already exists (idempotent re-run of close-out), proceed to the delete step (the archive was already written). If cas_delete fails (raced), raise LeaseConflict — operator inspects.
Add PR footer parsing. New helper in runtime.py exposing parse_lease_footer(pr_body: str) -> tuple[str, str] returning (task_id, lease_id). Regex-match against the exact format . Raises LeaseFooterMissing(pr_number=...) if not found or malformed.
Scaffold /sdlc:pr-respond. Create plugin/skills/pr-respond/SKILL.md following the same prose shape as task-work (numbered steps, structured stdout markers, single sub-agent dispatch for the actual feedback work). The skill is invoked with /sdlc:pr-respond <pr-number>.
Implement pr-respond Step 1: fetch PR body + parse footer. Shell out to gh pr view <pr-number> --json body -q .body, then call parse_lease_footer(). Missing/malformed → exit LEASE-FOOTER-MISSING pr=<n>.
Implement pr-respond Step 2: cd into worktree. Worktree must exist at .sdlc/worktrees/<task_id> (task-work created it at PR open and didn’t tear it down). If absent → exit WORKTREE-MISSING task=<task_id> and point at task-work as the entry. After cd, the skill is on task/<task_id> branch.
Implement pr-respond Step 3: fencing check + re-acquire. Call discover_lease(). Verify lease.lease_id == footer.lease_id — mismatch → exit LEASE-FENCING-MISMATCH expected=<footer_lease_id> actual=<discovered_lease_id> (the lease was rotated by another worker since this PR was opened; this PR no longer matches). On match, call reacquire_lease(task_id, "responding"). CAS-REPLACE failure → exit LEASE-CONFLICT ref=<ref> owner=<other-host-id>.
Implement pr-respond Steps 4-6: response sub-agent + heartbeat + push + transition back. Spawn the heartbeat thread (same shape as task-work — threading.Thread(daemon=True) at TTL/2 cadence). Dispatch an Agent sub-agent to apply review feedback. After the sub-agent completes, push commits, signal heartbeat shutdown, and call reacquire_lease(task_id, "awaiting-review"). Exit PR-RESPONSE-COMPLETE: pr=<n> task=<task_id>.
Wire lease re-acquisition into /sdlc:task-close-out. Add a new step at the top (after task resolution + worktree verification) calling reacquire_lease(task_id, "closing"). CAS-REPLACE failure → exit LEASE-CONFLICT. After existing close-out logic completes (worktree teardown, branch deletion, frontmatter closed/done, completion_note), call release_to_archive(task_id) as the final operation.
Update /sdlc:orchestrate to dispatch pr-respond. The orchestrate skill from PR #118 already runs /sdlc:pr-check per PR; this task adds the dispatch step for the NEEDS-RESPONSE verdict — Agent invocation of /sdlc:pr-respond <pr-number> with no lease env (lease is footer + discovery). MERGED verdict still dispatches /sdlc:task-close-out <slug>.
Tests. Library coverage: reacquire_lease happy path, expired-lease-steal path, cross-host-conflict path, missing-lease path; release_to_archive happy path, idempotent re-run path; parse_lease_footer happy / missing / malformed paths. Skill coverage: pr-respond fencing-mismatch and footer-missing paths; task-close-out reacquire-then-archive happy path and expired-lease steal path.
Manual smoke test. End-to-end: task-work opens a PR (footer planted) → operator posts a review comment → orchestrate’s pr-check returns NEEDS-RESPONSE → orchestrate dispatches pr-respond → pr-respond re-acquires + responds + transitions back → operator merges → orchestrate’s pr-check returns MERGED → orchestrate dispatches task-close-out → close-out re-acquires + tears down + archives. Capture the lease ref’s commit history at each step (should show 4-6 CAS-REPLACE entries plus the final archive ref).

Files to touch

Location	Kind	Change
`plugin/skills/pr-respond/SKILL.md`	new	New skill — parses PR footer, cd’s to worktree, fencing-checks via `discover_lease`, `reacquire_lease(_, "responding")`, dispatches response sub-agent, transitions back to `awaiting-review`
`plugin/skills/pr-respond/test_pr_respond.py`	new	Inline pytest — fencing-mismatch, footer-missing, worktree-missing, conflict paths
`plugin/skills/pr-respond/invariants.yaml`	new	Mirror the task-work pattern (lint-anchor cross-references)
`plugin/skills/task-close-out/SKILL.md`	modify	Add `reacquire_lease(_, "closing")` step at top of the lifecycle; add `release_to_archive` step at the end
`plugin/skills/task-close-out/test_close_out_lease.py`	new	Inline pytest — reacquire-then-archive happy path, expired-lease-steal path, missing-lease path
`plugin/skills/orchestrate/SKILL.md`	modify	Dispatch `/sdlc:pr-respond <pr-number>` on `pr-check` NEEDS-RESPONSE verdict (no lease env passed — pr-respond looks up the lease via footer + `discover_lease`)
`plugin/lib/lease/runtime.py`	modify	Add `reacquire_lease(task_id, target_phase)` (CAS-REPLACE-with-steal-on-expired), `release_to_archive(task_id)` (atomic archive+delete), `parse_lease_footer(pr_body)`
`plugin/lib/lease/tests/test_runtime.py`	modify	Add cases for `reacquire_lease` (happy / steal-on-expired / conflict / missing), `release_to_archive` (happy / idempotent-rerun), `parse_lease_footer` (happy / missing / malformed)
`plugin/lib/lease/__init__.py`	modify	Re-export `reacquire_lease`, `release_to_archive`, `parse_lease_footer`, `LeaseFooterMissing`
`plugin/conventions/lease-aware-skills.md`	modify	Document the re-acquire pattern, the fencing check (PR footer ↔ lease_id match), and the archive-ref convention (`refs/sdlc/archive/tasks/<id>`)

Acceptance criteria

Out of scope

Background heartbeat thread management. Both skills heartbeat during their active phases; a long-running daemon that heartbeats unattended leases is a separate concern (likely lives in slice 3’s reconcile or in a dedicated worker process).
Reconcile of leases left in responding or closing phases by crashed workers. Slice 3.
Auto-merging PRs. This task does not change merge gating; the human still merges, and task-close-out fires after the merge is observed.
Multi-PR-per-task workflows. The ADR assumes one PR per task lifecycle; supporting multiple PRs is a future revision and not in this task.
Operation-lease re-acquisition (for backlog-triage, reconcile, import-planning operation leases). Slice 4 — different lease shape, different ergonomics.

Dependencies

T-S0PK-add-lease-protocol-library-and-schemas — library + schemas
T-QC31-add-sdlc-lease-cli-commands — sdlc lease task transition is the re-acquire primitive
T-X24I-orchestrate-lease-aware-dispatch — convention doc + orchestrate dispatch path
T-TZSN-task-work-lease-integration-model-b — task-work plants the PR footer this task reads; task-work removes Steps 10–12 which this task replaces

Integration target: PRs from this task land on the lease-protocol-integration branch. Slice-2 cutover happens when the integration branch PRs to main with all three slice-2 tasks merged.

Discovery context

Realizes Rollout Plan item 7 of the ADR. The split is what makes lease ownership rotate cleanly across the PR lifecycle — pre-cutover, “the task is being worked” was a single state owned by one long-lived task-work sub-agent; post-cutover, it’s three states (working, awaiting-review, responding, closing) each owned by a distinct skill invocation that CAS-validates entry. The split also makes the protocol robust to operator restart: if pr-respond crashes mid-response, the next invocation re-acquires from responding cleanly (or steals if expired). The pre-cutover shape had no comparable recovery path — a crashed in-line response loop required manual operator intervention.

The two skills are bundled into one task (rather than four separate ones: extract-pr-respond, lease-wire-pr-respond, lease-wire-close-out, orchestrate-dispatch-respond) because they share the convention contract from T-X24I-orchestrate-lease-aware-dispatch and consume the same runtime.py helpers (discover_lease, acquire_lease from PR #124, and reacquire_lease + release_to_archive added by this task). Splitting further would multiply review surface without adding clarity. Per the user’s slice-2 directive, fewer-PRs-per-slice is the right trade for alpha-stage velocity.

← Back to Tasks