diff --git a/docs/guide/designing-before-building.md b/docs/guide/designing-before-building.md index c61bf0f95..88a720e91 100644 --- a/docs/guide/designing-before-building.md +++ b/docs/guide/designing-before-building.md @@ -40,6 +40,9 @@ is to attack the draft. A design that survives the critic exits as a build-ready critic rejects (or that comes out genuinely trivial) is handled accordingly, and anything the loop cannot settle confidently is handed back to your interactive queue rather than forced through. +Such a stub shows as **auto-groom blocked — needs you** on the board. Once you have supplied the +missing context you can groom it yourself, or re-arm it so the autonomous groomer picks it up +again — the interactive groom skill offers both. Two things are deliberately never autonomous: killing a change and deferring one. Those are judgment calls that stay with you. Like interactive grooming, auto-groom writes markdown only — diff --git a/docs/results/2026-09-27-changecreaterequest-typed-auto-groomable-branch-prefix-scalars-results.md b/docs/results/2026-09-27-changecreaterequest-typed-auto-groomable-branch-prefix-scalars-results.md new file mode 100644 index 000000000..90936c22e --- /dev/null +++ b/docs/results/2026-09-27-changecreaterequest-typed-auto-groomable-branch-prefix-scalars-results.md @@ -0,0 +1,40 @@ + +> ↩ **[Change 0382 — ChangeCreateRequest should accept typed auto_groomable / branch_prefix scalars](https://github.com/danielhanold/docket/blob/docket/docs/changes/active/0382-changecreaterequest-typed-auto-groomable-branch-prefix-scalars.md)** + +# ChangeCreateRequest should accept typed auto_groomable / branch_prefix scalars — Results + +**Human action:** No action is required before merge beyond the normal PR review. One behavior change is worth a glance: presence markers such as `## Auto-groom blocked` inside fenced code blocks no longer count on the board (see Known issues). + +## Outcome + +Before this change, three skill steps edited a change's `auto_groomable` and `branch_prefix` fields with plain git: `docket-new-change` after creating a change, `docket-auto-groom` when it abstained, and a human re-arming an abstained stub. The two auto-groom edits left `BOARD.md` stale, and a malformed branch prefix only failed later, inside an autonomous implement run. + +Now: + +- `docket change create` takes optional `auto_groomable` (true, false, or unset) and `branch_prefix` fields. The prefix is trimmed, has one trailing `/` removed, and is lowercased. An invalid prefix is refused up front with `invalid-branch_prefix`. +- `docket change groom` has two new outcomes. `abstain` sets `auto_groomable: false` and appends a dated `## Auto-groom blocked` entry. `rearm` sets it back to `true` and removes that section. Both re-render the board in the same commit. +- The four skills (new-change, auto-groom, groom-next, convention) use these typed operations instead of hand edits. + +Review fixes applied in-branch: + +- The board and `rearm` now agree on what counts as a blocked marker. Heading-shaped lines inside fenced code are ignored by both. +- A hand-typed non-boolean `auto_groomable` (for example `yes`) is reported as a warning. It is not an error, so it cannot block PR publication or finalize on a record that used to be valid. +- New tests cover abstain/re-arm when another section follows the blocked section. The `docket-groom-next` skill now documents the re-arm exit. + +The `docket-groom-next/SKILL.md` word ceiling was raised from 1889 to 1996 with human approval. + +## Verification performed + +- Full suite (`go run ./cmd/docket development test`) passed on the pre-review head f1c117c6 (54/54 files). The suite was run again on the final head and its result is in the PR's build-evidence block. +- A deep-rung whole-branch review returned 4 minor findings and no blockers or important findings. All four were fixed (commits 529d7f32, 6b24de39), and focused package tests passed after each fix. +- The new tests for a section following the blocked section were not mutation-checked against the section-boundary logic. + +## Known issues and follow-ups + +### Fenced presence markers no longer show on the board + +This affects any change file that has `## Run halted`, `## Auto-groom blocked`, `## Finalize blocked`, or `## Publish deferred` only inside a fenced code block (for example, a change that discusses those headings). Before this change the board treated such a record as halted or blocked. Now it does not. This matches how the typed operations that write and remove those sections already behaved, so it is an intended, confirmed change. Nothing needs to be done unless a real marker was ever written inside a fence, which the writers never do. + +### Suite budget watch lines + +The pre-review full suite printed several `BUDGET WATCH` lines, for example `test_go_race` at 407s and `test_go_toolchain` at 390s under -j11. These are screening findings, and none of them is a serially confirmed breach. No action is needed unless they repeat. diff --git a/docs/superpowers/plans/2026-09-27-0382-typed-auto-groomable-branch-prefix-and-abstain-rearm.md b/docs/superpowers/plans/2026-09-27-0382-typed-auto-groomable-branch-prefix-and-abstain-rearm.md new file mode 100644 index 000000000..268d7019b --- /dev/null +++ b/docs/superpowers/plans/2026-09-27-0382-typed-auto-groomable-branch-prefix-and-abstain-rearm.md @@ -0,0 +1,1656 @@ + +> ↩ **[Change 0382 — ChangeCreateRequest should accept typed auto_groomable / branch_prefix scalars](https://github.com/danielhanold/docket/blob/docket/docs/changes/active/0382-changecreaterequest-typed-auto-groomable-branch-prefix-scalars.md)** + +# Typed auto_groomable / branch_prefix at create, and typed abstain / re-arm 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. (In this repo the plan is executed by the `docket-build` role; each task is one `docket-build-task` unit.) + +**Goal:** Every write of a change's `auto_groomable` and `branch_prefix` scalars goes through a typed, validated, board-refreshing docket operation: `change.create` accepts both scalars (with Go-owned branch-prefix normalization), and `change.groom` gains `outcome: abstain` and `outcome: rearm`, retiring the plain-git frontmatter edits in the skills. + +**Architecture:** A new pure `domain.NormalizeBranchPrefix` sits beside `domain.ValidBranchComponent` (the claim-time validator it feeds). The domain model gains a tri-state `OptionalBool` `AutoGroomable` on `ChangeSpec`, decoded by the repository decoder and seeded by the change builder. `render.ChangeRecord` emits both scalars when set. `internal/app/change_create.go` validates, normalizes, digests (normalized, `omitempty` so pre-0382 digests are unchanged), and renders them. `internal/app/change_groom.go` adds two outcomes inside the existing needs-brainstorm gate, reusing `render.ApplySectionEdits`, the existing `namedSectionBody` / `namedSectionPresent` helpers (`internal/app/finalize_block.go`), `upsertField`, and the existing inline-board re-render — so every abstain/re-arm lands the record and `BOARD.md` in one CAS-pinned commit. Skills and the convention are then rewritten to call the typed paths. + +**Tech Stack:** Go (`internal/domain`, `internal/repository`, `internal/render`, `internal/app`, `internal/cli`, `internal/repoguard`); table-driven tests; Markdown skill bodies under `skills/` with the embedded bundle regenerated by `go generate ./internal/assets/`. + +**Spec:** `docs/superpowers/specs/2026-09-27-changecreaterequest-typed-auto-groomable-branch-prefix-scalars-design.md` (on the `docket` metadata branch; readable at `.docket/docs/superpowers/specs/…` from the primary checkout). + +## Global Constraints + +- `domain.NormalizeBranchPrefix(raw string) (normalized string, ok bool)`, in order: trim surrounding whitespace; strip exactly one trailing `/`; lowercase; empty ⇒ `("", true)` (unset); otherwise require `ValidBranchComponent`. Never rewrite `refs/heads/` to ``. +- Claim-time validation in `domain.Claim` is unchanged and stays strict. Existing records are never normalized. +- `ChangeCreateRequest` gains `AutoGroomable *bool` (`json:"auto_groomable"`; nil ⇒ unset/inherit) and `BranchPrefix string` (`json:"branch_prefix"`; empty after normalization ⇒ unset). The **normalized** prefix is what is digested and stored. +- New registered finding codes (const + `AllFindingCodes`, sorted): `invalid-branch_prefix`, `empty-blocked_note`, `invalid-blocked_note`, `invalid-sections`, `nothing-to-rearm`. All live in `internal/app/finding_codes.go`, the only sanctioned literal site (`TestNoInlineFindingCodeLiterals`). +- `change.groom` outcomes become `spec | trivial | revise | abstain | rearm`; the `groom_outcomes` vocabulary (`internal/app/schema_vocab.go`), the invalid-outcome message, `HumanText`, and the CLI help line all name the two new outcomes. +- `abstain` and `rearm` share the existing groom gate (`proposed`, no `spec:`, not `trivial`) and its existing `not-groomable` refusal; they never touch `spec:` or `trivial:`. +- `abstain`: requires `blocked_note`; refuses `sections`, `spec_markdown`, `spec_version`, and every relationship field; entry format is `Recorded (UTC).`, blank line, the note; a second abstain appends a new entry after the earlier ones. +- `rearm`: sets `auto_groomable: true`, removes `## Auto-groom blocked` if present, applies optional owned-section edits; refuses `blocked_note`, `spec_markdown`, `spec_version`; refuses `nothing-to-rearm` when there is no blocked section AND `auto_groomable` is already `true`. +- A spec or trivial groom of an abstained stub is unchanged (the section rides along; every reader of it is scoped to needs-brainstorm rows, so the presence-encoded marker is discharged, not stale). +- Out of scope (do not build): any change to what the scalars *mean*, moving auto-groom selection/eligibility into Go, normalizing existing records, other `change.create` fields. +- Cross-references in maintained source anchor on symbol names or quoted clauses, never line numbers (AGENTS.md). +- Skill edits are made in `skills/…` only, then `go generate ./internal/assets/` regenerates `internal/assets/embedded/` in the same task (`TestEmbeddedMatchesAuthored` is the drift guard). +- Per-task test runs below are focused checks, always `-count=1` (a cached PASS proves nothing about a mutated tree). The build gate runs the whole suite via the resolved `build.test_command`. + +## Review Focus + +1. **A pre-0382 `change.create` request re-run under its original `request_id`** — a caller expects a replay, not an idempotency conflict. Adding the two fields to the digest payload without `omitempty` would change every legacy digest. → Task 4 pins that the marshalled payload carries neither key when both are unset. +2. **A messily typed prefix (`" Hotfix/ "`, `HOTFIX/`)** — a human expects it stored as `hotfix`, and a retry typed as `hotfix` to replay rather than conflict. → Task 1 (table + fuzz idempotence/claim-validity property) and Task 4 (real-git create-then-replay). +3. **A `blocked_note` carrying a column-zero `## ` heading or an unterminated code fence** — an autonomous author could easily write `## Missing context`; embedded as-is it would split the section, and a smuggled owned heading (`## Why`) would make every later section edit refuse on a duplicate. Expected: refused `invalid-blocked_note` before any engine call. → Task 5. +4. **A record with no `auto_groomable:` key at all** (hand-authored or pre-template) — abstain/re-arm must insert the field rather than internal-erroring on a missing patch target. → Tasks 5 and 6 (`groomableChange` has no such key). +5. **A re-arm request whose `sections` edits `## Auto-groom blocked`** — that collides with the removal the op performs and would otherwise surface as an opaque `section-edit-failed` "duplicate edit". Expected: refused at shape with `invalid-section-heading`. → Task 6. + +(Also pinned in Task 2, below the top five: a stored non-boolean `auto_groomable: yes` decodes as malformed with a `field-malformed` finding — never silently read as `true`.) + +--- + +### Task 1: `domain.NormalizeBranchPrefix` + +**Files:** +- Modify: `internal/domain/types.go` (add `NormalizeBranchPrefix` directly after `ValidBranchComponent`) +- Test: `internal/domain/types_test.go` + +**Interfaces:** +- Consumes: `domain.ValidBranchComponent(s string) bool`. +- Produces: `func NormalizeBranchPrefix(raw string) (string, bool)` — used by Task 4. + +- [ ] **Step 1: Write the failing tests** + +Change the import block of `internal/domain/types_test.go` from `import "testing"` to: + +```go +import ( + "strings" + "testing" +) +``` + +Append: + +```go +func TestNormalizeBranchPrefix(t *testing.T) { + cases := []struct { + raw string + want string + ok bool + }{ + {"hotfix", "hotfix", true}, + {" hotfix ", "hotfix", true}, + {"hotfix/", "hotfix", true}, + {"Hotfix", "hotfix", true}, + {"HOTFIX/", "hotfix", true}, + {" Hotfix/ ", "hotfix", true}, + {"hotfix//", "", false}, + {"/hotfix", "", false}, + {"team/hotfix", "", false}, + {"refs", "", false}, + {"REFS", "", false}, + {"refs/heads/x", "", false}, + {"-x", "", false}, + {"x.lock", "", false}, + {"hot fix", "", false}, + {"", "", true}, + {" ", "", true}, + {"/", "", true}, + } + for _, c := range cases { + got, ok := NormalizeBranchPrefix(c.raw) + if got != c.want || ok != c.ok { + t.Errorf("NormalizeBranchPrefix(%q) = (%q, %v), want (%q, %v)", c.raw, got, ok, c.want, c.ok) + } + } +} + +// FuzzNormalizeBranchPrefix ties the normalizer to the reader it feeds: every +// accepted non-empty output must pass claim-time ValidBranchComponent, be +// lowercase, and be a fixed point — so a stored prefix can never fail at claim +// and a retry of the stored value replays. +func FuzzNormalizeBranchPrefix(f *testing.F) { + for _, s := range []string{"hotfix", " Hotfix/ ", "a/b", "refs", "", "/", "x.lock", "HOTFIX//"} { + f.Add(s) + } + f.Fuzz(func(t *testing.T, raw string) { + got, ok := NormalizeBranchPrefix(raw) + if !ok { + if got != "" { + t.Fatalf("refused %q but returned %q", raw, got) + } + return + } + if got == "" { + return + } + if !ValidBranchComponent(got) { + t.Fatalf("normalized %q -> %q fails claim-time ValidBranchComponent", raw, got) + } + if got != strings.ToLower(got) { + t.Fatalf("normalized %q -> %q is not lowercase", raw, got) + } + if again, ok2 := NormalizeBranchPrefix(got); !ok2 || again != got { + t.Fatalf("not idempotent: %q -> %q -> (%q, %v)", raw, got, again, ok2) + } + }) +} +``` + +- [ ] **Step 2: Run to verify failure** + +Run: `go test ./internal/domain/ -run 'TestNormalizeBranchPrefix|FuzzNormalizeBranchPrefix' -count=1` +Expected: FAIL — `undefined: NormalizeBranchPrefix`. + +- [ ] **Step 3: Implement** + +In `internal/domain/types.go`, after `ValidBranchComponent`: + +```go +// NormalizeBranchPrefix canonicalizes an authored branch_prefix before it is +// stored: it trims surrounding whitespace, strips exactly one trailing "/" +// (a presentation-only spelling), and lowercases the result — branch prefixes +// are lowercase-only, like the change-type token grammar. An empty result means +// unset ("", true). Anything else must satisfy ValidBranchComponent, the same +// rule domain.Claim applies at mint time, so a stored prefix can never fail at +// claim. A slash-embedded or refs/-qualified value is refused, never rewritten. +func NormalizeBranchPrefix(raw string) (string, bool) { + s := strings.TrimSpace(raw) + s = strings.TrimSuffix(s, "/") + s = strings.ToLower(s) + if s == "" { + return "", true + } + if !ValidBranchComponent(s) { + return "", false + } + return s, true +} +``` + +- [ ] **Step 4: Run to verify pass** + +Run: `go test ./internal/domain/ -run 'TestNormalizeBranchPrefix|FuzzNormalizeBranchPrefix|TestValidBranchComponent|TestMintBranch' -count=1` +Expected: PASS. Mutation spot-check: delete the `strings.TrimSpace` line and confirm the `" hotfix "` row reddens; restore. + +- [ ] **Step 5: Commit** + +```bash +git add internal/domain/types.go internal/domain/types_test.go +git commit -m "feat(domain): NormalizeBranchPrefix owns branch-prefix normalization (change 0382)" +``` + +--- + +### Task 2: Tri-state `auto_groomable` in the domain model and decoder + +**Files:** +- Modify: `internal/domain/entities.go` (new `OptionalBool` type beside `OptionalInt`; `ChangeSpec.AutoGroomable`; accessor) +- Modify: `internal/domain/actions.go` (`newChangeBuilder` seeds `AutoGroomable`) +- Modify: `internal/repository/decode.go` (`changeWire.AutoGroomable`; new `(*decoder).optionalBool`; `decodeChange` sets it) +- Test: `internal/repository/decode_test.go`, `internal/domain/lease_test.go` + +**Interfaces:** +- Produces: `type OptionalBool struct { State FieldState; Value bool; Raw string }`, `ChangeSpec.AutoGroomable OptionalBool`, `func (c Change) AutoGroomable() OptionalBool` — used by Tasks 4 and 6. + +- [ ] **Step 1: Write the failing tests** + +Append to `internal/repository/decode_test.go`: + +```go +// TestDecodeChangeAutoGroomable — auto_groomable decodes as a tri-state: absent, +// valueless (inherit), or an explicit true/false override. A non-boolean value +// is malformed with a finding, never a silent true or false. +func TestDecodeChangeAutoGroomable(t *testing.T) { + cases := []struct { + name string + line string // frontmatter line; "" = key absent + want domain.OptionalBool + malformed bool + }{ + {"absent", "", domain.OptionalBool{}, false}, + {"valueless", "auto_groomable:\n", domain.OptionalBool{State: domain.FieldEmpty}, false}, + {"true", "auto_groomable: true\n", domain.OptionalBool{State: domain.FieldPresent, Value: true, Raw: "true"}, false}, + {"false", "auto_groomable: false\n", domain.OptionalBool{State: domain.FieldPresent, Value: false, Raw: "false"}, false}, + {"non-boolean", "auto_groomable: yes\n", domain.OptionalBool{State: domain.FieldMalformed, Raw: "yes"}, true}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + src := "---\nid: 1\nslug: s\n" + c.line + "---\n\nbody\n" + change, findings := decodeChange(input(t, KindChange, LocationActive, "docs/changes/active/0001-s.md", src)) + if got := change.AutoGroomable(); got != c.want { + t.Errorf("AutoGroomable = %+v, want %+v", got, c.want) + } + if got := hasFinding(findings, CodeFieldMalformed, "auto_groomable"); got != c.malformed { + t.Errorf("field-malformed(auto_groomable) = %v, want %v; findings %v", got, c.malformed, findingCodes(findings)) + } + }) + } +} +``` + +Append to `internal/domain/lease_test.go`: + +```go +func TestReclaimPreservesAutoGroomable(t *testing.T) { + // The change builder seeds every durable input, auto_groomable included: a + // domain transition must never drop the human's override. + want := OptionalBool{State: FieldPresent, Value: true, Raw: "true"} + c := NewChange(ChangeSpec{ + ID: 7, + Slug: "lease-slug", + Type: "fix", + Status: StatusInProgress, + RawStatus: string(StatusInProgress), + ClaimedAt: leaseStamp(-10 * time.Hour), + AutoGroomable: want, + }) + got, fail := Reclaim(c, leaseNow, leaseTTL, leaseBranches()) + if fail != nil { + t.Fatalf("Reclaim failed: %v", fail) + } + if ag := got.Change.AutoGroomable(); ag != want { + t.Errorf("auto_groomable = %+v, want preserved %+v", ag, want) + } +} +``` + +- [ ] **Step 2: Run to verify failure** + +Run: `go test ./internal/repository/ ./internal/domain/ -run 'TestDecodeChangeAutoGroomable|TestReclaimPreservesAutoGroomable' -count=1` +Expected: FAIL — `undefined: domain.OptionalBool` / `change.AutoGroomable undefined`. + +- [ ] **Step 3: Implement** + +`internal/domain/entities.go`, after `OptionalInt`: + +```go +// OptionalBool is an optional tri-state boolean field: absent or valueless +// (inherit a default) stays distinguishable from an explicit false. Raw carries +// the stored text whenever State != FieldAbsent, so a malformed value stays +// reportable; Value is meaningful only when State is FieldPresent. +type OptionalBool struct { + State FieldState + Value bool + Raw string +} +``` + +In `ChangeSpec`, directly after `Trivial bool`: + +```go + AutoGroomable OptionalBool // per-change auto-groom override; unset ⇒ inherit auto_groom +``` + +Accessor, next to `Trivial()`: + +```go +// AutoGroomable returns the optional per-change auto-groom override. Unset +// (absent or valueless) means the repository's auto_groom knob applies. +func (c Change) AutoGroomable() OptionalBool { return c.spec.AutoGroomable } +``` + +`internal/domain/actions.go`, in `newChangeBuilder`'s `ChangeSpec` literal, after `Trivial: c.Trivial(),`: + +```go + AutoGroomable: c.AutoGroomable(), +``` + +`internal/repository/decode.go` — in `changeWire`, after `Trivial`: + +```go + AutoGroomable scalar `yaml:"auto_groomable"` +``` + +After `(*decoder).boolean`: + +```go +// optionalBool converts a scalar into a tri-state boolean: absent and valueless +// stay distinguishable from an explicit false, and a value that is not a YAML +// boolean is a finding, never a silent true or false. +func (d *decoder) optionalBool(name string, s scalar) domain.OptionalBool { + switch d.state(name, s) { + case domain.FieldAbsent: + return domain.OptionalBool{} + case domain.FieldEmpty: + return domain.OptionalBool{State: domain.FieldEmpty} + case domain.FieldMalformed: + d.malformed(name, s.raw) + return domain.OptionalBool{State: domain.FieldMalformed, Raw: s.raw} + } + switch s.raw { + case "true": + return domain.OptionalBool{State: domain.FieldPresent, Value: true, Raw: s.raw} + case "false": + return domain.OptionalBool{State: domain.FieldPresent, Value: false, Raw: s.raw} + } + d.malformed(name, s.raw) + return domain.OptionalBool{State: domain.FieldMalformed, Raw: s.raw} +} +``` + +In `decodeChange`, after `spec.Trivial = d.boolean("trivial", wire.Trivial)`: + +```go + spec.AutoGroomable = d.optionalBool("auto_groomable", wire.AutoGroomable) +``` + +- [ ] **Step 4: Run to verify pass** + +Run: `go test ./internal/repository/ ./internal/domain/ ./internal/render/ -count=1` +Expected: PASS (the corpus fixtures carry only valueless/`true`/`false` spellings, so no new findings appear). Mutation spot-check: delete the `AutoGroomable: c.AutoGroomable(),` builder line and confirm `TestReclaimPreservesAutoGroomable` reddens; restore. + +- [ ] **Step 5: Commit** + +```bash +git add internal/domain/entities.go internal/domain/actions.go internal/domain/lease_test.go internal/repository/decode.go internal/repository/decode_test.go +git commit -m "feat(domain): model auto_groomable as a decoded tri-state (change 0382)" +``` + +--- + +### Task 3: `render.ChangeRecord` emits the draft-time scalars + +**Files:** +- Modify: `internal/render/record.go` (`NewChangeRecord` fields; `ChangeRecord` field specs) +- Test: `internal/render/record_test.go` + +**Interfaces:** +- Produces: `NewChangeRecord.AutoGroomable *bool`, `NewChangeRecord.BranchPrefix string` (already-normalized; empty ⇒ null) — used by Task 4. + +- [ ] **Step 1: Write the failing test** + +Append to `internal/render/record_test.go`: + +```go +// TestChangeRecordDraftScalars pins the two create-time scalars: unset renders +// the canonical null (the golden's shape), a set value renders a boolean or a +// single-quoted string by construction. +func TestChangeRecordDraftScalars(t *testing.T) { + yes, no := true, false + cases := []struct { + name string + auto *bool + prefix string + wantAuto string + wantPrefix string + }{ + {"unset", nil, "", "\nauto_groomable:\n", "\nbranch_prefix:\n"}, + {"true with prefix", &yes, "hotfix", "\nauto_groomable: true\n", "\nbranch_prefix: 'hotfix'\n"}, + {"explicit false", &no, "", "\nauto_groomable: false\n", "\nbranch_prefix:\n"}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + got, err := render.ChangeRecord(render.NewChangeRecord{ + ID: 1, Slug: "s", Title: "T", Type: "feat", Priority: "medium", Created: goldenDate, + AutoGroomable: c.auto, BranchPrefix: c.prefix, + Why: "w", WhatChanges: "c", OutOfScope: "o", + }) + if err != nil { + t.Fatalf("ChangeRecord: %v", err) + } + s := string(got) + if !strings.Contains(s, c.wantAuto) || !strings.Contains(s, c.wantPrefix) { + t.Errorf("record missing %q / %q:\n%s", c.wantAuto, c.wantPrefix, s) + } + if _, err := document.Parse(got); err != nil { + t.Errorf("record does not reparse: %v", err) + } + }) + } +} +``` + +Add `"strings"` to the file's import block if absent. + +- [ ] **Step 2: Run to verify failure** + +Run: `go test ./internal/render/ -run TestChangeRecordDraftScalars -count=1` +Expected: FAIL — unknown fields `AutoGroomable` / `BranchPrefix`. + +- [ ] **Step 3: Implement** + +In `NewChangeRecord`, after `ADRs`: + +```go + AutoGroomable *bool // nil ⇒ null (inherit the repo's auto_groom) + BranchPrefix string // already normalized by the app layer; "" ⇒ null +``` + +In `ChangeRecord`, replace the two null field specs: + +```go + {Name: "auto_groomable", Value: document.Null()}, + {Name: "branch_prefix", Value: document.Null()}, +``` + +with + +```go + {Name: "auto_groomable", Value: optionalBoolValue(r.AutoGroomable)}, + {Name: "branch_prefix", Value: optionalStringValue(r.BranchPrefix)}, +``` + +and add below `ChangeRecord`: + +```go +// optionalBoolValue renders a tri-state create-time scalar: nil is the +// canonical null, anything else an explicit boolean. +func optionalBoolValue(v *bool) document.Value { + if v == nil { + return document.Null() + } + return document.Bool(*v) +} + +// optionalStringValue renders an optional create-time text scalar: empty is the +// canonical null, anything else a quoted-by-construction string (ADR-0071). +func optionalStringValue(s string) document.Value { + if s == "" { + return document.Null() + } + return document.String(s) +} +``` + +- [ ] **Step 4: Run to verify pass** + +Run: `go test ./internal/render/ -count=1` +Expected: PASS, including the unchanged `TestChangeRecordMatchesGolden`. + +- [ ] **Step 5: Commit** + +```bash +git add internal/render/record.go internal/render/record_test.go +git commit -m "feat(render): change records carry auto_groomable and branch_prefix when set (change 0382)" +``` + +--- + +### Task 4: `change.create` accepts, validates, normalizes, digests, and stores both scalars + +**Files:** +- Modify: `internal/app/change_create.go` (request fields; payload fields; `normalizedBranchPrefix`; shape check; `Plan`) +- Modify: `internal/app/finding_codes.go` (`FCInvalidBranchPrefix` const + `AllFindingCodes`) +- Test: `internal/app/change_create_test.go`, `internal/app/schema_test.go` (`TestReflectDescriptorChangeCreateRequest`), `internal/app/finding_codes_test.go` (`TestShapeValidatorCodesAreRegistered`) + +**Interfaces:** +- Consumes: `domain.NormalizeBranchPrefix` (Task 1); `domain.Change.AutoGroomable()` (Task 2, test round trip); `render.NewChangeRecord.AutoGroomable/BranchPrefix` (Task 3). +- Produces: `ChangeCreateRequest.AutoGroomable *bool` (`auto_groomable`), `ChangeCreateRequest.BranchPrefix string` (`branch_prefix`), `FCInvalidBranchPrefix FindingCode = "invalid-branch_prefix"`, `normalizedBranchPrefix(req ChangeCreateRequest) string`. + +- [ ] **Step 1: Write the failing tests** + +Append to `internal/app/change_create_test.go`: + +```go +func TestChangeCreateRejectsInvalidBranchPrefixWithoutEngineCall(t *testing.T) { + for _, raw := range []string{"team/hotfix", "refs/heads/x", "hotfix//", "-x", "x.lock", "hot fix"} { + t.Run(raw, func(t *testing.T) { + req := validChangeCreateRequest() + req.BranchPrefix = raw + engine := &recordingEngine{} + reader := &fakeChangeReader{pin: mainModePin([]string{"inline"})} + deps := PlanningDeps{Engine: engine, Reader: reader, Clock: testClock()} + + res := ChangeCreate(context.Background(), deps, "", req) + + if res.Result != ResultInvalidInput { + t.Fatalf("result = %q, want invalid-input", res.Result) + } + if len(engine.calls) != 0 { + t.Errorf("engine called %d times on an invalid prefix, want 0", len(engine.calls)) + } + var msg string + for _, f := range res.Findings { + if f.Code == "invalid-branch_prefix" { + msg = f.Message + } + } + if msg == "" { + t.Fatalf("missing invalid-branch_prefix; got %v", res.Findings) + } + if !strings.Contains(msg, fmt.Sprintf("%q", raw)) { + t.Errorf("message %q does not name the rejected value %q", msg, raw) + } + }) + } +} + +func TestChangeCreatePlanWritesDraftScalars(t *testing.T) { + yes, no := true, false + recPath := "docs/changes/active/0002-add-a-widget.md" + cases := []struct { + name string + auto *bool + prefix string + wantAuto string + wantPrefix string + wantAG domain.OptionalBool + wantBranch string + }{ + {"absent", nil, "", "\nauto_groomable:\n", "\nbranch_prefix:\n", + domain.OptionalBool{State: domain.FieldEmpty}, "feat/add-a-widget"}, + {"true with messy prefix", &yes, " Hotfix/ ", "\nauto_groomable: true\n", "\nbranch_prefix: 'hotfix'\n", + domain.OptionalBool{State: domain.FieldPresent, Value: true, Raw: "true"}, "hotfix/add-a-widget"}, + {"explicit false", &no, "", "\nauto_groomable: false\n", "\nbranch_prefix:\n", + domain.OptionalBool{State: domain.FieldPresent, Value: false, Raw: "false"}, "feat/add-a-widget"}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + op := baseOp([]string{}) + op.req.AutoGroomable = c.auto + op.req.BranchPrefix = c.prefix + plan, opRes := planFor(t, map[string]string{ + "docs/changes/active/0001-first.md": fixtureChange(1, "first"), + }, op) + if opRes.Refused { + t.Fatalf("unexpected refusal: %v", opRes.Findings) + } + rec := groomedRecordBytes(t, plan, recPath) + if s := string(rec); !strings.Contains(s, c.wantAuto) || !strings.Contains(s, c.wantPrefix) { + t.Errorf("record missing %q / %q:\n%s", c.wantAuto, c.wantPrefix, s) + } + // Round trip through the real decoder, then mint exactly as claim does. + snap, err := buildCandidateSnapshot(op.eff, nil, rec, recPath) + if err != nil { + t.Fatalf("buildCandidateSnapshot: %v", err) + } + ch, out := snap.Change(2) + if out != domain.LookupFound { + t.Fatalf("created record not decodable as change 2") + } + if got := ch.AutoGroomable(); got != c.wantAG { + t.Errorf("decoded AutoGroomable = %+v, want %+v", got, c.wantAG) + } + if got := domain.MintBranch(ch.Type(), ch.BranchPrefix(), ch.Slug()); got != c.wantBranch { + t.Errorf("minted branch = %q, want %q", got, c.wantBranch) + } + }) + } +} + +func TestChangeCreateDigestBindsNormalizedDraftScalars(t *testing.T) { + digest := func(mut func(*ChangeCreateRequest)) transaction.RequestDigest { + t.Helper() + req := validChangeCreateRequest() + mut(&req) + d, err := canonicalDigest(OperationChangeCreate, changeCreateSemanticPayload(req)) + if err != nil { + t.Fatalf("canonicalDigest: %v", err) + } + return d + } + yes, no := true, false + none := digest(func(*ChangeCreateRequest) {}) + if digest(func(r *ChangeCreateRequest) { r.BranchPrefix = "Hotfix/" }) != digest(func(r *ChangeCreateRequest) { r.BranchPrefix = "hotfix" }) { + t.Error("Hotfix/ and hotfix must digest identically (the normalized prefix is bound), so a retry replays") + } + if digest(func(r *ChangeCreateRequest) { r.BranchPrefix = "hotfix" }) == none { + t.Error("a set prefix must change the digest") + } + if digest(func(r *ChangeCreateRequest) { r.AutoGroomable = &yes }) == digest(func(r *ChangeCreateRequest) { r.AutoGroomable = &no }) { + t.Error("differing auto_groomable under one request_id must conflict, not replay") + } + if digest(func(r *ChangeCreateRequest) { r.AutoGroomable = &no }) == none { + t.Error("an explicit false must differ from unset (inherit)") + } +} + +// TestChangeCreatePayloadOmitsUnsetDraftScalars — Review Focus 1: a request +// carrying neither new field must digest exactly as it did before change 0382, +// so re-running a pre-0382 request under its request_id still replays. +func TestChangeCreatePayloadOmitsUnsetDraftScalars(t *testing.T) { + b, err := json.Marshal(changeCreateSemanticPayload(validChangeCreateRequest())) + if err != nil { + t.Fatalf("marshal: %v", err) + } + for _, k := range []string{`"auto_groomable"`, `"branch_prefix"`} { + if strings.Contains(string(b), k) { + t.Errorf("unset %s appears in the digest payload %s; it would change every pre-0382 digest", k, b) + } + } +} + +// TestChangeCreateNormalizedPrefixReplaysRealGit drives the real engine: a +// create with a messy prefix stores the normalized value, the same request_id +// retyped as the normalized spelling replays, and a differing auto_groomable +// under that id does not apply. +func TestChangeCreateNormalizedPrefixReplaysRealGit(t *testing.T) { + requireRealGit(t) + repo := newWorkingRepo(t, map[string]string{ + "docs/changes/active/0001-first.md": fixtureChange(1, "first"), + }) + node := planningDepsFor(t, repo.invocation) + yes, no := true, false + + req := validChangeCreateRequest() + req.BranchPrefix, req.AutoGroomable = "Hotfix/", &yes + first := ChangeCreate(context.Background(), node.deps, node.dir, req) + if first.Result != ResultApplied || first.Replayed { + t.Fatalf("first create = %q replayed=%v (findings %v), want a fresh apply", first.Result, first.Replayed, first.Findings) + } + body, ok := originFile(t, repo.origin, "docket", first.Path) + if !ok || !strings.Contains(body, "\nbranch_prefix: 'hotfix'\n") || !strings.Contains(body, "\nauto_groomable: true\n") { + t.Fatalf("created record lacks the normalized scalars:\n%s", body) + } + tip := originTip(t, repo.origin, "docket") + + req.BranchPrefix = "hotfix" + second := ChangeCreate(context.Background(), node.deps, node.dir, req) + if second.Result != ResultApplied || !second.Replayed || second.ID != first.ID { + t.Fatalf("retyped create = %q replayed=%v id=%d (findings %v), want a replay of %d", second.Result, second.Replayed, second.ID, second.Findings, first.ID) + } + + req.AutoGroomable = &no + third := ChangeCreate(context.Background(), node.deps, node.dir, req) + if third.Result == ResultApplied { + t.Fatalf("a differing auto_groomable under the same request_id applied (replayed=%v)", third.Replayed) + } + if got := originTip(t, repo.origin, "docket"); got != tip { + t.Errorf("replay/conflict moved the metadata branch %s -> %s", tip, got) + } +} +``` + +In `internal/app/schema_test.go` `TestReflectDescriptorChangeCreateRequest`, change `want` to: + +```go + want := []string{ + "request_id", "title", "type", "priority", + "why", "what_changes", "out_of_scope", + "depends_on", "stacked_on", "related", "discovered_from", "adrs", + "auto_groomable", "branch_prefix", + } +``` + +and after the `stacked_on` check add: + +```go + if f := fieldByKey(t, d, "auto_groomable"); f.Repeated || f.Type != "bool" { + t.Errorf("auto_groomable = %+v, want optional scalar bool", f) + } + if f := fieldByKey(t, d, "branch_prefix"); f.Repeated || f.Type != "string" { + t.Errorf("branch_prefix = %+v, want optional string", f) + } +``` + +(the existing `wantRequired` loop already asserts both are not required). + +In `internal/app/finding_codes_test.go` `TestShapeValidatorCodesAreRegistered`, after the `validateChangeCreateShape(ChangeCreateRequest{StackedOn: &zero})` line add: + +```go + emitted = append(emitted, validateChangeCreateShape(ChangeCreateRequest{BranchPrefix: "a/b"})...) +``` + +and add `FCInvalidBranchPrefix,` to the `floor` list after `FCInvalidStackedOn,`. + +- [ ] **Step 2: Run to verify failure** + +Run: `go test ./internal/app/ -run 'TestChangeCreate|TestReflectDescriptorChangeCreateRequest|TestShapeValidatorCodesAreRegistered' -count=1` +Expected: FAIL to compile — unknown fields `AutoGroomable` / `BranchPrefix`, undefined `FCInvalidBranchPrefix`. + +- [ ] **Step 3: Implement** + +`internal/app/finding_codes.go`: in the "change create request-shape findings" const group add + +```go + FCInvalidBranchPrefix FindingCode = "invalid-branch_prefix" +``` + +and insert `FCInvalidBranchPrefix,` into `AllFindingCodes` in sorted position (immediately before `FCInvalidChangeDotID`, i.e. after `FCInvalidAttempt`; `TestFindingCodeRegistryIntegrity` enforces the order). + +`internal/app/change_create.go`: + +1. `ChangeCreateRequest`, after `ADRs`: + +```go + // AutoGroomable is the optional per-change auto-groom override: nil leaves + // the record unset (inherit the repo's auto_groom); true/false are explicit. + AutoGroomable *bool `json:"auto_groomable"` + // BranchPrefix is the optional mint-prefix override, as the human typed it. + // It is normalized by domain.NormalizeBranchPrefix before it is validated, + // digested, or stored; empty after normalization means unset. + BranchPrefix string `json:"branch_prefix"` +``` + +2. `changeCreatePayload`, after `ADRs` — `omitempty` keeps every pre-0382 digest unchanged: + +```go + AutoGroomable *bool `json:"auto_groomable,omitempty"` + BranchPrefix string `json:"branch_prefix,omitempty"` +``` + +3. `changeCreateSemanticPayload`, add to the literal: + +```go + AutoGroomable: req.AutoGroomable, + BranchPrefix: normalizedBranchPrefix(req), +``` + +and add below it: + +```go +// normalizedBranchPrefix is the stored, digested spelling of the request's +// branch_prefix. The request was shape-validated first, so a refused value never +// reaches the digest or Plan; the discarded ok is re-checked there. +func normalizedBranchPrefix(req ChangeCreateRequest) string { + p, _ := domain.NormalizeBranchPrefix(req.BranchPrefix) + return p +} +``` + +4. `validateChangeCreateShape`, after the `StackedOn` check: + +```go + if _, ok := domain.NormalizeBranchPrefix(req.BranchPrefix); !ok { + addShape(FCInvalidBranchPrefix, fmt.Sprintf( + "branch_prefix %q is not a usable branch prefix: after trimming whitespace, one trailing \"/\", and lowercasing, it must be a single git ref component — no \"/\" (never refs/-qualified), not \"refs\", no \"..\", \"@{\", whitespace, or any of ~^:?*[\\, not starting with \"-\" or \".\", not ending with \".\" or \".lock\"", + req.BranchPrefix)) + } +``` + +5. `changeCreateOp.Plan`, in the `render.NewChangeRecord` literal after `ADRs`: + +```go + AutoGroomable: o.req.AutoGroomable, + BranchPrefix: normalizedBranchPrefix(o.req), +``` + +- [ ] **Step 4: Run to verify pass** + +Run: `go test ./internal/app/ -run 'TestChangeCreate|TestReflectDescriptor|TestShapeValidatorCodesAreRegistered|TestFindingCodeRegistryIntegrity|TestNoInlineFindingCodeLiterals|TestSchema|TestRequired' -count=1` +Expected: PASS. Mutation spot-checks (restore after each): drop `,omitempty` from the payload's `branch_prefix` tag → `TestChangeCreatePayloadOmitsUnsetDraftScalars` reddens; change `normalizedBranchPrefix(req)` to `req.BranchPrefix` in the payload → the `Hotfix/`-vs-`hotfix` digest assert reddens. + +- [ ] **Step 5: Commit** + +```bash +git add internal/app/change_create.go internal/app/change_create_test.go internal/app/finding_codes.go internal/app/finding_codes_test.go internal/app/schema_test.go +git commit -m "feat(app): change.create accepts typed auto_groomable and normalized branch_prefix (change 0382)" +``` + +--- + +### Task 5: `change.groom` `outcome: abstain` + +**Files:** +- Modify: `internal/app/change_groom.go` (outcome const, `autoGroomBlockedHeading`, `BlockedNote` field, shape rules, `blockedNoteBodyDiagnostic`, `autoGroomBlockedMarkdown`, Plan branch, `HumanText`, file header comment) +- Modify: `internal/app/finding_codes.go` (`FCEmptyBlockedNote`, `FCInvalidBlockedNote`, `FCInvalidSections`) +- Modify: `internal/app/schema_vocab.go` (`groom_outcomes` gains `GroomAbstain`) +- Test: `internal/app/change_groom_test.go`, `internal/app/schema_vocab_test.go`, `internal/app/finding_codes_test.go`, `internal/app/schema_test.go` + +**Interfaces:** +- Consumes: existing `namedSectionBody(src []byte, heading string) (string, bool, error)` (`internal/app/finalize_block.go`), `upsertField`, `boundAuthored`, `render.ValidateSectionBody`, `render.ErrSectionBodyUnterminatedFence`. +- Produces: `GroomAbstain GroomOutcome = "abstain"`, `const autoGroomBlockedHeading = "## Auto-groom blocked"`, `ChangeGroomRequest.BlockedNote string` (`blocked_note`), `FCEmptyBlockedNote`/`FCInvalidBlockedNote`/`FCInvalidSections`; test helpers `abstainRequest()` and `abstainedChange(id int, slug string) string` — Task 6 reuses them. + +- [ ] **Step 1: Write the failing tests** + +Append to `internal/app/change_groom_test.go`: + +```go +// abstainRequest is a well-formed abstain request against the groomable fixture +// at id 2 / slug add-a-widget. The note uses a ### subsection, which is legal. +func abstainRequest() ChangeGroomRequest { + return ChangeGroomRequest{ + ChangeID: 2, + Path: groomPath(2, "add-a-widget"), + Version: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + Outcome: GroomAbstain, + BlockedNote: "The storage decision needs a human.\n\n### What to supply\n\nPick the backend.\n", + } +} + +// abstainedChange is the groomable fixture after one earlier abstain: an +// explicit auto_groomable: false and a one-entry ## Auto-groom blocked section. +func abstainedChange(id int, slug string) string { + return strings.Replace(groomableChange(id, slug), "trivial: false\n", "trivial: false\nauto_groomable: false\n", 1) + + "\n## Auto-groom blocked\n\nRecorded 2026-08-01 (UTC).\n\nFirst note.\n" +} + +func TestChangeGroomAbstainShapeValidation(t *testing.T) { + one := 1 + cases := []struct { + name string + mut func(*ChangeGroomRequest) + code string // "" means the request must pass shape validation + }{ + {"valid abstain passes", func(r *ChangeGroomRequest) {}, ""}, + {"blank blocked_note", func(r *ChangeGroomRequest) { r.BlockedNote = " \n" }, "empty-blocked_note"}, + {"blocked_note smuggling a structural heading", func(r *ChangeGroomRequest) { + r.BlockedNote = "Context.\n\n## Why\n\nsmuggled\n" + }, "invalid-blocked_note"}, + {"blocked_note with an unterminated fence", func(r *ChangeGroomRequest) { + r.BlockedNote = "Context.\n\n```\nnever closed\n" + }, "invalid-blocked_note"}, + {"abstain with sections", func(r *ChangeGroomRequest) { + r.Sections = []SectionEditRequest{{Heading: "## Why", Intent: "replace", Markdown: "rewrite\n"}} + }, "invalid-sections"}, + {"abstain with spec_markdown", func(r *ChangeGroomRequest) { r.SpecMarkdown = "# Design\n" }, "invalid-spec_markdown"}, + {"abstain with spec_version", func(r *ChangeGroomRequest) { r.SpecVersion = "a" }, "invalid-spec_version"}, + {"abstain with depends_on", func(r *ChangeGroomRequest) { r.DependsOn = []int{1} }, "invalid-depends_on"}, + {"abstain with an explicit empty related", func(r *ChangeGroomRequest) { r.Related = []int{} }, "invalid-related"}, + {"abstain with discovered_from", func(r *ChangeGroomRequest) { r.DiscoveredFrom = []int{1} }, "invalid-discovered_from"}, + {"abstain with adrs", func(r *ChangeGroomRequest) { r.ADRs = []int{1} }, "invalid-adrs"}, + {"abstain with stacked_on", func(r *ChangeGroomRequest) { r.StackedOn = &one }, "invalid-stacked_on"}, + {"blocked_note on the spec outcome", func(r *ChangeGroomRequest) { + r.Outcome, r.SpecMarkdown = GroomSpec, "# Design\n" + }, "invalid-blocked_note"}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + req := abstainRequest() + c.mut(&req) + findings := validateChangeGroomShape(req) + if c.code == "" { + if len(findings) != 0 { + t.Fatalf("want no findings, got %v", findings) + } + return + } + if !hasFindingCode(findings, c.code) { + t.Errorf("missing finding %q; got %v", c.code, findings) + } + }) + } +} + +func TestChangeGroomAbstainBadNoteRefusedWithoutEngineCall(t *testing.T) { + req := abstainRequest() + req.BlockedNote = "Context.\n\n## Why\n\nsmuggled\n" + engine := &recordingEngine{} + deps := PlanningDeps{Engine: engine, Reader: &fakeChangeReader{pin: mainModePin([]string{"inline"})}, Clock: testClock()} + + res := ChangeGroom(context.Background(), deps, "", req) + + if res.Result != ResultInvalidInput || len(engine.calls) != 0 { + t.Fatalf("result = %q with %d engine calls, want invalid-input and none", res.Result, len(engine.calls)) + } +} + +func TestChangeGroomPlanAbstainSetsFlagSectionAndBoard(t *testing.T) { + cases := []struct { + name string + rec string + }{ + // Review Focus 4: a record with no auto_groomable key gets it inserted. + {"field absent", groomableChange(2, "add-a-widget")}, + {"field armed true", strings.Replace(groomableChange(2, "add-a-widget"), "trivial: false\n", "trivial: false\nauto_groomable: true\n", 1)}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + files := map[string]string{ + groomPath(2, "add-a-widget"): c.rec, + "docs/changes/BOARD.md": "# Backlog\n\nold\n", + } + plan, opRes := groomPlanFor(t, files, baseGroomOp([]string{"inline"}, abstainRequest())) + if opRes.Refused { + t.Fatalf("unexpected refusal: %v", opRes.Findings) + } + assertPlanPaths(t, plan, map[string]transaction.MutationKind{ + groomPath(2, "add-a-widget"): transaction.MutationReplace, + "docs/changes/BOARD.md": transaction.MutationReplace, + }) + rec := string(groomedRecordBytes(t, plan, groomPath(2, "add-a-widget"))) + for _, want := range []string{ + "\nauto_groomable: false\n", + "updated: '2026-08-16'", + "## Auto-groom blocked\n\nRecorded 2026-08-16 (UTC).\n\nThe storage decision needs a human.", + "### What to supply", + "\nspec:\n", "trivial: false", // groom scalars untouched + "Original why.", "An open question.", // proposal untouched + } { + if !strings.Contains(rec, want) { + t.Errorf("record missing %q:\n%s", want, rec) + } + } + if strings.Contains(rec, "auto_groomable: true") { + t.Errorf("abstain left the stub armed:\n%s", rec) + } + board := string(groomedRecordBytes(t, plan, "docs/changes/BOARD.md")) + if !strings.Contains(board, "auto-groom blocked — needs you") { + t.Errorf("board row did not flip to auto-groom blocked in the same plan:\n%s", board) + } + var receipt changeGroomReceipt + if err := json.Unmarshal(plan.Receipt, &receipt); err != nil || receipt.Outcome != "abstain" || receipt.SpecPath != "" { + t.Errorf("receipt = %s (%v), want outcome abstain and no spec_path", plan.Receipt, err) + } + }) + } +} + +func TestChangeGroomPlanAbstainAppendsSecondEntry(t *testing.T) { + files := map[string]string{groomPath(2, "add-a-widget"): abstainedChange(2, "add-a-widget")} + plan, opRes := groomPlanFor(t, files, baseGroomOp([]string{}, abstainRequest())) + if opRes.Refused { + t.Fatalf("a second abstain must append, not refuse: %v", opRes.Findings) + } + rec := string(groomedRecordBytes(t, plan, groomPath(2, "add-a-widget"))) + if n := strings.Count(rec, "## Auto-groom blocked"); n != 1 { + t.Fatalf("section heading appears %d times, want exactly 1:\n%s", n, rec) + } + first := strings.Index(rec, "Recorded 2026-08-01 (UTC).\n\nFirst note.") + second := strings.Index(rec, "Recorded 2026-08-16 (UTC).\n\nThe storage decision needs a human.") + if first < 0 || second < 0 || first > second { + t.Errorf("entries missing or out of order (first=%d second=%d):\n%s", first, second, rec) + } +} + +func TestChangeGroomPlanAbstainRefusesNonGroomable(t *testing.T) { + cases := []struct { + name string + rec string + }{ + {"spec'd", revisableChange(2, "add-a-widget", reviseSpecPath)}, + {"trivial", trivialChange(2, "add-a-widget")}, + {"not proposed", strings.Replace(groomableChange(2, "add-a-widget"), "status: proposed\n", "status: blocked\nblocked_by: 'waiting on infra'\n", 1)}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + files := map[string]string{groomPath(2, "add-a-widget"): c.rec} + if c.name == "spec'd" { + files[reviseSpecPath] = reviseFixtureFiles()[reviseSpecPath] + } + plan, opRes := groomPlanFor(t, files, baseGroomOp([]string{}, abstainRequest())) + if !opRes.Refused || len(plan.Files) != 0 { + t.Fatalf("want a refusal writing nothing, got refused=%v files=%v", opRes.Refused, planPaths(plan)) + } + found := false + for _, f := range opRes.Findings { + found = found || f.Code == "not-groomable" + } + if !found { + t.Errorf("missing not-groomable; got %v", opRes.Findings) + } + }) + } +} + +func TestChangeGroomResultHumanTextAbstain(t *testing.T) { + r := newChangeGroomResult(ResultApplied, ChangeGroomResult{ID: 7, Outcome: string(GroomAbstain), Revision: "cafe"}) + if got, want := r.HumanText(), "change 0007 auto-groom abstained — cafe"; got != want { + t.Errorf("HumanText = %q, want %q", got, want) + } +} +``` + +In `internal/app/schema_vocab_test.go` `TestSchemaVocabulariesCore`, change the groom line to: + +```go + assertVocabMembers(t, v, "groom_outcomes", []string{"spec", "trivial", "revise", "abstain"}) +``` + +In `internal/app/schema_test.go` `TestReflectDescriptorChangeGroomRequest`, after the `spec_version` check add: + +```go + if f := fieldByKey(t, d, "blocked_note"); f.Required || f.Type != "string" { + t.Errorf("blocked_note = %+v, want optional string", f) + } +``` + +In `internal/app/finding_codes_test.go` `TestShapeValidatorCodesAreRegistered`, after the `GroomOutcome("bogus")` probe add: + +```go + emitted = append(emitted, validateChangeGroomShape(ChangeGroomRequest{Outcome: GroomAbstain, Sections: []SectionEditRequest{{Heading: "## Why", Intent: "remove"}}})...) + emitted = append(emitted, validateChangeGroomShape(ChangeGroomRequest{Outcome: GroomTrivial, BlockedNote: "x"})...) +``` + +and add `FCEmptyBlockedNote, FCInvalidBlockedNote, FCInvalidSections,` to `floor`. + +- [ ] **Step 2: Run to verify failure** + +Run: `go test ./internal/app/ -run 'TestChangeGroom|TestSchemaVocabulariesCore|TestReflectDescriptorChangeGroomRequest|TestShapeValidatorCodesAreRegistered' -count=1` +Expected: FAIL to compile — undefined `GroomAbstain`, unknown field `BlockedNote`, undefined finding constants. + +- [ ] **Step 3: Implement** + +`internal/app/finding_codes.go` — in the request-shape const group (after `FCEmptyRevise`) add: + +```go + FCEmptyBlockedNote FindingCode = "empty-blocked_note" + FCInvalidBlockedNote FindingCode = "invalid-blocked_note" + FCInvalidSections FindingCode = "invalid-sections" +``` + +and insert each into `AllFindingCodes` in sorted position: `FCEmptyBlockedNote` after `FCEmptyAttempt`; `FCInvalidBlockedNote` after `FCInvalidAttempt` (before `FCInvalidBranchPrefix`); `FCInvalidSections` after `FCInvalidSectionMarkdown` (before `FCInvalidSlug`). + +`internal/app/schema_vocab.go`: + +```go + v["groom_outcomes"] = Vocabulary{Members: []string{string(GroomSpec), string(GroomTrivial), string(GroomRevise), string(GroomAbstain)}} +``` + +`internal/app/change_groom.go`: + +1. Import `"errors"`. +2. Const block, after `GroomRevise`: + +```go + // GroomAbstain records an autonomous groom's abstain on a needs-brainstorm + // change: it sets auto_groomable: false and appends one dated entry to the + // ## Auto-groom blocked section. It never writes spec: or trivial:, and it + // accepts no section, spec, or relationship edits — an autonomous caller + // cannot rewrite the proposal through it. + GroomAbstain GroomOutcome = "abstain" +``` + +3. Below `specsDir`: + +```go +// autoGroomBlockedHeading is the presence-encoded abstain section: the abstain +// outcome appends to it, and the board's "auto-groom blocked — needs you" cell +// keys on its presence (domain.ReadyAutoGroomBlocked). +const autoGroomBlockedHeading = "## Auto-groom blocked" +``` + +4. `ChangeGroomRequest`, after `SpecVersion`: + +```go + // BlockedNote is the authored body of one ## Auto-groom blocked entry. The + // abstain outcome requires it and no other outcome accepts it; the operation + // owns the heading and the dated "Recorded (UTC)." lead line, so the + // note must not carry a column-zero "## " heading or an unterminated fence. + BlockedNote string `json:"blocked_note,omitempty"` +``` + +5. `HumanText`, inside `case ResultApplied:` before the revise check: + +```go + if r.Outcome == string(GroomAbstain) { + return fmt.Sprintf("change %04d auto-groom abstained — %s", r.ID, r.Revision) + } +``` + +6. `validateChangeGroomShape` — add a case before `default:` and update the default message: + +```go + case GroomAbstain: + if strings.TrimSpace(req.BlockedNote) == "" { + addShape(FCEmptyBlockedNote, "blocked_note must be non-empty for the abstain outcome") + } else if err := render.ValidateSectionBody([]byte(req.BlockedNote)); err != nil { + addShape(FCInvalidBlockedNote, blockedNoteBodyDiagnostic(err)) + } + if strings.TrimSpace(req.SpecMarkdown) != "" { + addShape(FCInvalidSpecMarkdown, "spec_markdown is not accepted by the abstain outcome") + } + if len(req.Sections) > 0 { + addShape(FCInvalidSections, "sections are not accepted by the abstain outcome; an abstain cannot rewrite the proposal") + } + for _, rel := range []struct { + name string + set bool + code FindingCode + }{ + {"depends_on", req.DependsOn != nil, FCInvalidDependsOn}, + {"related", req.Related != nil, FCInvalidRelated}, + {"discovered_from", req.DiscoveredFrom != nil, FCInvalidDiscoveredFrom}, + {"adrs", req.ADRs != nil, FCInvalidADRs}, + {"stacked_on", req.StackedOn != nil, FCInvalidStackedOn}, + } { + if rel.set { + addShape(rel.code, rel.name+" is not accepted by the abstain outcome") + } + } + default: + addShape(FCInvalidOutcome, fmt.Sprintf("outcome %q must be one of spec, trivial, revise, abstain", req.Outcome)) + } +``` + +After the switch (before the spec_version block): + +```go + // blocked_note is the abstain entry's body; nothing else reads it, so it is + // refused anywhere else rather than silently ignored. + if req.Outcome != GroomAbstain && req.BlockedNote != "" { + addShape(FCInvalidBlockedNote, "blocked_note applies only to the abstain outcome") + } + boundAuthored(&findings, "blocked_note", req.BlockedNote) +``` + +7. New helpers (below `specMarkdownShapeProblem`): + +```go +// blockedNoteBodyDiagnostic maps a section-body validation error onto an +// actionable, static diagnostic that never echoes the authored note. +func blockedNoteBodyDiagnostic(err error) string { + if errors.Is(err, render.ErrSectionBodyUnterminatedFence) { + return "blocked_note leaves a code fence unterminated; close the fence so the sections after \"## Auto-groom blocked\" stay visible" + } + return "blocked_note carries a column-zero \"## \" heading outside fenced code; the operation owns the \"## Auto-groom blocked\" heading — author body text, lists, or \"###\"-or-deeper subsections, and put heading examples inside closed code fences" +} + +// autoGroomBlockedMarkdown is the ## Auto-groom blocked body after one more +// abstain: the prior entries (when the section exists) followed by one new +// entry — "Recorded (UTC).", a blank line, then the note — so earlier +// entries are preserved, never replaced. +func autoGroomBlockedMarkdown(oldBody string, present bool, date, note string) string { + entry := "Recorded " + date + " (UTC).\n\n" + strings.TrimRight(note, "\r\n") + if present { + if trimmed := strings.Trim(oldBody, "\r\n"); trimmed != "" { + return trimmed + "\n\n" + entry + } + } + return entry +} +``` + +8. `Plan` — replace + +```go + edited, err := render.ApplySectionEdits(src, render.ChangeOwnedHeadings, toSectionEdits(o.req.Sections)) +``` + +with + +```go + edits := toSectionEdits(o.req.Sections) + if o.req.Outcome == GroomAbstain { + // Append one dated entry, preserving earlier ones. The body is sliced at + // its NAMED terminator (the next top-level heading) by the same + // fence-aware scan the splice uses; a duplicate heading refuses. + oldBody, present, err := namedSectionBody(src, autoGroomBlockedHeading) + if err != nil { + return refuseGroom(string(FCSectionEditFailed), err.Error()) + } + edits = append(edits, render.SectionEdit{ + Heading: autoGroomBlockedHeading, Intent: render.SectionReplace, + Markdown: autoGroomBlockedMarkdown(oldBody, present, o.clock.Now().UTC().Format("2006-01-02"), o.req.BlockedNote), + }) + } + edited, err := render.ApplySectionEdits(src, render.ChangeOwnedHeadings, edits) +``` + +and after the `if o.req.Outcome == GroomTrivial { … }` block add: + +```go + if o.req.Outcome == GroomAbstain { + // upsertField: a record without the key (hand-authored or pre-template) + // gets it inserted rather than failing on a missing patch target. + upsertField(&ps, doc1, "auto_groomable", document.Bool(false)) + } +``` + +9. Update the file-header comment's outcome sentence to mention the abstain outcome (it records an auto-groom abstain on a needs-design change: `auto_groomable: false` plus a dated `## Auto-groom blocked` entry, under the same groom gate). + +- [ ] **Step 4: Run to verify pass** + +Run: `go test ./internal/app/ -run 'TestChangeGroom|TestSchemaVocab|TestVocabulary|TestReflectDescriptor|TestShapeValidatorCodesAreRegistered|TestFindingCodeRegistryIntegrity|TestNoInlineFindingCodeLiterals|TestRequired' -count=1` +Expected: PASS. Mutation spot-checks (restore after each): make `autoGroomBlockedMarkdown` return `entry` unconditionally → `TestChangeGroomPlanAbstainAppendsSecondEntry` reddens; delete the `render.ValidateSectionBody` branch → the two `invalid-blocked_note` body rows redden. + +- [ ] **Step 5: Commit** + +```bash +git add internal/app/change_groom.go internal/app/change_groom_test.go internal/app/finding_codes.go internal/app/finding_codes_test.go internal/app/schema_vocab.go internal/app/schema_vocab_test.go internal/app/schema_test.go +git commit -m "feat(app): change.groom outcome abstain writes the flag, the dated entry, and the board atomically (change 0382)" +``` + +--- + +### Task 6: `change.groom` `outcome: rearm`, CLI help, and the real-git end-to-end proof + +**Files:** +- Modify: `internal/app/change_groom.go` (outcome const, shape rules, Plan branch, `HumanText`, invalid-outcome message, header comment) +- Modify: `internal/app/finding_codes.go` (`FCNothingToRearm`) +- Modify: `internal/app/schema_vocab.go` (`groom_outcomes` gains `GroomRearm`) +- Modify: `internal/cli/change.go` (groom `Short` help) +- Test: `internal/app/change_groom_test.go`, `internal/app/schema_vocab_test.go` + +**Interfaces:** +- Consumes: `abstainRequest()`, `abstainedChange()`, `autoGroomBlockedHeading` (Task 5); `domain.Change.AutoGroomable()` (Task 2); existing `namedSectionPresent(src []byte, heading string) bool` (`internal/app/finalize_block.go`); real-git helpers `newWorkingRepo`, `planningDepsFor`, `blobVersionAt`, `originTip`, `originFile`, `originCommitPaths`. +- Produces: `GroomRearm GroomOutcome = "rearm"`, `FCNothingToRearm FindingCode = "nothing-to-rearm"`. + +- [ ] **Step 1: Write the failing tests** + +Append to `internal/app/change_groom_test.go`: + +```go +// rearmRequest is a well-formed rearm request against the fixture at id 2. +func rearmRequest() ChangeGroomRequest { + return ChangeGroomRequest{ + ChangeID: 2, + Path: groomPath(2, "add-a-widget"), + Version: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + Outcome: GroomRearm, + } +} + +func TestChangeGroomRearmShapeValidation(t *testing.T) { + cases := []struct { + name string + mut func(*ChangeGroomRequest) + code string + }{ + {"bare rearm passes", func(r *ChangeGroomRequest) {}, ""}, + {"rearm with owned-section edits passes", func(r *ChangeGroomRequest) { + r.Sections = []SectionEditRequest{{Heading: "## Open questions", Intent: "replace", Markdown: "Resolved.\n"}} + }, ""}, + {"rearm with blocked_note", func(r *ChangeGroomRequest) { r.BlockedNote = "x\n" }, "invalid-blocked_note"}, + {"rearm with spec_markdown", func(r *ChangeGroomRequest) { r.SpecMarkdown = "# Design\n" }, "invalid-spec_markdown"}, + {"rearm with spec_version", func(r *ChangeGroomRequest) { r.SpecVersion = "a" }, "invalid-spec_version"}, + // Review Focus 5: the op removes this section itself. + {"rearm editing ## Auto-groom blocked", func(r *ChangeGroomRequest) { + r.Sections = []SectionEditRequest{{Heading: "## Auto-groom blocked", Intent: "replace", Markdown: "x\n"}} + }, "invalid-section-heading"}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + req := rearmRequest() + c.mut(&req) + findings := validateChangeGroomShape(req) + if c.code == "" { + if len(findings) != 0 { + t.Fatalf("want no findings, got %v", findings) + } + return + } + if !hasFindingCode(findings, c.code) { + t.Errorf("missing finding %q; got %v", c.code, findings) + } + }) + } +} + +func TestChangeGroomPlanRearmClearsSectionSetsFlagAndBoard(t *testing.T) { + files := map[string]string{ + groomPath(2, "add-a-widget"): abstainedChange(2, "add-a-widget"), + "docs/changes/BOARD.md": "# Backlog\n\nold\n", + } + plan, opRes := groomPlanFor(t, files, baseGroomOp([]string{"inline"}, rearmRequest())) + if opRes.Refused { + t.Fatalf("unexpected refusal: %v", opRes.Findings) + } + rec := string(groomedRecordBytes(t, plan, groomPath(2, "add-a-widget"))) + if strings.Contains(rec, "## Auto-groom blocked") || strings.Contains(rec, "First note.") { + t.Errorf("re-arm left the presence-encoded section behind:\n%s", rec) + } + if !strings.Contains(rec, "\nauto_groomable: true\n") || strings.Contains(rec, "auto_groomable: false") { + t.Errorf("re-arm did not set auto_groomable: true:\n%s", rec) + } + board := string(groomedRecordBytes(t, plan, "docs/changes/BOARD.md")) + if strings.Contains(board, "auto-groom blocked — needs you") || !strings.Contains(board, "needs-brainstorm") { + t.Errorf("board row did not return to needs-brainstorm in the same plan:\n%s", board) + } + var receipt changeGroomReceipt + if err := json.Unmarshal(plan.Receipt, &receipt); err != nil || receipt.Outcome != "rearm" { + t.Errorf("receipt = %s (%v), want outcome rearm", plan.Receipt, err) + } +} + +func TestChangeGroomPlanRearmAppliesSectionEditsInOneRecord(t *testing.T) { + req := rearmRequest() + req.Sections = []SectionEditRequest{{Heading: "## Open questions", Intent: "replace", Markdown: "Resolved: use SQLite.\n"}} + files := map[string]string{groomPath(2, "add-a-widget"): abstainedChange(2, "add-a-widget")} + plan, opRes := groomPlanFor(t, files, baseGroomOp([]string{}, req)) + if opRes.Refused { + t.Fatalf("unexpected refusal: %v", opRes.Findings) + } + assertPlanPaths(t, plan, map[string]transaction.MutationKind{groomPath(2, "add-a-widget"): transaction.MutationReplace}) + rec := string(groomedRecordBytes(t, plan, groomPath(2, "add-a-widget"))) + if !strings.Contains(rec, "Resolved: use SQLite.") || strings.Contains(rec, "An open question.") || strings.Contains(rec, "## Auto-groom blocked") { + t.Errorf("section edit and section removal did not both land:\n%s", rec) + } +} + +func TestChangeGroomPlanRearmArmsWithoutABlockedSection(t *testing.T) { + cases := []struct { + name string + rec string + }{ + // Review Focus 4: the key is inserted when absent (inherit ⇒ explicit true). + {"field absent", groomableChange(2, "add-a-widget")}, + {"opted out", strings.Replace(groomableChange(2, "add-a-widget"), "trivial: false\n", "trivial: false\nauto_groomable: false\n", 1)}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + plan, opRes := groomPlanFor(t, map[string]string{groomPath(2, "add-a-widget"): c.rec}, baseGroomOp([]string{}, rearmRequest())) + if opRes.Refused { + t.Fatalf("unexpected refusal: %v", opRes.Findings) + } + if rec := string(groomedRecordBytes(t, plan, groomPath(2, "add-a-widget"))); !strings.Contains(rec, "\nauto_groomable: true\n") { + t.Errorf("auto_groomable: true not written:\n%s", rec) + } + }) + } +} + +func TestChangeGroomPlanRearmRefusals(t *testing.T) { + armed := strings.Replace(groomableChange(2, "add-a-widget"), "trivial: false\n", "trivial: false\nauto_groomable: true\n", 1) + cases := []struct { + name string + files map[string]string + code string + }{ + {"nothing to re-arm", map[string]string{groomPath(2, "add-a-widget"): armed}, "nothing-to-rearm"}, + {"trivial", map[string]string{groomPath(2, "add-a-widget"): trivialChange(2, "add-a-widget")}, "not-groomable"}, + {"spec'd", reviseFixtureFiles(), "not-groomable"}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + plan, opRes := groomPlanFor(t, c.files, baseGroomOp([]string{}, rearmRequest())) + if !opRes.Refused || len(plan.Files) != 0 { + t.Fatalf("want a refusal writing nothing, got refused=%v files=%v", opRes.Refused, planPaths(plan)) + } + found := false + for _, f := range opRes.Findings { + found = found || f.Code == c.code + } + if !found { + t.Errorf("missing %q; got %v", c.code, opRes.Findings) + } + }) + } +} + +func TestChangeGroomResultHumanTextRearm(t *testing.T) { + r := newChangeGroomResult(ResultApplied, ChangeGroomResult{ID: 7, Outcome: string(GroomRearm), Revision: "cafe"}) + if got, want := r.HumanText(), "change 0007 re-armed for auto-groom — cafe"; got != want { + t.Errorf("HumanText = %q, want %q", got, want) + } +} + +// TestChangeGroomAbstainThenRearmRealGit drives both outcomes through the real +// engine and a bare origin: the abstain lands the record and BOARD.md in ONE +// commit; a re-arm pinned to the pre-abstain version contends and writes +// nothing; a re-arm at the current version restores needs-brainstorm. +func TestChangeGroomAbstainThenRearmRealGit(t *testing.T) { + requireRealGit(t) + recPath := groomPath(2, "add-a-widget") + repo := newWorkingRepo(t, map[string]string{recPath: groomableChange(2, "add-a-widget")}) + node := planningDepsFor(t, repo.invocation) + + ab := abstainRequest() + ab.Version = blobVersionAt(t, repo.origin, "docket", recPath) + if res := ChangeGroom(context.Background(), node.deps, node.dir, ab); res.Result != ResultApplied { + t.Fatalf("abstain = %q (findings %v), want applied", res.Result, res.Findings) + } + tip := originTip(t, repo.origin, "docket") + paths := originCommitPaths(t, repo.origin, tip) + if !slices.Contains(paths, recPath) || !slices.Contains(paths, "docs/changes/BOARD.md") { + t.Fatalf("abstain commit paths = %v, want the record and BOARD.md in one commit", paths) + } + if board, _ := originFile(t, repo.origin, "docket", "docs/changes/BOARD.md"); !strings.Contains(board, "auto-groom blocked — needs you") { + t.Errorf("committed board does not show the abstain:\n%s", board) + } + + stale := rearmRequest() + stale.Version = ab.Version // pre-abstain pin + if res := ChangeGroom(context.Background(), node.deps, node.dir, stale); res.Result != ResultContended { + t.Fatalf("stale re-arm = %q (findings %v), want contended", res.Result, res.Findings) + } + if got := originTip(t, repo.origin, "docket"); got != tip { + t.Fatalf("a contended re-arm moved the metadata branch %s -> %s", tip, got) + } + + fresh := rearmRequest() + fresh.Version = blobVersionAt(t, repo.origin, "docket", recPath) + if res := ChangeGroom(context.Background(), node.deps, node.dir, fresh); res.Result != ResultApplied { + t.Fatalf("re-arm = %q (findings %v), want applied", res.Result, res.Findings) + } + rec, _ := originFile(t, repo.origin, "docket", recPath) + board, _ := originFile(t, repo.origin, "docket", "docs/changes/BOARD.md") + if strings.Contains(rec, "## Auto-groom blocked") || !strings.Contains(rec, "\nauto_groomable: true\n") { + t.Errorf("re-armed record:\n%s", rec) + } + if strings.Contains(board, "auto-groom blocked — needs you") { + t.Errorf("committed board still shows the abstain after re-arm:\n%s", board) + } +} +``` + +Add `"slices"` to `change_groom_test.go`'s imports. In `internal/app/schema_vocab_test.go` change the groom line to `[]string{"spec", "trivial", "revise", "abstain", "rearm"}`. + +- [ ] **Step 2: Run to verify failure** + +Run: `go test ./internal/app/ -run 'TestChangeGroom|TestSchemaVocabulariesCore' -count=1` +Expected: FAIL to compile — undefined `GroomRearm`. + +- [ ] **Step 3: Implement** + +`internal/app/finding_codes.go`: add to the state-shape const group (beside `FCNotFound`): + +```go + FCNothingToRearm FindingCode = "nothing-to-rearm" +``` + +and insert `FCNothingToRearm,` into `AllFindingCodes` directly after `FCNotFound`. + +`internal/app/schema_vocab.go`: append `string(GroomRearm)` to the `groom_outcomes` members. + +`internal/app/change_groom.go`: + +1. Const block, after `GroomAbstain`: + +```go + // GroomRearm re-arms a needs-brainstorm change for autonomous grooming: it + // sets auto_groomable: true, removes the ## Auto-groom blocked section when + // present, and applies any owned-section edits (typically the context the + // abstain asked for) in the same commit. Human-typed or human-attended only. + GroomRearm GroomOutcome = "rearm" +``` + +2. `HumanText`, next to the abstain check: + +```go + if r.Outcome == string(GroomRearm) { + return fmt.Sprintf("change %04d re-armed for auto-groom — %s", r.ID, r.Revision) + } +``` + +3. `validateChangeGroomShape`: add before `default:` + +```go + case GroomRearm: + if strings.TrimSpace(req.SpecMarkdown) != "" { + addShape(FCInvalidSpecMarkdown, "spec_markdown is not accepted by the rearm outcome") + } + for _, s := range req.Sections { + if s.Heading == autoGroomBlockedHeading { + addShape(FCInvalidSectionHeading, "the rearm outcome removes \"## Auto-groom blocked\" itself; a section edit may not name it") + } + } +``` + +and change the default message to `"outcome %q must be one of spec, trivial, revise, abstain, rearm"`. (`blocked_note` and `spec_version` on rearm are already refused by the generic post-switch rules.) + +4. `Plan`, directly after the `if o.req.Outcome == GroomAbstain { … }` edits block from Task 5: + +```go + if o.req.Outcome == GroomRearm { + // Every transition out of the abstained state removes the marker whose + // presence encodes it (the board keys on it). With no marker and the flag + // already true there is nothing to re-arm. + blocked := namedSectionPresent(src, autoGroomBlockedHeading) + if ag := c.AutoGroomable(); !blocked && ag.State == domain.FieldPresent && ag.Value { + return refuseGroom(string(FCNothingToRearm), + fmt.Sprintf("change %04d has no %s section and is already auto_groomable: true; there is nothing to re-arm", o.req.ChangeID, autoGroomBlockedHeading)) + } + if blocked { + edits = append(edits, render.SectionEdit{Heading: autoGroomBlockedHeading, Intent: render.SectionRemove}) + } + } +``` + +and after the abstain `upsertField` block: + +```go + if o.req.Outcome == GroomRearm { + upsertField(&ps, doc1, "auto_groomable", document.Bool(true)) + } +``` + +5. File-header comment: mention the rearm outcome alongside abstain. + +`internal/cli/change.go`, groom `Short`: + +```go + "Groom a proposed change to build-ready (spec or trivial), record or clear an auto-groom abstain (abstain, rearm), or revise an already-groomed one, from a JSON request", +``` + +- [ ] **Step 4: Run to verify pass** + +Run: `go test ./internal/app/ -run 'TestChangeGroom|TestSchemaVocab|TestVocabulary|TestShapeValidatorCodesAreRegistered|TestFindingCodeRegistryIntegrity|TestNoInlineFindingCodeLiterals' -count=1 && go test ./internal/cli/ -count=1` +Expected: PASS. Mutation spot-check: delete the `edits = append(… SectionRemove …)` line → `TestChangeGroomPlanRearmClearsSectionSetsFlagAndBoard` and the real-git test redden; restore. + +- [ ] **Step 5: Commit** + +```bash +git add internal/app/change_groom.go internal/app/change_groom_test.go internal/app/finding_codes.go internal/app/schema_vocab.go internal/app/schema_vocab_test.go internal/cli/change.go +git commit -m "feat(app): change.groom outcome rearm clears the abstain marker and re-renders the board (change 0382)" +``` + +--- + +### Task 7: `docket-new-change` passes the scalars in `change.create` + +**Files:** +- Modify: `skills/docket-new-change/SKILL.md` (the "Two draft-time scalars" paragraph under step 4) +- Modify: `internal/repoguard/prose_contracts_test.go` (new row) +- Modify: `internal/repoguard/budgets_test.go` (lower the `docket-new-change/SKILL.md` ceiling if it shrank) +- Regenerate: `internal/assets/embedded/` via `go generate ./internal/assets/` + +**Interfaces:** +- Consumes: the `change.create` request keys `auto_groomable` / `branch_prefix` and finding `invalid-branch_prefix` (Task 4). + +- [ ] **Step 1: Write the failing guard row** + +In `internal/repoguard/prose_contracts_test.go`, append to `proseContracts` (after the `change_0400_readme_landing` row): + +```go + // change 0382 — both draft-time scalars ride in the change.create request; + // the post-create plain-git frontmatter edit and the skill-owned + // normalization rules are gone (normalization is domain.NormalizeBranchPrefix). + {sentinel: "change_0382_typed_create_scalars", file: "skills/docket-new-change/SKILL.md", + present: []string{"`invalid-branch_prefix`"}, + absent: []string{"Two draft-time scalars `create` does not carry", "strip one presentation-only trailing slash"}}, +``` + +- [ ] **Step 2: Run to verify failure** + +Run: `go test ./internal/repoguard/ -run 'TestProseContracts$' -count=1` +Expected: FAIL — the new row's `present` phrase is missing and both `absent` phrases are present. + +- [ ] **Step 3: Rewrite the paragraph** + +Replace the whole paragraph beginning ` **Two draft-time scalars \`create\` does not carry**` with: + +```markdown + **Two draft-time scalars ride in the same request** — `auto_groomable` and `branch_prefix`. When the human says the change may be designed without them, send `auto_groomable: true` (so `docket-auto-groom` carries it to build-ready; `false` opts it out). When they name a branch prefix ("use the `hotfix/` prefix"), send it as typed in `branch_prefix` — the operation normalizes it and refuses an unusable value with `invalid-branch_prefix` before anything is written: show the human that finding and ask for a new value. Omit either field to inherit the repo default. +``` + +- [ ] **Step 4: Regenerate, rebaseline, and verify** + +Run: + +```bash +go generate ./internal/assets/ +go test ./internal/repoguard/ ./internal/assets/ -count=1 +``` + +Expected: PASS. If `TestSkillSizeBudgets` reports the file is now under its ceiling by a margin, lower the `docket-new-change/SKILL.md` row's word ceiling to the measured count in the same commit, appending `// 0382: draft-time scalars moved into change.create (ceiling 1706 -> )` to its existing trailing comment. Mutation-check the row: back up the edited skill (`bak=$(mktemp "${TMPDIR:-/tmp}/nc-skill.XXXXXX") && cp skills/docket-new-change/SKILL.md "$bak"`), restore the committed text with `git show HEAD:skills/docket-new-change/SKILL.md > skills/docket-new-change/SKILL.md`, run the prose-contract test and confirm it reddens, then `mv -f "$bak" skills/docket-new-change/SKILL.md`. + +- [ ] **Step 5: Commit** + +```bash +git add skills/docket-new-change/SKILL.md internal/repoguard/prose_contracts_test.go internal/repoguard/budgets_test.go internal/assets/embedded +git commit -m "docs(skills): new-change sends auto_groomable and branch_prefix through change.create (change 0382)" +``` + +--- + +### Task 8: Auto-groom abstain, convention, groom-next re-arm, and the guide use the typed outcomes + +**Files:** +- Modify: `skills/docket-auto-groom/SKILL.md` (Step 3 no-verdict posture clause; Step 4 exit 3; Step 5) +- Modify: `skills/docket-convention/SKILL.md` (the `## Auto-groom blocked` roster bullet; *Autonomous grooming* ownership sentence and **Abstain rule** paragraph) +- Modify: `skills/docket-groom-next/SKILL.md` (Step 4 header, "All five exits", new exit 6) +- Modify: `docs/guide/designing-before-building.md` (*Grooming a batch with no human*) +- Modify: `internal/repoguard/prose_contracts_test.go` (new rows) +- Modify: `internal/repoguard/budgets_test.go` (re-baseline rows that grow, with `// 0382:` notes) +- Regenerate: `internal/assets/embedded/` + +Site derivation (do not hand-extend): a whole-repo `grep -rn -i -e abstain -e "auto-groom blocked" -e "change.groom" -e branch_prefix` over `README.md docs/guide docs/reference docs/concepts skills agents cursor-rules .docket.example.yml` finds prose that WRITES these scalars only in the four skill files plus the guide's auto-groom section; `skills/docket-status/SKILL.md`, `skills/docket-convention/references/dummy-mode.md`, `agents/*`, and `cursor-rules/*` only read or name the section and stay unchanged. README and `docs/reference` enumerate no groom outcomes. Re-run that grep before editing and treat any new hit as in scope. + +- [ ] **Step 1: Write the failing guard rows** + +Append to `proseContracts`: + +```go + // change 0382 — the auto-groom abstain and the human re-arm are typed + // change.groom outcomes that re-render the board in the same commit; the + // plain-git abstain, its false "no board-visible cell" claim, and the + // hand-edit re-arm are gone. + {sentinel: "change_0382_typed_abstain", file: "skills/docket-auto-groom/SKILL.md", + present: []string{"`outcome: abstain`", "`blocked_note`"}, + absent: []string{"changes no board-visible cell", "there is no typed groom for this outcome"}}, + {sentinel: "change_0382_typed_abstain", file: "skills/docket-convention/SKILL.md", + present: []string{"`outcome: abstain`", "`outcome: rearm`"}, + absent: []string{"flips the flag back to `true`, and DELETES"}}, + {sentinel: "change_0382_typed_abstain", file: "skills/docket-groom-next/SKILL.md", + present: []string{"`outcome: rearm`", "`nothing-to-rearm`"}}, +``` + +- [ ] **Step 2: Run to verify failure** + +Run: `go test ./internal/repoguard/ -count=1` +Expected: FAIL on the three new rows. + +- [ ] **Step 3: Edit the prose** + +`skills/docket-auto-groom/SKILL.md`: + +- In Step 3's no-verdict posture, replace `recording the return-channel diagnostic in the \`## Auto-groom blocked\` section, the human's re-arm cue` with `recording the return-channel diagnostic in the \`blocked_note\`, the human's re-arm cue`. +- Replace Step 4 item 3 with: + +```markdown +3. **Abstain** — any needs-human-context verdict, or Step 3's exhausted no-verdict posture: emit NO spec; apply the `change.groom` operation with `--repo-dir .docket --request ` with `outcome: abstain`, the pinned `path` + `version`, and a `blocked_note` — the undecidable decision(s), what context is missing, what a human should supply, and any recommendation (including "this should probably be killed/deferred because …"), with subsections at `###` or deeper. The transaction sets `auto_groomable: false` + `updated:`, appends a dated entry to the `## Auto-groom blocked` section, and re-renders the inline board — the row flips to **auto-groom blocked — needs you** in that same commit. It accepts no section, spec, or relationship edits. The stub stays needs-brainstorm, first in `docket-groom-next`'s queue. +``` + +- Replace the whole of Step 5's two body paragraphs with: + +```markdown +Every exit's Step-4 `change.groom` operation is the whole write — it re-checks the pinned `version` and commits the record, any spec, the `## Artifacts` block, and the inline board in one metadata commit pushed under an exact-lease push, so there is **no separate Board pass** and no hand-staged commit. On a `contended` refusal it writes nothing: re-sync (re-run the `repository.prepare` operation), re-read the stub's `path` + `version` from the `status` operation, and if it is no longer autonomous-eligible (groomed, killed, claimed, or opted out) DISCARD this iteration's draft (delete any just-drafted spec markdown) and loop; otherwise re-author and retry. Loop to step 1. +``` + +`skills/docket-convention/SKILL.md`: + +- Roster bullet: replace `dated abstain record appended by \`docket-auto-groom\`; contents and lifecycle (including removal on re-arm)` with `dated abstain record written by \`change.groom\` \`outcome: abstain\`; contents and lifecycle (including removal by \`outcome: rearm\`)`. +- *Autonomous grooming*: replace `\`docket-auto-groom\`'s abstain is the single agent write (it flips the override to \`false\`).` with `\`docket-auto-groom\`'s abstain is the single agent write (\`change.groom\` \`outcome: abstain\` flips the override to \`false\`).` +- Replace the **Abstain rule.** paragraph with: + +```markdown +**Abstain rule.** When autonomous grooming cannot safely default a decision, it emits NO spec; it applies `change.groom` with `outcome: abstain`, which flips `auto_groomable: false`, appends a dated entry to the `## Auto-groom blocked` body section, and re-renders the board in one commit. The stub stays needs-brainstorm — out of the autonomous queue, still in the interactive one. Re-arm = a human supplies the missing context and applies `change.groom` with `outcome: rearm` (optionally with owned-section edits carrying that context): it sets the flag back to `true` and removes the `## Auto-groom blocked` section in the same commit — never a hand edit (git history keeps the section; its presence drives the board's needs-you cell, so a stale one would mislabel a re-armed stub). Kill and defer are never autonomous: they surface inside the blocked section as recommendations. +``` + +`skills/docket-groom-next/SKILL.md`: + +- `### Step 4 — Exit (one of five; the human confirms which)` → `### Step 4 — Exit (one of six; the human confirms which)`; `All five exits reuse` → `All six exits reuse`. +- Append a sixth item after item 5: + +```markdown +6. **Re-arm** (abstained or opted-out stubs): the human has supplied the context an abstain asked for and wants `docket-auto-groom` to take the stub rather than grooming it now — apply the `change.groom` operation with `--repo-dir .docket --request ` with `outcome: rearm`, the pinned `path` + `version`, and any owned-section `sections` edits carrying the new context (typically `## Open questions`). The transaction sets `auto_groomable: true`, removes the `## Auto-groom blocked` section, and re-renders the inline board atomically; the stub stays needs-brainstorm, back in the autonomous queue. A `nothing-to-rearm` refusal (no blocked section, already `true`) writes nothing. A spec or trivial groom of an abstained stub needs no re-arm. +``` + +`docs/guide/designing-before-building.md`, after the sentence ending `…handed back to your interactive queue rather than forced through.` add: + +```markdown +Such a stub shows as **auto-groom blocked — needs you** on the board. Once you have supplied the +missing context you can groom it yourself, or re-arm it so the autonomous groomer picks it up +again — the interactive groom skill offers both. +``` + +- [ ] **Step 4: Regenerate, rebaseline, and verify** + +Run: + +```bash +go generate ./internal/assets/ +go test ./internal/repoguard/ ./internal/assets/ -count=1 +``` + +Expected: the prose rows pass. If `TestSkillSizeBudgets` reports a file over its ceiling, raise that row to the reported line/word count and append a `// 0382: …` note naming the added prose (e.g. `0382: +typed rearm exit (word ceiling 1889 -> )` for `docket-groom-next/SKILL.md`); if a file shrank (likely `docket-auto-groom/SKILL.md`), lower its ceiling instead. Re-run until green. Mutation-check the three new rows as in Task 7 (backup via templated `mktemp`, restore `HEAD` text, confirm red, `mv -f` back). + +- [ ] **Step 5: Commit** + +```bash +git add skills/docket-auto-groom/SKILL.md skills/docket-convention/SKILL.md skills/docket-groom-next/SKILL.md docs/guide/designing-before-building.md internal/repoguard/prose_contracts_test.go internal/repoguard/budgets_test.go internal/assets/embedded +git commit -m "docs(skills): auto-groom abstain and human re-arm use typed change.groom outcomes (change 0382)" +``` + +--- + +## Self-review record + +- **Spec coverage:** §1 request shape + digest → Task 4; §2 normalization + `invalid-branch_prefix` → Tasks 1, 4; §3 rendering → Task 3; §4 domain tri-state → Task 2; §5 abstain → Task 5; §6 rearm (+ `nothing-to-rearm`, vocabulary, message, HumanText) → Task 6; §7 skills/convention/guide → Tasks 7, 8; Error handling (typed refusals, contended) → Tasks 4–6; Testing section rows → Tasks 1, 4, 5, 6 (normalization table incl. `/`, `" "`, `""`; create true/false/absent; round-trip MintBranch; replay/conflict; schema descriptor; abstain board + append + refusals; rearm board + sections + `nothing-to-rearm` + contended). +- **Type consistency:** `OptionalBool{State, Value, Raw}`, `Change.AutoGroomable()`, `NormalizeBranchPrefix(string) (string, bool)`, `normalizedBranchPrefix(ChangeCreateRequest) string`, `GroomAbstain`/`GroomRearm`, `autoGroomBlockedHeading`, `BlockedNote`/`blocked_note`, `FCInvalidBranchPrefix`/`FCEmptyBlockedNote`/`FCInvalidBlockedNote`/`FCInvalidSections`/`FCNothingToRearm` are spelled identically in every task that names them. +- **Intermediate buildability:** each task compiles and passes on its own; the vocabulary test is extended per outcome (Task 5 adds `abstain`, Task 6 adds `rearm`), and the doc tasks regenerate the embedded bundle in the same commit as the skill edit. diff --git a/internal/app/change_create.go b/internal/app/change_create.go index 27e543046..1c9056866 100644 --- a/internal/app/change_create.go +++ b/internal/app/change_create.go @@ -52,6 +52,14 @@ type ChangeCreateRequest struct { Related []int `json:"related"` DiscoveredFrom []int `json:"discovered_from"` ADRs []int `json:"adrs"` + + // AutoGroomable is the optional per-change auto-groom override: nil leaves + // the record unset (inherit the repo's auto_groom); true/false are explicit. + AutoGroomable *bool `json:"auto_groomable"` + // BranchPrefix is the optional mint-prefix override, as the human typed it. + // It is normalized by domain.NormalizeBranchPrefix before it is validated, + // digested, or stored; empty after normalization means unset. + BranchPrefix string `json:"branch_prefix"` } // ChangeCreateResult is the protocol-v1 document `change create` returns. It @@ -119,6 +127,10 @@ type changeCreatePayload struct { Related []int `json:"related"` DiscoveredFrom []int `json:"discovered_from"` ADRs []int `json:"adrs"` + // The draft-time scalars are omitempty so a request carrying neither digests + // exactly as it did before change 0382 and a pre-0382 retry still replays. + AutoGroomable *bool `json:"auto_groomable,omitempty"` + BranchPrefix string `json:"branch_prefix,omitempty"` } func changeCreateSemanticPayload(req ChangeCreateRequest) changeCreatePayload { @@ -134,9 +146,20 @@ func changeCreateSemanticPayload(req ChangeCreateRequest) changeCreatePayload { Related: req.Related, DiscoveredFrom: req.DiscoveredFrom, ADRs: req.ADRs, + AutoGroomable: req.AutoGroomable, + BranchPrefix: normalizedBranchPrefix(req), } } +// normalizedBranchPrefix is the stored, digested spelling of the request's +// branch_prefix. The request was shape-validated first, so a refused value never +// reaches the digest or Plan; the ok discarded here is enforced by +// validateChangeCreateShape. +func normalizedBranchPrefix(req ChangeCreateRequest) string { + p, _ := domain.NormalizeBranchPrefix(req.BranchPrefix) + return p +} + // ChangeCreate validates the request, pins authoritative context, and drives one // atomic transaction that lands the new change record and — when the inline // board surface is enabled — the re-rendered board. Every failure that predates @@ -294,6 +317,11 @@ func validateChangeCreateShape(req ChangeCreateRequest) []StatusFinding { if req.StackedOn != nil && *req.StackedOn <= 0 { addShape(FCInvalidStackedOn, "stacked_on must be a positive change id") } + if _, ok := domain.NormalizeBranchPrefix(req.BranchPrefix); !ok { + addShape(FCInvalidBranchPrefix, fmt.Sprintf( + "branch_prefix %q is not a usable branch prefix: after trimming whitespace, one trailing \"/\", and lowercasing, it must be a single git ref component — no \"/\" (never refs/-qualified), not \"refs\", no \"..\", \"@{\", whitespace, or any of ~^:?*[\\, not starting with \"-\" or \".\", not ending with \".\" or \".lock\"", + req.BranchPrefix)) + } return findings } @@ -458,6 +486,8 @@ func (o changeCreateOp) Plan(ctx context.Context, st transaction.AttemptState) ( Related: toChangeIDs(o.req.Related), DiscoveredFrom: toChangeIDs(o.req.DiscoveredFrom), ADRs: toADRIDs(o.req.ADRs), + AutoGroomable: o.req.AutoGroomable, + BranchPrefix: normalizedBranchPrefix(o.req), Why: o.req.Why, WhatChanges: o.req.WhatChanges, OutOfScope: o.req.OutOfScope, diff --git a/internal/app/change_create_test.go b/internal/app/change_create_test.go index 7da7d7f47..84a958cf8 100644 --- a/internal/app/change_create_test.go +++ b/internal/app/change_create_test.go @@ -466,3 +466,171 @@ func TestChangeCreateRecordHasNoRepairFindings(t *testing.T) { t.Fatalf("change.create output must have zero repair findings, got %+v\n%s", fs, rec) } } + +func TestChangeCreateRejectsInvalidBranchPrefixWithoutEngineCall(t *testing.T) { + for _, raw := range []string{"team/hotfix", "refs/heads/x", "hotfix//", "-x", "x.lock", "hot fix"} { + t.Run(raw, func(t *testing.T) { + req := validChangeCreateRequest() + req.BranchPrefix = raw + engine := &recordingEngine{} + reader := &fakeChangeReader{pin: mainModePin([]string{"inline"})} + deps := PlanningDeps{Engine: engine, Reader: reader, Clock: testClock()} + + res := ChangeCreate(context.Background(), deps, "", req) + + if res.Result != ResultInvalidInput { + t.Fatalf("result = %q, want invalid-input", res.Result) + } + if len(engine.calls) != 0 { + t.Errorf("engine called %d times on an invalid prefix, want 0", len(engine.calls)) + } + var msg string + for _, f := range res.Findings { + if f.Code == "invalid-branch_prefix" { + msg = f.Message + } + } + if msg == "" { + t.Fatalf("missing invalid-branch_prefix; got %v", res.Findings) + } + if !strings.Contains(msg, fmt.Sprintf("%q", raw)) { + t.Errorf("message %q does not name the rejected value %q", msg, raw) + } + }) + } +} + +func TestChangeCreatePlanWritesDraftScalars(t *testing.T) { + yes, no := true, false + recPath := "docs/changes/active/0002-add-a-widget.md" + cases := []struct { + name string + auto *bool + prefix string + wantAuto string + wantPrefix string + wantAG domain.OptionalBool + wantBranch string + }{ + {"absent", nil, "", "\nauto_groomable:\n", "\nbranch_prefix:\n", + domain.OptionalBool{State: domain.FieldEmpty}, "feat/add-a-widget"}, + {"true with messy prefix", &yes, " Hotfix/ ", "\nauto_groomable: true\n", "\nbranch_prefix: 'hotfix'\n", + domain.OptionalBool{State: domain.FieldPresent, Value: true, Raw: "true"}, "hotfix/add-a-widget"}, + {"explicit false", &no, "", "\nauto_groomable: false\n", "\nbranch_prefix:\n", + domain.OptionalBool{State: domain.FieldPresent, Value: false, Raw: "false"}, "feat/add-a-widget"}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + op := baseOp([]string{}) + op.req.AutoGroomable = c.auto + op.req.BranchPrefix = c.prefix + plan, opRes := planFor(t, map[string]string{ + "docs/changes/active/0001-first.md": fixtureChange(1, "first"), + }, op) + if opRes.Refused { + t.Fatalf("unexpected refusal: %v", opRes.Findings) + } + rec := groomedRecordBytes(t, plan, recPath) + if s := string(rec); !strings.Contains(s, c.wantAuto) || !strings.Contains(s, c.wantPrefix) { + t.Errorf("record missing %q / %q:\n%s", c.wantAuto, c.wantPrefix, s) + } + // Round trip through the real decoder, then mint exactly as claim does. + snap, err := buildCandidateSnapshot(op.eff, nil, rec, recPath) + if err != nil { + t.Fatalf("buildCandidateSnapshot: %v", err) + } + ch, out := snap.Change(2) + if out != domain.LookupFound { + t.Fatalf("created record not decodable as change 2") + } + if got := ch.AutoGroomable(); got != c.wantAG { + t.Errorf("decoded AutoGroomable = %+v, want %+v", got, c.wantAG) + } + if got := domain.MintBranch(ch.Type(), ch.BranchPrefix(), ch.Slug()); got != c.wantBranch { + t.Errorf("minted branch = %q, want %q", got, c.wantBranch) + } + }) + } +} + +func TestChangeCreateDigestBindsNormalizedDraftScalars(t *testing.T) { + digest := func(mut func(*ChangeCreateRequest)) transaction.RequestDigest { + t.Helper() + req := validChangeCreateRequest() + mut(&req) + d, err := canonicalDigest(OperationChangeCreate, changeCreateSemanticPayload(req)) + if err != nil { + t.Fatalf("canonicalDigest: %v", err) + } + return d + } + yes, no := true, false + none := digest(func(*ChangeCreateRequest) {}) + if digest(func(r *ChangeCreateRequest) { r.BranchPrefix = "Hotfix/" }) != digest(func(r *ChangeCreateRequest) { r.BranchPrefix = "hotfix" }) { + t.Error("Hotfix/ and hotfix must digest identically (the normalized prefix is bound), so a retry replays") + } + if digest(func(r *ChangeCreateRequest) { r.BranchPrefix = "hotfix" }) == none { + t.Error("a set prefix must change the digest") + } + if digest(func(r *ChangeCreateRequest) { r.AutoGroomable = &yes }) == digest(func(r *ChangeCreateRequest) { r.AutoGroomable = &no }) { + t.Error("differing auto_groomable under one request_id must conflict, not replay") + } + if digest(func(r *ChangeCreateRequest) { r.AutoGroomable = &no }) == none { + t.Error("an explicit false must differ from unset (inherit)") + } +} + +// TestChangeCreatePayloadOmitsUnsetDraftScalars — Review Focus 1: a request +// carrying neither new field must digest exactly as it did before change 0382, +// so re-running a pre-0382 request under its request_id still replays. +func TestChangeCreatePayloadOmitsUnsetDraftScalars(t *testing.T) { + b, err := json.Marshal(changeCreateSemanticPayload(validChangeCreateRequest())) + if err != nil { + t.Fatalf("marshal: %v", err) + } + for _, k := range []string{`"auto_groomable"`, `"branch_prefix"`} { + if strings.Contains(string(b), k) { + t.Errorf("unset %s appears in the digest payload %s; it would change every pre-0382 digest", k, b) + } + } +} + +// TestChangeCreateNormalizedPrefixReplaysRealGit drives the real engine: a +// create with a messy prefix stores the normalized value, the same request_id +// retyped as the normalized spelling replays, and a differing auto_groomable +// under that id does not apply. +func TestChangeCreateNormalizedPrefixReplaysRealGit(t *testing.T) { + requireRealGit(t) + repo := newWorkingRepo(t, map[string]string{ + "docs/changes/active/0001-first.md": fixtureChange(1, "first"), + }) + node := planningDepsFor(t, repo.invocation) + yes, no := true, false + + req := validChangeCreateRequest() + req.BranchPrefix, req.AutoGroomable = "Hotfix/", &yes + first := ChangeCreate(context.Background(), node.deps, node.dir, req) + if first.Result != ResultApplied || first.Replayed { + t.Fatalf("first create = %q replayed=%v (findings %v), want a fresh apply", first.Result, first.Replayed, first.Findings) + } + body, ok := originFile(t, repo.origin, "docket", first.Path) + if !ok || !strings.Contains(body, "\nbranch_prefix: 'hotfix'\n") || !strings.Contains(body, "\nauto_groomable: true\n") { + t.Fatalf("created record lacks the normalized scalars:\n%s", body) + } + tip := originTip(t, repo.origin, "docket") + + req.BranchPrefix = "hotfix" + second := ChangeCreate(context.Background(), node.deps, node.dir, req) + if second.Result != ResultApplied || !second.Replayed || second.ID != first.ID { + t.Fatalf("retyped create = %q replayed=%v id=%d (findings %v), want a replay of %d", second.Result, second.Replayed, second.ID, second.Findings, first.ID) + } + + req.AutoGroomable = &no + third := ChangeCreate(context.Background(), node.deps, node.dir, req) + if third.Result == ResultApplied { + t.Fatalf("a differing auto_groomable under the same request_id applied (replayed=%v)", third.Replayed) + } + if got := originTip(t, repo.origin, "docket"); got != tip { + t.Errorf("replay/conflict moved the metadata branch %s -> %s", tip, got) + } +} diff --git a/internal/app/change_groom.go b/internal/app/change_groom.go index a6f6016cc..53a4bcaad 100644 --- a/internal/app/change_groom.go +++ b/internal/app/change_groom.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "encoding/json" + "errors" "fmt" "path" "sort" @@ -24,8 +25,12 @@ import ( // spec, or a trivial verdict — or, by the third outcome (revise), adjusts an // already-groomed proposed change in place: a whole-body replace of its existing // linked spec and/or owned proposal-section edits, never touching spec: or -// trivial:. Every outcome lands the change's source mutation and every -// affected v1-owned derived view (the change record's owned proposal sections, +// trivial:. The abstain outcome records an autonomous groom's abstain on a +// needs-design change — auto_groomable: false plus one dated +// "## Auto-groom blocked" entry — under the same groom gate; the rearm outcome +// clears it — auto_groomable: true, the section removed — under that gate too, +// optionally with owned-section edits in the same commit. Every outcome +// lands the change's source mutation and every affected v1-owned derived view (the change record's owned proposal sections, // its typed fields, its artifact block; a new spec file for the spec outcome, a // replaced one for a spec-body revise; the inline board) as one validated // atomic transaction. Grooming is a @@ -54,6 +59,17 @@ const ( // both. It never writes spec: or trivial:, so a change can never flip // between spec'd and trivial through this outcome. GroomRevise GroomOutcome = "revise" + // GroomAbstain records an autonomous groom's abstain on a needs-brainstorm + // change: it sets auto_groomable: false and appends one dated entry to the + // ## Auto-groom blocked section. It never writes spec: or trivial:, and it + // accepts no section, spec, or relationship edits — an autonomous caller + // cannot rewrite the proposal through it. + GroomAbstain GroomOutcome = "abstain" + // GroomRearm re-arms a needs-brainstorm change for autonomous grooming: it + // sets auto_groomable: true, removes the ## Auto-groom blocked section when + // present, and applies any owned-section edits (typically the context the + // abstain asked for) in the same commit. Human-typed or human-attended only. + GroomRearm GroomOutcome = "rearm" ) // reasonSpecVersionMismatch is the Plan refusal for a stale spec_version on a @@ -64,6 +80,11 @@ const reasonSpecVersionMismatch = "spec-version-mismatch" // location (the Bash grooming skills write here); it is not configurable. const specsDir = "docs/superpowers/specs" +// autoGroomBlockedHeading is the presence-encoded abstain section: the abstain +// outcome appends to it, and the board's "auto-groom blocked — needs you" cell +// keys on its presence (domain.ReadyAutoGroomBlocked). +const autoGroomBlockedHeading = "## Auto-groom blocked" + // ChangeGroomRequest is the closed, caller-supplied request for one groom. Path // and Version pin the exact submitted record; the relationship collections are // the complete desired values (a nil collection is left unchanged, an explicit @@ -85,6 +106,12 @@ type ChangeGroomRequest struct { // contends instead of being silently clobbered. SpecVersion string `json:"spec_version,omitempty"` + // BlockedNote is the authored body of one ## Auto-groom blocked entry. The + // abstain outcome requires it and no other outcome accepts it; the operation + // owns the heading and the dated "Recorded (UTC)." lead line, so the + // note must not carry a column-zero "## " heading or an unterminated fence. + BlockedNote string `json:"blocked_note,omitempty"` + DependsOn []int `json:"depends_on"` Related []int `json:"related"` DiscoveredFrom []int `json:"discovered_from"` @@ -118,6 +145,12 @@ type ChangeGroomResult struct { func (r ChangeGroomResult) HumanText() string { switch r.Result { case ResultApplied: + if r.Outcome == string(GroomAbstain) { + return fmt.Sprintf("change %04d auto-groom abstained — %s", r.ID, r.Revision) + } + if r.Outcome == string(GroomRearm) { + return fmt.Sprintf("change %04d re-armed for auto-groom — %s", r.ID, r.Revision) + } if r.Outcome == string(GroomRevise) { return fmt.Sprintf("change %04d revised — %s", r.ID, r.Revision) } @@ -292,9 +325,52 @@ func validateChangeGroomShape(req ChangeGroomRequest) []StatusFinding { } else if !hasEffectiveSectionEdit(req.Sections) { addShape(FCEmptyRevise, "the revise outcome requires a non-empty spec_markdown or at least one replace/remove section edit") } + case GroomAbstain: + if strings.TrimSpace(req.BlockedNote) == "" { + addShape(FCEmptyBlockedNote, "blocked_note must be non-empty for the abstain outcome") + } else if err := render.ValidateSectionBody([]byte(req.BlockedNote)); err != nil { + addShape(FCInvalidBlockedNote, blockedNoteBodyDiagnostic(err)) + } + if strings.TrimSpace(req.SpecMarkdown) != "" { + addShape(FCInvalidSpecMarkdown, "spec_markdown is not accepted by the abstain outcome") + } + if len(req.Sections) > 0 { + addShape(FCInvalidSections, "sections are not accepted by the abstain outcome; an abstain cannot rewrite the proposal") + } + for _, rel := range []struct { + name string + set bool + code FindingCode + }{ + {"depends_on", req.DependsOn != nil, FCInvalidDependsOn}, + {"related", req.Related != nil, FCInvalidRelated}, + {"discovered_from", req.DiscoveredFrom != nil, FCInvalidDiscoveredFrom}, + {"adrs", req.ADRs != nil, FCInvalidADRs}, + {"stacked_on", req.StackedOn != nil, FCInvalidStackedOn}, + } { + if rel.set { + addShape(rel.code, rel.name+" is not accepted by the abstain outcome") + } + } + case GroomRearm: + if strings.TrimSpace(req.SpecMarkdown) != "" { + addShape(FCInvalidSpecMarkdown, "spec_markdown is not accepted by the rearm outcome") + } + for _, s := range req.Sections { + if s.Heading == autoGroomBlockedHeading { + addShape(FCInvalidSectionHeading, "the rearm outcome removes \"## Auto-groom blocked\" itself; a section edit may not name it") + } + } default: - addShape(FCInvalidOutcome, fmt.Sprintf("outcome %q must be one of spec, trivial, revise", req.Outcome)) + addShape(FCInvalidOutcome, fmt.Sprintf("outcome %q must be one of spec, trivial, revise, abstain, rearm", req.Outcome)) + } + + // blocked_note is the abstain entry's body; nothing else reads it, so it is + // refused anywhere else rather than silently ignored. + if req.Outcome != GroomAbstain && req.BlockedNote != "" { + addShape(FCInvalidBlockedNote, "blocked_note applies only to the abstain outcome") } + boundAuthored(&findings, "blocked_note", req.BlockedNote) // A spec-body revise overwrites the linked spec file, so it must pin that // file's version exactly like the record. Nothing else checks spec_version, @@ -328,6 +404,29 @@ func specMarkdownShapeProblem(markdown string) string { return "" } +// blockedNoteBodyDiagnostic maps a section-body validation error onto an +// actionable, static diagnostic that never echoes the authored note. +func blockedNoteBodyDiagnostic(err error) string { + if errors.Is(err, render.ErrSectionBodyUnterminatedFence) { + return "blocked_note leaves a code fence unterminated; close the fence so the sections after \"## Auto-groom blocked\" stay visible" + } + return "blocked_note carries a column-zero \"## \" heading outside fenced code; the operation owns the \"## Auto-groom blocked\" heading — author body text, lists, or \"###\"-or-deeper subsections, and put heading examples inside closed code fences" +} + +// autoGroomBlockedMarkdown is the ## Auto-groom blocked body after one more +// abstain: the prior entries (when the section exists) followed by one new +// entry — "Recorded (UTC).", a blank line, then the note — so earlier +// entries are preserved, never replaced. +func autoGroomBlockedMarkdown(oldBody string, present bool, date, note string) string { + entry := "Recorded " + date + " (UTC).\n\n" + strings.TrimRight(note, "\r\n") + if present { + if trimmed := strings.Trim(oldBody, "\r\n"); trimmed != "" { + return trimmed + "\n\n" + entry + } + } + return entry +} + // hasAuthoredRationale reports whether the section edits carry at least one // replace with a non-empty body — the trivial outcome's required rationale. func hasAuthoredRationale(sections []SectionEditRequest) bool { @@ -482,7 +581,34 @@ func (o changeGroomOp) Plan(ctx context.Context, st transaction.AttemptState) (t } // Splice the owned proposal sections first, over the exact source bytes. - edited, err := render.ApplySectionEdits(src, render.ChangeOwnedHeadings, toSectionEdits(o.req.Sections)) + edits := toSectionEdits(o.req.Sections) + if o.req.Outcome == GroomAbstain { + // Append one dated entry, preserving earlier ones. The body is sliced at + // its NAMED terminator (the next top-level heading) by the same + // fence-aware scan the splice uses; a duplicate heading refuses. + oldBody, present, err := namedSectionBody(src, autoGroomBlockedHeading) + if err != nil { + return refuseGroom(string(FCSectionEditFailed), err.Error()) + } + edits = append(edits, render.SectionEdit{ + Heading: autoGroomBlockedHeading, Intent: render.SectionReplace, + Markdown: autoGroomBlockedMarkdown(oldBody, present, o.clock.Now().UTC().Format("2006-01-02"), o.req.BlockedNote), + }) + } + if o.req.Outcome == GroomRearm { + // Every transition out of the abstained state removes the marker whose + // presence encodes it (the board keys on it). With no marker and the flag + // already true there is nothing to re-arm. + blocked := namedSectionPresent(src, autoGroomBlockedHeading) + if ag := c.AutoGroomable(); !blocked && ag.State == domain.FieldPresent && ag.Value { + return refuseGroom(string(FCNothingToRearm), + fmt.Sprintf("change %04d has no %s section and is already auto_groomable: true; there is nothing to re-arm", o.req.ChangeID, autoGroomBlockedHeading)) + } + if blocked { + edits = append(edits, render.SectionEdit{Heading: autoGroomBlockedHeading, Intent: render.SectionRemove}) + } + } + edited, err := render.ApplySectionEdits(src, render.ChangeOwnedHeadings, edits) if err != nil { return refuseGroom("section-edit-failed", err.Error()) } @@ -512,6 +638,14 @@ func (o changeGroomOp) Plan(ctx context.Context, st transaction.AttemptState) (t if o.req.Outcome == GroomTrivial { ps.SetField("trivial", document.Bool(true)) } + if o.req.Outcome == GroomAbstain { + // upsertField: a record without the key (hand-authored or pre-template) + // gets it inserted rather than failing on a missing patch target. + upsertField(&ps, doc1, "auto_groomable", document.Bool(false)) + } + if o.req.Outcome == GroomRearm { + upsertField(&ps, doc1, "auto_groomable", document.Bool(true)) + } // upsertField (not bare SetField): the updated: field is inserted when a record // lacks it (a Bash-era or hand-authored record), so this op degrades like the // ADR ops, which upsert the same field, rather than internal-erroring with a diff --git a/internal/app/change_groom_test.go b/internal/app/change_groom_test.go index ae1d01606..d849370cb 100644 --- a/internal/app/change_groom_test.go +++ b/internal/app/change_groom_test.go @@ -6,6 +6,7 @@ import ( "github.com/danielhanold/docket/internal/domain" "github.com/danielhanold/docket/internal/render" "github.com/danielhanold/docket/internal/repository/transaction" + "slices" "strings" "testing" ) @@ -959,3 +960,451 @@ func TestChangeGroomResultHumanTextRevise(t *testing.T) { t.Errorf("HumanText = %q, want %q", got, want) } } + +// abstainRequest is a well-formed abstain request against the groomable fixture +// at id 2 / slug add-a-widget. The note uses a ### subsection, which is legal. +func abstainRequest() ChangeGroomRequest { + return ChangeGroomRequest{ + ChangeID: 2, + Path: groomPath(2, "add-a-widget"), + Version: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + Outcome: GroomAbstain, + BlockedNote: "The storage decision needs a human.\n\n### What to supply\n\nPick the backend.\n", + } +} + +// abstainedChange is the groomable fixture after one earlier abstain: an +// explicit auto_groomable: false and a one-entry ## Auto-groom blocked section. +func abstainedChange(id int, slug string) string { + return strings.Replace(groomableChange(id, slug), "trivial: false\n", "trivial: false\nauto_groomable: false\n", 1) + + "\n## Auto-groom blocked\n\nRecorded 2026-08-01 (UTC).\n\nFirst note.\n" +} + +func TestChangeGroomAbstainShapeValidation(t *testing.T) { + one := 1 + cases := []struct { + name string + mut func(*ChangeGroomRequest) + code string // "" means the request must pass shape validation + }{ + {"valid abstain passes", func(r *ChangeGroomRequest) {}, ""}, + {"blank blocked_note", func(r *ChangeGroomRequest) { r.BlockedNote = " \n" }, "empty-blocked_note"}, + {"blocked_note smuggling a structural heading", func(r *ChangeGroomRequest) { + r.BlockedNote = "Context.\n\n## Why\n\nsmuggled\n" + }, "invalid-blocked_note"}, + {"blocked_note with an unterminated fence", func(r *ChangeGroomRequest) { + r.BlockedNote = "Context.\n\n```\nnever closed\n" + }, "invalid-blocked_note"}, + {"abstain with sections", func(r *ChangeGroomRequest) { + r.Sections = []SectionEditRequest{{Heading: "## Why", Intent: "replace", Markdown: "rewrite\n"}} + }, "invalid-sections"}, + {"abstain with spec_markdown", func(r *ChangeGroomRequest) { r.SpecMarkdown = "# Design\n" }, "invalid-spec_markdown"}, + {"abstain with spec_version", func(r *ChangeGroomRequest) { r.SpecVersion = "a" }, "invalid-spec_version"}, + {"abstain with depends_on", func(r *ChangeGroomRequest) { r.DependsOn = []int{1} }, "invalid-depends_on"}, + {"abstain with an explicit empty related", func(r *ChangeGroomRequest) { r.Related = []int{} }, "invalid-related"}, + {"abstain with discovered_from", func(r *ChangeGroomRequest) { r.DiscoveredFrom = []int{1} }, "invalid-discovered_from"}, + {"abstain with adrs", func(r *ChangeGroomRequest) { r.ADRs = []int{1} }, "invalid-adrs"}, + {"abstain with stacked_on", func(r *ChangeGroomRequest) { r.StackedOn = &one }, "invalid-stacked_on"}, + {"blocked_note on the spec outcome", func(r *ChangeGroomRequest) { + r.Outcome, r.SpecMarkdown = GroomSpec, "# Design\n" + }, "invalid-blocked_note"}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + req := abstainRequest() + c.mut(&req) + findings := validateChangeGroomShape(req) + if c.code == "" { + if len(findings) != 0 { + t.Fatalf("want no findings, got %v", findings) + } + return + } + if !hasFindingCode(findings, c.code) { + t.Errorf("missing finding %q; got %v", c.code, findings) + } + }) + } +} + +func TestChangeGroomAbstainBadNoteRefusedWithoutEngineCall(t *testing.T) { + req := abstainRequest() + req.BlockedNote = "Context.\n\n## Why\n\nsmuggled\n" + engine := &recordingEngine{} + deps := PlanningDeps{Engine: engine, Reader: &fakeChangeReader{pin: mainModePin([]string{"inline"})}, Clock: testClock()} + + res := ChangeGroom(context.Background(), deps, "", req) + + if res.Result != ResultInvalidInput || len(engine.calls) != 0 { + t.Fatalf("result = %q with %d engine calls, want invalid-input and none", res.Result, len(engine.calls)) + } + // The refusal must come from the shape check itself: an empty repoDir also + // yields invalid-input further down, so the result alone would pass even if + // blocked_note were never validated. + if !hasFindingCode(res.Findings, "invalid-blocked_note") { + t.Errorf("missing invalid-blocked_note; got %v", res.Findings) + } +} + +func TestChangeGroomPlanAbstainSetsFlagSectionAndBoard(t *testing.T) { + cases := []struct { + name string + rec string + }{ + // Review Focus 4: a record with no auto_groomable key gets it inserted. + {"field absent", groomableChange(2, "add-a-widget")}, + {"field armed true", strings.Replace(groomableChange(2, "add-a-widget"), "trivial: false\n", "trivial: false\nauto_groomable: true\n", 1)}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + files := map[string]string{ + groomPath(2, "add-a-widget"): c.rec, + "docs/changes/BOARD.md": "# Backlog\n\nold\n", + } + plan, opRes := groomPlanFor(t, files, baseGroomOp([]string{"inline"}, abstainRequest())) + if opRes.Refused { + t.Fatalf("unexpected refusal: %v", opRes.Findings) + } + assertPlanPaths(t, plan, map[string]transaction.MutationKind{ + groomPath(2, "add-a-widget"): transaction.MutationReplace, + "docs/changes/BOARD.md": transaction.MutationReplace, + }) + rec := string(groomedRecordBytes(t, plan, groomPath(2, "add-a-widget"))) + for _, want := range []string{ + "\nauto_groomable: false\n", + "updated: '2026-08-16'", + "## Auto-groom blocked\n\nRecorded 2026-08-16 (UTC).\n\nThe storage decision needs a human.", + "### What to supply", + "\nspec:\n", "trivial: false", // groom scalars untouched + "Original why.", "An open question.", // proposal untouched + } { + if !strings.Contains(rec, want) { + t.Errorf("record missing %q:\n%s", want, rec) + } + } + if strings.Contains(rec, "auto_groomable: true") { + t.Errorf("abstain left the stub armed:\n%s", rec) + } + board := string(groomedRecordBytes(t, plan, "docs/changes/BOARD.md")) + if !strings.Contains(board, "auto-groom blocked — needs you") { + t.Errorf("board row did not flip to auto-groom blocked in the same plan:\n%s", board) + } + var receipt changeGroomReceipt + if err := json.Unmarshal(plan.Receipt, &receipt); err != nil || receipt.Outcome != "abstain" || receipt.SpecPath != "" { + t.Errorf("receipt = %s (%v), want outcome abstain and no spec_path", plan.Receipt, err) + } + }) + } +} + +func TestChangeGroomPlanAbstainAppendsSecondEntry(t *testing.T) { + files := map[string]string{groomPath(2, "add-a-widget"): abstainedChange(2, "add-a-widget")} + plan, opRes := groomPlanFor(t, files, baseGroomOp([]string{}, abstainRequest())) + if opRes.Refused { + t.Fatalf("a second abstain must append, not refuse: %v", opRes.Findings) + } + rec := string(groomedRecordBytes(t, plan, groomPath(2, "add-a-widget"))) + if n := strings.Count(rec, "## Auto-groom blocked"); n != 1 { + t.Fatalf("section heading appears %d times, want exactly 1:\n%s", n, rec) + } + first := strings.Index(rec, "Recorded 2026-08-01 (UTC).\n\nFirst note.") + second := strings.Index(rec, "Recorded 2026-08-16 (UTC).\n\nThe storage decision needs a human.") + if first < 0 || second < 0 || first > second { + t.Errorf("entries missing or out of order (first=%d second=%d):\n%s", first, second, rec) + } +} + +func TestChangeGroomPlanAbstainRefusesNonGroomable(t *testing.T) { + cases := []struct { + name string + rec string + }{ + {"spec'd", revisableChange(2, "add-a-widget", reviseSpecPath)}, + {"trivial", trivialChange(2, "add-a-widget")}, + {"not proposed", strings.Replace(groomableChange(2, "add-a-widget"), "status: proposed\n", "status: blocked\nblocked_by: 'waiting on infra'\n", 1)}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + files := map[string]string{groomPath(2, "add-a-widget"): c.rec} + if c.name == "spec'd" { + files[reviseSpecPath] = reviseFixtureFiles()[reviseSpecPath] + } + plan, opRes := groomPlanFor(t, files, baseGroomOp([]string{}, abstainRequest())) + if !opRes.Refused || len(plan.Files) != 0 { + t.Fatalf("want a refusal writing nothing, got refused=%v files=%v", opRes.Refused, planPaths(plan)) + } + found := false + for _, f := range opRes.Findings { + found = found || f.Code == "not-groomable" + } + if !found { + t.Errorf("missing not-groomable; got %v", opRes.Findings) + } + }) + } +} + +func TestChangeGroomResultHumanTextAbstain(t *testing.T) { + r := newChangeGroomResult(ResultApplied, ChangeGroomResult{ID: 7, Outcome: string(GroomAbstain), Revision: "cafe"}) + if got, want := r.HumanText(), "change 0007 auto-groom abstained — cafe"; got != want { + t.Errorf("HumanText = %q, want %q", got, want) + } +} + +// rearmRequest is a well-formed rearm request against the fixture at id 2. +func rearmRequest() ChangeGroomRequest { + return ChangeGroomRequest{ + ChangeID: 2, + Path: groomPath(2, "add-a-widget"), + Version: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + Outcome: GroomRearm, + } +} + +func TestChangeGroomRearmShapeValidation(t *testing.T) { + cases := []struct { + name string + mut func(*ChangeGroomRequest) + code string + }{ + {"bare rearm passes", func(r *ChangeGroomRequest) {}, ""}, + {"rearm with owned-section edits passes", func(r *ChangeGroomRequest) { + r.Sections = []SectionEditRequest{{Heading: "## Open questions", Intent: "replace", Markdown: "Resolved.\n"}} + }, ""}, + {"rearm with blocked_note", func(r *ChangeGroomRequest) { r.BlockedNote = "x\n" }, "invalid-blocked_note"}, + {"rearm with spec_markdown", func(r *ChangeGroomRequest) { r.SpecMarkdown = "# Design\n" }, "invalid-spec_markdown"}, + {"rearm with spec_version", func(r *ChangeGroomRequest) { r.SpecVersion = "a" }, "invalid-spec_version"}, + // Review Focus 5: the op removes this section itself. + {"rearm editing ## Auto-groom blocked", func(r *ChangeGroomRequest) { + r.Sections = []SectionEditRequest{{Heading: "## Auto-groom blocked", Intent: "replace", Markdown: "x\n"}} + }, "invalid-section-heading"}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + req := rearmRequest() + c.mut(&req) + findings := validateChangeGroomShape(req) + if c.code == "" { + if len(findings) != 0 { + t.Fatalf("want no findings, got %v", findings) + } + return + } + if !hasFindingCode(findings, c.code) { + t.Errorf("missing finding %q; got %v", c.code, findings) + } + }) + } +} + +func TestChangeGroomPlanRearmClearsSectionSetsFlagAndBoard(t *testing.T) { + files := map[string]string{ + groomPath(2, "add-a-widget"): abstainedChange(2, "add-a-widget"), + "docs/changes/BOARD.md": "# Backlog\n\nold\n", + } + plan, opRes := groomPlanFor(t, files, baseGroomOp([]string{"inline"}, rearmRequest())) + if opRes.Refused { + t.Fatalf("unexpected refusal: %v", opRes.Findings) + } + rec := string(groomedRecordBytes(t, plan, groomPath(2, "add-a-widget"))) + if strings.Contains(rec, "## Auto-groom blocked") || strings.Contains(rec, "First note.") { + t.Errorf("re-arm left the presence-encoded section behind:\n%s", rec) + } + if !strings.Contains(rec, "\nauto_groomable: true\n") || strings.Contains(rec, "auto_groomable: false") { + t.Errorf("re-arm did not set auto_groomable: true:\n%s", rec) + } + board := string(groomedRecordBytes(t, plan, "docs/changes/BOARD.md")) + if strings.Contains(board, "auto-groom blocked — needs you") || !strings.Contains(board, "needs-brainstorm") { + t.Errorf("board row did not return to needs-brainstorm in the same plan:\n%s", board) + } + var receipt changeGroomReceipt + if err := json.Unmarshal(plan.Receipt, &receipt); err != nil || receipt.Outcome != "rearm" { + t.Errorf("receipt = %s (%v), want outcome rearm", plan.Receipt, err) + } +} + +func TestChangeGroomPlanRearmAppliesSectionEditsInOneRecord(t *testing.T) { + req := rearmRequest() + req.Sections = []SectionEditRequest{{Heading: "## Open questions", Intent: "replace", Markdown: "Resolved: use SQLite.\n"}} + files := map[string]string{groomPath(2, "add-a-widget"): abstainedChange(2, "add-a-widget")} + plan, opRes := groomPlanFor(t, files, baseGroomOp([]string{}, req)) + if opRes.Refused { + t.Fatalf("unexpected refusal: %v", opRes.Findings) + } + assertPlanPaths(t, plan, map[string]transaction.MutationKind{groomPath(2, "add-a-widget"): transaction.MutationReplace}) + rec := string(groomedRecordBytes(t, plan, groomPath(2, "add-a-widget"))) + if !strings.Contains(rec, "Resolved: use SQLite.") || strings.Contains(rec, "An open question.") || strings.Contains(rec, "## Auto-groom blocked") { + t.Errorf("section edit and section removal did not both land:\n%s", rec) + } +} + +func TestChangeGroomPlanRearmArmsWithoutABlockedSection(t *testing.T) { + cases := []struct { + name string + rec string + }{ + // Review Focus 4: the key is inserted when absent (inherit ⇒ explicit true). + {"field absent", groomableChange(2, "add-a-widget")}, + {"opted out", strings.Replace(groomableChange(2, "add-a-widget"), "trivial: false\n", "trivial: false\nauto_groomable: false\n", 1)}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + plan, opRes := groomPlanFor(t, map[string]string{groomPath(2, "add-a-widget"): c.rec}, baseGroomOp([]string{}, rearmRequest())) + if opRes.Refused { + t.Fatalf("unexpected refusal: %v", opRes.Findings) + } + if rec := string(groomedRecordBytes(t, plan, groomPath(2, "add-a-widget"))); !strings.Contains(rec, "\nauto_groomable: true\n") { + t.Errorf("auto_groomable: true not written:\n%s", rec) + } + }) + } +} + +func TestChangeGroomPlanRearmRefusals(t *testing.T) { + armed := strings.Replace(groomableChange(2, "add-a-widget"), "trivial: false\n", "trivial: false\nauto_groomable: true\n", 1) + cases := []struct { + name string + files map[string]string + code string + }{ + {"nothing to re-arm", map[string]string{groomPath(2, "add-a-widget"): armed}, "nothing-to-rearm"}, + {"trivial", map[string]string{groomPath(2, "add-a-widget"): trivialChange(2, "add-a-widget")}, "not-groomable"}, + {"spec'd", reviseFixtureFiles(), "not-groomable"}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + plan, opRes := groomPlanFor(t, c.files, baseGroomOp([]string{}, rearmRequest())) + if !opRes.Refused || len(plan.Files) != 0 { + t.Fatalf("want a refusal writing nothing, got refused=%v files=%v", opRes.Refused, planPaths(plan)) + } + found := false + for _, f := range opRes.Findings { + found = found || f.Code == c.code + } + if !found { + t.Errorf("missing %q; got %v", c.code, opRes.Findings) + } + }) + } +} + +// TestChangeGroomRearmFencedMarkerAgreesWithBoard — a heading-shaped +// "## Auto-groom blocked" line inside fenced code is not the abstain section: +// the decoded record (which the board's cell keys on) and rearm's section scan +// must agree, so the board never shows "needs you" on a record rearm reports +// as having nothing to re-arm. +func TestChangeGroomRearmFencedMarkerAgreesWithBoard(t *testing.T) { + rec := strings.Replace(groomableChange(2, "add-a-widget"), "trivial: false\n", "trivial: false\nauto_groomable: true\n", 1) + + "\n## Notes\n\n```md\n## Auto-groom blocked\n```\n" + files := map[string]string{groomPath(2, "add-a-widget"): rec} + before, err := newPlanningLoader(planningTestConfig([]string{})).Load(context.Background(), newFakeTree(files)) + if err != nil { + t.Fatalf("loader.Load: %v", err) + } + c, out := before.Snapshot.Change(2) + if out != domain.LookupFound { + t.Fatalf("change 2 lookup = %v", out) + } + if c.HasAutoGroomBlocked() { + t.Errorf("decoded record reports a fenced heading as the abstain marker; the board would show it blocked") + } + plan, opRes := groomPlanFor(t, files, baseGroomOp([]string{}, rearmRequest())) + found := false + for _, f := range opRes.Findings { + found = found || f.Code == "nothing-to-rearm" + } + if !opRes.Refused || len(plan.Files) != 0 || !found { + t.Errorf("rearm over a fenced marker: refused=%v files=%v findings=%v, want nothing-to-rearm", opRes.Refused, planPaths(plan), opRes.Findings) + } +} + +func TestChangeGroomResultHumanTextRearm(t *testing.T) { + r := newChangeGroomResult(ResultApplied, ChangeGroomResult{ID: 7, Outcome: string(GroomRearm), Revision: "cafe"}) + if got, want := r.HumanText(), "change 0007 re-armed for auto-groom — cafe"; got != want { + t.Errorf("HumanText = %q, want %q", got, want) + } +} + +// TestChangeGroomAbstainThenRearmRealGit drives both outcomes through the real +// engine and a bare origin: the abstain lands the record and BOARD.md in ONE +// commit; a re-arm pinned to the pre-abstain version contends and writes +// nothing; a re-arm at the current version restores needs-brainstorm. +func TestChangeGroomAbstainThenRearmRealGit(t *testing.T) { + requireRealGit(t) + recPath := groomPath(2, "add-a-widget") + repo := newWorkingRepo(t, map[string]string{recPath: groomableChange(2, "add-a-widget")}) + node := planningDepsFor(t, repo.invocation) + + ab := abstainRequest() + ab.Version = blobVersionAt(t, repo.origin, "docket", recPath) + if res := ChangeGroom(context.Background(), node.deps, node.dir, ab); res.Result != ResultApplied { + t.Fatalf("abstain = %q (findings %v), want applied", res.Result, res.Findings) + } + tip := originTip(t, repo.origin, "docket") + paths := originCommitPaths(t, repo.origin, tip) + if !slices.Contains(paths, recPath) || !slices.Contains(paths, "docs/changes/BOARD.md") { + t.Fatalf("abstain commit paths = %v, want the record and BOARD.md in one commit", paths) + } + if board, _ := originFile(t, repo.origin, "docket", "docs/changes/BOARD.md"); !strings.Contains(board, "auto-groom blocked — needs you") { + t.Errorf("committed board does not show the abstain:\n%s", board) + } + + stale := rearmRequest() + stale.Version = ab.Version // pre-abstain pin + if res := ChangeGroom(context.Background(), node.deps, node.dir, stale); res.Result != ResultContended { + t.Fatalf("stale re-arm = %q (findings %v), want contended", res.Result, res.Findings) + } + if got := originTip(t, repo.origin, "docket"); got != tip { + t.Fatalf("a contended re-arm moved the metadata branch %s -> %s", tip, got) + } + + fresh := rearmRequest() + fresh.Version = blobVersionAt(t, repo.origin, "docket", recPath) + if res := ChangeGroom(context.Background(), node.deps, node.dir, fresh); res.Result != ResultApplied { + t.Fatalf("re-arm = %q (findings %v), want applied", res.Result, res.Findings) + } + rec, _ := originFile(t, repo.origin, "docket", recPath) + board, _ := originFile(t, repo.origin, "docket", "docs/changes/BOARD.md") + if strings.Contains(rec, "## Auto-groom blocked") || !strings.Contains(rec, "\nauto_groomable: true\n") { + t.Errorf("re-armed record:\n%s", rec) + } + if strings.Contains(board, "auto-groom blocked — needs you") { + t.Errorf("committed board still shows the abstain after re-arm:\n%s", board) + } +} + +// followedSection is a non-final section placed after ## Auto-groom blocked so +// abstain-append and rearm-removal are proven not to consume what follows. +const followedSection = "## Reconcile log\n\nKept entry.\n" + +func TestChangeGroomPlanAbstainAppendsBeforeFollowingSection(t *testing.T) { + files := map[string]string{groomPath(2, "add-a-widget"): abstainedChange(2, "add-a-widget") + "\n" + followedSection} + plan, opRes := groomPlanFor(t, files, baseGroomOp([]string{}, abstainRequest())) + if opRes.Refused { + t.Fatalf("unexpected refusal: %v", opRes.Findings) + } + rec := string(groomedRecordBytes(t, plan, groomPath(2, "add-a-widget"))) + first := strings.Index(rec, "Recorded 2026-08-01 (UTC).\n\nFirst note.") + second := strings.Index(rec, "Recorded 2026-08-16 (UTC).\n\nThe storage decision needs a human.") + if first < 0 || second < 0 || first > second { + t.Errorf("entries missing or out of order (first=%d second=%d):\n%s", first, second, rec) + } + if strings.Count(rec, followedSection) != 1 || !strings.HasSuffix(rec, followedSection) { + t.Errorf("following section not preserved byte-identically:\n%s", rec) + } +} + +func TestChangeGroomPlanRearmRemovesSectionBeforeFollowingSection(t *testing.T) { + files := map[string]string{groomPath(2, "add-a-widget"): abstainedChange(2, "add-a-widget") + "\n" + followedSection} + plan, opRes := groomPlanFor(t, files, baseGroomOp([]string{}, rearmRequest())) + if opRes.Refused { + t.Fatalf("unexpected refusal: %v", opRes.Findings) + } + rec := string(groomedRecordBytes(t, plan, groomPath(2, "add-a-widget"))) + if strings.Contains(rec, "## Auto-groom blocked") || strings.Contains(rec, "First note.") { + t.Errorf("re-arm left the blocked section behind:\n%s", rec) + } + if strings.Count(rec, followedSection) != 1 || !strings.HasSuffix(rec, followedSection) { + t.Errorf("following section not preserved byte-identically:\n%s", rec) + } +} diff --git a/internal/app/finding_codes.go b/internal/app/finding_codes.go index 4258c48bb..32096861e 100644 --- a/internal/app/finding_codes.go +++ b/internal/app/finding_codes.go @@ -39,6 +39,7 @@ const ( // State-shape refusals built by refuseLifecycle inside a Plan closure. FCArtifactRenderFailed FindingCode = "artifact-render-failed" FCNotFound FindingCode = "not-found" + FCNothingToRearm FindingCode = "nothing-to-rearm" FCPathMismatch FindingCode = "path-mismatch" FCSectionEditFailed FindingCode = "section-edit-failed" @@ -102,6 +103,7 @@ const ( // can produce, so the vocabulary is closed over every value that can appear. FCInvalidRequestID FindingCode = "invalid-request_id" FCInvalidStackedOn FindingCode = "invalid-stacked_on" + FCInvalidBranchPrefix FindingCode = "invalid-branch_prefix" FCEmptyTitle FindingCode = "empty-title" FCEmptyWhy FindingCode = "empty-why" FCEmptyWhatChanges FindingCode = "empty-what_changes" @@ -126,6 +128,9 @@ const ( FCInvalidSpecVersion FindingCode = "invalid-spec_version" FCMissingRationale FindingCode = "missing-rationale" FCEmptyRevise FindingCode = "empty-revise" + FCEmptyBlockedNote FindingCode = "empty-blocked_note" + FCInvalidBlockedNote FindingCode = "invalid-blocked_note" + FCInvalidSections FindingCode = "invalid-sections" FCInvalidOutcome FindingCode = "invalid-outcome" FCInvalidSpecSectionHeading FindingCode = "invalid-spec-section-heading" FCEmptyReconcileLogEntry FindingCode = "empty-reconcile_log_entry" @@ -193,6 +198,7 @@ var AllFindingCodes = []FindingCode{ FCEmptyAlternatives, FCEmptyApply, FCEmptyAttempt, + FCEmptyBlockedNote, FCEmptyChangePath, FCEmptyChangeVersion, FCEmptyChildPRVersion, @@ -228,6 +234,8 @@ var AllFindingCodes = []FindingCode{ FindingCode(ReasonStatusInterrupted), FCInvalidADRs, FCInvalidAttempt, + FCInvalidBlockedNote, + FCInvalidBranchPrefix, FCInvalidChangeDotID, FCInvalidChangeID, FCInvalidChanges, @@ -247,6 +255,7 @@ var AllFindingCodes = []FindingCode{ FCInvalidSectionHeading, FCInvalidSectionIntent, FCInvalidSectionMarkdown, + FCInvalidSections, FCInvalidSlug, FCInvalidSpecSectionHeading, FCInvalidSpecMarkdown, @@ -266,6 +275,7 @@ var AllFindingCodes = []FindingCode{ FindingCode("missing-claim-stamp"), FCMissingRationale, FCNotFound, + FCNothingToRearm, FCParseFailed, FCPathMismatch, FCPrepareLocalStateUnknown, diff --git a/internal/app/finding_codes_test.go b/internal/app/finding_codes_test.go index be761ec91..bccac5249 100644 --- a/internal/app/finding_codes_test.go +++ b/internal/app/finding_codes_test.go @@ -224,6 +224,7 @@ func TestShapeValidatorCodesAreRegistered(t *testing.T) { var emitted []StatusFinding emitted = append(emitted, validateChangeCreateShape(ChangeCreateRequest{StackedOn: &zero})...) + emitted = append(emitted, validateChangeCreateShape(ChangeCreateRequest{BranchPrefix: "a/b"})...) emitted = append(emitted, validateADRRecordShape(adrContent)...) emitted = append(emitted, validateADRReplaceShape(ADRReplaceRequest{Target: ADRTarget{ID: 0}, Successor: adrContent})...) emitted = append(emitted, validateLearningRecordShape(LearningRecordRequest{Topics: []string{""}})...) @@ -233,6 +234,8 @@ func TestShapeValidatorCodesAreRegistered(t *testing.T) { emitted = append(emitted, validateChangeGroomShape(ChangeGroomRequest{Outcome: GroomRevise, SpecMarkdown: "# x\n"})...) emitted = append(emitted, validateChangeGroomShape(ChangeGroomRequest{Outcome: GroomTrivial, SpecVersion: "a"})...) emitted = append(emitted, validateChangeGroomShape(ChangeGroomRequest{Outcome: GroomOutcome("bogus")})...) + emitted = append(emitted, validateChangeGroomShape(ChangeGroomRequest{Outcome: GroomAbstain, Sections: []SectionEditRequest{{Heading: "## Why", Intent: "remove"}}})...) + emitted = append(emitted, validateChangeGroomShape(ChangeGroomRequest{Outcome: GroomTrivial, BlockedNote: "x"})...) emitted = append(emitted, validateChangeReconcileShape(ChangeReconcileRequest{ Sections: map[string]string{"## Not Owned": "x"}, SpecSections: map[string]string{"not a heading": "x"}, @@ -251,7 +254,7 @@ func TestShapeValidatorCodesAreRegistered(t *testing.T) { // Floor set: concrete codes registered by this fix that these requests must // reach. A miss means the mint path drifted from the registered constant. floor := []FindingCode{ - FCInvalidRequestID, FCInvalidStackedOn, + FCInvalidRequestID, FCInvalidStackedOn, FCInvalidBranchPrefix, FCEmptyTitle, FCEmptyWhy, FCEmptyWhatChanges, FCEmptyOutOfScope, FCEmptyContext, FCEmptyDecision, FCEmptyConsequences, FCEmptyAlternatives, FCInvalidChangeDotID, FCEmptyChangePath, FCEmptyChangeVersion, @@ -259,6 +262,7 @@ func TestShapeValidatorCodesAreRegistered(t *testing.T) { FCEmptyHook, FCEmptyApply, FCEmptyWarStory, FCInvalidTopics, FCEmptySpecMarkdown, FCEmptySpecVersion, FCInvalidSpecVersion, FCMissingRationale, FCInvalidOutcome, FCInvalidSpecSectionHeading, FCEmptyReconcileLogEntry, + FCEmptyBlockedNote, FCInvalidBlockedNote, FCInvalidSections, FCInvalidPRNumber, FCInvalidAttempt, FCEmptyHead, } for _, c := range floor { diff --git a/internal/app/schema_test.go b/internal/app/schema_test.go index b178b21b1..3776e456f 100644 --- a/internal/app/schema_test.go +++ b/internal/app/schema_test.go @@ -101,6 +101,9 @@ func TestReflectDescriptorChangeGroomRequest(t *testing.T) { if f := fieldByKey(t, d, "spec_version"); f.Required || f.Type != "string" { t.Errorf("spec_version = %+v, want optional string", f) } + if f := fieldByKey(t, d, "blocked_note"); f.Required || f.Type != "string" { + t.Errorf("blocked_note = %+v, want optional string", f) + } // No caller-supplied spec path: the spec pinned is always the one the // record's spec: field links. for _, f := range d.Fields { @@ -140,6 +143,7 @@ func TestReflectDescriptorChangeCreateRequest(t *testing.T) { "request_id", "title", "type", "priority", "why", "what_changes", "out_of_scope", "depends_on", "stacked_on", "related", "discovered_from", "adrs", + "auto_groomable", "branch_prefix", } if !reflect.DeepEqual(keys, want) { t.Fatalf("keys = %v, want %v", keys, want) @@ -156,6 +160,12 @@ func TestReflectDescriptorChangeCreateRequest(t *testing.T) { if so.Repeated || so.Type != "int" { t.Errorf("stacked_on = %+v, want non-repeated int", so) } + if f := fieldByKey(t, d, "auto_groomable"); f.Repeated || f.Type != "bool" { + t.Errorf("auto_groomable = %+v, want optional scalar bool", f) + } + if f := fieldByKey(t, d, "branch_prefix"); f.Repeated || f.Type != "string" { + t.Errorf("branch_prefix = %+v, want optional string", f) + } // Required set per Task 4's tags. wantRequired := map[string]bool{ diff --git a/internal/app/schema_vocab.go b/internal/app/schema_vocab.go index 793204aa6..fba26a701 100644 --- a/internal/app/schema_vocab.go +++ b/internal/app/schema_vocab.go @@ -68,7 +68,7 @@ func SchemaVocabularies(effects []string) map[string]Vocabulary { v["section_intents"] = Vocabulary{Members: members(len(render.AllSectionIntents), func(i int) string { return string(render.AllSectionIntents[i]) })} - v["groom_outcomes"] = Vocabulary{Members: []string{string(GroomSpec), string(GroomTrivial), string(GroomRevise)}} + v["groom_outcomes"] = Vocabulary{Members: []string{string(GroomSpec), string(GroomTrivial), string(GroomRevise), string(GroomAbstain), string(GroomRearm)}} v["change_types"] = Vocabulary{Pattern: changeTypePattern} v["effects"] = Vocabulary{Members: effects} diff --git a/internal/app/schema_vocab_test.go b/internal/app/schema_vocab_test.go index f362d3581..514a64fa7 100644 --- a/internal/app/schema_vocab_test.go +++ b/internal/app/schema_vocab_test.go @@ -45,7 +45,7 @@ func TestSchemaVocabulariesCore(t *testing.T) { assertVocabMembers(t, v, "priorities", []string{"critical", "high", "medium", "low"}) assertVocabMembers(t, v, "section_intents", []string{"preserve", "replace", "remove"}) - assertVocabMembers(t, v, "groom_outcomes", []string{"spec", "trivial", "revise"}) + assertVocabMembers(t, v, "groom_outcomes", []string{"spec", "trivial", "revise", "abstain", "rearm"}) assertVocabMembers(t, v, "statuses", []string{ "proposed", "in-progress", "blocked", "deferred", "implemented", "stacked-merged", "done", "killed", diff --git a/internal/assets/embedded/manifest.json b/internal/assets/embedded/manifest.json index 8902df6c7..7f27deaf3 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:f707a7ef0b87623c80f942469aafc407317973b44ea64e52c5fffad18d5cfdb4", + "asset_set_id": "sha256:2bcf93ecad4fae175b3bfba5657d2abe90bbaf431b324da317ef451c9e1b5a1e", "entries": [ { "path": ".docket.example.yml", @@ -287,8 +287,8 @@ "path": "skills/docket-auto-groom/SKILL.md", "role": "skill", "mode": 420, - "size": 11745, - "sha256": "e75f32bbace723851dbdae1473ade8817a57f913252276d6d8f8373a679d6138" + "size": 11285, + "sha256": "1e29726ae744d85735ecb021e60ea1bd000048e39d50dd51160bdfd78114c063" }, { "path": "skills/docket-brainstorm/SKILL.md", @@ -343,8 +343,8 @@ "path": "skills/docket-convention/SKILL.md", "role": "skill", "mode": 420, - "size": 57097, - "sha256": "eea0bb2c4f9792287320364d02b4c281a029ad52dc7acb769c5a0000d338be58" + "size": 57405, + "sha256": "633761a9a49770209132d72d256f5c40f85d79d7ff363aa449618f4d86928e5f" }, { "path": "skills/docket-convention/references/agent-layer.md", @@ -399,8 +399,8 @@ "path": "skills/docket-groom-next/SKILL.md", "role": "skill", "mode": 420, - "size": 12889, - "sha256": "59bf547fd16711a60708531573549bd70a5a2d0f325050f7323a4389e54233d8" + "size": 13628, + "sha256": "fd6785c3d9f88f22fbf81fe271b8f4af338ed7992de2991978ece9513231132d" }, { "path": "skills/docket-implement-next/SKILL.md", @@ -434,8 +434,8 @@ "path": "skills/docket-new-change/SKILL.md", "role": "skill", "mode": 420, - "size": 11796, - "sha256": "8df6492648b47579aeafd2e1f6809ccd423b60857fa1ac184cf4ce44228a8dd1" + "size": 11588, + "sha256": "68d0c6f6203d00971df81aa4d7564631dc515f7ec9dd99e65e9c7d7aa44e8f41" }, { "path": "skills/docket-new-change/change-template.md", diff --git a/internal/assets/embedded/tree/skills/docket-auto-groom/SKILL.md b/internal/assets/embedded/tree/skills/docket-auto-groom/SKILL.md index 73f6f1eaa..a636aea69 100644 --- a/internal/assets/embedded/tree/skills/docket-auto-groom/SKILL.md +++ b/internal/assets/embedded/tree/skills/docket-auto-groom/SKILL.md @@ -41,21 +41,19 @@ Dispatch the dedicated **`docket-auto-groom-critic`** subagent (foreground, at t **Receiving the verdict.** The verdict is read from the critic's **return** — its final report, which the groom is actively blocking on; the groom never backgrounds the critic. The groom never waits for a message, a notification, or any other out-of-band delivery: nothing is registered to deliver one, so that wait never ends. -**No-verdict posture (bounded — two steps, then out).** If the dispatch returns no legible verdict — a malformed return, pre-yield prose, or a backgrounded child's bare completion — make **one collect attempt** (read the child's completed final report where the harness surfaces it), and failing that **one fresh foreground re-dispatch** of the critic over the same draft, issued through whatever mechanism makes the parent block on the return — if none does, that leg would only repeat the first, so skip it straight to Tier B. Still no verdict ⇒ treat it as a failed dispatch attempt under the convention's *Dispatch-capability resolution*: **Tier B**, so the groom **abstains** for this stub (→ Step 4's **Abstain** exit in full, the `auto_groomable: false` flip included — left armed, the stub stays autonomous-eligible and the drain re-selects it, forfeiting *Termination & concurrency*), recording the return-channel diagnostic in the `## Auto-groom blocked` section, the human's re-arm cue. Never a third dispatch; never an indefinite wait. Re-dispatching a critic is safe where a build worker is not — it is read-only over prose, holds no worktree, and writes no git state, so `yielded-worker-return-closes-every-door`'s closed-doors analysis does not bind here. +**No-verdict posture (bounded — two steps, then out).** If the dispatch returns no legible verdict — a malformed return, pre-yield prose, or a backgrounded child's bare completion — make **one collect attempt** (read the child's completed final report where the harness surfaces it), and failing that **one fresh foreground re-dispatch** of the critic over the same draft, issued through whatever mechanism makes the parent block on the return — if none does, that leg would only repeat the first, so skip it straight to Tier B. Still no verdict ⇒ treat it as a failed dispatch attempt under the convention's *Dispatch-capability resolution*: **Tier B**, so the groom **abstains** for this stub (→ Step 4's **Abstain** exit in full, the `auto_groomable: false` flip included — left armed, the stub stays autonomous-eligible and the drain re-selects it, forfeiting *Termination & concurrency*), recording the return-channel diagnostic in the `blocked_note`, the human's re-arm cue. Never a third dispatch; never an indefinite wait. Re-dispatching a critic is safe where a build worker is not — it is read-only over prose, holds no worktree, and writes no git state, so `yielded-worker-return-closes-every-door`'s closed-doors analysis does not bind here. ### Step 4 — Exit (one of three) 1. **Spec** — every assumption survived: apply one atomic transaction — the `change.groom` operation with `--repo-dir .docket --request ` — carrying the change id, the pinned `path` + `version` from Step 1, `outcome: spec`, the `spec_markdown` (the settled design plus its `## Assumptions` block), the owned proposal-section rewrites (proposal altitude, resolved `## Open questions` removed), and the desired `depends_on`/`related`/`adrs`/`discovered_from`/`stacked_on`. In one metadata commit it writes the spec file, sets `spec:` + `updated:`, stamps the spec's `docket:backlink` block, and re-renders the `## Artifacts` block and inline board; a typed refusal (not-groomable, spec-path-taken, malformed markers, version mismatch) writes nothing. Build-ready. 2. **Trivial** — the critic confirmed no hidden design decisions: apply the `change.groom` operation with `--repo-dir .docket --request ` with `outcome: trivial`, the pinned `path` + `version`, and an owned-section edit carrying the tightened body and its reasoning as the trivial rationale. The transaction sets `trivial: true` + `updated:` and re-renders the `## Artifacts` block and inline board atomically. Build-ready, no spec. -3. **Abstain** — any needs-human-context verdict, or Step 3's exhausted no-verdict posture: emit NO spec; there is no typed groom for this outcome, so write it directly on the metadata tree — flip `auto_groomable: false` and append a dated `## Auto-groom blocked` section (the undecidable decision(s), what context is missing, what a human should supply, and any recommendation — including "this should probably be killed/deferred because …") — then commit that change-file edit with plain git plumbing (Step 5). The abstain changes no board-visible cell, so no board render is needed; the stub stays needs-brainstorm, first in `docket-groom-next`'s queue. +3. **Abstain** — any needs-human-context verdict, or Step 3's exhausted no-verdict posture: emit NO spec; apply the `change.groom` operation with `--repo-dir .docket --request ` with `outcome: abstain`, the pinned `path` + `version`, and a `blocked_note` — the undecidable decision(s), what context is missing, what a human should supply, and any recommendation (including "this should probably be killed/deferred because …"), with subsections at `###` or deeper. The transaction sets `auto_groomable: false` + `updated:`, appends a dated entry to the `## Auto-groom blocked` section, and re-renders the inline board — the row flips to **auto-groom blocked — needs you** in that same commit. It accepts no section, spec, or relationship edits. The stub stays needs-brainstorm, first in `docket-groom-next`'s queue. **Kill and defer are NEVER autonomous.** Verdict authority over the backlog's composition stays human; the strongest the drain may say is an abstain-with-recommendation. ### Step 5 — The outcome lands (no separate board pass) -For a **spec** or **trivial** exit the Step-4 `change.groom` operation is the whole write — it re-checks the pinned `version` and commits the record, the spec, the `## Artifacts` block, and the inline board in one metadata commit pushed under an exact-lease push, so there is **no separate Board pass**. On a `contended` refusal it writes nothing: re-sync (re-run the `repository.prepare` operation), re-read the stub's `path` + `version` from the `status` operation, and if it is no longer autonomous-eligible (groomed, killed, claimed, or opted out) DISCARD this iteration's draft (delete the just-drafted spec markdown) and loop; otherwise re-author and retry. - -For an **abstain** exit, commit the change-file edit (`auto_groomable: false` + the `## Auto-groom blocked` section) with plain git plumbing in the metadata working tree; push `origin/docket`. **Stage by explicit path** — that tree is shared, so a bare `add -A` commits another agent's staged work under your message. On a non-fast-forward rejection: re-sync (re-run the `repository.prepare` operation), and if the rebase brought in commits touching this stub's file, RE-READ it — no longer autonomous-eligible ⇒ DISCARD this iteration's writes for it (`git -C .docket restore -- `) and loop. Loop to step 1. +Every exit's Step-4 `change.groom` operation is the whole write — it re-checks the pinned `version` and commits the record, any spec, the `## Artifacts` block, and the inline board in one metadata commit pushed under an exact-lease push, so there is **no separate Board pass** and no hand-staged commit. On a `contended` refusal it writes nothing: re-sync (re-run the `repository.prepare` operation), re-read the stub's `path` + `version` from the `status` operation, and if it is no longer autonomous-eligible (groomed, killed, claimed, or opted out) DISCARD this iteration's draft (delete any just-drafted spec markdown) and loop; otherwise re-author and retry. Loop to step 1. ### Step 6 — Report diff --git a/internal/assets/embedded/tree/skills/docket-convention/SKILL.md b/internal/assets/embedded/tree/skills/docket-convention/SKILL.md index 4573a39dd..55800aa27 100644 --- a/internal/assets/embedded/tree/skills/docket-convention/SKILL.md +++ b/internal/assets/embedded/tree/skills/docket-convention/SKILL.md @@ -222,7 +222,7 @@ change, never in the merged artifact. - `## Reconcile log` — dated entries appended by the implementer's reconcile pass. - `## Closeout notes` — terminal-only, **optional**, and the **final authored body section** of a terminal record. Written solely by the `finalize.closeout` operation from its structured request (`verification_outcomes` / `late_findings`, rendered as `### Verification` / `### Late findings` bullet lists); never hand-edited, copied to a stacked descendant, or a link-bearing artifact. The merged `results:` file stays a frozen build record — the freeze rule above is unchanged. - `## Reclaim log` — dated entries appended by the `change.reclaim` operation when an expired-lease, no-branch claim self-heals back to `proposed`. -- `## Auto-groom blocked` — dated abstain record appended by `docket-auto-groom`; contents and lifecycle (including removal on re-arm) are defined by the *Autonomous grooming* shared definition below. +- `## Auto-groom blocked` — dated abstain record written by `change.groom` `outcome: abstain`; contents and lifecycle (including removal by `outcome: rearm`) are defined by the *Autonomous grooming* shared definition below. - `## Publish deferred` — dated record left by earlier docket versions when a terminal close-out's publish step was expected but deferred or blocked (change 0083). **Read-only historical evidence:** publication-deferral marking is deferred from Go v1 — existing `publish-deferred` markers remain as historical evidence, and the `publish-deferred` health check keeps them visible; no maintained script writes or removes one. Never hand-authored. - `## Finalize blocked` — dated record appended by `docket-finalize-change` when a gate failure leaves a change needing a human; presence drives the board's `finalize blocked — needs you` cell and makes later **auto-detect** finalize runs skip the change. A human retries a marked change by **naming its id**, which overrides the skip. The clearing rule is owned by `docket-finalize-change` and not restated here. - `## Run halted` — record appended (heading **bare**, never dated — the reader is a whole-line match, so the date belongs inside the body) by an autonomous run that stops needing a human (the `halted` disposition). **Presence-encoded state**, in the same family as `## Auto-groom blocked` and `## Finalize blocked`: the run clears `verify-run`'s gate by *writing this section and committing it*, which is what makes a `halted` disposition verifiable in git rather than a claim in a completion report. Removal is owned by `docket-implement-next`'s Step 2 claim — the only transition back into a live run — and is stated there, not restated here. @@ -290,11 +290,11 @@ A change is **build-ready** — eligible for `docket-implement-next` — only wh ### Autonomous grooming (shared definition) -A change's **effective auto-groomable** value is its `auto_groomable:` override when explicitly set, else the repo's `auto_groom` knob (default `false`). The field is human input with one exception: `docket-auto-groom`'s abstain is the single agent write (it flips the override to `false`). +A change's **effective auto-groomable** value is its `auto_groomable:` override when explicitly set, else the repo's `auto_groom` knob (default `false`). The field is human input with one exception: `docket-auto-groom`'s abstain is the single agent write (`change.groom` `outcome: abstain` flips the override to `false`). A stub is **autonomous-eligible** — selectable by `docket-auto-groom` — when it is needs-brainstorm (`proposed`, no `spec:`, not `trivial: true`) AND effective auto-groomable. Unsatisfied `depends_on` does NOT exclude it (the same design-ahead rule as interactive grooming; the implementer's reconcile re-validates at build time). Ranking is the same deterministic selection order as build-ready selection. -**Abstain rule.** When autonomous grooming cannot safely default a decision, it emits NO spec; it flips `auto_groomable: false` and appends a dated `## Auto-groom blocked` body section. The stub stays needs-brainstorm — out of the autonomous queue, still in the interactive one. Re-arm = a human supplies the missing context, flips the flag back to `true`, and DELETES the `## Auto-groom blocked` section (git history keeps it; the section's presence drives the board's needs-you cell, so a stale one would mislabel a re-armed stub). Kill and defer are never autonomous: they surface inside the blocked section as recommendations. +**Abstain rule.** When autonomous grooming cannot safely default a decision, it emits NO spec; it applies `change.groom` with `outcome: abstain`, which flips `auto_groomable: false`, appends a dated entry to the `## Auto-groom blocked` body section, and re-renders the board in one commit. The stub stays needs-brainstorm — out of the autonomous queue, still in the interactive one. Re-arm = a human supplies the missing context and applies `change.groom` with `outcome: rearm` (optionally with owned-section edits carrying that context): it sets the flag back to `true` and removes the `## Auto-groom blocked` section in the same commit — never a hand edit (git history keeps the section; its presence drives the board's needs-you cell, so a stale one would mislabel a re-armed stub). Kill and defer are never autonomous: they surface inside the blocked section as recommendations. **Interactive selection bands.** `docket-groom-next` still sees every needs-brainstorm stub, but its default order prefers stubs that need a human: (1) abstained (`## Auto-groom blocked` present), (2) effective `auto_groomable: false`, (3) effective auto-groomable — flagged "docket-auto-groom will handle it unless you want it now." Within each band, the deterministic selection order applies. The board renders abstained stubs as **auto-groom blocked — needs you**, distinct from plain needs-brainstorm. diff --git a/internal/assets/embedded/tree/skills/docket-groom-next/SKILL.md b/internal/assets/embedded/tree/skills/docket-groom-next/SKILL.md index b1725de85..d3b78ac7f 100644 --- a/internal/assets/embedded/tree/skills/docket-groom-next/SKILL.md +++ b/internal/assets/embedded/tree/skills/docket-groom-next/SKILL.md @@ -1,6 +1,6 @@ --- name: docket-groom-next -description: Use when stubs are sitting at needs-brainstorm on the docket board and you want the next one designed — selecting the next needs-brainstorm change (proposed, no spec, not trivial) deterministically and grooming it to build-ready through an interactive brainstorm with the human, exiting with a linked spec, a trivial verdict, a kill, or a defer — or revising an already-groomed proposed change by explicit id. Selection is autonomous; the design conversation is not. Writes markdown only — never branches, worktrees, or code. +description: Use when stubs are sitting at needs-brainstorm on the docket board and you want the next one designed — selecting the next needs-brainstorm change (proposed, no spec, not trivial) deterministically and grooming it to build-ready through an interactive brainstorm with the human, exiting with a linked spec, a trivial verdict, a kill, a defer, or a re-arm — or revising an already-groomed proposed change by explicit id. Selection is autonomous; the design conversation is not. Writes markdown only — never branches, worktrees, or code. --- # docket-groom-next — the groomer (interactive) @@ -56,19 +56,20 @@ The recap is an introduction, not a confirmation gate — flow directly into the Then run the **resolved brainstorm skill** — `$SKILL_BRAINSTORM` from the Step-0 config export (default `superpowers:brainstorming`) — WITH THE HUMAN, seeded with the stub's body and its `## Open questions` — the open questions are the session's starting agenda. If it resolves to `auto` or cannot be invoked, apply the brainstorm auto-fallback per the convention's *Skill layer* (design inline with the human, warning prominently on unavailability) — the artifact is unchanged: a spec, then stop. If the human asks for a consultant-written spec, invoke `docket-brainstorm` for this run regardless of `$SKILL_BRAINSTORM` — human steering of an interactive session always wins (see the README's consultant-brainstorm section). STOP AT THE SPEC — do NOT continue to `superpowers:writing-plans` (planning is build-time, owned by `docket-implement-next`). -### Step 4 — Exit (one of five; the human confirms which) +### Step 4 — Exit (one of six; the human confirms which) -All five exits reuse existing transitions — this skill introduces no new lifecycle status: +All six exits reuse existing transitions — this skill introduces no new lifecycle status: 1. **Spec** (the normal exit): **author** the settled design as the spec's Markdown body, and any owned proposal-section rewrites (proposal altitude, resolved `## Open questions` removed). Apply it with one atomic transaction — the `change.groom` operation with `--repo-dir .docket --request ` — carrying the change id, the pinned record `path` + `version` from Step 1, `outcome: spec`, the `spec_markdown`, the owned-section edits, and the desired `depends_on`/`related`/`adrs`/`discovered_from`/`stacked_on` (authored Markdown travels in the request file, never a shell-escaped flag). In one metadata commit it writes the spec file, sets `spec:` + `updated:`, stamps the spec's `docket:backlink` block, and re-renders the `## Artifacts` block and inline board — so there is no separate render, back-link, or Board pass. A typed refusal (not-groomable, spec-path-taken, malformed markers, version mismatch) writes nothing — surface it. The change is now build-ready. 2. **Trivial verdict**: the brainstorm concludes there is no real design question — apply the `change.groom` operation with `--repo-dir .docket --request ` with `outcome: trivial`, the pinned `path` + `version`, and an owned-section edit carrying the tightened body as the trivial rationale. The transaction sets `trivial: true` + `updated:`, re-renders the `## Artifacts` block, and re-renders the inline board atomically — no spec file, no separate Board pass. Also build-ready. 3. **Kill**: the stub is obsolete, a duplicate, or decided against — follow the proposed-kill sub-path in `docket-new-change` (it owns the kill mechanics; do not restate them here). 4. **Defer**: right idea, wrong time — apply the `change.defer` operation with `--repo-dir .docket --request ` with the pinned `path` + `version` and the authored `why_deferred` section body. The transaction sets `status: deferred`, splices the `## Why deferred` section, sets `updated:`, and re-renders the inline board atomically. 5. **Revise** (explicit-id route only): the change is already groomed and the human wants it adjusted — apply the `change.groom` operation with `--repo-dir .docket --request ` with `outcome: revise`, the pinned `path` + `version`, and whichever of `spec_markdown` (a whole-body replace of the *existing* linked spec, minus its `docket:backlink` block, which the transaction re-stamps; it never mints a new path) and owned-section `sections` edits the adjustment touched, plus any relationship-field updates. With `spec_markdown`, also send `spec_version`, the linked spec's blob object id read by `git -C .docket rev-parse HEAD:` (`` is the record's `spec:` value) — a concurrent spec edit then contends instead of being overwritten. It never sets `spec:` or `trivial:`, so a change cannot flip between spec'd and trivial here. Repeatable while the change stays `proposed`. A typed refusal (`not-revisable`, `spec-not-linked`, `spec-file-missing`, `empty-revise`, `empty-spec_version`, version mismatch) writes nothing — surface it. The change stays build-ready. +6. **Re-arm** (abstained or opted-out stubs): the human supplied the context an abstain asked for and wants `docket-auto-groom` to take the stub — apply the `change.groom` operation with `--repo-dir .docket --request ` with `outcome: rearm`, the pinned `path` + `version`, and any owned-section `sections` edits carrying the new context. The transaction sets `auto_groomable: true`, removes the `## Auto-groom blocked` section, and re-renders the inline board atomically, returning the stub to the autonomous queue. A `nothing-to-rearm` refusal (no blocked section, already `true`) writes nothing. A spec or trivial groom of an abstained stub needs no re-arm. ### Step 5 — The transaction lands (no separate board pass) -The Step-4 typed op is the whole write: it re-checks the pinned exact `version`, commits the change record, the spec, the `## Artifacts` block, and the inline board in one metadata commit pushed to `origin/docket` under an exact-lease push — so there is no separate hand-staged commit and **no separate Board pass** (the readiness cell flips from needs-brainstorm, or the row leaves the Proposed section on a kill or defer, in that same commit; a revise keeps the row build-ready). On a `contended` refusal the op writes nothing: re-sync (re-run the `repository.prepare` operation), re-read the record's `path` + `version` from the `status` operation, and — if it is no longer needs-brainstorm (someone else groomed, killed, or claimed it) — STOP and report rather than overwrite; otherwise re-author and retry. On a revise, also re-read the spec (a fresh `spec_version`) and stop only if the change is no longer an already-groomed `proposed` change. STOP — grooming never implements. +The Step-4 typed op is the whole write: it re-checks the pinned exact `version`, commits the change record, the spec, the `## Artifacts` block, and the inline board in one metadata commit pushed to `origin/docket` under an exact-lease push — so there is no separate hand-staged commit and **no separate Board pass** (the readiness cell flips from needs-brainstorm, or the row leaves the Proposed section on a kill or defer, in that same commit; a revise keeps the row build-ready; a re-arm returns an abstained row to needs-brainstorm). On a `contended` refusal the op writes nothing: re-sync (re-run the `repository.prepare` operation), re-read the record's `path` + `version` from the `status` operation, and — if it is no longer needs-brainstorm (someone else groomed, killed, or claimed it) — STOP and report rather than overwrite; otherwise re-author and retry. On a revise, also re-read the spec (a fresh `spec_version`) and stop only if the change is no longer an already-groomed `proposed` change. STOP — grooming never implements. ## Concurrency — no claim diff --git a/internal/assets/embedded/tree/skills/docket-new-change/SKILL.md b/internal/assets/embedded/tree/skills/docket-new-change/SKILL.md index 6bc88eb33..7864716d5 100644 --- a/internal/assets/embedded/tree/skills/docket-new-change/SKILL.md +++ b/internal/assets/embedded/tree/skills/docket-new-change/SKILL.md @@ -36,7 +36,7 @@ The default path for any non-trivial new change. Five steps: 4. **Create the proposed change** — submit the record through one atomic transaction: the `change.create` operation (resolve argv from the capability catalog) with `--repo-dir .docket --request `. Its closed JSON request carries `title`, `type` (one configured `change_type` — `create` refuses an unknown or empty type, so no created change is ever left `untyped`, and there is no template comment to replace), `priority` (default `medium`), the PM-altitude `why`/`what_changes`/`out_of_scope` body distilled from the brainstorm (design detail lives in the linked spec, NOT here), the resolved `depends_on`/`related`/`adrs`/`discovered_from` from step 3, `stacked_on` when the work builds on another change's **unmerged** branch (set it and **read [stacked-changes.md](../docket-convention/references/stacked-changes.md) now (blocking)** first — stacking changes how it is built, merged, and closed out), and the stable `request_id` from step 1. The transaction allocates the id, derives the slug, serializes the canonical `status: proposed` record (`created`/`updated` = the commit's UTC date), renders its `## Artifacts` block, and re-renders the inline board — one metadata commit under an exact-lease push, returning the new `id`/`slug`/`path`. - **Two draft-time scalars `create` does not carry** — `auto_groomable` and `branch_prefix`. When the human says the change may be designed without them, or names a branch prefix ("use the `hotfix/` prefix"), set the scalar directly on the created record's frontmatter and commit it with plain git plumbing (**Stage by explicit path** — that tree is shared, so never `add -A`) — no derived view depends on either, so no re-render: `auto_groomable: true` (so `docket-auto-groom` carries it to build-ready), and/or the normalized `branch_prefix: hotfix` (strip one presentation-only trailing slash, but **refuse and ask the human** on a slash-embedded or `refs/`-qualified value — claim consumes it at mint time, never prose). Leave a field unset to inherit the repo default. + **Two draft-time scalars ride in the same request** — `auto_groomable` and `branch_prefix`. When the human says the change may be designed without them, send `auto_groomable: true` (so `docket-auto-groom` carries it to build-ready; `false` opts it out). When they name a branch prefix ("use the `hotfix/` prefix"), send it as typed in `branch_prefix` — the operation normalizes it and refuses an unusable value with `invalid-branch_prefix` before anything is written: show the human that finding and ask for a new value. Omit either field to inherit the repo default. 5. **Groom to build-ready & land** — read the created record's `path` + exact **entity version** (blob object id) from the `status` operation (with `--json`), then apply the spec atomically: the `change.groom` operation with `--repo-dir .docket --request ` carrying the change id, the pinned `path` + `version`, `outcome: spec`, the `spec_markdown` from step 2, and any owned proposal-section rewrites. In one metadata commit the transaction writes the spec file, sets `spec:` + `updated:`, stamps the spec's `docket:backlink` block, re-renders the `## Artifacts` block, and re-renders the inline board (pushed under an exact-lease push) — **no separate Board pass**. A version-mismatch / `contended` refusal writes nothing: re-sync (re-run the `repository.prepare` operation), re-read `path` + `version`, retry. STOP. Never implements. To adjust a just-landed spec or its owned sections afterwards, run `docket-groom-next ` — its explicit-id path routes an already-groomed change to `change.groom` with `outcome: revise`; never re-run this skill or hand-edit the spec file. diff --git a/internal/cli/change.go b/internal/cli/change.go index 879242f69..8f568aa0e 100644 --- a/internal/cli/change.go +++ b/internal/cli/change.go @@ -60,7 +60,7 @@ func newChangeCommand(setResult func(app.OperationResult)) *cobra.Command { }, EffectMetadataWrite) groom := changeSubcommand("change", "groom", - "Groom a proposed change to build-ready (spec or trivial), or revise an already-groomed one, from a JSON request", + "Groom a proposed change to build-ready (spec or trivial), record or clear an auto-groom abstain (abstain, rearm), or revise an already-groomed one, from a JSON request", func(c *cobra.Command, deps app.PlanningDeps, repoDir string) error { var req app.ChangeGroomRequest if err := decodeRequestFlag(c, &req); err != nil { diff --git a/internal/domain/actions.go b/internal/domain/actions.go index d26d4a2ef..627c5418a 100644 --- a/internal/domain/actions.go +++ b/internal/domain/actions.go @@ -424,6 +424,7 @@ func newChangeBuilder(c Change) *changeBuilder { Plan: c.Plan(), Results: c.Results(), Trivial: c.Trivial(), + AutoGroomable: c.AutoGroomable(), BranchPrefix: c.BranchPrefix(), Branch: c.Branch(), ClaimedAt: c.ClaimedAt(), diff --git a/internal/domain/entities.go b/internal/domain/entities.go index dd3c67e7a..f65324e79 100644 --- a/internal/domain/entities.go +++ b/internal/domain/entities.go @@ -35,6 +35,16 @@ type OptionalInt struct { Raw string } +// OptionalBool is an optional tri-state boolean field: absent or valueless +// (inherit a default) stays distinguishable from an explicit false. Raw carries +// the stored text whenever State != FieldAbsent, so a malformed value stays +// reportable; Value is meaningful only when State is FieldPresent. +type OptionalBool struct { + State FieldState + Value bool + Raw string +} + // OptionalTime is an optional date or timestamp field. Raw carries the stored // text whenever State != FieldAbsent; Value is meaningful only when the state // is FieldPresent. @@ -80,6 +90,7 @@ type ChangeSpec struct { Plan OptionalString Results OptionalString Trivial bool + AutoGroomable OptionalBool // per-change auto-groom override; unset ⇒ inherit auto_groom BranchPrefix OptionalString // per-change mint-prefix override; durable input Branch OptionalString ClaimedAt OptionalTime // second-precision UTC; Raw kept @@ -170,6 +181,10 @@ func (c Change) Results() OptionalString { return c.spec.Results } // Trivial reports whether the change is marked trivial. func (c Change) Trivial() bool { return c.spec.Trivial } +// AutoGroomable returns the optional per-change auto-groom override. Unset +// (absent or valueless) means the repository's auto_groom knob applies. +func (c Change) AutoGroomable() OptionalBool { return c.spec.AutoGroomable } + // BranchPrefix returns the optional per-change mint-prefix override. It is // durable human input consumed only at claim time; once branch: is populated // it is informational and inert. diff --git a/internal/domain/lease_test.go b/internal/domain/lease_test.go index db1c812c9..a83536ca0 100644 --- a/internal/domain/lease_test.go +++ b/internal/domain/lease_test.go @@ -359,3 +359,25 @@ func TestReclaimPreservesBranchPrefix(t *testing.T) { t.Errorf("branch_prefix = %+v, want preserved {Present hotfix}", bp) } } + +func TestReclaimPreservesAutoGroomable(t *testing.T) { + // The change builder seeds every durable input, auto_groomable included: a + // domain transition must never drop the human's override. + want := OptionalBool{State: FieldPresent, Value: true, Raw: "true"} + c := NewChange(ChangeSpec{ + ID: 7, + Slug: "lease-slug", + Type: "fix", + Status: StatusInProgress, + RawStatus: string(StatusInProgress), + ClaimedAt: leaseStamp(-10 * time.Hour), + AutoGroomable: want, + }) + got, fail := Reclaim(c, leaseNow, leaseTTL, leaseBranches()) + if fail != nil { + t.Fatalf("Reclaim failed: %v", fail) + } + if ag := got.Change.AutoGroomable(); ag != want { + t.Errorf("auto_groomable = %+v, want preserved %+v", ag, want) + } +} diff --git a/internal/domain/types.go b/internal/domain/types.go index e0aae4568..d9418abc1 100644 --- a/internal/domain/types.go +++ b/internal/domain/types.go @@ -276,6 +276,26 @@ func ValidBranchComponent(s string) bool { return true } +// NormalizeBranchPrefix canonicalizes an authored branch_prefix before it is +// stored: it trims surrounding whitespace, strips exactly one trailing "/" +// (a presentation-only spelling), and lowercases the result — branch prefixes +// are lowercase-only, like the change-type token grammar. An empty result means +// unset ("", true). Anything else must satisfy ValidBranchComponent, the same +// rule domain.Claim applies at mint time, so a stored prefix can never fail at +// claim. A slash-embedded or refs/-qualified value is refused, never rewritten. +func NormalizeBranchPrefix(raw string) (string, bool) { + s := strings.TrimSpace(raw) + s = strings.TrimSuffix(s, "/") + s = strings.ToLower(s) + if s == "" { + return "", true + } + if !ValidBranchComponent(s) { + return "", false + } + return s, true +} + // MintBranch constructs the full feature-branch name a claim records: // (branch_prefix when present and non-empty, otherwise the change type) + // "/" + slug. This is the ONLY branch-name constructor; every post-claim diff --git a/internal/domain/types_test.go b/internal/domain/types_test.go index 78886711d..8c1c3e6dc 100644 --- a/internal/domain/types_test.go +++ b/internal/domain/types_test.go @@ -1,6 +1,9 @@ package domain -import "testing" +import ( + "strings" + "testing" +) func TestParseStatus(t *testing.T) { tests := []struct { @@ -211,3 +214,67 @@ func TestMintBranch(t *testing.T) { t.Fatalf("empty prefix mint = %q, want chore/s", got) } } + +func TestNormalizeBranchPrefix(t *testing.T) { + cases := []struct { + raw string + want string + ok bool + }{ + {"hotfix", "hotfix", true}, + {" hotfix ", "hotfix", true}, + {"hotfix/", "hotfix", true}, + {"Hotfix", "hotfix", true}, + {"HOTFIX/", "hotfix", true}, + {" Hotfix/ ", "hotfix", true}, + {"hotfix//", "", false}, + {"/hotfix", "", false}, + {"team/hotfix", "", false}, + {"refs", "", false}, + {"REFS", "", false}, + {"refs/heads/x", "", false}, + {"-x", "", false}, + {"x.lock", "", false}, + {"hot fix", "", false}, + {"", "", true}, + {" ", "", true}, + {"/", "", true}, + } + for _, c := range cases { + got, ok := NormalizeBranchPrefix(c.raw) + if got != c.want || ok != c.ok { + t.Errorf("NormalizeBranchPrefix(%q) = (%q, %v), want (%q, %v)", c.raw, got, ok, c.want, c.ok) + } + } +} + +// FuzzNormalizeBranchPrefix ties the normalizer to the reader it feeds: every +// accepted non-empty output must pass claim-time ValidBranchComponent, be +// lowercase, and be a fixed point — so a stored prefix can never fail at claim +// and a retry of the stored value replays. +func FuzzNormalizeBranchPrefix(f *testing.F) { + for _, s := range []string{"hotfix", " Hotfix/ ", "a/b", "refs", "", "/", "x.lock", "HOTFIX//"} { + f.Add(s) + } + f.Fuzz(func(t *testing.T, raw string) { + got, ok := NormalizeBranchPrefix(raw) + if !ok { + if got != "" { + t.Fatalf("refused %q but returned %q", raw, got) + } + return + } + if got == "" { + return + } + if !ValidBranchComponent(got) { + t.Fatalf("normalized %q -> %q fails claim-time ValidBranchComponent", raw, got) + } + if got != strings.ToLower(got) { + t.Fatalf("normalized %q -> %q is not lowercase", raw, got) + } + if again, ok2 := NormalizeBranchPrefix(got); !ok2 || again != got { + t.Fatalf("not idempotent: %q -> %q -> (%q, %v)", raw, got, again, ok2) + } + }) +} diff --git a/internal/render/record.go b/internal/render/record.go index a2a4e31ac..8e51cfe33 100644 --- a/internal/render/record.go +++ b/internal/render/record.go @@ -28,6 +28,8 @@ type NewChangeRecord struct { Related []domain.ChangeID DiscoveredFrom []domain.ChangeID ADRs []domain.ADRID + AutoGroomable *bool // nil ⇒ null (inherit the repo's auto_groom) + BranchPrefix string // already normalized by the app layer; "" ⇒ null Why string // markdown body, no heading line WhatChanges string // markdown body, no heading line OutOfScope string // markdown body, no heading line @@ -59,8 +61,8 @@ func ChangeRecord(r NewChangeRecord) ([]byte, error) { {Name: "plan", Value: document.Null()}, {Name: "results", Value: document.Null()}, {Name: "trivial", Value: document.Bool(false)}, - {Name: "auto_groomable", Value: document.Null()}, - {Name: "branch_prefix", Value: document.Null()}, + {Name: "auto_groomable", Value: optionalBoolValue(r.AutoGroomable)}, + {Name: "branch_prefix", Value: optionalStringValue(r.BranchPrefix)}, {Name: "branch", Value: document.Null()}, {Name: "pr", Value: document.Null()}, {Name: "blocked_by", Value: document.Null()}, @@ -75,6 +77,24 @@ func ChangeRecord(r NewChangeRecord) ([]byte, error) { return document.New(fields, body) } +// optionalBoolValue renders a tri-state create-time scalar: nil is the +// canonical null, anything else an explicit boolean. +func optionalBoolValue(v *bool) document.Value { + if v == nil { + return document.Null() + } + return document.Bool(*v) +} + +// optionalStringValue renders an optional create-time text scalar: empty is the +// canonical null, anything else a quoted-by-construction string (ADR-0071). +func optionalStringValue(s string) document.Value { + if s == "" { + return document.Null() + } + return document.String(s) +} + // NewLearningRecord describes one canonical newly-recorded learning finding. type NewLearningRecord struct { Slug, Hook string diff --git a/internal/render/record_test.go b/internal/render/record_test.go index 7b8b563ba..6c3eeed4c 100644 --- a/internal/render/record_test.go +++ b/internal/render/record_test.go @@ -4,6 +4,7 @@ import ( "bytes" "os" "path/filepath" + "strings" "testing" "time" @@ -198,3 +199,40 @@ func TestRelationshipCollectionsRenderFlowSeqs(t *testing.T) { t.Fatalf("populated depends_on not rendered as [3, 5]:\n%s", full) } } + +// TestChangeRecordDraftScalars pins the two create-time scalars: unset renders +// the canonical null (the golden's shape), a set value renders a boolean or a +// single-quoted string by construction. +func TestChangeRecordDraftScalars(t *testing.T) { + yes, no := true, false + cases := []struct { + name string + auto *bool + prefix string + wantAuto string + wantPrefix string + }{ + {"unset", nil, "", "\nauto_groomable:\n", "\nbranch_prefix:\n"}, + {"true with prefix", &yes, "hotfix", "\nauto_groomable: true\n", "\nbranch_prefix: 'hotfix'\n"}, + {"explicit false", &no, "", "\nauto_groomable: false\n", "\nbranch_prefix:\n"}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + got, err := render.ChangeRecord(render.NewChangeRecord{ + ID: 1, Slug: "s", Title: "T", Type: "feat", Priority: "medium", Created: goldenDate, + AutoGroomable: c.auto, BranchPrefix: c.prefix, + Why: "w", WhatChanges: "c", OutOfScope: "o", + }) + if err != nil { + t.Fatalf("ChangeRecord: %v", err) + } + s := string(got) + if !strings.Contains(s, c.wantAuto) || !strings.Contains(s, c.wantPrefix) { + t.Errorf("record missing %q / %q:\n%s", c.wantAuto, c.wantPrefix, s) + } + if _, err := document.Parse(got); err != nil { + t.Errorf("record does not reparse: %v", err) + } + }) + } +} diff --git a/internal/repoguard/budgets_test.go b/internal/repoguard/budgets_test.go index 396b9179a..72db4c099 100644 --- a/internal/repoguard/budgets_test.go +++ b/internal/repoguard/budgets_test.go @@ -175,7 +175,7 @@ type skillBudget struct { var skillBudgets = []skillBudget{ {"docket-adr/SKILL.md", 110, 1600}, {"docket-adr/adr-template.md", 26, 90}, - {"docket-auto-groom/SKILL.md", 70, 1750}, + {"docket-auto-groom/SKILL.md", 70, 1627}, // 0382: typed change.groom abstain replaces the plain-git abstain commit prose (word ceiling 1750 -> 1627) {"docket-brainstorm/SKILL.md", 84, 692}, {"docket-build/SKILL.md", 432, 4391}, // 0459: +continuation carries the claimed verdict, closed-scope statement, and fresh prepare-scope bundle (424/4284 -> 432/4391); 0405: sequential-drive contract; 0420: shell-safe capture; 0421: budgeted repair cycle (see note above) // 0154: docket-build/references/delegation-execution.md removed — it was the @@ -196,13 +196,13 @@ var skillBudgets = []skillBudget{ {"docket-convention/references/terminal-close-out.md", 240, 2150}, {"docket-finalize-change/SKILL.md", 239, 5647}, // 0455: +record-invalid refusal (structural scope, findings remedy, merged-outside-docket precedence) in step 8 (word ceiling 5520 -> 5647); 0442: +post-publication base-advance guidance (word ceiling 5421 -> 5520); de-duplicated the shared forward-rebase mechanic against the 0438 unpublished-case paragraph (reclaimed 57 words), but the distinct published-refresh facts plus the retained 0438 guidance cannot fit the old ceiling without deleting required guidance; 0411: +reconciliation-write recovery exception paragraph in the resolver loop (ceilings 238/5232 -> 239/5421); 0413: +generated-bundle mixed-conflict handoff sentence in the resolver-loop block (word ceiling 5200 -> 5232); 0419: +repair-attempt budget payload line and rewired repair contract (line ceiling 236 -> 238); 0393: +exact payload, marker, and direct-dispatch lines atop 0349/0410 (see note above) {"docket-finalize-change/references/gate-failure.md", 147, 1901}, // 0411: +reconciliation-write exception section and abort-set carve-out (ceilings 135/1472 -> 147/1901); 0413: +conflicted_paths-lists-authored-only rule in the resolver-report section (line ceiling 133 -> 135, word ceiling 1465 -> 1472); 0419: +repair-attempt budget payload and rewired repair contract prose (word ceiling 1450 -> 1465); 0349: +reserve-before-dispatch resolver protocol prose; 0375: +worktree-slot note for the scopeless finalize gate (120/1300 -> 133/1450) - {"docket-groom-next/SKILL.md", 77, 1889}, // 0445: +revise route for already-groomed explicit ids (Step 1) and the fifth Step-4 exit (word ceiling 1650 -> 1813); +spec_version pin for a spec-body revise (1813 -> 1849); +revise spec_markdown excludes the backlink block (1849 -> 1850); +revise in the description and the revise contended/board clauses (1850 -> 1889) + {"docket-groom-next/SKILL.md", 77, 1996}, // 0382: +typed rearm exit (word ceiling 1889 -> 1996); 0445: +revise route for already-groomed explicit ids (Step 1) and the fifth Step-4 exit (word ceiling 1650 -> 1813); +spec_version pin for a spec-body revise (1813 -> 1849); +revise spec_markdown excludes the backlink block (1849 -> 1850); +revise in the description and the revise contended/board clauses (1850 -> 1889) {"docket-implement-next/SKILL.md", 214, 8080}, // 0455: +pr.publish record-invalid refusal clause (word ceiling 8025 -> 8080); 0448: +named-invocation branch; bounded own-dependency closeout moved to edge-paths.md (ceilings 210/7716 -> 214/8270 -> 214/8025); 0393: +exact payload, marker, and direct-dispatch lines atop 0410/0354/0376; 0375: +gate-epoch resume pointer (word ceiling 7530 -> 7547); 0440: reader-first results prose {"docket-implement-next/references/edge-paths.md", 118, 1554}, // 0410: +resume/recovery + required-results reconciliation; 0375: +gate-epoch resume refusals (78/1091 -> 93/1261); 0448: +named own-dependency closeout moved from SKILL.md (93/1261 -> 118/1554) {"docket-implement-next/references/fix-loop.md", 190, 1958}, // 0410: +findings-to-results checkpoint linkage (see note above) {"docket-implement-next/results-template.md", 64, 446}, // 0440: reader-first template — action statement + merged Known issues (see note above) {"docket-review/SKILL.md", 110, 913}, // 0410: +findings-return capture contract (see note above) - {"docket-new-change/SKILL.md", 61, 1706}, // 0445: +pointer to the docket-groom-next revise path after landing (word ceiling 1700 -> 1706) + {"docket-new-change/SKILL.md", 61, 1675}, // 0445: +pointer to the docket-groom-next revise path after landing (word ceiling 1700 -> 1706); 0382: draft-time scalars moved into change.create (ceiling 1706 -> 1675) {"docket-new-change/change-template.md", 51, 250}, {"docket-status/SKILL.md", 140, 3065}, // 0388: +sync-integration prose (see note above) } diff --git a/internal/repoguard/prose_contracts_test.go b/internal/repoguard/prose_contracts_test.go index d676bab37..277fa0e0c 100644 --- a/internal/repoguard/prose_contracts_test.go +++ b/internal/repoguard/prose_contracts_test.go @@ -212,6 +212,24 @@ var proseContracts = []proseContract{ // guide by change 0402 — and the comparison page). {sentinel: "change_0400_readme_landing", file: "README.md", present: []string{"](docs/README.md)", "](docs/comparison/ai-native-sdlc-playbook.md)"}}, + // change 0382 — both draft-time scalars ride in the change.create request; + // the post-create plain-git frontmatter edit and the skill-owned + // normalization rules are gone (normalization is domain.NormalizeBranchPrefix). + {sentinel: "change_0382_typed_create_scalars", file: "skills/docket-new-change/SKILL.md", + present: []string{"`invalid-branch_prefix`"}, + absent: []string{"Two draft-time scalars `create` does not carry", "strip one presentation-only trailing slash"}}, + // change 0382 — the auto-groom abstain and the human re-arm are typed + // change.groom outcomes that re-render the board in the same commit; the + // plain-git abstain, its false "no board-visible cell" claim, and the + // hand-edit re-arm are gone. + {sentinel: "change_0382_typed_abstain", file: "skills/docket-auto-groom/SKILL.md", + present: []string{"`outcome: abstain`", "`blocked_note`"}, + absent: []string{"changes no board-visible cell", "there is no typed groom for this outcome"}}, + {sentinel: "change_0382_typed_abstain", file: "skills/docket-convention/SKILL.md", + present: []string{"`outcome: abstain`", "`outcome: rearm`"}, + absent: []string{"flips the flag back to `true`, and DELETES"}}, + {sentinel: "change_0382_typed_abstain", file: "skills/docket-groom-next/SKILL.md", + present: []string{"`outcome: rearm`", "`nothing-to-rearm`"}}, // change 0389 — implementation-scope sweep + the two completion barriers. // docket-status owns the COMMAND barrier: a backgrounded sweep is observed // to its terminal envelope, never declared done by proxy signals; and an diff --git a/internal/repository/decode.go b/internal/repository/decode.go index 9f57dde71..5f9994c46 100644 --- a/internal/repository/decode.go +++ b/internal/repository/decode.go @@ -120,6 +120,7 @@ type changeWire struct { Plan scalar `yaml:"plan"` Results scalar `yaml:"results"` Trivial scalar `yaml:"trivial"` + AutoGroomable scalar `yaml:"auto_groomable"` BranchPrefix scalar `yaml:"branch_prefix"` Branch scalar `yaml:"branch"` ClaimedAt scalar `yaml:"claimed_at"` @@ -178,6 +179,12 @@ func (d *decoder) malformed(field, raw string) { d.report(CodeFieldMalformed, field, domain.SeverityError, map[string]string{"raw": raw}) } +// malformedWarning is malformed at warning severity, for a field whose bad +// value degrades to "unset" rather than invalidating the record. +func (d *decoder) malformedWarning(field, raw string) { + d.report(CodeFieldMalformed, field, domain.SeverityWarning, map[string]string{"raw": raw}) +} + // state classifies how a captured scalar appeared. The located frontmatter // entry is consulted for the "key present, no value" shape, so a key the YAML // tree resolves to null and a key the byte locator sees as valueless agree. @@ -283,6 +290,32 @@ func (d *decoder) boolean(name string, s scalar) bool { return false } +// optionalBool converts a scalar into a tri-state boolean: absent and valueless +// stay distinguishable from an explicit false, and a value that is not a YAML +// boolean is a finding, never a silent true or false. The finding is a +// warning: the only optional boolean is auto_groomable, human input that +// gates no record validity, so a bad value must not make publish or finalize +// refuse a record that was otherwise valid — it is simply not a true. +func (d *decoder) optionalBool(name string, s scalar) domain.OptionalBool { + switch d.state(name, s) { + case domain.FieldAbsent: + return domain.OptionalBool{} + case domain.FieldEmpty: + return domain.OptionalBool{State: domain.FieldEmpty} + case domain.FieldMalformed: + d.malformedWarning(name, s.raw) + return domain.OptionalBool{State: domain.FieldMalformed, Raw: s.raw} + } + switch s.raw { + case "true": + return domain.OptionalBool{State: domain.FieldPresent, Value: true, Raw: s.raw} + case "false": + return domain.OptionalBool{State: domain.FieldPresent, Value: false, Raw: s.raw} + } + d.malformedWarning(name, s.raw) + return domain.OptionalBool{State: domain.FieldMalformed, Raw: s.raw} +} + // integer converts a required numeric identity field, reporting an unusable // value rather than substituting one. func (d *decoder) integer(name string, s scalar) int { @@ -405,16 +438,49 @@ const ( ) // hasHeading reports whether text carries heading as a whole line, matched -// exactly — no leading whitespace, no trailing text, CRLF tolerated. +// exactly — no leading whitespace, no trailing text, CRLF tolerated. A line +// inside a fenced code block is content, not a section: the operations that +// write and remove these sections scan headings the same fence-aware way +// (internal/app scanTopHeadings), so the board and those operations agree on +// whether a marker is present. func hasHeading(text, heading string) bool { + fence := "" for line := range strings.SplitSeq(text, "\n") { - if strings.TrimSuffix(line, "\r") == heading { + line = strings.TrimSuffix(line, "\r") + if run, ok := fenceRun(line); ok { + switch { + case fence == "": + fence = run + case run[0] == fence[0] && len(run) >= len(fence) && strings.TrimSpace(line) == run: + fence = "" + } + continue + } + if fence == "" && line == heading { return true } } return false } +// fenceRun returns the leading delimiter run of a code-fence line — three or +// more backticks or tildes after at most three spaces — and whether the line +// is one. It mirrors internal/app fenceRunBytes. +func fenceRun(line string) (string, bool) { + s := strings.TrimLeft(line, " ") + if len(line)-len(s) > 3 || len(s) < 3 || (s[0] != '`' && s[0] != '~') { + return "", false + } + n := 0 + for n < len(s) && s[n] == s[0] { + n++ + } + if n < 3 { + return "", false + } + return s[:n], true +} + // archiveDate parses the "YYYY-MM-DD-" prefix an archived record's filename // carries. A record read from anywhere else has no archive date at all; an // archived record whose filename lacks a usable prefix is malformed, with the @@ -495,6 +561,7 @@ func decodeChange(in InputDocument) (domain.Change, []domain.Finding) { spec.Plan = d.optionalString("plan", wire.Plan) spec.Results = d.optionalString("results", wire.Results) spec.Trivial = d.boolean("trivial", wire.Trivial) + spec.AutoGroomable = d.optionalBool("auto_groomable", wire.AutoGroomable) spec.BranchPrefix = d.optionalString("branch_prefix", wire.BranchPrefix) spec.Branch = d.optionalString("branch", wire.Branch) spec.ClaimedAt = d.optionalTime("claimed_at", wire.ClaimedAt, stampLayout) diff --git a/internal/repository/decode_test.go b/internal/repository/decode_test.go index fab793879..4738368cd 100644 --- a/internal/repository/decode_test.go +++ b/internal/repository/decode_test.go @@ -400,6 +400,14 @@ func TestDecodeChangePresenceMarkers(t *testing.T) { {"deeper heading level does not count", "### Auto-groom blocked\n", [4]bool{false, false, false, false}}, {"prose mention does not count", "The run halted; see ## Run halted below.\n", [4]bool{false, false, false, false}}, {"CRLF body still matches", "## Run halted\r\n", [4]bool{true, false, false, false}}, + // A heading-shaped line inside fenced code is content, not a section: the + // board and the section-editing operations (rearm) must agree on it. + {"backtick-fenced heading does not count", "```\n## Auto-groom blocked\n## Run halted\n```\n", [4]bool{false, false, false, false}}, + {"tilde-fenced heading does not count", "~~~md\n## Finalize blocked\n## Publish deferred\n~~~\n", [4]bool{false, false, false, false}}, + {"shorter run does not close the fence", "````\n```\n## Auto-groom blocked\n````\n", [4]bool{false, false, false, false}}, + {"info string does not close the fence", "```\n```go\n## Auto-groom blocked\n```\n", [4]bool{false, false, false, false}}, + {"heading after a closed fence counts", "```\nx\n```\n\n## Auto-groom blocked\n", [4]bool{false, true, false, false}}, + {"unterminated fence hides the rest", "```\n## Run halted\n", [4]bool{false, false, false, false}}, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { @@ -804,3 +812,41 @@ func TestDecodeChangeNonScalarWhereScalarExpected(t *testing.T) { t.Errorf("no field-malformed finding for branch: %v", findingCodes(findings)) } } + +// TestDecodeChangeAutoGroomable — auto_groomable decodes as a tri-state: absent, +// valueless (inherit), or an explicit true/false override. A non-boolean value +// is malformed with a finding, never a silent true or false. +func TestDecodeChangeAutoGroomable(t *testing.T) { + cases := []struct { + name string + line string // frontmatter line; "" = key absent + want domain.OptionalBool + malformed bool + }{ + {"absent", "", domain.OptionalBool{}, false}, + {"valueless", "auto_groomable:\n", domain.OptionalBool{State: domain.FieldEmpty}, false}, + {"true", "auto_groomable: true\n", domain.OptionalBool{State: domain.FieldPresent, Value: true, Raw: "true"}, false}, + {"false", "auto_groomable: false\n", domain.OptionalBool{State: domain.FieldPresent, Value: false, Raw: "false"}, false}, + {"non-boolean", "auto_groomable: yes\n", domain.OptionalBool{State: domain.FieldMalformed, Raw: "yes"}, true}, + {"non-scalar", "auto_groomable: [true]\n", domain.OptionalBool{State: domain.FieldMalformed}, true}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + src := "---\nid: 1\nslug: s\n" + c.line + "---\n\nbody\n" + change, findings := decodeChange(input(t, KindChange, LocationActive, "docs/changes/active/0001-s.md", src)) + if got := change.AutoGroomable(); got != c.want { + t.Errorf("AutoGroomable = %+v, want %+v", got, c.want) + } + if got := hasFinding(findings, CodeFieldMalformed, "auto_groomable"); got != c.malformed { + t.Errorf("field-malformed(auto_groomable) = %v, want %v; findings %v", got, c.malformed, findingCodes(findings)) + } + // auto_groomable is human input nothing gates on: a bad value is a + // warning, never an error that makes publish/finalize refuse the record. + for _, f := range findings { + if f.Field == "auto_groomable" && f.Severity != domain.SeverityWarning { + t.Errorf("auto_groomable finding %s severity = %v, want warning", f.Code, f.Severity) + } + } + }) + } +} diff --git a/skills/docket-auto-groom/SKILL.md b/skills/docket-auto-groom/SKILL.md index 73f6f1eaa..a636aea69 100644 --- a/skills/docket-auto-groom/SKILL.md +++ b/skills/docket-auto-groom/SKILL.md @@ -41,21 +41,19 @@ Dispatch the dedicated **`docket-auto-groom-critic`** subagent (foreground, at t **Receiving the verdict.** The verdict is read from the critic's **return** — its final report, which the groom is actively blocking on; the groom never backgrounds the critic. The groom never waits for a message, a notification, or any other out-of-band delivery: nothing is registered to deliver one, so that wait never ends. -**No-verdict posture (bounded — two steps, then out).** If the dispatch returns no legible verdict — a malformed return, pre-yield prose, or a backgrounded child's bare completion — make **one collect attempt** (read the child's completed final report where the harness surfaces it), and failing that **one fresh foreground re-dispatch** of the critic over the same draft, issued through whatever mechanism makes the parent block on the return — if none does, that leg would only repeat the first, so skip it straight to Tier B. Still no verdict ⇒ treat it as a failed dispatch attempt under the convention's *Dispatch-capability resolution*: **Tier B**, so the groom **abstains** for this stub (→ Step 4's **Abstain** exit in full, the `auto_groomable: false` flip included — left armed, the stub stays autonomous-eligible and the drain re-selects it, forfeiting *Termination & concurrency*), recording the return-channel diagnostic in the `## Auto-groom blocked` section, the human's re-arm cue. Never a third dispatch; never an indefinite wait. Re-dispatching a critic is safe where a build worker is not — it is read-only over prose, holds no worktree, and writes no git state, so `yielded-worker-return-closes-every-door`'s closed-doors analysis does not bind here. +**No-verdict posture (bounded — two steps, then out).** If the dispatch returns no legible verdict — a malformed return, pre-yield prose, or a backgrounded child's bare completion — make **one collect attempt** (read the child's completed final report where the harness surfaces it), and failing that **one fresh foreground re-dispatch** of the critic over the same draft, issued through whatever mechanism makes the parent block on the return — if none does, that leg would only repeat the first, so skip it straight to Tier B. Still no verdict ⇒ treat it as a failed dispatch attempt under the convention's *Dispatch-capability resolution*: **Tier B**, so the groom **abstains** for this stub (→ Step 4's **Abstain** exit in full, the `auto_groomable: false` flip included — left armed, the stub stays autonomous-eligible and the drain re-selects it, forfeiting *Termination & concurrency*), recording the return-channel diagnostic in the `blocked_note`, the human's re-arm cue. Never a third dispatch; never an indefinite wait. Re-dispatching a critic is safe where a build worker is not — it is read-only over prose, holds no worktree, and writes no git state, so `yielded-worker-return-closes-every-door`'s closed-doors analysis does not bind here. ### Step 4 — Exit (one of three) 1. **Spec** — every assumption survived: apply one atomic transaction — the `change.groom` operation with `--repo-dir .docket --request ` — carrying the change id, the pinned `path` + `version` from Step 1, `outcome: spec`, the `spec_markdown` (the settled design plus its `## Assumptions` block), the owned proposal-section rewrites (proposal altitude, resolved `## Open questions` removed), and the desired `depends_on`/`related`/`adrs`/`discovered_from`/`stacked_on`. In one metadata commit it writes the spec file, sets `spec:` + `updated:`, stamps the spec's `docket:backlink` block, and re-renders the `## Artifacts` block and inline board; a typed refusal (not-groomable, spec-path-taken, malformed markers, version mismatch) writes nothing. Build-ready. 2. **Trivial** — the critic confirmed no hidden design decisions: apply the `change.groom` operation with `--repo-dir .docket --request ` with `outcome: trivial`, the pinned `path` + `version`, and an owned-section edit carrying the tightened body and its reasoning as the trivial rationale. The transaction sets `trivial: true` + `updated:` and re-renders the `## Artifacts` block and inline board atomically. Build-ready, no spec. -3. **Abstain** — any needs-human-context verdict, or Step 3's exhausted no-verdict posture: emit NO spec; there is no typed groom for this outcome, so write it directly on the metadata tree — flip `auto_groomable: false` and append a dated `## Auto-groom blocked` section (the undecidable decision(s), what context is missing, what a human should supply, and any recommendation — including "this should probably be killed/deferred because …") — then commit that change-file edit with plain git plumbing (Step 5). The abstain changes no board-visible cell, so no board render is needed; the stub stays needs-brainstorm, first in `docket-groom-next`'s queue. +3. **Abstain** — any needs-human-context verdict, or Step 3's exhausted no-verdict posture: emit NO spec; apply the `change.groom` operation with `--repo-dir .docket --request ` with `outcome: abstain`, the pinned `path` + `version`, and a `blocked_note` — the undecidable decision(s), what context is missing, what a human should supply, and any recommendation (including "this should probably be killed/deferred because …"), with subsections at `###` or deeper. The transaction sets `auto_groomable: false` + `updated:`, appends a dated entry to the `## Auto-groom blocked` section, and re-renders the inline board — the row flips to **auto-groom blocked — needs you** in that same commit. It accepts no section, spec, or relationship edits. The stub stays needs-brainstorm, first in `docket-groom-next`'s queue. **Kill and defer are NEVER autonomous.** Verdict authority over the backlog's composition stays human; the strongest the drain may say is an abstain-with-recommendation. ### Step 5 — The outcome lands (no separate board pass) -For a **spec** or **trivial** exit the Step-4 `change.groom` operation is the whole write — it re-checks the pinned `version` and commits the record, the spec, the `## Artifacts` block, and the inline board in one metadata commit pushed under an exact-lease push, so there is **no separate Board pass**. On a `contended` refusal it writes nothing: re-sync (re-run the `repository.prepare` operation), re-read the stub's `path` + `version` from the `status` operation, and if it is no longer autonomous-eligible (groomed, killed, claimed, or opted out) DISCARD this iteration's draft (delete the just-drafted spec markdown) and loop; otherwise re-author and retry. - -For an **abstain** exit, commit the change-file edit (`auto_groomable: false` + the `## Auto-groom blocked` section) with plain git plumbing in the metadata working tree; push `origin/docket`. **Stage by explicit path** — that tree is shared, so a bare `add -A` commits another agent's staged work under your message. On a non-fast-forward rejection: re-sync (re-run the `repository.prepare` operation), and if the rebase brought in commits touching this stub's file, RE-READ it — no longer autonomous-eligible ⇒ DISCARD this iteration's writes for it (`git -C .docket restore -- `) and loop. Loop to step 1. +Every exit's Step-4 `change.groom` operation is the whole write — it re-checks the pinned `version` and commits the record, any spec, the `## Artifacts` block, and the inline board in one metadata commit pushed under an exact-lease push, so there is **no separate Board pass** and no hand-staged commit. On a `contended` refusal it writes nothing: re-sync (re-run the `repository.prepare` operation), re-read the stub's `path` + `version` from the `status` operation, and if it is no longer autonomous-eligible (groomed, killed, claimed, or opted out) DISCARD this iteration's draft (delete any just-drafted spec markdown) and loop; otherwise re-author and retry. Loop to step 1. ### Step 6 — Report diff --git a/skills/docket-convention/SKILL.md b/skills/docket-convention/SKILL.md index 4573a39dd..55800aa27 100644 --- a/skills/docket-convention/SKILL.md +++ b/skills/docket-convention/SKILL.md @@ -222,7 +222,7 @@ change, never in the merged artifact. - `## Reconcile log` — dated entries appended by the implementer's reconcile pass. - `## Closeout notes` — terminal-only, **optional**, and the **final authored body section** of a terminal record. Written solely by the `finalize.closeout` operation from its structured request (`verification_outcomes` / `late_findings`, rendered as `### Verification` / `### Late findings` bullet lists); never hand-edited, copied to a stacked descendant, or a link-bearing artifact. The merged `results:` file stays a frozen build record — the freeze rule above is unchanged. - `## Reclaim log` — dated entries appended by the `change.reclaim` operation when an expired-lease, no-branch claim self-heals back to `proposed`. -- `## Auto-groom blocked` — dated abstain record appended by `docket-auto-groom`; contents and lifecycle (including removal on re-arm) are defined by the *Autonomous grooming* shared definition below. +- `## Auto-groom blocked` — dated abstain record written by `change.groom` `outcome: abstain`; contents and lifecycle (including removal by `outcome: rearm`) are defined by the *Autonomous grooming* shared definition below. - `## Publish deferred` — dated record left by earlier docket versions when a terminal close-out's publish step was expected but deferred or blocked (change 0083). **Read-only historical evidence:** publication-deferral marking is deferred from Go v1 — existing `publish-deferred` markers remain as historical evidence, and the `publish-deferred` health check keeps them visible; no maintained script writes or removes one. Never hand-authored. - `## Finalize blocked` — dated record appended by `docket-finalize-change` when a gate failure leaves a change needing a human; presence drives the board's `finalize blocked — needs you` cell and makes later **auto-detect** finalize runs skip the change. A human retries a marked change by **naming its id**, which overrides the skip. The clearing rule is owned by `docket-finalize-change` and not restated here. - `## Run halted` — record appended (heading **bare**, never dated — the reader is a whole-line match, so the date belongs inside the body) by an autonomous run that stops needing a human (the `halted` disposition). **Presence-encoded state**, in the same family as `## Auto-groom blocked` and `## Finalize blocked`: the run clears `verify-run`'s gate by *writing this section and committing it*, which is what makes a `halted` disposition verifiable in git rather than a claim in a completion report. Removal is owned by `docket-implement-next`'s Step 2 claim — the only transition back into a live run — and is stated there, not restated here. @@ -290,11 +290,11 @@ A change is **build-ready** — eligible for `docket-implement-next` — only wh ### Autonomous grooming (shared definition) -A change's **effective auto-groomable** value is its `auto_groomable:` override when explicitly set, else the repo's `auto_groom` knob (default `false`). The field is human input with one exception: `docket-auto-groom`'s abstain is the single agent write (it flips the override to `false`). +A change's **effective auto-groomable** value is its `auto_groomable:` override when explicitly set, else the repo's `auto_groom` knob (default `false`). The field is human input with one exception: `docket-auto-groom`'s abstain is the single agent write (`change.groom` `outcome: abstain` flips the override to `false`). A stub is **autonomous-eligible** — selectable by `docket-auto-groom` — when it is needs-brainstorm (`proposed`, no `spec:`, not `trivial: true`) AND effective auto-groomable. Unsatisfied `depends_on` does NOT exclude it (the same design-ahead rule as interactive grooming; the implementer's reconcile re-validates at build time). Ranking is the same deterministic selection order as build-ready selection. -**Abstain rule.** When autonomous grooming cannot safely default a decision, it emits NO spec; it flips `auto_groomable: false` and appends a dated `## Auto-groom blocked` body section. The stub stays needs-brainstorm — out of the autonomous queue, still in the interactive one. Re-arm = a human supplies the missing context, flips the flag back to `true`, and DELETES the `## Auto-groom blocked` section (git history keeps it; the section's presence drives the board's needs-you cell, so a stale one would mislabel a re-armed stub). Kill and defer are never autonomous: they surface inside the blocked section as recommendations. +**Abstain rule.** When autonomous grooming cannot safely default a decision, it emits NO spec; it applies `change.groom` with `outcome: abstain`, which flips `auto_groomable: false`, appends a dated entry to the `## Auto-groom blocked` body section, and re-renders the board in one commit. The stub stays needs-brainstorm — out of the autonomous queue, still in the interactive one. Re-arm = a human supplies the missing context and applies `change.groom` with `outcome: rearm` (optionally with owned-section edits carrying that context): it sets the flag back to `true` and removes the `## Auto-groom blocked` section in the same commit — never a hand edit (git history keeps the section; its presence drives the board's needs-you cell, so a stale one would mislabel a re-armed stub). Kill and defer are never autonomous: they surface inside the blocked section as recommendations. **Interactive selection bands.** `docket-groom-next` still sees every needs-brainstorm stub, but its default order prefers stubs that need a human: (1) abstained (`## Auto-groom blocked` present), (2) effective `auto_groomable: false`, (3) effective auto-groomable — flagged "docket-auto-groom will handle it unless you want it now." Within each band, the deterministic selection order applies. The board renders abstained stubs as **auto-groom blocked — needs you**, distinct from plain needs-brainstorm. diff --git a/skills/docket-groom-next/SKILL.md b/skills/docket-groom-next/SKILL.md index b1725de85..d3b78ac7f 100644 --- a/skills/docket-groom-next/SKILL.md +++ b/skills/docket-groom-next/SKILL.md @@ -1,6 +1,6 @@ --- name: docket-groom-next -description: Use when stubs are sitting at needs-brainstorm on the docket board and you want the next one designed — selecting the next needs-brainstorm change (proposed, no spec, not trivial) deterministically and grooming it to build-ready through an interactive brainstorm with the human, exiting with a linked spec, a trivial verdict, a kill, or a defer — or revising an already-groomed proposed change by explicit id. Selection is autonomous; the design conversation is not. Writes markdown only — never branches, worktrees, or code. +description: Use when stubs are sitting at needs-brainstorm on the docket board and you want the next one designed — selecting the next needs-brainstorm change (proposed, no spec, not trivial) deterministically and grooming it to build-ready through an interactive brainstorm with the human, exiting with a linked spec, a trivial verdict, a kill, a defer, or a re-arm — or revising an already-groomed proposed change by explicit id. Selection is autonomous; the design conversation is not. Writes markdown only — never branches, worktrees, or code. --- # docket-groom-next — the groomer (interactive) @@ -56,19 +56,20 @@ The recap is an introduction, not a confirmation gate — flow directly into the Then run the **resolved brainstorm skill** — `$SKILL_BRAINSTORM` from the Step-0 config export (default `superpowers:brainstorming`) — WITH THE HUMAN, seeded with the stub's body and its `## Open questions` — the open questions are the session's starting agenda. If it resolves to `auto` or cannot be invoked, apply the brainstorm auto-fallback per the convention's *Skill layer* (design inline with the human, warning prominently on unavailability) — the artifact is unchanged: a spec, then stop. If the human asks for a consultant-written spec, invoke `docket-brainstorm` for this run regardless of `$SKILL_BRAINSTORM` — human steering of an interactive session always wins (see the README's consultant-brainstorm section). STOP AT THE SPEC — do NOT continue to `superpowers:writing-plans` (planning is build-time, owned by `docket-implement-next`). -### Step 4 — Exit (one of five; the human confirms which) +### Step 4 — Exit (one of six; the human confirms which) -All five exits reuse existing transitions — this skill introduces no new lifecycle status: +All six exits reuse existing transitions — this skill introduces no new lifecycle status: 1. **Spec** (the normal exit): **author** the settled design as the spec's Markdown body, and any owned proposal-section rewrites (proposal altitude, resolved `## Open questions` removed). Apply it with one atomic transaction — the `change.groom` operation with `--repo-dir .docket --request ` — carrying the change id, the pinned record `path` + `version` from Step 1, `outcome: spec`, the `spec_markdown`, the owned-section edits, and the desired `depends_on`/`related`/`adrs`/`discovered_from`/`stacked_on` (authored Markdown travels in the request file, never a shell-escaped flag). In one metadata commit it writes the spec file, sets `spec:` + `updated:`, stamps the spec's `docket:backlink` block, and re-renders the `## Artifacts` block and inline board — so there is no separate render, back-link, or Board pass. A typed refusal (not-groomable, spec-path-taken, malformed markers, version mismatch) writes nothing — surface it. The change is now build-ready. 2. **Trivial verdict**: the brainstorm concludes there is no real design question — apply the `change.groom` operation with `--repo-dir .docket --request ` with `outcome: trivial`, the pinned `path` + `version`, and an owned-section edit carrying the tightened body as the trivial rationale. The transaction sets `trivial: true` + `updated:`, re-renders the `## Artifacts` block, and re-renders the inline board atomically — no spec file, no separate Board pass. Also build-ready. 3. **Kill**: the stub is obsolete, a duplicate, or decided against — follow the proposed-kill sub-path in `docket-new-change` (it owns the kill mechanics; do not restate them here). 4. **Defer**: right idea, wrong time — apply the `change.defer` operation with `--repo-dir .docket --request ` with the pinned `path` + `version` and the authored `why_deferred` section body. The transaction sets `status: deferred`, splices the `## Why deferred` section, sets `updated:`, and re-renders the inline board atomically. 5. **Revise** (explicit-id route only): the change is already groomed and the human wants it adjusted — apply the `change.groom` operation with `--repo-dir .docket --request ` with `outcome: revise`, the pinned `path` + `version`, and whichever of `spec_markdown` (a whole-body replace of the *existing* linked spec, minus its `docket:backlink` block, which the transaction re-stamps; it never mints a new path) and owned-section `sections` edits the adjustment touched, plus any relationship-field updates. With `spec_markdown`, also send `spec_version`, the linked spec's blob object id read by `git -C .docket rev-parse HEAD:` (`` is the record's `spec:` value) — a concurrent spec edit then contends instead of being overwritten. It never sets `spec:` or `trivial:`, so a change cannot flip between spec'd and trivial here. Repeatable while the change stays `proposed`. A typed refusal (`not-revisable`, `spec-not-linked`, `spec-file-missing`, `empty-revise`, `empty-spec_version`, version mismatch) writes nothing — surface it. The change stays build-ready. +6. **Re-arm** (abstained or opted-out stubs): the human supplied the context an abstain asked for and wants `docket-auto-groom` to take the stub — apply the `change.groom` operation with `--repo-dir .docket --request ` with `outcome: rearm`, the pinned `path` + `version`, and any owned-section `sections` edits carrying the new context. The transaction sets `auto_groomable: true`, removes the `## Auto-groom blocked` section, and re-renders the inline board atomically, returning the stub to the autonomous queue. A `nothing-to-rearm` refusal (no blocked section, already `true`) writes nothing. A spec or trivial groom of an abstained stub needs no re-arm. ### Step 5 — The transaction lands (no separate board pass) -The Step-4 typed op is the whole write: it re-checks the pinned exact `version`, commits the change record, the spec, the `## Artifacts` block, and the inline board in one metadata commit pushed to `origin/docket` under an exact-lease push — so there is no separate hand-staged commit and **no separate Board pass** (the readiness cell flips from needs-brainstorm, or the row leaves the Proposed section on a kill or defer, in that same commit; a revise keeps the row build-ready). On a `contended` refusal the op writes nothing: re-sync (re-run the `repository.prepare` operation), re-read the record's `path` + `version` from the `status` operation, and — if it is no longer needs-brainstorm (someone else groomed, killed, or claimed it) — STOP and report rather than overwrite; otherwise re-author and retry. On a revise, also re-read the spec (a fresh `spec_version`) and stop only if the change is no longer an already-groomed `proposed` change. STOP — grooming never implements. +The Step-4 typed op is the whole write: it re-checks the pinned exact `version`, commits the change record, the spec, the `## Artifacts` block, and the inline board in one metadata commit pushed to `origin/docket` under an exact-lease push — so there is no separate hand-staged commit and **no separate Board pass** (the readiness cell flips from needs-brainstorm, or the row leaves the Proposed section on a kill or defer, in that same commit; a revise keeps the row build-ready; a re-arm returns an abstained row to needs-brainstorm). On a `contended` refusal the op writes nothing: re-sync (re-run the `repository.prepare` operation), re-read the record's `path` + `version` from the `status` operation, and — if it is no longer needs-brainstorm (someone else groomed, killed, or claimed it) — STOP and report rather than overwrite; otherwise re-author and retry. On a revise, also re-read the spec (a fresh `spec_version`) and stop only if the change is no longer an already-groomed `proposed` change. STOP — grooming never implements. ## Concurrency — no claim diff --git a/skills/docket-new-change/SKILL.md b/skills/docket-new-change/SKILL.md index 6bc88eb33..7864716d5 100644 --- a/skills/docket-new-change/SKILL.md +++ b/skills/docket-new-change/SKILL.md @@ -36,7 +36,7 @@ The default path for any non-trivial new change. Five steps: 4. **Create the proposed change** — submit the record through one atomic transaction: the `change.create` operation (resolve argv from the capability catalog) with `--repo-dir .docket --request `. Its closed JSON request carries `title`, `type` (one configured `change_type` — `create` refuses an unknown or empty type, so no created change is ever left `untyped`, and there is no template comment to replace), `priority` (default `medium`), the PM-altitude `why`/`what_changes`/`out_of_scope` body distilled from the brainstorm (design detail lives in the linked spec, NOT here), the resolved `depends_on`/`related`/`adrs`/`discovered_from` from step 3, `stacked_on` when the work builds on another change's **unmerged** branch (set it and **read [stacked-changes.md](../docket-convention/references/stacked-changes.md) now (blocking)** first — stacking changes how it is built, merged, and closed out), and the stable `request_id` from step 1. The transaction allocates the id, derives the slug, serializes the canonical `status: proposed` record (`created`/`updated` = the commit's UTC date), renders its `## Artifacts` block, and re-renders the inline board — one metadata commit under an exact-lease push, returning the new `id`/`slug`/`path`. - **Two draft-time scalars `create` does not carry** — `auto_groomable` and `branch_prefix`. When the human says the change may be designed without them, or names a branch prefix ("use the `hotfix/` prefix"), set the scalar directly on the created record's frontmatter and commit it with plain git plumbing (**Stage by explicit path** — that tree is shared, so never `add -A`) — no derived view depends on either, so no re-render: `auto_groomable: true` (so `docket-auto-groom` carries it to build-ready), and/or the normalized `branch_prefix: hotfix` (strip one presentation-only trailing slash, but **refuse and ask the human** on a slash-embedded or `refs/`-qualified value — claim consumes it at mint time, never prose). Leave a field unset to inherit the repo default. + **Two draft-time scalars ride in the same request** — `auto_groomable` and `branch_prefix`. When the human says the change may be designed without them, send `auto_groomable: true` (so `docket-auto-groom` carries it to build-ready; `false` opts it out). When they name a branch prefix ("use the `hotfix/` prefix"), send it as typed in `branch_prefix` — the operation normalizes it and refuses an unusable value with `invalid-branch_prefix` before anything is written: show the human that finding and ask for a new value. Omit either field to inherit the repo default. 5. **Groom to build-ready & land** — read the created record's `path` + exact **entity version** (blob object id) from the `status` operation (with `--json`), then apply the spec atomically: the `change.groom` operation with `--repo-dir .docket --request ` carrying the change id, the pinned `path` + `version`, `outcome: spec`, the `spec_markdown` from step 2, and any owned proposal-section rewrites. In one metadata commit the transaction writes the spec file, sets `spec:` + `updated:`, stamps the spec's `docket:backlink` block, re-renders the `## Artifacts` block, and re-renders the inline board (pushed under an exact-lease push) — **no separate Board pass**. A version-mismatch / `contended` refusal writes nothing: re-sync (re-run the `repository.prepare` operation), re-read `path` + `version`, retry. STOP. Never implements. To adjust a just-landed spec or its owned sections afterwards, run `docket-groom-next ` — its explicit-id path routes an already-groomed change to `change.groom` with `outcome: revise`; never re-run this skill or hand-edit the spec file.