feat(spec-builder): add approvals and per-task runs

[kyleseaman]

Problem / Motivation

Spec Builder could only hand off an entire task list. It did not persist phase approvals, run one task at a time, or offer non-destructive lifecycle controls for labels, copies, and archived specs.

Why it matters

Users need review evidence tied to the exact document version they approved, plus enough execution control to run or resume an individual task without starting the whole autonomous build. They also need to correct labels and organize old specs without deleting their work or conversation.

What changed

• Records requirements and design approvals against the stored document hash and reports when an approved document has changed; starting the build remains the task-plan approval handoff.
• Adds per-task execution from tasks.md, with task-text hashes, fresh agent-state checks, and handoff/task arbitration immediately before dispatch.
• Adds editable display labels, archive/restore, and duplicate. Duplicate uses create-only writes, a reserved destination, atomic staged publication, rollback, and persisted startup recovery so races and process interruption cannot expose or strand a partial copy. It starts a fresh conversation and sends the copied context as an orientation turn.
• Keeps spec documents read-only in the dashboard because the agent and IDE remain uncontrolled writers, without exposing a dead document-write API.
• Reports duplicate capability explicitly and omits the action on Windows, where CPython cannot provide the descriptor-relative operations required for crash-safe publication.
• Keeps the actionable task checklist as the default while making the complete tasks.md prose and headings reachable from the same tab.
• Keeps mobile and narrow-pane headers bounded: lifecycle actions live in one overflow menu, while rename and duplicate forms use full-width stacked rows below the fixed header.
• Threads abort signals through reads, keeps writes non-cancellable once submitted, rejects empty task handoff, and teaches the workflow skill to read repository guidance before drafting.

Tests

• 828 passed across the complete Spec Builder backend route suite and repo-level route coverage.
• 73 passed across the focused Spec Builder frontend files.
• Passed Python formatting gate, isort, flake8, targeted mypy, TypeScript build, targeted ESLint, and the production frontend build.
• Added deterministic regressions for task/handoff arbitration, destination-setting races, create-only rollback, pre- and post-publication crash recovery, abandoned staging cleanup, read-only document metadata, and narrow-pane editor placement.

Manual verification

Ran the app through an isolated gateway with a fake ACP backend in a real browser. Verified desktop task progress and per-task controls, the 320px stacked duplicate flow, no blocking overlay, no page errors, and no horizontal overflow (width=320, scrollWidth=320).

Screenshots / video

Desktop task controls:

320px duplicate flow:

Related Issues

No linked issue: this closes gaps found in the Spec Builder surface audit rather than a filed issue.

Checklist

• Single commit with a Conventional Commits title
• Existing affected tests pass and regressions cover new behavior
• Self-review and independent local review completed
• Workflow documentation updated
• No secrets, credentials, or internal references in the diff

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of 924998cb73f45b28c10a9dea23057be1a9087731 and found no blocking issues.

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 924998c

False positive or not applicable? A repository writer can comment:
/ai-review override gpt 924998cb73f45b28c10a9dea23057be1a9087731: <one-sentence reason>

[github-actions]

Design Review (Fable 5) — 🟡 CONCERNS

Design-level review of 924998cb73f45b28c10a9dea23057be1a9087731 — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Design-Verdict: CONCERNS

Duplicate's crash-safe machinery is disproportionate to copying three markdown files, and its rename-based design is what costs Windows the feature entirely.

Watch

