From e60021e6f0f217e6dcd83cdbfb5bba3dd05bc2d7 Mon Sep 17 00:00:00 2001 From: Daniel Hanold Date: Fri, 25 Sep 2026 07:14:39 -0400 Subject: [PATCH 1/8] docs(plan): implementation plan for change 0450 Docket-Plan-Path: docs/superpowers/plans/2026-09-25-typed-change-unblock-operation-to-reverse-change-block.md --- ...block-operation-to-reverse-change-block.md | 453 ++++++++++++++++++ 1 file changed, 453 insertions(+) create mode 100644 docs/superpowers/plans/2026-09-25-typed-change-unblock-operation-to-reverse-change-block.md diff --git a/docs/superpowers/plans/2026-09-25-typed-change-unblock-operation-to-reverse-change-block.md b/docs/superpowers/plans/2026-09-25-typed-change-unblock-operation-to-reverse-change-block.md new file mode 100644 index 000000000..eb834428d --- /dev/null +++ b/docs/superpowers/plans/2026-09-25-typed-change-unblock-operation-to-reverse-change-block.md @@ -0,0 +1,453 @@ + +> ↩ **[Change 0450 — Typed change.unblock operation to reverse change.block](https://github.com/danielhanold/docket/blob/docket/docs/changes/active/0450-typed-change-unblock-operation-to-reverse-change-block.md)** + +# Typed `change.unblock` and `change.revive` Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Expose the existing `domain.Unblock` (`blocked` → `in-progress`, clearing `blocked_by`) and `domain.Revive` (`deferred` → `proposed`) transitions as typed operations `change.unblock` / `change.revive` through the existing `executeChangeLifecycle` driver, replacing the hand-edit workflow. + +**Architecture:** No new mechanism. Two new request types and entry points in `internal/app/change_lifecycle.go` compose the same shared driver `change.block` / `change.defer` use (exact-blob version pin, domain legality gate, `updated:` refresh, artifact-block and inline-board re-render, one metadata commit). Wiring mirrors `change.defer` at every enumeration site: CLI subcommands, schema registry, shadow/schema-tag/CLI/asset-independence tests, and the docket-convention lifecycle prose. + +**Tech Stack:** Go (stdlib + repo-internal packages only), Cobra CLI, the repo's transaction engine, `go generate ./internal/assets/` for the embedded skill bundle. + +**Spec:** `docs/superpowers/specs/2026-09-24-typed-change-unblock-operation-to-reverse-change-block-design.md` (on the `docket` metadata branch; synchronized copy at `.docket/docs/superpowers/specs/…` in the primary checkout). + +## Global Constraints + +- Change id: 0450. Feature branch: `chore/typed-change-unblock-operation-to-reverse-change-block`. Work only in this feature worktree; never write docket metadata. +- Operation ids are exactly `change.unblock` and `change.revive`; CLI subcommands are exactly `docket change unblock` / `docket change revive`. +- No `reason` field on either request (spec: YAGNI). Requests carry only `change_id`, `path`, `version`. +- Result type is the existing `ChangeLifecycleResult`, unchanged. +- Wrong source status is refused via the domain's `requireStatus` failure mapped to `invalid-state`; nothing is written. +- Revive leaves the body alone: `## Why deferred` kept, `branch:` and `claimed_at:` byte-intact. Do not change `change.block` or `domain.Revive` semantics. +- Out of scope: automatic unblocking, `domain.KillStackParent` recovery, `finalize.block`/`finalize.clear-block`. +- The final build gate is the whole suite via `go run ./cmd/docket development test` (run once by the build skill's gate, not per task). Per-task verification uses focused `go test` commands as written in each task; note `internal/app/change_integration_test.go` is behind `//go:build integration`, so those runs need `-tags integration`. +- `git add` only the exact files each task names — never `git add -A` (shared-loop discipline). + +## Review Focus + +Spec-implied conditions no single task's happy path covers; each has its pinning test added to the owning task: + +1. **Unblock/revive on a github board surface** must refuse before any engine call (users on the github surface would otherwise half-write) — Task 1 fence tests. +2. **Version drift between read and submit** must map to `contended`, not a write over a moved record — Task 2 recording-engine drift assertions. +3. **A halted-then-blocked change (the 0444 path)** must come all the way back: halt → block → unblock → resume-halted removes `## Run halted` — Task 2 resume regression. +4. **A Bash-era record lacking `updated:`** must not internal-error on unblock (the driver upserts) — Task 1 missing-updated test for unblock. +5. **Revive of a record whose `## Why deferred` is absent** (deferred by an old tool or hand edit) must still apply — revive passes no section edits, so absence is legal — Task 1 plan test uses a record without the section. + +--- + +### Task 1: `change.unblock` / `change.revive` app operations + +**Files:** +- Modify: `internal/app/change_lifecycle.go` +- Test: `internal/app/change_lifecycle_test.go` + +**Interfaces:** +- Consumes: `executeChangeLifecycle`, `validateLifecycleShape`, `newChangeLifecycleResult`, `domain.Unblock`, `domain.Revive` — all already present. +- Produces: `OperationChangeUnblock = "change.unblock"`, `OperationChangeRevive = "change.revive"`, `type ChangeUnblockRequest struct { ChangeID int; Path string; Version string }` (json/docket tags as below), `type ChangeReviveRequest` (identical fields), `func ChangeUnblock(ctx, deps PlanningDeps, repoDir string, req ChangeUnblockRequest) ChangeLifecycleResult`, `func ChangeRevive(…, req ChangeReviveRequest) ChangeLifecycleResult`. Tasks 2–3 use these names exactly. + +- [ ] **Step 1: Write the failing tests** + +In `internal/app/change_lifecycle_test.go`, mirror the existing block/defer tests (same file, same helpers). Add: + +```go +func TestChangeUnblockRejectsBadShapeWithoutEngineCall(t *testing.T) { + // Clone TestChangeBlockRejectsBadShapeWithoutEngineCall's cases minus the + // empty-reason case: non-positive id, empty path, empty version each yield + // ResultInvalidInput with the same finding codes (invalidIDCode("change_id"), + // FCEmptyPath, FCEmptyVersion) and zero engine calls. +} + +func TestChangeReviveRejectsBadShapeWithoutEngineCall(t *testing.T) { /* same */ } + +func TestChangeUnblockFencesGithubBoardSurface(t *testing.T) { + // Clone TestChangeBlockFencesGithubBoardSurface for ChangeUnblock. +} + +func TestChangeReviveFencesGithubBoardSurface(t *testing.T) { /* same */ } +``` + +Add op helpers beside `blockOp`/`deferOp` (follow their exact shape at the `baseLifecycleOp` call sites): + +```go +func unblockOp(surfaces []string, id int, recPath string) changeLifecycleOp { + return baseLifecycleOp(OperationChangeUnblock, surfaces, id, recPath, + func(c domain.Change) (domain.ActionResult, *domain.PolicyFailure) { return domain.Unblock(c) }, nil) +} + +func reviveOp(surfaces []string, id int, recPath string) changeLifecycleOp { + return baseLifecycleOp(OperationChangeRevive, surfaces, id, recPath, + func(c domain.Change) (domain.ActionResult, *domain.PolicyFailure) { return domain.Revive(c) }, nil) +} +``` + +Plan-behaviour tests, following `TestChangeBlockPlanFileSet` / `TestChangeBlockPlanSourceStatusMatrix` byte-for-byte in structure: + +```go +func TestChangeUnblockPlanFileSet(t *testing.T) { + // Fixture: a record with status: 'blocked' and blocked_by: 'waiting on 0446'. + // Assert the mutated record has status: 'in-progress', blocked_by: (bare + // null form — lifecycleFieldValue("") renders document.Null()), updated: + // refreshed to the test clock date, the docket:artifacts block re-rendered, + // and — inline surface — BOARD.md in the file set. Commit subject is + // "change 0003 → in-progress" (fmt "change %04d → %s"); receipt decodes to + // changeLifecycleReceipt{ID: 3, Op: "change.unblock", Status: "in-progress"}. +} + +func TestChangeRevivePlanFileSet(t *testing.T) { + // Fixture: status: 'deferred' WITHOUT a ## Why deferred section (Review + // Focus 5), carrying branch: and claimed_at:. Assert status: 'proposed', + // updated: refreshed, branch:/claimed_at: byte-intact, no section added. +} + +func TestChangeRevivePlanPreservesWhyDeferredAndClaim(t *testing.T) { + // Fixture: status: 'deferred' WITH "## Why deferred\n\nParked for X." plus + // branch: 'feat/widget' and claimed_at:. Assert the section body, branch, + // and claimed_at survive byte-identical in the mutated record. +} + +func TestChangeUnblockPlanSourceStatusMatrix(t *testing.T) { + // Clone TestChangeBlockPlanSourceStatusMatrix: every non-'blocked' status + // refuses with the domain's requireStatus reason token as the finding code, + // Refused: true, no files planned. +} + +func TestChangeRevivePlanSourceStatusMatrix(t *testing.T) { + // Same for every non-'deferred' status. +} + +func TestChangeUnblockPlanToleratesMissingUpdatedField(t *testing.T) { + // Clone TestChangeBlockPlanToleratesMissingUpdatedField for unblock + // (Review Focus 4): a record without updated: gains the field, no error. +} +``` + +- [ ] **Step 2: Run tests to verify they fail** + +Run: `go test ./internal/app/ -run 'TestChangeUnblock|TestChangeRevive' -count=1` +Expected: FAIL to compile — `OperationChangeUnblock`, `ChangeUnblockRequest`, `ChangeUnblock`, etc. undefined. + +- [ ] **Step 3: Implement the operations** + +In `internal/app/change_lifecycle.go`: + +```go +const ( + OperationChangeBlock = "change.block" + OperationChangeDefer = "change.defer" + OperationChangeUnblock = "change.unblock" + OperationChangeRevive = "change.revive" +) + +// ChangeUnblockRequest is the closed, caller-supplied request for one unblock. +// Path and Version pin the exact submitted record. No reason field: the commit +// subject records the transition and the cleared blocked_by text stays in git +// history. +type ChangeUnblockRequest struct { + ChangeID int `json:"change_id" docket:"required"` + Path string `json:"path" docket:"required"` + Version string `json:"version" docket:"required"` +} + +// ChangeReviveRequest is the closed, caller-supplied request for one revive. +type ChangeReviveRequest struct { + ChangeID int `json:"change_id" docket:"required"` + Path string `json:"path" docket:"required"` + Version string `json:"version" docket:"required"` +} + +// ChangeUnblock validates the request, pins authoritative context, and drives +// one atomic transaction that unblocks the change (blocked → in-progress, +// clearing blocked_by) and — when inline is enabled — re-renders the board. +func ChangeUnblock(ctx context.Context, deps PlanningDeps, repoDir string, req ChangeUnblockRequest) ChangeLifecycleResult { + findings := validateLifecycleShape("change_id", req.ChangeID, req.Path, req.Version) + if len(findings) > 0 { + return newChangeLifecycleResult(OperationChangeUnblock, ResultInvalidInput, ChangeLifecycleResult{Findings: findings}) + } + action := func(c domain.Change) (domain.ActionResult, *domain.PolicyFailure) { + return domain.Unblock(c) + } + return executeChangeLifecycle(ctx, deps, repoDir, OperationChangeUnblock, req.ChangeID, req.Path, req.Version, action, nil) +} + +// ChangeRevive validates the request, pins authoritative context, and drives +// one atomic transaction that revives the change (deferred → proposed). The +// ## Why deferred section, branch, and claim stamp are left untouched, per +// domain.Revive's contract. +func ChangeRevive(ctx context.Context, deps PlanningDeps, repoDir string, req ChangeReviveRequest) ChangeLifecycleResult { + findings := validateLifecycleShape("change_id", req.ChangeID, req.Path, req.Version) + if len(findings) > 0 { + return newChangeLifecycleResult(OperationChangeRevive, ResultInvalidInput, ChangeLifecycleResult{Findings: findings}) + } + action := func(c domain.Change) (domain.ActionResult, *domain.PolicyFailure) { + return domain.Revive(c) + } + return executeChangeLifecycle(ctx, deps, repoDir, OperationChangeRevive, req.ChangeID, req.Path, req.Version, action, nil) +} +``` + +Update the two comments the spec names so they describe all four transitions: + +- The file-header comment (`// This file is the `change block` and `change defer` planning operations…`) → "the `change block`, `change defer`, `change unblock`, and `change revive` planning operations", keeping the rest of its claims accurate (defer is still the only one with an authored section; unblock/revive edit no sections). +- The `lifecycleFieldValue` trailing sentence ("Block and defer only ever set string-valued owned fields (status, blocked_by).") → "The lifecycle transitions only ever set string-valued owned fields (status, blocked_by); unblock clears blocked_by via the empty target." + +- [ ] **Step 4: Run tests to verify they pass** + +Run: `go test ./internal/app/ -run 'TestChange(Block|Defer|Unblock|Revive)|TestLifecycle' -count=1` +Expected: PASS (new and pre-existing lifecycle tests). + +- [ ] **Step 5: Commit** + +```bash +git add internal/app/change_lifecycle.go internal/app/change_lifecycle_test.go +git commit -m "feat(app): typed change.unblock and change.revive operations (change 0450)" +``` + +--- + +### Task 2: Integration coverage — applied results, drift, and the 0444 resume regression + +**Files:** +- Test: `internal/app/change_integration_test.go` + +**Interfaces:** +- Consumes: `ChangeUnblock`, `ChangeRevive`, `ChangeUnblockRequest`, `ChangeReviveRequest` (Task 1); existing helpers `recordingEngine`, `fakeChangeReader`, `mainModePin`, `mustMarshal`, `changeLifecycleReceipt`, `newGitClient`, `testClock`, `setupHaltedFixture`, `planRepoModes`, `originFile`, `groomPath`, and the existing `validBlockRequest()` pattern. +- Produces: nothing consumed later; pure test coverage. + +- [ ] **Step 1: Write the failing tests** + +All in `internal/app/change_integration_test.go` (note `//go:build integration`). Mirror `TestIntegrationChangeAuthoringBlockAppliedResult` / `…DeferAppliedResultCarriesDeferStatus`: + +```go +func validUnblockRequest() ChangeUnblockRequest { + // Same id/path/version literals validBlockRequest uses, minus Reason. +} +func validReviveRequest() ChangeReviveRequest { /* same */ } + +func TestIntegrationChangeAuthoringUnblockAppliedResult(t *testing.T) { + // recordingEngine returns DispositionApplied with receipt + // changeLifecycleReceipt{ID: 3, Op: OperationChangeUnblock, Status: "in-progress"}. + // Assert Result applied; ID 3; Status "in-progress"; Revision equals the + // engine's AppliedCommit; Operation == OperationChangeUnblock; exactly one + // engine call whose single entity expectation pins the request's path at + // the exact blob version (clone the Expected assertions from the block test). +} + +func TestIntegrationChangeAuthoringReviveAppliedResultCarriesProposedStatus(t *testing.T) { + // Same shape; receipt Status "proposed"; Operation == OperationChangeRevive. +} + +func TestIntegrationChangeAuthoringUnblockContendedOnVersionDrift(t *testing.T) { + // recordingEngine returns transaction.Result{Disposition: transaction.DispositionContended}; + // assert res.Result == ResultContended for ChangeUnblock (Review Focus 2). +} + +func TestIntegrationChangeAuthoringReviveContendedOnVersionDrift(t *testing.T) { /* same */ } +``` + +The 0444 regression (Review Focus 3), beside `TestIntegrationChangeRuntimeResumeHalted` and reusing its fixture: + +```go +// TestIntegrationChangeRuntimeUnblockThenResumeHalted proves the 0444 path as +// typed operations end to end: a halted in-progress change is blocked, then +// unblocked (status back to in-progress, blocked_by cleared, ## Run halted +// preserved), then resume-halted succeeds and removes exactly the marker. +func TestIntegrationChangeRuntimeUnblockThenResumeHalted(t *testing.T) { + for _, m := range planRepoModes() { + t.Run(m.name, func(t *testing.T) { + f := setupHaltedFixture(t, m) + // 1. Block through the real engine. Re-read the record's current blob + // version from origin before each step (originFile gives the bytes; + // hash or re-list for the blob id the same way the fixture derives + // f.version — follow whatever helper setupHaltedFixture uses). + // 2. Assert origin record: status: 'blocked', blocked_by recorded, + // "## Run halted" still present. + // 3. Unblock through the real engine with the post-block version. + // Assert applied; origin record: status: 'in-progress', + // blocked_by: cleared (bare null form), "## Run halted" preserved. + // 4. ChangeResumeHalted with a quiescent fake workspace (clone the + // "quiescent-resumes" subtest's call) and the post-unblock version. + // Assert applied and "## Run halted" gone from the origin record. + }) + } +} +``` + +- [ ] **Step 2: Run tests to verify they fail** + +Run: `go test -tags integration ./internal/app/ -run 'TestIntegrationChangeAuthoringUnblock|TestIntegrationChangeAuthoringRevive|TestIntegrationChangeRuntimeUnblockThenResumeHalted' -count=1` +Expected: FAIL only if Task 1 is absent (compile) — with Task 1 in place these should PASS if the driver truly needs no new code. A failure here is a real finding about the driver, not a test to weaken; debug it (superpowers:systematic-debugging) before touching non-test code. + +- [ ] **Step 3: Make them pass** + +Expected implementation delta: none (the operations exist from Task 1). Fix only genuine defects the tests expose. + +- [ ] **Step 4: Run the package's integration tests** + +Run: `go test -tags integration ./internal/app/ -run 'TestIntegrationChange' -count=1` +Expected: PASS. + +- [ ] **Step 5: Commit** + +```bash +git add internal/app/change_integration_test.go +git commit -m "test(app): unblock/revive integration and 0444 resume regression (change 0450)" +``` + +--- + +### Task 3: Wiring — CLI subcommands, schema registry, shadow/schema-tag/asset enumerations + +**Files:** +- Modify: `internal/cli/change.go` +- Modify: `internal/cli/install.go` +- Modify: `internal/app/schema_registry.go` +- Test: `internal/cli/change_test.go`, `internal/app/schema_tags_test.go`, `internal/app/shadow_test.go` + +**Interfaces:** +- Consumes: `app.ChangeUnblock` / `app.ChangeRevive` and their request types (Task 1); `changeSubcommand`, `decodeRequestFlag`, `setResult`, `EffectMetadataWrite` (existing CLI plumbing). +- Produces: catalog-visible operations `change unblock` / `change revive`. `TestAssetIndependentSetExact` (`internal/cli/root_test.go`) enforces the `assetIndependent` ↔ Cobra-tree correspondence both ways — it needs no edit, only the map entries. + +- [ ] **Step 1: Extend the enumerating tests first** + +`internal/cli/change_test.go`: +- Subcommand list (`for _, sub := range []string{"create", "groom", "block", "defer", "kill"}`): add `"unblock", "revive"`. +- Operation-id pairs table (`{"block", "change.block"}, {"defer", "change.defer"}`): add `{"unblock", "change.unblock"}, {"revive", "change.revive"}`. +- Catalog-key list (`"change block", "change defer", …`): add `"change unblock", "change revive"`. + +`internal/app/schema_tags_test.go` — two new rows following the block row exactly: + +```go +{"change.unblock", ChangeUnblockRequest{}, func() []StatusFinding { + return ChangeUnblock(context.Background(), PlanningDeps{}, "", ChangeUnblockRequest{}).Findings +}}, +{"change.revive", ChangeReviveRequest{}, func() []StatusFinding { + return ChangeRevive(context.Background(), PlanningDeps{}, "", ChangeReviveRequest{}).Findings +}}, +``` + +`internal/app/shadow_test.go` — two rows beside the block/defer ones: + +```go +{"change.unblock", newChangeLifecycleResult(OperationChangeUnblock, ResultApplied, ChangeLifecycleResult{})}, +{"change.revive", newChangeLifecycleResult(OperationChangeRevive, ResultApplied, ChangeLifecycleResult{})}, +``` + +- [ ] **Step 2: Run tests to verify they fail** + +Run: `go test ./internal/cli/ ./internal/app/ -run 'TestChangeSubcommands|TestChange.*Operation|TestAssetIndependentSetExact|TestSchemaTags|TestShadow' -count=1` +(Adjust `-run` to the actual enclosing test names at the edited tables; run the two packages without `-run` if in doubt — they are fast.) +Expected: FAIL — unknown subcommands, unregistered schema ids, missing asset-independent entries. + +- [ ] **Step 3: Wire the operations** + +`internal/cli/change.go` — two subcommands cloned from `block`, registered in the `changeCmd.AddCommand(…)` list after `deferCmd`: + +```go +unblock := changeSubcommand("change", "unblock", + "Unblock a blocked change back to in-progress, clearing blocked_by, from a JSON request", + func(c *cobra.Command, deps app.PlanningDeps, repoDir string) error { + var req app.ChangeUnblockRequest + if err := decodeRequestFlag(c, &req); err != nil { + return err + } + setResult(app.ChangeUnblock(c.Context(), deps, repoDir, req)) + return nil + }, EffectMetadataWrite) + +revive := changeSubcommand("change", "revive", + "Revive a deferred change back to proposed, from a JSON request", + func(c *cobra.Command, deps app.PlanningDeps, repoDir string) error { + var req app.ChangeReviveRequest + if err := decodeRequestFlag(c, &req); err != nil { + return err + } + setResult(app.ChangeRevive(c.Context(), deps, repoDir, req)) + return nil + }, EffectMetadataWrite) +``` + +`internal/app/schema_registry.go` — two rows beside the block/defer rows, same comment style: + +```go +{ID: "change.unblock", Request: ChangeUnblockRequest{}, Result: ChangeLifecycleResult{}}, // ChangeUnblock +{ID: "change.revive", Request: ChangeReviveRequest{}, Result: ChangeLifecycleResult{}}, // ChangeRevive +``` + +`internal/cli/install.go` — in `assetIndependent`, beside `"change block"` / `"change defer"`: + +```go +"change unblock": true, +"change revive": true, +``` + +(match the file's existing alignment). + +- [ ] **Step 4: Run both packages' unit tests** + +Run: `go test ./internal/cli/ ./internal/app/ -count=1` +Expected: PASS, including `TestAssetIndependentSetExact` (both directions of the correspondence) and the schema round-trip tests. + +- [ ] **Step 5: Commit** + +```bash +git add internal/cli/change.go internal/cli/install.go internal/cli/change_test.go internal/app/schema_registry.go internal/app/schema_tags_test.go internal/app/shadow_test.go +git commit -m "feat(cli): wire change unblock and change revive into the catalog (change 0450)" +``` + +--- + +### Task 4: Convention prose and embedded-asset regeneration + +**Files:** +- Modify: `skills/docket-convention/SKILL.md` +- Regenerate: `internal/assets/embedded/` (via `go generate ./internal/assets/` — commit whatever it rewrites, including the manifest) + +**Interfaces:** +- Consumes: nothing from earlier tasks (prose only). +- Produces: the shipped convention text agents follow; `TestEmbeddedMatchesAuthored` (`internal/assets/embedded_test.go`) reds on any authored-vs-embedded drift. + +- [ ] **Step 1: Sweep for every hand-edit instruction** + +Per the spec, grep the shipped skill/agent text and README before editing (never hand-list sites): + +Run: `PAT='one-line frontmatter edit'; OUT=$(grep -rn -F -e "$PAT" skills/ agents/ cursor-rules/ README.md docs/ internal/assets/embedded/ 2>/dev/null); printf '%s\n' "$OUT"` + +Expected hits: `skills/docket-convention/SKILL.md` (the Rules sentence), its mirror under `internal/assets/embedded/tree/…` (regenerated, never hand-edited), and point-in-time records under `docs/superpowers/` (specs/plans — historical, leave untouched). Also probe for other phrasings of the same instruction, e.g. `grep -rniE -e 'unblock|reviv' skills/ agents/ cursor-rules/ README.md | grep -viE 'change (unblock|revive)|docket-'` and read the survivors. Only maintained instructional text gets edited; anything unexpected gets fixed the same way as the Rules sentence. + +- [ ] **Step 2: Edit the Rules sentence** + +In `skills/docket-convention/SKILL.md`, in the `**Rules.**` paragraph (currently containing "…and revived to `proposed`; clearing a blocker or reviving is a one-line frontmatter edit, no move."), replace that clause so it names the operations: + +``` +`deferred` may be entered from `proposed` or `in-progress` (add `## Why deferred`) and revived to `proposed`; clear a blocker with `change.unblock` (`blocked` → `in-progress`, clearing `blocked_by`) and revive with `change.revive` (`deferred` → `proposed`) — typed operations, no file move. +``` + +Leave the lifecycle diagram's edges untouched (they already show `blocked ──clears──▶ in-progress` and revive → `proposed`). + +- [ ] **Step 3: Regenerate the embedded bundle** + +Run: `go generate ./internal/assets/` +Then: `git status --porcelain` — expect the embedded mirror of `SKILL.md` and the asset manifest changed; nothing else. + +- [ ] **Step 4: Run the guards** + +Run: `go test ./internal/assets/ -count=1` +Expected: PASS (`TestEmbeddedMatchesAuthored` proves authored and embedded agree). Also run `go test ./internal/repoguard/ -count=1` (prose/anchor guards over maintained text). + +- [ ] **Step 5: Commit** + +```bash +git add skills/docket-convention/SKILL.md internal/assets/embedded/ +git commit -m "docs(convention): clearing a blocker / reviving are typed operations (change 0450)" +``` + +--- + +## Self-review notes + +- Spec coverage: operations (Task 1), request shape + refusals + preservation (Task 1), applied/contended integration + receipt/commit-subject + 0444 resume regression (Task 2), all five wiring sites and enumeration tests (Task 3), documentation sweep + convention edit (Task 4). Board-row assertion rides the inline file-set tests in Task 1 (`BOARD.md` in the planned file set), matching how the existing block/defer tests pin it. +- The spec's "one applied unblock and one applied revive through the real engine": the existing `TestIntegrationChangeAuthoring*Applied*` siblings use the recording engine for receipt/result assertions, while the real-engine end-to-end coverage (commit subject on origin, record bytes, marker survival) lands in `TestIntegrationChangeRuntimeUnblockThenResumeHalted`, which drives block → unblock → resume through `setupHaltedFixture`'s real repositories. Together they cover the spec's intent in this suite's own idiom. +- Types: `ChangeUnblockRequest`/`ChangeReviveRequest` fields and tags are identical across Tasks 1–3. +- No placeholders: every step names its exact file, code, command, and expected outcome. From 7fbb3d7ac06733cbac795d962552d5d2e9a62d1a Mon Sep 17 00:00:00 2001 From: Daniel Hanold Date: Fri, 25 Sep 2026 07:17:54 -0400 Subject: [PATCH 2/8] feat(app): typed change.unblock and change.revive operations (change 0450) --- internal/app/change_lifecycle.go | 106 ++++++--- internal/app/change_lifecycle_test.go | 306 ++++++++++++++++++++++++++ 2 files changed, 387 insertions(+), 25 deletions(-) diff --git a/internal/app/change_lifecycle.go b/internal/app/change_lifecycle.go index 76bc65df3..acec2c281 100644 --- a/internal/app/change_lifecycle.go +++ b/internal/app/change_lifecycle.go @@ -16,24 +16,28 @@ import ( "github.com/danielhanold/docket/internal/repository/transaction" ) -// This file is the `change block` and `change defer` planning operations: two -// non-allocating lifecycle transitions that land one change's owned frontmatter -// changes and every affected v1-owned derived view (the change record's typed -// lifecycle fields, its refreshed updated date, its re-rendered artifact block, -// and — for defer — its ## Why deferred authored section; plus the inline board) -// as one validated atomic transaction. The domain owns legality: domain.Block -// and domain.Defer decide whether the current status may take the transition and -// yield the exact FieldChanges to apply, so this layer decides no lifecycle -// policy of its own. Both operations edit an existing record, so each pins the -// submitted record version with an exact-blob entity expectation rather than an -// idempotency key. Neither inspects any process, branch, worktree, or PR state. - -// OperationChangeBlock and OperationChangeDefer are the operation keys the two -// lifecycle transitions record in their result envelopes and transaction -// trailers. +// This file is the `change block`, `change defer`, `change unblock`, and +// `change revive` planning operations: four non-allocating lifecycle transitions +// that land one change's owned frontmatter changes and every affected v1-owned +// derived view (the change record's typed lifecycle fields, its refreshed +// updated date, its re-rendered artifact block, and — for defer only — its +// ## Why deferred authored section; unblock and revive edit no sections; plus +// the inline board) as one validated atomic transaction. The domain owns +// legality: domain.Block, domain.Defer, domain.Unblock, and domain.Revive decide +// whether the current status may take the transition and yield the exact +// FieldChanges to apply, so this layer decides no lifecycle policy of its own. +// Every operation edits an existing record, so each pins the submitted record +// version with an exact-blob entity expectation rather than an idempotency key. +// None inspects any process, branch, worktree, or PR state. + +// OperationChangeBlock, OperationChangeDefer, OperationChangeUnblock, and +// OperationChangeRevive are the operation keys the lifecycle transitions record +// in their result envelopes and transaction trailers. const ( - OperationChangeBlock = "change.block" - OperationChangeDefer = "change.defer" + OperationChangeBlock = "change.block" + OperationChangeDefer = "change.defer" + OperationChangeUnblock = "change.unblock" + OperationChangeRevive = "change.revive" ) // whyDeferredHeading is the owned authored section `change defer` replaces (or @@ -61,8 +65,27 @@ type ChangeDeferRequest struct { WhyDeferred string `json:"why_deferred" docket:"required"` } -// ChangeLifecycleResult is the protocol-v1 document `change block` and -// `change defer` return. It embeds the envelope; Status carries the resulting +// ChangeUnblockRequest is the closed, caller-supplied request for one unblock. +// Path and Version pin the exact submitted record. No reason field: the commit +// subject records the transition and the cleared blocked_by text stays in git +// history. +type ChangeUnblockRequest struct { + ChangeID int `json:"change_id" docket:"required"` + Path string `json:"path" docket:"required"` + Version string `json:"version" docket:"required"` +} + +// ChangeReviveRequest is the closed, caller-supplied request for one revive. +// Path and Version pin the exact submitted record. +type ChangeReviveRequest struct { + ChangeID int `json:"change_id" docket:"required"` + Path string `json:"path" docket:"required"` + Version string `json:"version" docket:"required"` +} + +// ChangeLifecycleResult is the protocol-v1 document every change lifecycle +// transition (`change block`, `change defer`, `change unblock`, `change revive`) +// returns. It embeds the envelope; Status carries the resulting // stored status on a successful apply, and Findings carries every refusal or // validation diagnostic (marshalled as [] never null). type ChangeLifecycleResult struct { @@ -146,7 +169,39 @@ func ChangeDefer(ctx context.Context, deps PlanningDeps, repoDir string, req Cha return executeChangeLifecycle(ctx, deps, repoDir, OperationChangeDefer, req.ChangeID, req.Path, req.Version, action, sections) } -// executeChangeLifecycle is the shared driver both transitions compose after +// ChangeUnblock validates the request, pins authoritative context, and drives +// one atomic transaction that unblocks the change (blocked → in-progress, +// clearing blocked_by) and — when inline is enabled — re-renders the board. +// Every failure that predates the transaction (bad request shape, a github +// board surface) returns without an engine call. +func ChangeUnblock(ctx context.Context, deps PlanningDeps, repoDir string, req ChangeUnblockRequest) ChangeLifecycleResult { + findings := validateLifecycleShape("change_id", req.ChangeID, req.Path, req.Version) + if len(findings) > 0 { + return newChangeLifecycleResult(OperationChangeUnblock, ResultInvalidInput, ChangeLifecycleResult{Findings: findings}) + } + action := func(c domain.Change) (domain.ActionResult, *domain.PolicyFailure) { + return domain.Unblock(c) + } + return executeChangeLifecycle(ctx, deps, repoDir, OperationChangeUnblock, req.ChangeID, req.Path, req.Version, action, nil) +} + +// ChangeRevive validates the request, pins authoritative context, and drives +// one atomic transaction that revives the change (deferred → proposed). The +// ## Why deferred section, branch, and claim stamp are left untouched, per +// domain.Revive's contract. Every failure that predates the transaction (bad +// request shape, a github board surface) returns without an engine call. +func ChangeRevive(ctx context.Context, deps PlanningDeps, repoDir string, req ChangeReviveRequest) ChangeLifecycleResult { + findings := validateLifecycleShape("change_id", req.ChangeID, req.Path, req.Version) + if len(findings) > 0 { + return newChangeLifecycleResult(OperationChangeRevive, ResultInvalidInput, ChangeLifecycleResult{Findings: findings}) + } + action := func(c domain.Change) (domain.ActionResult, *domain.PolicyFailure) { + return domain.Revive(c) + } + return executeChangeLifecycle(ctx, deps, repoDir, OperationChangeRevive, req.ChangeID, req.Path, req.Version, action, nil) +} + +// executeChangeLifecycle is the shared driver every transition composes after // their own request-shape validation: it pins context, fences the board // surface, discovers the repository, and submits one exact-version transaction // carrying the supplied domain action and section edits. @@ -210,7 +265,7 @@ func executeChangeLifecycle(ctx context.Context, deps PlanningDeps, repoDir, opK } // lifecycleResultFromOutcome folds a transaction outcome into the result -// document. A refusal from either transition is always state-shaped (an illegal +// document. A refusal from any transition is always state-shaped (an illegal // source status, a not-found record), so the refusal maps onto invalid-state. func lifecycleResultFromOutcome(opKey string, res transaction.Result, execErr error) ChangeLifecycleResult { result, _ := mapOutcome(res, execErr, ResultInvalidState) @@ -228,8 +283,8 @@ func lifecycleResultFromOutcome(opKey string, res transaction.Result, execErr er return r } -// validateLifecycleShape runs the pinned-entity request checks common to both -// transitions: a positive change id and non-empty path and version. idKey is the +// validateLifecycleShape runs the pinned-entity request checks common to every +// transition: a positive change id and non-empty path and version. idKey is the // JSON key the caller's request actually decodes the id field under ("id" or // "change_id"), so the id-shape finding names the real key in its message; its // code is the registered FindingCode invalidIDCode selects for that key by a @@ -400,8 +455,9 @@ func (o changeLifecycleOp) Plan(ctx context.Context, st transaction.AttemptState // lifecycleFieldValue renders one FieldChange's target value as a document // value: a cleared field (empty target) becomes the bare null form, any other -// value a single-quoted string. Block and defer only ever set string-valued -// owned fields (status, blocked_by). +// value a single-quoted string. The lifecycle transitions only ever set +// string-valued owned fields (status, blocked_by); unblock clears blocked_by via +// the empty target. func lifecycleFieldValue(to string) document.Value { if to == "" { return document.Null() diff --git a/internal/app/change_lifecycle_test.go b/internal/app/change_lifecycle_test.go index da1e6ced4..bfcab4581 100644 --- a/internal/app/change_lifecycle_test.go +++ b/internal/app/change_lifecycle_test.go @@ -404,6 +404,312 @@ func TestChangeBlockPlanToleratesMissingUpdatedField(t *testing.T) { } } +// --- change unblock / change revive ------------------------------------------ + +func validUnblockRequest() ChangeUnblockRequest { + return ChangeUnblockRequest{ChangeID: 3, Path: groomPath(3, "widget"), Version: blobV} +} + +func validReviveRequest() ChangeReviveRequest { + return ChangeReviveRequest{ChangeID: 3, Path: groomPath(3, "widget"), Version: blobV} +} + +func unblockOp(surfaces []string, id int, recPath string) changeLifecycleOp { + return baseLifecycleOp(OperationChangeUnblock, surfaces, id, recPath, + func(c domain.Change) (domain.ActionResult, *domain.PolicyFailure) { return domain.Unblock(c) }, nil) +} + +func reviveOp(surfaces []string, id int, recPath string) changeLifecycleOp { + return baseLifecycleOp(OperationChangeRevive, surfaces, id, recPath, + func(c domain.Change) (domain.ActionResult, *domain.PolicyFailure) { return domain.Revive(c) }, nil) +} + +// pinnedShapeCases are the request-shape failures common to every pinned-entity +// lifecycle request without an authored payload (unblock, revive): each +// mutates the valid (id, path, version) triple and names the expected finding. +var pinnedShapeCases = []struct { + name string + mut func(id *int, path, version *string) + code string +}{ + {"non-positive change id", func(id *int, _, _ *string) { *id = 0 }, "invalid-change_id"}, + {"empty path", func(_ *int, p, _ *string) { *p = "" }, "empty-path"}, + {"empty version", func(_ *int, _, v *string) { *v = " " }, "empty-version"}, +} + +func TestChangeUnblockRejectsBadShapeWithoutEngineCall(t *testing.T) { + for _, c := range pinnedShapeCases { + t.Run(c.name, func(t *testing.T) { + req := validUnblockRequest() + c.mut(&req.ChangeID, &req.Path, &req.Version) + engine := &recordingEngine{} + reader := &fakeChangeReader{pin: mainModePin([]string{"inline"})} + deps := PlanningDeps{Engine: engine, Reader: reader, Clock: testClock()} + + res := ChangeUnblock(context.Background(), deps, "", req) + + if res.Result != ResultInvalidInput { + t.Fatalf("result = %q, want invalid-input", res.Result) + } + if res.Operation != OperationChangeUnblock { + t.Errorf("operation = %q, want %q", res.Operation, OperationChangeUnblock) + } + if len(engine.calls) != 0 { + t.Errorf("engine called %d times on a shape failure, want 0", len(engine.calls)) + } + if !hasFindingCode(res.Findings, c.code) { + t.Errorf("missing finding %q; got %v", c.code, res.Findings) + } + }) + } +} + +func TestChangeReviveRejectsBadShapeWithoutEngineCall(t *testing.T) { + for _, c := range pinnedShapeCases { + t.Run(c.name, func(t *testing.T) { + req := validReviveRequest() + c.mut(&req.ChangeID, &req.Path, &req.Version) + engine := &recordingEngine{} + reader := &fakeChangeReader{pin: mainModePin([]string{"inline"})} + deps := PlanningDeps{Engine: engine, Reader: reader, Clock: testClock()} + + res := ChangeRevive(context.Background(), deps, "", req) + + if res.Result != ResultInvalidInput { + t.Fatalf("result = %q, want invalid-input", res.Result) + } + if res.Operation != OperationChangeRevive { + t.Errorf("operation = %q, want %q", res.Operation, OperationChangeRevive) + } + if len(engine.calls) != 0 { + t.Errorf("engine called %d times on a shape failure, want 0", len(engine.calls)) + } + if !hasFindingCode(res.Findings, c.code) { + t.Errorf("missing finding %q; got %v", c.code, res.Findings) + } + }) + } +} + +func TestChangeUnblockFencesGithubBoardSurface(t *testing.T) { + engine := &recordingEngine{} + reader := &fakeChangeReader{pin: mainModePin([]string{"inline", "github"})} + deps := PlanningDeps{Engine: engine, Reader: reader, Clock: testClock()} + + res := ChangeUnblock(context.Background(), deps, "", validUnblockRequest()) + + if res.Result != ResultUnsupportedConfig { + t.Fatalf("result = %q, want unsupported-config", res.Result) + } + if len(engine.calls) != 0 { + t.Errorf("engine called despite a fenced board surface") + } +} + +func TestChangeReviveFencesGithubBoardSurface(t *testing.T) { + engine := &recordingEngine{} + reader := &fakeChangeReader{pin: mainModePin([]string{"github"})} + deps := PlanningDeps{Engine: engine, Reader: reader, Clock: testClock()} + + res := ChangeRevive(context.Background(), deps, "", validReviveRequest()) + + if res.Result != ResultUnsupportedConfig { + t.Fatalf("result = %q, want unsupported-config", res.Result) + } + if len(engine.calls) != 0 { + t.Errorf("engine called despite a fenced board surface") + } +} + +func TestChangeUnblockPlanFileSet(t *testing.T) { + recPath := groomPath(3, "widget") + src := strings.Replace(lifecycleChange(3, "widget", "blocked"), + "blocked_by: 'waiting on infra'\n", "blocked_by: 'waiting on 0446'\n", 1) + files := map[string]string{ + recPath: src, + "docs/changes/BOARD.md": "# Backlog\n\nold\n", + } + plan, opRes := lifecyclePlanFor(t, files, unblockOp([]string{"inline"}, 3, recPath)) + if opRes.Refused { + t.Fatalf("unexpected refusal: %v", opRes.Findings) + } + assertPlanPaths(t, plan, map[string]transaction.MutationKind{ + recPath: transaction.MutationReplace, + "docs/changes/BOARD.md": transaction.MutationReplace, + }) + + rec := lifecycleRecordBytes(t, plan, recPath) + if !strings.Contains(rec, "status: 'in-progress'") { + t.Errorf("status not set to in-progress:\n%s", rec) + } + if !strings.Contains(rec, "\nblocked_by:\n") || strings.Contains(rec, "waiting on 0446") { + t.Errorf("blocked_by not cleared to the bare null form:\n%s", rec) + } + if !strings.Contains(rec, "updated: '2026-08-16'") { + t.Errorf("updated not stamped from the clock:\n%s", rec) + } + if !strings.Contains(rec, "docket:artifacts:start") { + t.Errorf("artifact block missing:\n%s", rec) + } + if !strings.Contains(rec, "branch: feat/widget\n") || !strings.Contains(rec, "claimed_at: 2026-08-02T00:00:00Z\n") { + t.Errorf("unblock must leave the claim facts intact:\n%s", rec) + } + if plan.CommitSubject != "change 0003 → in-progress" { + t.Errorf("commit subject = %q, want %q", plan.CommitSubject, "change 0003 → in-progress") + } + rc, ok := decodeChangeLifecycleReceipt(plan.Receipt) + if !ok || rc != (changeLifecycleReceipt{ID: 3, Op: "change.unblock", Status: "in-progress"}) { + t.Errorf("receipt = %+v (ok=%v), want {3 change.unblock in-progress}", rc, ok) + } +} + +// deferredWithClaim renders a deferred record that still carries the branch and +// claim stamp a deferral of an in-progress change leaves behind. +func deferredWithClaim() string { + return strings.Replace(lifecycleChange(3, "widget", "deferred"), "trivial: false\n", + "trivial: false\nbranch: 'feat/widget'\nclaimed_at: 2026-08-02T00:00:00Z\n", 1) +} + +func TestChangeRevivePlanFileSet(t *testing.T) { + // A deferred record WITHOUT a ## Why deferred section (deferred by an old + // tool or a hand edit): revive passes no section edits, so it still applies. + recPath := groomPath(3, "widget") + src := deferredWithClaim() + if strings.Contains(src, "## Why deferred") { + t.Fatalf("fixture unexpectedly carries ## Why deferred:\n%s", src) + } + files := map[string]string{recPath: src} + plan, opRes := lifecyclePlanFor(t, files, reviveOp([]string{}, 3, recPath)) + if opRes.Refused { + t.Fatalf("unexpected refusal: %v", opRes.Findings) + } + assertPlanPaths(t, plan, map[string]transaction.MutationKind{ + recPath: transaction.MutationReplace, + }) + + rec := lifecycleRecordBytes(t, plan, recPath) + if !strings.Contains(rec, "status: 'proposed'") { + t.Errorf("status not set to proposed:\n%s", rec) + } + if !strings.Contains(rec, "updated: '2026-08-16'") { + t.Errorf("updated not stamped:\n%s", rec) + } + if !strings.Contains(rec, "branch: 'feat/widget'\n") || !strings.Contains(rec, "claimed_at: 2026-08-02T00:00:00Z\n") { + t.Errorf("revive must leave branch and claimed_at byte-intact:\n%s", rec) + } + if strings.Contains(rec, "## Why deferred") { + t.Errorf("revive must not add a ## Why deferred section:\n%s", rec) + } + if plan.CommitSubject != "change 0003 → proposed" { + t.Errorf("commit subject = %q, want %q", plan.CommitSubject, "change 0003 → proposed") + } + rc, ok := decodeChangeLifecycleReceipt(plan.Receipt) + if !ok || rc != (changeLifecycleReceipt{ID: 3, Op: "change.revive", Status: "proposed"}) { + t.Errorf("receipt = %+v (ok=%v), want {3 change.revive proposed}", rc, ok) + } +} + +func TestChangeRevivePlanPreservesWhyDeferredAndClaim(t *testing.T) { + recPath := groomPath(3, "widget") + src := deferredWithClaim() + "\n## Why deferred\n\nParked for X.\n" + files := map[string]string{recPath: src} + plan, opRes := lifecyclePlanFor(t, files, reviveOp([]string{}, 3, recPath)) + if opRes.Refused { + t.Fatalf("unexpected refusal: %v", opRes.Findings) + } + rec := lifecycleRecordBytes(t, plan, recPath) + if !strings.Contains(rec, "status: 'proposed'") { + t.Errorf("status not set to proposed:\n%s", rec) + } + if !strings.Contains(rec, "\n## Why deferred\n\nParked for X.\n") { + t.Errorf("## Why deferred section did not survive byte-identically:\n%s", rec) + } + if !strings.Contains(rec, "branch: 'feat/widget'\n") || !strings.Contains(rec, "claimed_at: 2026-08-02T00:00:00Z\n") { + t.Errorf("revive must leave branch and claimed_at byte-intact:\n%s", rec) + } +} + +func TestChangeUnblockPlanSourceStatusMatrix(t *testing.T) { + recPath := groomPath(3, "widget") + cases := []struct { + status string + refused bool + }{ + {"blocked", false}, + {"proposed", true}, + {"in-progress", true}, + {"deferred", true}, + } + for _, c := range cases { + t.Run(c.status, func(t *testing.T) { + files := map[string]string{recPath: lifecycleChange(3, "widget", c.status)} + plan, opRes := lifecyclePlanFor(t, files, unblockOp([]string{}, 3, recPath)) + if opRes.Refused != c.refused { + t.Fatalf("unblock from %q: refused=%v, want %v (findings %v)", c.status, opRes.Refused, c.refused, opRes.Findings) + } + if c.refused { + if !hasDomainFindingCode(opRes.Findings, "illegal-source-status") { + t.Errorf("unblock from %q: missing illegal-source-status finding; got %v", c.status, opRes.Findings) + } + if len(plan.Files) != 0 { + t.Errorf("unblock from %q: refused plan still carries files %v", c.status, planPaths(plan)) + } + } + }) + } +} + +func TestChangeRevivePlanSourceStatusMatrix(t *testing.T) { + recPath := groomPath(3, "widget") + cases := []struct { + status string + refused bool + }{ + {"deferred", false}, + {"proposed", true}, + {"in-progress", true}, + {"blocked", true}, + } + for _, c := range cases { + t.Run(c.status, func(t *testing.T) { + files := map[string]string{recPath: lifecycleChange(3, "widget", c.status)} + plan, opRes := lifecyclePlanFor(t, files, reviveOp([]string{}, 3, recPath)) + if opRes.Refused != c.refused { + t.Fatalf("revive from %q: refused=%v, want %v (findings %v)", c.status, opRes.Refused, c.refused, opRes.Findings) + } + if c.refused { + if !hasDomainFindingCode(opRes.Findings, "illegal-source-status") { + t.Errorf("revive from %q: missing illegal-source-status finding; got %v", c.status, opRes.Findings) + } + if len(plan.Files) != 0 { + t.Errorf("revive from %q: refused plan still carries files %v", c.status, planPaths(plan)) + } + } + }) + } +} + +// TestChangeUnblockPlanToleratesMissingUpdatedField pins that an unblock over a +// Bash-era record lacking updated: inserts the field rather than internal-erroring. +func TestChangeUnblockPlanToleratesMissingUpdatedField(t *testing.T) { + recPath := groomPath(3, "widget") + src := lifecycleChange(3, "widget", "blocked") + src = strings.Replace(src, "updated: 2026-08-02\n", "", 1) + if strings.Contains(src, "updated:") { + t.Fatalf("fixture still carries an updated field:\n%s", src) + } + + files := map[string]string{recPath: src} + plan, opRes := lifecyclePlanFor(t, files, unblockOp([]string{}, 3, recPath)) + if opRes.Refused { + t.Fatalf("unexpected refusal: %v", opRes.Findings) + } + rec := lifecycleRecordBytes(t, plan, recPath) + if !strings.Contains(rec, "updated: '2026-08-16'") { + t.Errorf("updated not inserted from the clock on a record lacking it:\n%s", rec) + } +} + // hasDomainFindingCode reports whether any domain finding carries code. func hasDomainFindingCode(findings []domain.Finding, code string) bool { for _, f := range findings { From 36b2f3bfd61115685a10bdb3c6530ac204a9f760 Mon Sep 17 00:00:00 2001 From: Daniel Hanold Date: Fri, 25 Sep 2026 07:22:27 -0400 Subject: [PATCH 3/8] test(app): unblock/revive integration and 0444 resume regression (change 0450) --- internal/app/change_integration_test.go | 205 ++++++++++++++++++++++++ 1 file changed, 205 insertions(+) diff --git a/internal/app/change_integration_test.go b/internal/app/change_integration_test.go index 38c78c47b..27dc50436 100644 --- a/internal/app/change_integration_test.go +++ b/internal/app/change_integration_test.go @@ -713,6 +713,132 @@ func TestIntegrationChangeAuthoringDeferAppliedResultCarriesDeferStatus(t *testi } } +func TestIntegrationChangeAuthoringUnblockAppliedResult(t *testing.T) { + repoDir := newWorkingRepo(t, nil).invocation + receipt := mustMarshal(t, changeLifecycleReceipt{ + ID: 3, Op: OperationChangeUnblock, Status: "in-progress", + }) + engine := &recordingEngine{result: transaction.Result{ + Disposition: transaction.DispositionApplied, + AppliedCommit: "abababababababababababababababababababab", + Receipt: receipt, + }} + reader := &fakeChangeReader{pin: mainModePin([]string{"inline"})} + deps := PlanningDeps{Client: newGitClient(t), Engine: engine, Reader: reader, Clock: testClock()} + + res := ChangeUnblock(context.Background(), deps, repoDir, validUnblockRequest()) + + if res.Result != ResultApplied { + t.Fatalf("result = %q, want applied", res.Result) + } + if res.ID != 3 || res.Status != "in-progress" { + t.Errorf("identity from receipt = (%d, %q)", res.ID, res.Status) + } + if res.Revision != "abababababababababababababababababababab" { + t.Errorf("revision = %q", res.Revision) + } + if res.Operation != OperationChangeUnblock { + t.Errorf("operation = %q, want %q", res.Operation, OperationChangeUnblock) + } + assertSingleLifecycleEngineCall(t, engine, OperationChangeUnblock) +} + +func TestIntegrationChangeAuthoringReviveAppliedResultCarriesProposedStatus(t *testing.T) { + repoDir := newWorkingRepo(t, nil).invocation + receipt := mustMarshal(t, changeLifecycleReceipt{ + ID: 3, Op: OperationChangeRevive, Status: "proposed", + }) + engine := &recordingEngine{result: transaction.Result{ + Disposition: transaction.DispositionApplied, + AppliedCommit: "cdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcd", + Receipt: receipt, + }} + reader := &fakeChangeReader{pin: mainModePin([]string{"inline"})} + deps := PlanningDeps{Client: newGitClient(t), Engine: engine, Reader: reader, Clock: testClock()} + + res := ChangeRevive(context.Background(), deps, repoDir, validReviveRequest()) + + if res.Result != ResultApplied { + t.Fatalf("result = %q, want applied", res.Result) + } + if res.ID != 3 || res.Status != "proposed" { + t.Errorf("identity from receipt = (%d, %q)", res.ID, res.Status) + } + if res.Revision != "cdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcd" { + t.Errorf("revision = %q", res.Revision) + } + if res.Operation != OperationChangeRevive { + t.Errorf("operation = %q, want %q", res.Operation, OperationChangeRevive) + } + assertSingleLifecycleEngineCall(t, engine, OperationChangeRevive) +} + +// assertSingleLifecycleEngineCall pins the one engine call a lifecycle +// transition submits: the operation key, the metadata target ref, no +// idempotency key, and exactly one entity expectation pinning the request's +// path at the exact submitted blob version. +func assertSingleLifecycleEngineCall(t *testing.T, engine *recordingEngine, op string) { + t.Helper() + if len(engine.calls) != 1 { + t.Fatalf("engine calls = %d, want 1", len(engine.calls)) + } + req := engine.calls[0] + if string(req.Operation.Key()) != op { + t.Errorf("operation key = %q, want %q", req.Operation.Key(), op) + } + if req.TargetRef != "refs/heads/docket" { + t.Errorf("target ref = %q, want refs/heads/docket", req.TargetRef) + } + if req.Idempotency != nil { + t.Errorf("lifecycle is non-allocating; it must carry no idempotency key, got %+v", req.Idempotency) + } + if len(req.Expected) != 1 { + t.Fatalf("expected %d entity expectations, want 1", len(req.Expected)) + } + exp := req.Expected[0] + if string(exp.Path) != groomPath(3, "widget") { + t.Errorf("expectation path = %q", exp.Path) + } + if exp.Version.Kind != transaction.VersionBlob || string(exp.Version.ObjectID) != blobV { + t.Errorf("expectation version = %+v, want blob %s", exp.Version, blobV) + } +} + +// Version drift between read and submit is a lost race: the engine reports +// contended and the lifecycle result must say so, never a write over a moved +// record (change 0450 Review Focus 2). +func TestIntegrationChangeAuthoringUnblockContendedOnVersionDrift(t *testing.T) { + repoDir := newWorkingRepo(t, nil).invocation + engine := &recordingEngine{result: transaction.Result{Disposition: transaction.DispositionContended}} + reader := &fakeChangeReader{pin: mainModePin([]string{"inline"})} + deps := PlanningDeps{Client: newGitClient(t), Engine: engine, Reader: reader, Clock: testClock()} + + res := ChangeUnblock(context.Background(), deps, repoDir, validUnblockRequest()) + + if res.Result != ResultContended { + t.Fatalf("result = %q, want contended", res.Result) + } + if res.Findings == nil { + t.Errorf("Findings must marshal as [], not nil") + } +} + +func TestIntegrationChangeAuthoringReviveContendedOnVersionDrift(t *testing.T) { + repoDir := newWorkingRepo(t, nil).invocation + engine := &recordingEngine{result: transaction.Result{Disposition: transaction.DispositionContended}} + reader := &fakeChangeReader{pin: mainModePin([]string{"inline"})} + deps := PlanningDeps{Client: newGitClient(t), Engine: engine, Reader: reader, Clock: testClock()} + + res := ChangeRevive(context.Background(), deps, repoDir, validReviveRequest()) + + if res.Result != ResultContended { + t.Fatalf("result = %q, want contended", res.Result) + } + if res.Findings == nil { + t.Errorf("Findings must marshal as [], not nil") + } +} + // TestEvidenceRecordFromPassedRun: a green terminal record plus a feature head // matching the request produces an immutable record carrying the OBSERVED gate // command (never a request field) and the exact head; the rendered block @@ -2943,6 +3069,85 @@ func TestIntegrationChangeRuntimeResumeHalted(t *testing.T) { } } +// TestIntegrationChangeRuntimeUnblockThenResumeHalted proves the 0444 path as +// typed operations end to end: a halted in-progress change is blocked, then +// unblocked (status back to in-progress, blocked_by cleared, ## Run halted +// preserved), then resume-halted succeeds and removes exactly the marker. +func TestIntegrationChangeRuntimeUnblockThenResumeHalted(t *testing.T) { + for _, m := range planRepoModes() { + t.Run(m.name, func(t *testing.T) { + f := setupHaltedFixture(t, m) + recPath := groomPath(f.id, f.slug) + + // 1. Block the halted change through the real engine. + blocked := ChangeBlock(context.Background(), f.deps, f.repo.invocation, ChangeBlockRequest{ + ChangeID: f.id, Path: recPath, Version: f.version, Reason: "waiting on 0446", + }) + if blocked.Result != ResultApplied || blocked.Status != "blocked" { + t.Fatalf("block = %q status %q (findings %v), want applied blocked", + blocked.Result, blocked.Status, blocked.Findings) + } + + // 2. The record is blocked, the reason recorded, the marker intact. + rec, _ := originFile(t, f.repo.origin, f.branch, recPath) + if !recordHasStatus(rec, "blocked") { + t.Fatalf("post-block record not blocked:\n%s", rec) + } + for _, want := range []string{"blocked_by: 'waiting on 0446'", "## Run halted"} { + if !strings.Contains(rec, want) { + t.Fatalf("post-block record missing %q:\n%s", want, rec) + } + } + + // 3. Unblock with the post-block version. + unblocked := ChangeUnblock(context.Background(), f.deps, f.repo.invocation, ChangeUnblockRequest{ + ChangeID: f.id, Path: recPath, Version: blobVersionAt(t, f.repo.origin, f.branch, recPath), + }) + if unblocked.Result != ResultApplied || unblocked.Status != "in-progress" { + t.Fatalf("unblock = %q status %q (findings %v), want applied in-progress", + unblocked.Result, unblocked.Status, unblocked.Findings) + } + rec, _ = originFile(t, f.repo.origin, f.branch, recPath) + if !recordHasStatus(rec, "in-progress") { + t.Errorf("unblock did not restore in-progress:\n%s", rec) + } + if !strings.Contains(rec, "\nblocked_by:\n") || strings.Contains(rec, "waiting on 0446") { + t.Errorf("blocked_by not cleared to the bare null form:\n%s", rec) + } + if !strings.Contains(rec, "## Run halted") { + t.Fatalf("unblock dropped the ## Run halted marker:\n%s", rec) + } + + // 4. Resume-halted with a quiescent workspace and the post-unblock + // version recovers the change and removes exactly the marker. + resumed := ChangeResumeHalted(context.Background(), f.deps, + WorkspaceDeps{Service: fakeResumeWorkspace{kind: workspace.StateReady, head: f.head}}, f.repo.invocation, + ResumeRequest{ID: f.id, Version: blobVersionAt(t, f.repo.origin, f.branch, recPath), AcknowledgeQuiescent: true}) + if resumed.Result != ResultApplied || resumed.Disposition != HaltDispResumed { + t.Fatalf("resume = %q disp %q reason %q", resumed.Result, resumed.Disposition, resumed.Reason) + } + rec, _ = originFile(t, f.repo.origin, f.branch, recPath) + if strings.Contains(rec, "## Run halted") { + t.Errorf("marker not removed on resume after unblock:\n%s", rec) + } + if !recordHasStatus(rec, "in-progress") { + t.Errorf("resume after unblock left status other than in-progress:\n%s", rec) + } + for _, want := range []string{"branch: feat/widget", "## Why\n\nOriginal why."} { + if !strings.Contains(rec, want) { + t.Errorf("resume after unblock missing %q:\n%s", want, rec) + } + } + }) + } +} + +// recordHasStatus reports whether the record's status line carries want, in +// either the bare or the writer's single-quoted scalar form. +func recordHasStatus(rec, want string) bool { + return strings.Contains(rec, "\nstatus: "+want+"\n") || strings.Contains(rec, "\nstatus: '"+want+"'\n") +} + // TestIntegrationChangeRuntimeResumeHaltedRemoteProbeErrors is change 0368's coverage // for the remote-probe ERROR arm of the pre-allocation recovery path — the // "refusal coverage for ... failed probes" the change's own spec promised. A From e6d3a5958701220d5523bfd994973b6e4cd3d485 Mon Sep 17 00:00:00 2001 From: Daniel Hanold Date: Fri, 25 Sep 2026 07:28:03 -0400 Subject: [PATCH 4/8] feat(cli): wire change unblock and change revive into the catalog (change 0450) --- internal/app/schema_registry.go | 2 ++ internal/app/schema_tags_test.go | 6 ++++++ internal/app/shadow_test.go | 2 ++ internal/cli/change.go | 24 +++++++++++++++++++++++- internal/cli/change_test.go | 8 +++++--- internal/cli/install.go | 2 ++ 6 files changed, 40 insertions(+), 4 deletions(-) diff --git a/internal/app/schema_registry.go b/internal/app/schema_registry.go index d102214ff..92061775b 100644 --- a/internal/app/schema_registry.go +++ b/internal/app/schema_registry.go @@ -56,6 +56,8 @@ var operationBindings = []OperationBinding{ {ID: "change.refresh-claim", Request: ChangeClaimRequest{}, Result: ChangeClaimResult{}}, // ChangeRefreshClaim {ID: "change.repair-identity", Request: RepairIdentityRequest{}, Result: RepairIdentityResult{}}, // RepairIdentity {ID: "change.resume-halted", Request: ResumeRequest{}, Result: HaltResult{}}, // ChangeResumeHalted + {ID: "change.revive", Request: ChangeReviveRequest{}, Result: ChangeLifecycleResult{}}, // ChangeRevive + {ID: "change.unblock", Request: ChangeUnblockRequest{}, Result: ChangeLifecycleResult{}}, // ChangeUnblock {ID: "context.finalize", Request: FinalizeContextRequest{}, Result: FinalizeContextResult{}}, // ContextFinalize {ID: "context.implementation", Request: ImplementationContextRequest{}, Result: ImplementationContextResult{}}, // ContextImplementation {ID: "development.install", Request: nil, Result: InstallResult{}}, // RunDevelopmentInstall diff --git a/internal/app/schema_tags_test.go b/internal/app/schema_tags_test.go index 8298a879d..4134ea111 100644 --- a/internal/app/schema_tags_test.go +++ b/internal/app/schema_tags_test.go @@ -27,6 +27,12 @@ func TestRequiredTagMatchesValidator(t *testing.T) { {"change.defer", ChangeDeferRequest{}, func() []StatusFinding { return ChangeDefer(context.Background(), PlanningDeps{}, "", ChangeDeferRequest{}).Findings }}, + {"change.unblock", ChangeUnblockRequest{}, func() []StatusFinding { + return ChangeUnblock(context.Background(), PlanningDeps{}, "", ChangeUnblockRequest{}).Findings + }}, + {"change.revive", ChangeReviveRequest{}, func() []StatusFinding { + return ChangeRevive(context.Background(), PlanningDeps{}, "", ChangeReviveRequest{}).Findings + }}, {"change.create", ChangeCreateRequest{}, func() []StatusFinding { return validateChangeCreateShape(ChangeCreateRequest{}) }}, diff --git a/internal/app/shadow_test.go b/internal/app/shadow_test.go index 6530bdab5..0c2fe94e6 100644 --- a/internal/app/shadow_test.go +++ b/internal/app/shadow_test.go @@ -69,6 +69,8 @@ func TestEnvelopeNotShadowed(t *testing.T) { {"change.groom", newChangeGroomResult(ResultApplied, ChangeGroomResult{})}, {"change.block", newChangeLifecycleResult(OperationChangeBlock, ResultApplied, ChangeLifecycleResult{})}, {"change.defer", newChangeLifecycleResult(OperationChangeDefer, ResultApplied, ChangeLifecycleResult{})}, + {"change.unblock", newChangeLifecycleResult(OperationChangeUnblock, ResultApplied, ChangeLifecycleResult{})}, + {"change.revive", newChangeLifecycleResult(OperationChangeRevive, ResultApplied, ChangeLifecycleResult{})}, {"change.kill", newChangeKillResult(ResultApplied, ChangeKillResult{})}, {"learning.record", newLearningResult(OperationLearningRecord, ResultApplied, LearningResult{})}, {"learning.update", newLearningResult(OperationLearningUpdate, ResultApplied, LearningResult{})}, diff --git a/internal/cli/change.go b/internal/cli/change.go index 999e4b41b..879242f69 100644 --- a/internal/cli/change.go +++ b/internal/cli/change.go @@ -92,6 +92,28 @@ func newChangeCommand(setResult func(app.OperationResult)) *cobra.Command { return nil }, EffectMetadataWrite) + unblock := changeSubcommand("change", "unblock", + "Unblock a blocked change back to in-progress, clearing blocked_by, from a JSON request", + func(c *cobra.Command, deps app.PlanningDeps, repoDir string) error { + var req app.ChangeUnblockRequest + if err := decodeRequestFlag(c, &req); err != nil { + return err + } + setResult(app.ChangeUnblock(c.Context(), deps, repoDir, req)) + return nil + }, EffectMetadataWrite) + + revive := changeSubcommand("change", "revive", + "Revive a deferred change back to proposed, from a JSON request", + func(c *cobra.Command, deps app.PlanningDeps, repoDir string) error { + var req app.ChangeReviveRequest + if err := decodeRequestFlag(c, &req); err != nil { + return err + } + setResult(app.ChangeRevive(c.Context(), deps, repoDir, req)) + return nil + }, EffectMetadataWrite) + kill := changeSubcommand("change", "kill", "Kill a change, archiving it, from a JSON request", func(c *cobra.Command, deps app.PlanningDeps, repoDir string) error { @@ -165,7 +187,7 @@ func newChangeCommand(setResult func(app.OperationResult)) *cobra.Command { repairIdentity := newRepairIdentitySubcommand(setResult) - changeCmd.AddCommand(create, groom, block, deferCmd, kill, claim, refreshClaim, reconcile, attachPlan, attachResults, halt, resumeHalted, reclaim, markImplemented, repairIdentity) + changeCmd.AddCommand(create, groom, block, deferCmd, unblock, revive, kill, claim, refreshClaim, reconcile, attachPlan, attachResults, halt, resumeHalted, reclaim, markImplemented, repairIdentity) return changeCmd } diff --git a/internal/cli/change_test.go b/internal/cli/change_test.go index 87a60b651..a388e589f 100644 --- a/internal/cli/change_test.go +++ b/internal/cli/change_test.go @@ -20,11 +20,11 @@ func runCLIStdin(t *testing.T, stdin string, args ...string) (stdout, stderr str } // TestChangeCommandsRegistered is the registration assertion: `docket change` -// carries exactly the five settled subcommands, each with a required --request +// carries the seven settled authoring subcommands, each with a required --request // flag and a --repo-dir flag, and the bare group reports a missing command. func TestChangeCommandsRegistered(t *testing.T) { root := captureTree(t) - for _, sub := range []string{"create", "groom", "block", "defer", "kill"} { + for _, sub := range []string{"create", "groom", "block", "defer", "unblock", "revive", "kill"} { cmd, _, err := root.Find([]string{"change", sub}) if err != nil || cmd == nil || cmd.Name() != sub { t.Fatalf("change %s not registered: cmd=%v err=%v", sub, cmd, err) @@ -81,6 +81,8 @@ func TestChangeCommandsReachOperation(t *testing.T) { {"groom", "change.groom"}, {"block", "change.block"}, {"defer", "change.defer"}, + {"unblock", "change.unblock"}, + {"revive", "change.revive"}, {"kill", "change.kill"}, } for _, c := range cases { @@ -134,7 +136,7 @@ func TestChangeRequestFileMissing(t *testing.T) { // never installed assets), so they are not refused on a machine with no // installation. func TestChangeCommandsAssetIndependent(t *testing.T) { - for _, key := range []string{"change", "change create", "change groom", "change block", "change defer", "change kill", "change claim", "change refresh-claim", "change reconcile"} { + for _, key := range []string{"change", "change create", "change groom", "change block", "change defer", "change unblock", "change revive", "change kill", "change claim", "change refresh-claim", "change reconcile"} { if !assetIndependent[key] { t.Errorf("%q is not registered asset-independent", key) } diff --git a/internal/cli/install.go b/internal/cli/install.go index 1116f8e70..9bcae2d2c 100644 --- a/internal/cli/install.go +++ b/internal/cli/install.go @@ -40,6 +40,8 @@ var assetIndependent = map[string]bool{ "change groom": true, "change block": true, "change defer": true, + "change unblock": true, + "change revive": true, "change kill": true, "change claim": true, "change refresh-claim": true, From 09b3b284c49499fae1c068d27158775b7840b051 Mon Sep 17 00:00:00 2001 From: Daniel Hanold Date: Fri, 25 Sep 2026 07:28:49 -0400 Subject: [PATCH 5/8] docs(convention): clearing a blocker / reviving are typed operations (change 0450) --- internal/assets/embedded/manifest.json | 6 +++--- .../assets/embedded/tree/skills/docket-convention/SKILL.md | 2 +- skills/docket-convention/SKILL.md | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/internal/assets/embedded/manifest.json b/internal/assets/embedded/manifest.json index 9950eb2ad..fc4628ba9 100644 --- a/internal/assets/embedded/manifest.json +++ b/internal/assets/embedded/manifest.json @@ -1,7 +1,7 @@ { "format_version": 1, "asset_protocol": 1, - "asset_set_id": "sha256:59310c05314464c379449e18f9be6b1c93d44492585c7ca29f2f94274c4adb95", + "asset_set_id": "sha256:731403057cb43dbe23441329450003b511900f190a85c685fc103544bbfb7cda", "entries": [ { "path": ".docket.example.yml", @@ -343,8 +343,8 @@ "path": "skills/docket-convention/SKILL.md", "role": "skill", "mode": 420, - "size": 56982, - "sha256": "dc8be5bcb398ad4d223752d1a40c2e9bf905c61f35773aafc098095b1dee18b8" + "size": 57097, + "sha256": "eea0bb2c4f9792287320364d02b4c281a029ad52dc7acb769c5a0000d338be58" }, { "path": "skills/docket-convention/references/agent-layer.md", diff --git a/internal/assets/embedded/tree/skills/docket-convention/SKILL.md b/internal/assets/embedded/tree/skills/docket-convention/SKILL.md index 7e5ac2ad0..4573a39dd 100644 --- a/internal/assets/embedded/tree/skills/docket-convention/SKILL.md +++ b/internal/assets/embedded/tree/skills/docket-convention/SKILL.md @@ -278,7 +278,7 @@ An `Accepted` ADR is immutable except its `status:` line; a non-reversing contex | `done` | PR merged, filed away (happy terminal) | `archive/` | | `killed` | abandoned — obsolete or never shipped (sad terminal) | `archive/` | -**Rules.** `active/` holds every non-terminal status; `archive/` holds the two terminal outcomes. `stacked-merged` is non-terminal for that reason: merged into its stack parent, not the integration branch, so it stays in `active/` until the stack close-out promotes it when the stack root lands. The single physical move (`active/ → archive/`, date-prefixed) happens once on the terminal transition and is **idempotent**: re-pull, re-read `status` on `metadata_branch`, no-op if already terminal. `deferred` may be entered from `proposed` or `in-progress` (add `## Why deferred`) and revived to `proposed`; clearing a blocker or reviving is a one-line frontmatter edit, no move. A change whose `depends_on` is unsatisfied is *implicitly* blocked — the selector skips it and the board shows it **waiting on #N**. A dependency is **satisfied when it reaches `done`**; if `#N` is still `implemented` the board flags **waiting on #N — needs your merge**, distinct from **waiting on #N — not yet built**. Reserve explicit `blocked` for external blockers the system can't infer. +**Rules.** `active/` holds every non-terminal status; `archive/` holds the two terminal outcomes. `stacked-merged` is non-terminal for that reason: merged into its stack parent, not the integration branch, so it stays in `active/` until the stack close-out promotes it when the stack root lands. The single physical move (`active/ → archive/`, date-prefixed) happens once on the terminal transition and is **idempotent**: re-pull, re-read `status` on `metadata_branch`, no-op if already terminal. `deferred` may be entered from `proposed` or `in-progress` (add `## Why deferred`) and revived to `proposed`; clear a blocker with `change.unblock` (`blocked` → `in-progress`, clearing `blocked_by`) and revive with `change.revive` (`deferred` → `proposed`) — typed operations, no file move. A change whose `depends_on` is unsatisfied is *implicitly* blocked — the selector skips it and the board shows it **waiting on #N**. A dependency is **satisfied when it reaches `done`**; if `#N` is still `implemented` the board flags **waiting on #N — needs your merge**, distinct from **waiting on #N — not yet built**. Reserve explicit `blocked` for external blockers the system can't infer. **Reclaim edge (`in-progress → proposed`).** An `in-progress` change whose claim lease (`claimed_at:` + `reclaim.lease_ttl`) has expired AND that has no feature branch is flipped back to `proposed` by the reclaim operation (opt-in via `reclaim.auto`, or an explicit `change.reclaim`), clearing `branch:`/`claimed_at:` and resetting `reconciled: false` so a fresh reconcile runs on re-claim. The has-branch case is never auto-reclaimed (it may carry real work) — it stays flagged for a human. diff --git a/skills/docket-convention/SKILL.md b/skills/docket-convention/SKILL.md index 7e5ac2ad0..4573a39dd 100644 --- a/skills/docket-convention/SKILL.md +++ b/skills/docket-convention/SKILL.md @@ -278,7 +278,7 @@ An `Accepted` ADR is immutable except its `status:` line; a non-reversing contex | `done` | PR merged, filed away (happy terminal) | `archive/` | | `killed` | abandoned — obsolete or never shipped (sad terminal) | `archive/` | -**Rules.** `active/` holds every non-terminal status; `archive/` holds the two terminal outcomes. `stacked-merged` is non-terminal for that reason: merged into its stack parent, not the integration branch, so it stays in `active/` until the stack close-out promotes it when the stack root lands. The single physical move (`active/ → archive/`, date-prefixed) happens once on the terminal transition and is **idempotent**: re-pull, re-read `status` on `metadata_branch`, no-op if already terminal. `deferred` may be entered from `proposed` or `in-progress` (add `## Why deferred`) and revived to `proposed`; clearing a blocker or reviving is a one-line frontmatter edit, no move. A change whose `depends_on` is unsatisfied is *implicitly* blocked — the selector skips it and the board shows it **waiting on #N**. A dependency is **satisfied when it reaches `done`**; if `#N` is still `implemented` the board flags **waiting on #N — needs your merge**, distinct from **waiting on #N — not yet built**. Reserve explicit `blocked` for external blockers the system can't infer. +**Rules.** `active/` holds every non-terminal status; `archive/` holds the two terminal outcomes. `stacked-merged` is non-terminal for that reason: merged into its stack parent, not the integration branch, so it stays in `active/` until the stack close-out promotes it when the stack root lands. The single physical move (`active/ → archive/`, date-prefixed) happens once on the terminal transition and is **idempotent**: re-pull, re-read `status` on `metadata_branch`, no-op if already terminal. `deferred` may be entered from `proposed` or `in-progress` (add `## Why deferred`) and revived to `proposed`; clear a blocker with `change.unblock` (`blocked` → `in-progress`, clearing `blocked_by`) and revive with `change.revive` (`deferred` → `proposed`) — typed operations, no file move. A change whose `depends_on` is unsatisfied is *implicitly* blocked — the selector skips it and the board shows it **waiting on #N**. A dependency is **satisfied when it reaches `done`**; if `#N` is still `implemented` the board flags **waiting on #N — needs your merge**, distinct from **waiting on #N — not yet built**. Reserve explicit `blocked` for external blockers the system can't infer. **Reclaim edge (`in-progress → proposed`).** An `in-progress` change whose claim lease (`claimed_at:` + `reclaim.lease_ttl`) has expired AND that has no feature branch is flipped back to `proposed` by the reclaim operation (opt-in via `reclaim.auto`, or an explicit `change.reclaim`), clearing `branch:`/`claimed_at:` and resetting `reconciled: false` so a fresh reconcile runs on re-claim. The has-branch case is never auto-reclaimed (it may carry real work) — it stays flagged for a human. From fb5a3c422693615725e7ee85fe3f9096376dcaf3 Mon Sep 17 00:00:00 2001 From: Daniel Hanold Date: Fri, 25 Sep 2026 07:29:31 -0400 Subject: [PATCH 6/8] docs(results): change 0450 results --- ...eration-to-reverse-change-block-results.md | 47 +++++++++++++++++++ 1 file changed, 47 insertions(+) create mode 100644 docs/results/2026-09-25-typed-change-unblock-operation-to-reverse-change-block-results.md diff --git a/docs/results/2026-09-25-typed-change-unblock-operation-to-reverse-change-block-results.md b/docs/results/2026-09-25-typed-change-unblock-operation-to-reverse-change-block-results.md new file mode 100644 index 000000000..bf06e9633 --- /dev/null +++ b/docs/results/2026-09-25-typed-change-unblock-operation-to-reverse-change-block-results.md @@ -0,0 +1,47 @@ + +> ↩ **[Change 0450 — Typed change.unblock operation to reverse change.block](https://github.com/danielhanold/docket/blob/docket/docs/changes/active/0450-typed-change-unblock-operation-to-reverse-change-block.md)** + +# Typed change.unblock operation to reverse change.block — Results + +**Human action:** None required to merge. One optional walkthrough is below if you want to see the two new commands work on a scratch repository. + +## Outcome + +Docket could move a change to `blocked` (`docket change block`) or `deferred` (`docket change defer`), but there was no command to undo either. Getting a change out meant hand-editing its frontmatter and committing on the `docket` branch, which skipped the version check and left `BOARD.md` stale until someone ran `docket repository migrate`. This happened on change 0444. + +Two new operations close that gap: + +- `docket change unblock` (`change.unblock`) moves a `blocked` change back to `in-progress` and clears `blocked_by`. +- `docket change revive` (`change.revive`) moves a `deferred` change back to `proposed`. + +Both work like `block` and `defer`: the request pins the record's exact version, and one metadata commit updates `updated:`, the `## Artifacts` block and the board together. A change in any other status is refused and nothing is written. Revive leaves the `## Why deferred` section, `branch:` and `claimed_at:` in place, as the spec required. A change that was halted and then blocked keeps its `## Run halted` section through unblock, so `change.resume-halted` works afterwards. That is the 0444 path, and it now has a regression test. + +Both commands appear in the capability catalog and the schema registry. The docket-convention lifecycle rules now point to these commands instead of the hand edit. + +There was one small departure from the plan: the new schema-registry rows sit in id-sorted position (after `change.resume-halted`), not next to block/defer, because a registry test requires sorted order. + +## Human actions and testing + +### Optional — Block, unblock, defer and revive a scratch change + +Use this to see the new commands work end to end. Automated tests already cover this behavior. + +Prerequisites: a throwaway repository initialized with docket, containing one `proposed` change that has `trivial: true`, and a docket binary built from this branch (`go build -o /tmp/docket ./cmd/docket`). + +1. Claim the change with `docket change claim`, then block it with `docket change block` using a reason. + Expected: the status is `blocked`, `blocked_by` holds your reason, and `BOARD.md` shows it as blocked. +2. Run `docket change unblock` with the change id, path and current version (from `docket status --json`) in the request file. + Expected: the status is `in-progress`, `blocked_by` is empty, and one new commit on `docket` updated the board. +3. Defer the change with `docket change defer`, then run `docket change revive`. + Expected: the status is `proposed`, and `## Why deferred` is still in the body. +4. Run `docket change unblock` on the now-`proposed` change. + Expected: the command refuses with `invalid-state` and no commit is made. + +Cleanup: delete the scratch repository. + +## Verification performed + +- Each of the four plan tasks ran focused tests through the gate driver: the lifecycle unit tests, the `internal/app` integration tests (`-tags integration`), the `internal/cli` and `internal/app` packages in full, and the `internal/assets` and `internal/repoguard` tests. All passed. +- Where it applied, each task showed a failing test first: the missing types, then the unregistered commands. +- Mutation check on the regression test: making `ChangeUnblock` call the revive transition caused the 0444 regression test to fail as intended. The mutation was reverted. +- The full-suite build gate and the whole-branch review happen after this checkpoint. The build evidence in the PR body records their outcome. From e5ba1ead8c3d73c7c585781cda572424d492f0ea Mon Sep 17 00:00:00 2001 From: Daniel Hanold Date: Fri, 25 Sep 2026 07:40:08 -0400 Subject: [PATCH 7/8] test(app): real-engine revive and unblock board/subject assertions (change 0450) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review finding (important): the spec's Testing section requires one applied unblock and one applied revive through the real engine, asserting the receipt, commit subject, and board row; the applied tests used recordingEngine and revive never ran through the real engine. Adds TestIntegrationChangeRuntimeDeferThenRevive (defer then revive via the real engine: receipt identity, committed revision == tip, subject 'change 0005 → proposed', BOARD.md row under Proposed with no Deferred section, board matches a fresh render) and adds the commit-subject and BOARD.md row assertions to the unblock step of TestIntegrationChangeRuntimeUnblockThenResumeHalted. --- internal/app/change_integration_test.go | 98 +++++++++++++++++++++++++ 1 file changed, 98 insertions(+) diff --git a/internal/app/change_integration_test.go b/internal/app/change_integration_test.go index 27dc50436..fac3a90c1 100644 --- a/internal/app/change_integration_test.go +++ b/internal/app/change_integration_test.go @@ -3117,6 +3117,11 @@ func TestIntegrationChangeRuntimeUnblockThenResumeHalted(t *testing.T) { if !strings.Contains(rec, "## Run halted") { t.Fatalf("unblock dropped the ## Run halted marker:\n%s", rec) } + // The unblock commit carries the transition subject and the + // re-rendered board: the change's row sits under In progress and the + // Blocked section is gone. + assertLifecycleCommit(t, f.repo.origin, f.branch, unblocked, f.id, "in-progress") + assertBoardRowUnder(t, f.repo.origin, f.branch, recPath, "## 🟢 In progress (", "## 🔴 Blocked (") // 4. Resume-halted with a quiescent workspace and the post-unblock // version recovers the change and removes exactly the marker. @@ -3142,6 +3147,99 @@ func TestIntegrationChangeRuntimeUnblockThenResumeHalted(t *testing.T) { } } +// TestIntegrationChangeRuntimeDeferThenRevive proves change revive end to end +// through the real transaction engine (change 0450 spec Testing): a proposed +// change is deferred, then revived — the record returns to proposed with its +// ## Why deferred rationale kept, the receipt names the change and status, the +// applied commit carries the `change NNNN → proposed` subject, and the +// committed BOARD.md lists the change under Proposed with no Deferred section. +func TestIntegrationChangeRuntimeDeferThenRevive(t *testing.T) { + for _, m := range planRepoModes() { + t.Run(m.name, func(t *testing.T) { + requireRealGit(t) + const id, slug = 5, "widget" + recPath := groomPath(id, slug) + repo := buildConfiguredRepo(t, m, recPath, lifecycleChange(id, slug, "proposed")) + node := planningDepsFor(t, repo.invocation) + ctx := context.Background() + + // 1. Defer the proposed change through the real engine. + deferred := ChangeDefer(ctx, node.deps, node.dir, ChangeDeferRequest{ + ChangeID: id, Path: recPath, Version: blobVersionAt(t, repo.origin, m.branch, recPath), + WhyDeferred: "Parked pending a decision.\n", + }) + if deferred.Result != ResultApplied || deferred.Status != "deferred" { + t.Fatalf("defer = %q status %q (findings %v), want applied deferred", + deferred.Result, deferred.Status, deferred.Findings) + } + assertBoardRowUnder(t, repo.origin, m.branch, recPath, "## ⚪ Deferred (", "## 🟡 Proposed (") + + // 2. Revive with the post-defer version. + revived := ChangeRevive(ctx, node.deps, node.dir, ChangeReviveRequest{ + ChangeID: id, Path: recPath, Version: blobVersionAt(t, repo.origin, m.branch, recPath), + }) + if revived.Result != ResultApplied || revived.Status != "proposed" { + t.Fatalf("revive = %q status %q (findings %v), want applied proposed", + revived.Result, revived.Status, revived.Findings) + } + if revived.ID != id || revived.Operation != OperationChangeRevive { + t.Errorf("revive receipt identity = (%d, %q), want (%d, %q)", + revived.ID, revived.Operation, id, OperationChangeRevive) + } + rec, _ := originFile(t, repo.origin, m.branch, recPath) + if !recordHasStatus(rec, "proposed") { + t.Errorf("revive did not restore proposed:\n%s", rec) + } + if !strings.Contains(rec, "## Why deferred\n\nParked pending a decision.\n") { + t.Errorf("revive dropped the ## Why deferred rationale:\n%s", rec) + } + assertLifecycleCommit(t, repo.origin, m.branch, revived, id, "proposed") + assertBoardRowUnder(t, repo.origin, m.branch, recPath, "## 🟡 Proposed (", "## ⚪ Deferred (") + assertBoardMatchesCommitted(t, repo.origin, m.branch, node.dir) + }) + } +} + +// assertLifecycleCommit proves an applied lifecycle result's committed revision +// is the metadata branch tip and that commit carries the transition subject +// `change NNNN → `. +func assertLifecycleCommit(t *testing.T, origin, branch string, res ChangeLifecycleResult, id int, status string) { + t.Helper() + tip := originTip(t, origin, branch) + if res.Revision != tip { + t.Errorf("committed revision = %q, want metadata tip %q", res.Revision, tip) + } + want := fmt.Sprintf("change %04d → %s", id, status) + if got := runGit(t, origin, "log", "-1", "--format=%s", tip); got != want { + t.Errorf("commit subject = %q, want %q", got, want) + } +} + +// assertBoardRowUnder proves the committed BOARD.md lists recPath's row inside +// the section whose heading starts with wantHeading, and carries no section +// whose heading starts with absentHeading (the change was its only member). +func assertBoardRowUnder(t *testing.T, origin, branch, recPath, wantHeading, absentHeading string) { + t.Helper() + board, ok := originFile(t, origin, branch, "docs/changes/BOARD.md") + if !ok { + t.Fatalf("BOARD.md absent on %s", branch) + } + start := strings.Index(board, wantHeading) + if start < 0 { + t.Fatalf("board lacks a %q section:\n%s", wantHeading, board) + } + section := board[start+len(wantHeading):] + if end := strings.Index(section, "\n## "); end >= 0 { + section = section[:end] + } + if row := "(active/" + filepath.Base(recPath) + ")"; !strings.Contains(section, row) { + t.Errorf("board section %q lacks the change row %q:\n%s", wantHeading, row, board) + } + if strings.Contains(board, absentHeading) { + t.Errorf("board still carries a %q section:\n%s", absentHeading, board) + } +} + // recordHasStatus reports whether the record's status line carries want, in // either the bare or the writer's single-quoted scalar form. func recordHasStatus(rec, want string) bool { From 098a277fe26ce8dad76bfc5c5e866e17667c3cd4 Mon Sep 17 00:00:00 2001 From: Daniel Hanold Date: Fri, 25 Sep 2026 07:40:43 -0400 Subject: [PATCH 8/8] docs(results): change 0450 review-fix checkpoint --- ...ange-unblock-operation-to-reverse-change-block-results.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/results/2026-09-25-typed-change-unblock-operation-to-reverse-change-block-results.md b/docs/results/2026-09-25-typed-change-unblock-operation-to-reverse-change-block-results.md index bf06e9633..0be7f145b 100644 --- a/docs/results/2026-09-25-typed-change-unblock-operation-to-reverse-change-block-results.md +++ b/docs/results/2026-09-25-typed-change-unblock-operation-to-reverse-change-block-results.md @@ -3,7 +3,7 @@ # Typed change.unblock operation to reverse change.block — Results -**Human action:** None required to merge. One optional walkthrough is below if you want to see the two new commands work on a scratch repository. +**Human action:** None required. The change is ready to merge. One optional walkthrough is below if you want to see the two new commands work on a scratch repository. ## Outcome @@ -44,4 +44,5 @@ Cleanup: delete the scratch repository. - Each of the four plan tasks ran focused tests through the gate driver: the lifecycle unit tests, the `internal/app` integration tests (`-tags integration`), the `internal/cli` and `internal/app` packages in full, and the `internal/assets` and `internal/repoguard` tests. All passed. - Where it applied, each task showed a failing test first: the missing types, then the unregistered commands. - Mutation check on the regression test: making `ChangeUnblock` call the revive transition caused the 0444 regression test to fail as intended. The mutation was reverted. -- The full-suite build gate and the whole-branch review happen after this checkpoint. The build evidence in the PR body records their outcome. +- The full build suite (`go run ./cmd/docket development test`) passed. Several integration and race test files printed `BUDGET WATCH` lines, meaning they ran longer than budget while tests ran in parallel. These are screening notes, not failures, and no serial run confirmed a breach. +- The whole-branch review returned one important finding. The spec asked for real-engine integration tests that check the board row for an applied unblock and revive, but the first applied tests used a fake engine. The finding was fixed in commit e5ba1ead: a new real-engine defer-then-revive test, plus checks of the commit subject and board row on the unblock step of the 0444 regression. A deliberately broken expectation made the new assertions fail, so they do catch mistakes. The full suite was then run again at the final head. The build evidence in the PR body records that run.