• Duplicate transaction proportionality. The staged directory + provenance marker + ctypes renameat2/renameatx_np seam + inode-pinned rollback + startup recovery (~1,000 lines) buys all-or-nothing external visibility for a copy of at most three small files. The _DUPLICATING index reservation already hides the partial copy from the app, and the reservation already carries documents hashes — per-file O_EXCL create-only writes into the reserved destination (the pattern _create_spec_doc already implements) plus hash-scoped recovery deletion would give equivalent no-strand/no-overwrite guarantees, drop the new platform_compat.rename_noreplace ctypes surface, and — per "omits the action on Windows, where CPython cannot provide the descriptor-relative operations" — would not exclude Windows at all. The shipped design is correct and honestly reported, but it trades a permanent per-platform capability gap for atomicity against a harm (transient partial visibility to the IDE) no other file copy on the machine defends against.
• Ownership of the transaction engine. routes.py is now 5,642 lines and hosts a generic crash-safe directory-publication engine inside one app's route module; atomic_write.py exists precisely so atomic-write logic isn't re-invented per call site. If this machinery stays, it belongs beside it, not in an app.
• Bundle revert granularity. Approvals, per-task runs, labels, archive, and the duplicate transaction land as one 6,700-line commit; a defect in the duplicate engine can only be reverted by taking four unrelated features with it. The messaging.md set_project hunk is entirely outside this PR's stated purpose and should move out.

Suggestions

• If the atomic-publication design stays, extract the stage/publish/recover primitives into a shared module next to atomic_write.py so the next app-level copy operation doesn't fork them.

[DESIGN-REVIEWED] 924998c

[github-actions]

First Principles Review (Fable 5) — 🟡 CONCERNS

Premise-level review of 924998cb73f45b28c10a9dea23057be1a9087731 — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

I have everything I need. All counts below were run against the checkout and the patch.

First-Principles-Verdict: CONCERNS

Every headline item earns its place, but this feature PR carries five undeclared riders and a duplicate path engineered far past the harm it removes.

What this change ships

Intent: let a user approve spec phases with evidence, run one task at a time, and relabel/archive/copy specs without destroying work — an ADDITION.

1. Approve records the phase against the document hash; a rewritten doc shows "changed since approval" — justified
2. Tasks tab becomes an addressable checklist with progress bar and per-task Run — justified
3. Start-build now refuses a tasks.md with no unchecked task — justified
4. Editable display label; the name stays the identity — justified
5. Archive/restore group in the rail — justified
6. Duplicate via staged dir, provenance marker, fsync choreography, startup recovery; hidden on Windows — oversized
7. Reads take AbortSignals, writes stay non-cancellable — justified
8. Creating a spec now auto-selects it (previously a known dropped-selection bug) — rides along, undeclared
9. messaging.md gains a channel set_project semantics paragraph; no messaging code changes here — undeclared
10. CliPanel test-reset export, App.test onboarding mock, eslint-disable removals, un-baselining session.py/test_session.py — rides along, undeclared

Capped at 10; SpecApiError (see Subtractions) is an eleventh.

Watch

• The description says documents stay read-only, yet the code keeps editor-shaped surface: _create_spec_doc's not_editable branch is unreachable (its only caller iterates the same _PHASE_FILES set), and the _sha256_text / _agent_is_writing docstrings justify themselves by "an editor sends back the hash it loaded" — a save API this PR deliberately does not ship.
• The messaging.md paragraph (item 9) documents behavior no code in this diff creates or changes; if it corrects doc drift it belongs with the change that caused it.

Subtractions

• Drop SpecApiError.code (website/src/apps/spec-builder/api.ts:166) — grep \.code across website/src/apps/spec-builder: 0 non-test consumers; only SpecBuilderWriteApi.test.tsx reads it. Throw the plain Error until a caller branches on a code.
• Shrink duplicate crash recovery: the adopt/discard/retry engine (_recover_abandoned_copy, marker/manifest helpers, ~500 lines) exists so a crashed copy of three markdown files strands nothing. The sibling mechanism already in this file — _load_index dropping a foreign _DELETING reservation (routes.py:525) — plus existing _discover_folder_specs re-finding a published copy covers the same harm; the residue is one hidden staging dir of user-visible markdown. Accepted-and-deferred if the adoption window is judged worth it, but the smaller shape was never weighed in the description.
• Revert the .github/black-baseline.txt removals of src/kiro_crew/session.py and test/test_session.py — neither file appears in this diff, and AGENTS.md routes baseline pruning through its own commit.

[FIRST-PRINCIPLES-REVIEWED] 924998c

[github-actions]

UX Review (Fable 5) — 🟡 CONCERNS

UX-level review of 924998cb73f45b28c10a9dea23057be1a9087731 — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

UX-Verdict: CONCERNS

The primary Approve button silently disables whenever the open tab isn't the current phase — after any reload it looks broken with no explanation.

Watch

• tab initializes to 'requirements' (SpecDetail.tsx:85) while the new || !reviewingPhase disable (SpecDetail.tsx) greys "Approve → Tasks" on every revisit of a design-phase spec; the tooltip still reads "Records your approval of this version…", describing the action it refuses. Every-visit frequency × primary-CTA confusion. Fix: add a tooltip branch "Open the Design tab to review it first" or initialize tab to detail.phase.
• commitDuplicate closes the form immediately (setDupDraft(null) right after mutate), so a multi-second staged copy runs with zero visible pending state before the jump to the new spec. Rare action, major operation, silent gap. Fix: keep the form open with the button reading "Duplicating…" until settled.
• "Changed since approval" warns but offers no adjacent action or way to clear it; users can't tell what remedies it. Add a hint (re-review path) or accept as known.

Suggestions

• Menu item rename_this_spec_label_the_folder_name_stays is two sentences inside a dropdown; use "Rename label" as the item and keep the folder-name caveat as the tooltip.
• "Pause the build before running a single task" also shows when a plain chat turn is in flight (busy = running || executing) — when no build exists and no Pause button renders; branch the copy on executing.

[UX-REVIEWED] 924998c

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

Reviewed 924998cb73f45b28c10a9dea23057be1a9087731 — this comment is updated in place on each push.

Review details

The candidate concerns whether an archived spec can still be executed. The backend handlers _handle_handoff and _handle_run_task indeed lack an archived check, and the frontend leaves Execute/Run controls active on a selected archived spec. But: archive is explicitly the non-destructive, recoverable counterpart to delete; the spec stays fully selectable with a visible Pause surface; and execution is user-initiated authorization of the user's own agent editing the user's own files. No security boundary is crossed, no crash, no data loss (the archive guard's own stated harm — "no surface left to stop it" — does not materialize because Pause remains available). This is a behavioral inconsistency, not a reachable concrete defect of a blocking class. The candidate's own confidence is "low," and it fails the (c) observable-wrong-outcome bar. Dropped.

No grounded new defect surfaced while falsifying.

No findings.

[OPUS-REVIEWED] 924998c

Verdict parsed from the review's SHA-scoped output markers for commit 924998cb73f45b28c10a9dea23057be1a9087731.

False positive or not applicable? A repository writer can comment:
/ai-review override fable 924998cb73f45b28c10a9dea23057be1a9087731: <one-sentence reason>

[kyleseaman]

/ai-review override gpt b95483c: Tasks approval is the Start building handoff by design; the PR description now accurately limits recorded phase approvals to Requirements and Design.

[github-actions]

Human judgment recorded

@kyleseaman marked the gpt AI finding as false positive, not applicable, or explicitly accepted for b95483c5d0cbbaa4bda71201f48cde5d749d0b26.

Tasks approval is the Start building handoff by design; the PR description now accurately limits recorded phase approvals to Requirements and Design.

This decision applies only to this commit. A new push requires a new judgment.

[bolichen97]

Open PR relationship audit

This is a consolidated, point-in-time code-level audit note. It compares complete merge-base diffs and current/merged code; it does not treat a shared topic as duplication or partial coverage as completion.

Relationship findings

• PR #5111 is PARTIALLY_COVERED relative to this PR. Coverage is explicitly incomplete; this finding is not a completion or closure claim. Recommended action for PR #5111: KEEP. Only the API client and backend halves of spec deletion are on main; the UI half exists nowhere else and is what this PR adds. Files: website/src/apps/spec-builder/api.ts, src/kiro_crew/apps/builtins/spec_builder/backend/routes.py.

No PR, Issue, label, branch, or review state was changed by the relationship-note portion of this audit.