From eb10a615a151eb53d9319380f24a39f644f9469b Mon Sep 17 00:00:00 2001 From: Andy Eschbacher Date: Mon, 24 Aug 2026 18:11:15 -0400 Subject: [PATCH 1/4] chore(planning): settle the vocabulary audits and correct a stale repair plan MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two audits close, both by handing their remainder to tasks that already existed — which is what `tracked` is for, and the first real use of it outside the branch that introduced it: * 2026-08-22-multi-workspace-atlas — M2/M4 were atlas FEATURES (branch badges, live filter), not defects in what shipped. Both already had tasks on epic 29. 12/12 settled, closed. * 2026-08-24-finding-note-and-vocabulary-selfreview — M3 folded into the acceptance-criterion vocabulary task as criterion 10. It is a decision about a shared word, and deciding it apart from that task's other open questions would be deciding it twice. 10/10 settled, closed. audit-lint-fix-for-legacy-finding-status-debt is rewritten rather than edited. Its scope had gone from stale to actively harmful: it specified mapping `tracked` → `superseded`, which would now DESTROY a handoff rather than repair anything, and it listed `landed` as legal. Its emoji-stripping half is largely obsolete since the parser became decoration-tolerant. The replacement leads with measuring what debt actually remains — plausibly none — and forbids any repair that silently changes a status's meaning, with its own former rule as the cautionary example. audit-finding-write-surface completes: 3 criteria met, 2 tracked to the candidate-list convention task. H2 of 2026-08-17-finding-status-surface is tracked to the lint-fix task above. --- ...fke23-2026-08-17-finding-status-surface.md | 8 ++- ...ye4qma-2026-08-22-multi-workspace-atlas.md | 15 ++++-- ...-finding-note-and-vocabulary-selfreview.md | 10 +++- ...ce-status-write-and-candidate-list-sync.md | 3 +- ...lint-fix-for-legacy-finding-status-debt.md | 50 +++++++++++++------ ...riterion-say-more-than-done-or-not-done.md | 9 ++++ 6 files changed, 74 insertions(+), 21 deletions(-) diff --git a/planning/audits/6g1397jfke23-2026-08-17-finding-status-surface.md b/planning/audits/6g1397jfke23-2026-08-17-finding-status-surface.md index b3dc1ea..f01bebc 100644 --- a/planning/audits/6g1397jfke23-2026-08-17-finding-status-surface.md +++ b/planning/audits/6g1397jfke23-2026-08-17-finding-status-surface.md @@ -64,7 +64,7 @@ atomic single write, `--json`. Agent-facing, like `audit append`. "on `fixed`, add a resolution block" convention that currently lives only in consumer docs. -#### H2. `lint --fix` advertises repairing audits but never looks at finding status · **Status:** open +#### H2. `lint --fix` advertises repairing audits but never looks at finding status · **Status:** tracked by 6fq9zy13wkdc **File:** `internal/cli/lint.go:16,28` | **Component:** cli / lint **Effort:** S · **Urgency:** soon @@ -93,6 +93,12 @@ one command instead of a scripted hand-repair. the existing global `--dry-run`. Separately, narrow the `lint` help text so "repairs tasks/audits" cannot be read as covering finding status. +**Resolution:** Carried by the audit-lint-fix task on epic 20, which is where +the legacy-debt repair belongs. Its scope needs revising first: the +emoji-stripping half is largely obsolete now that M2 made the parser +decoration-tolerant, and its declined/tracked→superseded mapping predates +tracked becoming a legal status. + #### M1. The status error names the offending value but not the legal set · **Status:** fixed **File:** `internal/domain/finding.go:114,116` | **Component:** domain / lint diff --git a/planning/audits/6g2k3qye4qma-2026-08-22-multi-workspace-atlas.md b/planning/audits/6g2k3qye4qma-2026-08-22-multi-workspace-atlas.md index ebff045..ba4ccc3 100644 --- a/planning/audits/6g2k3qye4qma-2026-08-22-multi-workspace-atlas.md +++ b/planning/audits/6g2k3qye4qma-2026-08-22-multi-workspace-atlas.md @@ -1,9 +1,10 @@ --- schema: 1 id: 6g2k3qye4qma -bucket: open +bucket: closed area: multi-workspace-atlas date: "2026-08-22" +updated_at: "2026-08-24" --- # Audit: multi-workspace-atlas — 2026-08-22 @@ -202,7 +203,7 @@ filesystem access was added to the render path. Tests: --- -#### M2. Atlas cards omit branch and worktree badges that `DescribeCheckout` already supplies · **Status:** open +#### M2. Atlas cards omit branch and worktree badges that `DescribeCheckout` already supplies · **Status:** tracked by 6g2nnkfk1em1 **File:** `internal/spacehealth/diagnose.go:119-127`, `internal/tui/atlas.go:439-444` | **Component:** spacehealth/tui **Effort:** S · **Urgency:** soon @@ -227,6 +228,10 @@ registered on this machine. --- +**Resolution:** Spun into a task on epic 29 rather than fixed in the atlas +restructure branch: the badges need DescribeCheckout wired through the atlas +projection, which is a wider change than the layout work this audit reviewed. + #### M3. The cross-space in-progress rail — the sketch's stated payload — is discarded · **Status:** fixed **File:** `internal/tui/atlas.go:344-392`, `internal/core/space_overview.go:36-40` | **Component:** tui @@ -261,7 +266,7 @@ rail, so it was left neither built nor excluded. --- -#### M4. No live filter (`/`) on the atlas · **Status:** open +#### M4. No live filter (`/`) on the atlas · **Status:** tracked by 6g2nnmmwp1gd **File:** `internal/tui/atlas.go:123-154` | **Component:** tui **Effort:** S · **Urgency:** soon @@ -278,6 +283,10 @@ the dashboard and tabs elsewhere) and `g`/`G`. Worth folding into the same pass. --- +**Resolution:** Spun into a task on epic 29. A live filter is a feature the +atlas did not yet have rather than a defect in what shipped, so it is queued +rather than carried by this audit. + #### M5. Visiting the atlas and returning resets focus and zoom for the space you never left · **Status:** fixed **File:** `internal/tui/atlas.go:156-169`, `internal/tui/atlas.go:140-141` | **Component:** tui diff --git a/planning/audits/6g3ahpetw89g-2026-08-24-finding-note-and-vocabulary-selfreview.md b/planning/audits/6g3ahpetw89g-2026-08-24-finding-note-and-vocabulary-selfreview.md index 4148e52..90e805e 100644 --- a/planning/audits/6g3ahpetw89g-2026-08-24-finding-note-and-vocabulary-selfreview.md +++ b/planning/audits/6g3ahpetw89g-2026-08-24-finding-note-and-vocabulary-selfreview.md @@ -1,7 +1,7 @@ --- schema: 1 id: 6g3ahpetw89g -bucket: open +bucket: closed area: finding-note-and-vocabulary-selfreview date: "2026-08-24" updated_at: "2026-08-24" @@ -102,7 +102,7 @@ transcribing them, matching what the `schema audit` conventions line now does. already publishes finding_statuses, and the two wire descriptions point at it instead of transcribing a list that had already fallen a word behind. -#### M3. A `tracked` criterion needs no destination, while a `tracked` finding does · **Status:** open +#### M3. A `tracked` criterion needs no destination, while a `tracked` finding does · **Status:** tracked by 6g31g9f8x4cv **File:** `internal/domain/resolution.go` · `internal/cli/task.go` | **Component:** domain / vocabulary **Effort:** S · **Urgency:** soon @@ -125,6 +125,12 @@ a `tracked` criterion's reason, or document why a criterion's destination is sof finding's. The asymmetry is defensible — an audit concludes its interest on handoff while a task's work merely moves — but it is currently accidental, not stated. +**Resolution:** Folded in as criterion 10 of the acceptance-criterion vocabulary +task, where the shared-word decision belongs. It is a decision about the +vocabulary rather than a defect in this branch's code, and deciding it in +isolation from the rest of that task's open questions would be deciding it +twice. + #### M4. A note containing the literal label can wrap into a false duplicate · **Status:** fixed 2026-08-24 **File:** `internal/domain/finding.go` (`wrapNote` / `DuplicateNotes`) | **Component:** domain / lint diff --git a/planning/tasks/6feeygw00jmx-audit-finding-write-surface-status-write-and-candidate-list-sync.md b/planning/tasks/6feeygw00jmx-audit-finding-write-surface-status-write-and-candidate-list-sync.md index e3d8c0a..9f9e6c0 100644 --- a/planning/tasks/6feeygw00jmx-audit-finding-write-surface-status-write-and-candidate-list-sync.md +++ b/planning/tasks/6feeygw00jmx-audit-finding-write-surface-status-write-and-candidate-list-sync.md @@ -1,6 +1,6 @@ --- schema: 1 -status: in-progress +status: completed epic: 20-cli-ux-and-ergonomics description: 'audit finding --status write + audit sync + candidate drift lint — items 3+5 carved from the finding-level read task (grammar transcribed in-repo)' effort: Unknown @@ -12,6 +12,7 @@ created: "2026-06-21" updated_at: "2026-08-24" id: 6feeygw00jmx started_at: "2026-08-24" +completed_at: "2026-08-24" --- # Audit finding write surface — status write + candidate-list sync diff --git a/planning/tasks/6fq9zy13wkdc-audit-lint-fix-for-legacy-finding-status-debt.md b/planning/tasks/6fq9zy13wkdc-audit-lint-fix-for-legacy-finding-status-debt.md index 4f2a1db..d3704dd 100644 --- a/planning/tasks/6fq9zy13wkdc-audit-lint-fix-for-legacy-finding-status-debt.md +++ b/planning/tasks/6fq9zy13wkdc-audit-lint-fix-for-legacy-finding-status-debt.md @@ -10,27 +10,49 @@ priority: medium autonomy_level: 3 tags: [audit, lint] created: "2026-07-18" -updated_at: "2026-08-23" +updated_at: "2026-08-24" --- -# audit lint --fix for legacy finding-status debt + document the vocabulary - ## Objective -`audit lint` enforces a strict finding-status vocabulary -(deferred/fixed/in-progress/landed/open/superseded/wontfix) but there is no -`audit lint --fix` to normalize older audits (emoji ✅/⏳/⛔, legacy words like -`tracked`/`declined`, or pre-`Status:` findings). Separately, `schema audit` -documents the Status-line *format* but not the allowed *vocabulary* (only the -top-level `schema` lists it). +`audit lint` enforces a strict finding-status vocabulary, and `audit finding` now +writes it, but there is still no `audit lint --fix` to repair audits written before +either existed. A file with a malformed status is flagged forever and repaired by hand, +which is the practice this whole surface has been retiring. + +**This task was rewritten on 2026-08-24.** Its original scope was authored against a +vocabulary that has since changed, and two of its three premises are now wrong: + +- It called `tracked` a legacy word to be mapped to `superseded`. `tracked` is now a + first-class status meaning "handed to a task", and that mapping would DESTROY the + handoff — the opposite of a repair. +- It listed `landed` as legal. `landed` was dropped (M3 of + `2026-08-17-finding-status-surface`) after the corpus showed zero uses of it. +- Emoji-stripping was its largest item. M2 of the same audit made the parser + decoration-tolerant, so `**Status:** ✅ fixed` now reads correctly and lints clean. + A `--fix` that rewrites those lines is now cosmetic rather than corrective. + +What is genuinely left is narrower, and worth checking against a real corpus before +building: there may be very little actual debt remaining. ## Acceptance criteria -- [ ] `audit lint --fix` normalizes finding statuses (strip emoji, map declined→wontfix / tracked→superseded, backfill missing) -- [ ] `schema audit` lists the finding-status vocabulary +- [x] `schema audit` names the finding-status vocabulary. **met:** the conventions line is + built from `FindingStatuses()` rather than transcribed, so it cannot fall behind. +- [ ] The remaining debt is MEASURED before it is repaired — how many audits in a real + corpus still fail `audit lint`, and for what. If the answer is "none", this task + closes as `wontfix` rather than growing a repair nobody needs. +- [ ] `audit lint --fix` repairs what that measurement actually finds, honouring + `--dry-run` and reporting per-file changes the way `lint --fix` already does. +- [ ] No repair silently changes a status's MEANING. A backfill of a missing status is + safe; a mapping between two legal words is not, and must be refused rather than + guessed — the `tracked → superseded` rule this task used to carry is the cautionary + example. +- [ ] Errors wrap the domain sentinels; suite + lint green; docs updated. ## Notes -- Was masked in the wild by the P2 abort — whole-tree `audit lint` never - completed until the invalid-id files were fixed, hiding ~10 audits of debt. -- Confirmed: no `--fix` on `audit lint`; `schema audit` omits the vocab. +- Was masked in the wild by the P2 abort — whole-tree `audit lint` never completed until + the invalid-id files were fixed, hiding ~10 audits of debt. That abort is fixed, so the + measurement above is now possible where it was not before. +- H2 of `2026-08-17-finding-status-surface` is tracked here. - Source: https://github.com/andy-esch/taskflow/issues/105 (P3, Medium) diff --git a/planning/tasks/6g31g9f8x4cv-let-an-acceptance-criterion-say-more-than-done-or-not-done.md b/planning/tasks/6g31g9f8x4cv-let-an-acceptance-criterion-say-more-than-done-or-not-done.md index 4f3acdc..7eb6d3b 100644 --- a/planning/tasks/6g31g9f8x4cv-let-an-acceptance-criterion-say-more-than-done-or-not-done.md +++ b/planning/tasks/6g31g9f8x4cv-let-an-acceptance-criterion-say-more-than-done-or-not-done.md @@ -119,6 +119,15 @@ the surface that shows unmet criteria is the surface that would explain a refusa - [ ] Criterion states reuse the finding glyph/colour vocabulary rather than introducing a · **deferred:** no glyph vocabulary rendered yet; falls out of the TUI roll-up in criterion 9 parallel one. - [ ] A task's acceptance-criteria roll-up is visible near the top of its TUI detail view, · **deferred:** TUI detail-header roll-up not built yet; the CLI tally landed first +- [ ] A decision is recorded on whether a `tracked` CRITERION must name its destination the + way a `tracked` FINDING does. `SetFindingStatus` refuses a bare `tracked` and lint + flags one, but `task ac --tracked --reason "just because"` is accepted — the same + word carrying a weaker guarantee on one of the two entities that share it. The + asymmetry is defensible (an audit concludes its interest on handoff; a task's work + merely moves) but it is currently accidental rather than stated, and a shared + vocabulary whose guarantees differ per entity has already begun to drift. Either + require an id-shaped token in the reason, or write down why a criterion's destination + is softer. Raised as M3 of `2026-08-24-finding-note-and-vocabulary-selfreview`. not only by scrolling into the body. ## Decisions, 2026-08-24 — settled before implementation From 7540f0cf9da1c5561c9c6afb1eb99d862d91aad3 Mon Sep 17 00:00:00 2001 From: Andy Eschbacher Date: Mon, 24 Aug 2026 18:42:39 -0400 Subject: [PATCH 2/4] fix(domain): a criterion is not one line MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Criteria wrap — the corpus wraps them — but the scanner only ever captured the marker line, so every wrapped criterion was truncated at the fold in `task ac --list`, `task show`, and the JSON: "Criterion states reuse the finding glyph/colour vocabulary rather than introducing a" with "parallel one." silently dropped. Worse, the state writer appended its suffix to the end of that same first line, splitting the sentence and stranding the tail beneath the reason: - [ ] …rather than introducing a · **deferred:** no glyph vocabulary rendered yet parallel one. The scanner now tracks a criterion's last line, joining continuations — indented, non-blank, not themselves list items — and stopping where a markdown reader stops: a blank line, a new list item, a heading, or a fence. Text is the whole criterion. The writer strips an existing suffix from wherever it sits and appends the new one to the criterion's LAST line. Two bugs only surfaced by re-setting a state: stripping the suffix trimmed the continuation's indentation, which made the line stop being a continuation, so the NEXT write left its suffix stranded and `met` no longer cleared it. Indentation is now preserved through the strip. TestSetAcceptanceCriterion_MultiLine asserted the truncated text, so it had been holding the bug in place rather than catching it. Repairs the one file in the corpus this damaged — this task's own criteria 8 and 9, found by starting the task and reading them. --- internal/domain/body.go | 75 +++++++++++++--- internal/domain/body_test.go | 88 ++++++++++++++++++- ...riterion-say-more-than-done-or-not-done.md | 11 +-- 3 files changed, 154 insertions(+), 20 deletions(-) diff --git a/internal/domain/body.go b/internal/domain/body.go index ac810c8..a76badf 100644 --- a/internal/domain/body.go +++ b/internal/domain/body.go @@ -40,10 +40,17 @@ type Criterion struct { Suffix CriterionState } -// acCheckbox is an acceptance-criteria checkbox located in a body: its 0-based line -// index (so a flip can rewrite exactly that line) and current state/text. +// acCheckbox is an acceptance-criteria checkbox located in a body: the 0-based line +// index of its marker, the index of its LAST line, and current state/text. +// +// A criterion is not necessarily one line. The corpus wraps them, and treating only the +// marker line as the criterion truncated every wrapped one — `task ac --list`, `task show` +// and the JSON all showed "…rather than introducing a" and silently dropped the rest — +// while the state writer appended its suffix mid-sentence, leaving the remainder stranded +// on the line below. text is the criterion's full logical text with the wrapping collapsed. type acCheckbox struct { - line int + line int // the marker line + end int // the criterion's last line (== line when it does not wrap) checked bool text string } @@ -157,12 +164,15 @@ func scanAcceptanceCheckboxes(body string) (lines []string, boxes []acCheckbox) fence fenceScanner inSection bool sectionLvl int + open = -1 // index of the criterion still accepting continuation lines ) for i, ln := range lines { if fence.inCode(ln) { + open = -1 continue } if m := bodyHeadingRe.FindStringSubmatch(ln); m != nil { + open = -1 lvl := len(m[1]) switch { case !inSection: @@ -176,13 +186,33 @@ func scanAcceptanceCheckboxes(body string) (lines []string, boxes []acCheckbox) } if inSection { if m := bodyCheckboxRe.FindStringSubmatch(ln); m != nil { - boxes = append(boxes, acCheckbox{line: i, checked: m[1] == "x" || m[1] == "X", text: checkboxText(ln)}) + boxes = append(boxes, acCheckbox{line: i, end: i, checked: m[1] == "x" || m[1] == "X", text: checkboxText(ln)}) + open = len(boxes) - 1 + continue + } + // A wrapped criterion continues on the following indented, non-list lines. A + // blank line, a new list item, or a heading ends it — the same rule a markdown + // reader applies, kept in this one pass so the fence tracker stays in step. + if open >= 0 && isCriterionContinuation(ln) { + boxes[open].end = i + boxes[open].text += " " + strings.TrimSpace(ln) + continue } + open = -1 } } return lines, boxes } +// isCriterionContinuation reports whether ln continues the criterion above it: indented, +// non-blank, and not itself a list item (a nested bullet is a sub-list, not more sentence). +func isCriterionContinuation(ln string) bool { + if strings.TrimSpace(ln) == "" || !strings.HasPrefix(ln, " ") && !strings.HasPrefix(ln, "\t") { + return false + } + return !acListItemRe.MatchString(ln) +} + // CountAcceptanceCriteria tallies the acceptance-criteria checkboxes. No such // section — or none with checkboxes — yields a zero tally. func CountAcceptanceCriteria(body string) ACCount { @@ -248,20 +278,39 @@ func SetCriterionState(body string, n int, state CriterionState, reason string) return "", fmt.Errorf("%w: criterion %d out of range (have %d)", ErrValidation, n, len(boxes)) } box := boxes[n-1] - line := lines[box.line] - // Strip whatever suffix is there before writing the new one, so repeated calls are - // idempotent rather than stacking markers. - text := checkboxText(line) - if stripped, _, _, ok := criterionSuffix(text); ok { - text = stripped // replace the existing disposition rather than stacking another - } + // Strip whatever suffix is there before writing the new one, so repeated calls replace + // rather than stack. It can sit on any line of a wrapped criterion — including the + // wrong one, if it was written before the writer knew criteria wrap. + for j := box.line; j <= box.end; j++ { + lines[j] = stripCriterionSuffixLine(lines[j]) + } + lines[box.line] = replaceCheckboxLine(lines[box.line], state.Met(), strings.TrimSpace(checkboxText(lines[box.line]))) + // The suffix belongs at the END of the criterion, which on a wrapped one is not the + // marker line: appending it there splits the sentence and leaves its tail dangling + // under the reason. if state.NeedsReason() { - text = strings.TrimSpace(text) + fmt.Sprintf(" · **%s:** %s", state, strings.TrimSpace(reason)) + lines[box.end] = strings.TrimRight(lines[box.end], " \t") + + fmt.Sprintf(" · **%s:** %s", state, strings.TrimSpace(reason)) } - lines[box.line] = replaceCheckboxLine(line, state.Met(), strings.TrimSpace(text)) return strings.Join(lines, "\n"), nil } +// stripCriterionSuffixLine removes a trailing disposition suffix from one line, leaving a +// line that carries none untouched. Line-level rather than text-level because a wrapped +// criterion's suffix may be on a continuation line, which has no checkbox marker to parse. +func stripCriterionSuffixLine(line string) string { + stripped, _, _, ok := criterionSuffix(line) + if !ok { + return line + } + // criterionSuffix trims the whole text, indentation included. Re-attaching the line's + // own indent is what keeps a continuation line a continuation: strip it and the next + // scan no longer sees the line as part of the criterion, so the following write leaves + // its suffix behind and `met` stops clearing it. + indent := line[:len(line)-len(strings.TrimLeft(line, " \t"))] + return indent + strings.TrimRight(stripped, " \t") +} + // replaceCheckboxLine rebuilds one checkbox line, preserving its original indentation and // bullet so a nested or differently-bulleted list survives a state change untouched. func replaceCheckboxLine(line string, checked bool, text string) string { diff --git a/internal/domain/body_test.go b/internal/domain/body_test.go index 3714290..a10d11b 100644 --- a/internal/domain/body_test.go +++ b/internal/domain/body_test.go @@ -201,11 +201,15 @@ func TestSetAcceptanceCriterion_NoSection(t *testing.T) { // A multi-line criterion (a checkbox with an indented continuation line — the shape // real tasks use) is ONE criterion: the continuation isn't a separate checkbox, and a // flip touches only the checkbox line, leaving the continuation intact. +// +// Its Text is the WHOLE criterion. This previously asserted the truncated first line, +// which is how `task ac --list`, `task show`, and the JSON all came to show +// "…rather than introducing a" and silently drop the rest of the sentence. func TestSetAcceptanceCriterion_MultiLine(t *testing.T) { body := "## Acceptance criteria\n\n- [ ] first criterion spans\n a continuation line\n- [x] second is done\n" cs := ListAcceptanceCriteria(body) - if len(cs) != 2 || cs[0].Text != "first criterion spans" { - t.Fatalf("multi-line criterion should count once: %+v", cs) + if len(cs) != 2 || cs[0].Text != "first criterion spans a continuation line" { + t.Fatalf("multi-line criterion should count once and read whole: %+v", cs) } out, err := SetAcceptanceCriterion(body, 1, true) if err != nil { @@ -434,3 +438,83 @@ func TestCriterionSuffixIsTrailingAndSingleLine(t *testing.T) { t.Error("a newline in a reason was accepted") } } + +// A wrapped criterion's state suffix belongs at the END of the criterion, not the end of +// its marker line. Appending it to the first line splits the sentence and leaves its tail +// dangling beneath the reason: +// +// - [ ] Criterion states reuse the finding glyph vocabulary rather than introducing a · **deferred:** not yet +// parallel one. +// +// which is what this repo's own planning file looked like until the scanner learned that +// criteria wrap. +func TestSetCriterionState_WrappedCriterion(t *testing.T) { + const body = "## Acceptance criteria\n\n" + + "- [ ] Criterion states reuse the finding glyph vocabulary rather than introducing a\n" + + " parallel one.\n" + + "- [ ] A short one.\n" + const whole = "Criterion states reuse the finding glyph vocabulary rather than introducing a parallel one." + + deferred, err := SetCriterionState(body, 1, CriterionDeferred, "not yet") + if err != nil { + t.Fatal(err) + } + if !strings.Contains(deferred, " parallel one. · **deferred:** not yet\n") { + t.Errorf("suffix must land at the end of the criterion, indentation intact:\n%s", deferred) + } + c := ListAcceptanceCriteria(deferred)[0] + if c.Text != whole || c.State != CriterionDeferred || c.Reason != "not yet" { + t.Errorf("round-trip: text=%q state=%q reason=%q", c.Text, c.State, c.Reason) + } + + // Re-setting REPLACES the suffix wherever it sits, and must not strip the + // continuation's indent — losing it would make the line stop being a continuation, so + // the next write would leave its suffix stranded and `met` would not clear it. + wont, err := SetCriterionState(deferred, 1, CriterionWontFix, "changed my mind") + if err != nil { + t.Fatal(err) + } + if n := strings.Count(wont, "· **"); n != 1 { + t.Errorf("want exactly one suffix after re-setting, got %d:\n%s", n, wont) + } + if !strings.Contains(wont, " parallel one. · **wontfix:** changed my mind\n") { + t.Errorf("re-set lost the continuation's indentation:\n%s", wont) + } + + // met clears the suffix from wherever it is and ticks the box — the whole criterion + // returns to its original text. + met, err := SetCriterionState(wont, 1, CriterionMet, "") + if err != nil { + t.Fatal(err) + } + if strings.Contains(met, "· **") { + t.Errorf("met must drop the suffix:\n%s", met) + } + if met != strings.Replace(body, "- [ ] Criterion", "- [x] Criterion", 1) { + t.Errorf("met should restore the criterion's text exactly:\n%s", met) + } +} + +// The continuation rule stops where a markdown reader stops: a blank line, a new list +// item, a heading, or a fence. Without those bounds one criterion would swallow the rest +// of the section. +func TestListAcceptanceCriteria_ContinuationBounds(t *testing.T) { + body := "## Acceptance criteria\n\n" + + "- [ ] wraps\n onto here\n\n" + // blank line ends it + " orphaned prose that is not part of it\n" + + "- [ ] next one\n" + + " - a nested bullet is a sub-list, not more sentence\n" + + "- [ ] third\n" + + "## Next section\n" + + " not a criterion either\n" + cs := ListAcceptanceCriteria(body) + want := []string{"wraps onto here", "next one", "third"} + if len(cs) != len(want) { + t.Fatalf("got %d criteria, want %d: %+v", len(cs), len(want), cs) + } + for i, w := range want { + if cs[i].Text != w { + t.Errorf("criterion %d = %q, want %q", i+1, cs[i].Text, w) + } + } +} diff --git a/planning/tasks/6g31g9f8x4cv-let-an-acceptance-criterion-say-more-than-done-or-not-done.md b/planning/tasks/6g31g9f8x4cv-let-an-acceptance-criterion-say-more-than-done-or-not-done.md index 7eb6d3b..ca826fb 100644 --- a/planning/tasks/6g31g9f8x4cv-let-an-acceptance-criterion-say-more-than-done-or-not-done.md +++ b/planning/tasks/6g31g9f8x4cv-let-an-acceptance-criterion-say-more-than-done-or-not-done.md @@ -1,7 +1,7 @@ --- schema: 1 id: 6g31g9f8x4cv -status: next-up +status: in-progress epic: 20-cli-ux-and-ergonomics description: Criteria are a binary checkbox while findings carry a seven-state vocabulary; an unchecked box cannot distinguish not-yet from won't-do from deferred. effort: M @@ -11,6 +11,7 @@ autonomy_level: 3 tags: [cli, domain, planning-model] created: "2026-08-23" updated_at: "2026-08-24" +started_at: "2026-08-24" --- # Let an acceptance criterion say more than done or not-done @@ -116,9 +117,10 @@ the surface that shows unmet criteria is the surface that would explain a refusa - [x] The state vocabulary is defined once in `domain` and shared with finding status — either the same set or a declared subset — with a test that fails if the two drift apart. -- [ ] Criterion states reuse the finding glyph/colour vocabulary rather than introducing a · **deferred:** no glyph vocabulary rendered yet; falls out of the TUI roll-up in criterion 9 - parallel one. -- [ ] A task's acceptance-criteria roll-up is visible near the top of its TUI detail view, · **deferred:** TUI detail-header roll-up not built yet; the CLI tally landed first +- [ ] Criterion states reuse the finding glyph/colour vocabulary rather than introducing a + parallel one. · **deferred:** no glyph vocabulary rendered yet; falls out of the TUI roll-up in criterion 9 +- [ ] A task's acceptance-criteria roll-up is visible near the top of its TUI detail view, + not only by scrolling into the body. · **deferred:** TUI detail-header roll-up not built yet; the CLI tally landed first - [ ] A decision is recorded on whether a `tracked` CRITERION must name its destination the way a `tracked` FINDING does. `SetFindingStatus` refuses a bare `tracked` and lint flags one, but `task ac --tracked --reason "just because"` is accepted — the same @@ -128,7 +130,6 @@ the surface that shows unmet criteria is the surface that would explain a refusa vocabulary whose guarantees differ per entity has already begun to drift. Either require an id-shaped token in the reason, or write down why a criterion's destination is softer. Raised as M3 of `2026-08-24-finding-note-and-vocabulary-selfreview`. - not only by scrolling into the body. ## Decisions, 2026-08-24 — settled before implementation From 238bcf8a7b195e23419baabb6ea22d1820d0f4bd Mon Sep 17 00:00:00 2001 From: Andy Eschbacher Date: Mon, 24 Aug 2026 18:47:58 -0400 Subject: [PATCH 3/4] feat(tui): surface acceptance criteria in the task detail header MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Once a criterion can say `deferred` or `wontfix`, that state is decision-shaped and belongs where decisions are read — not several screens into the body prose. The task detail header now carries a roll-up: acceptance ███████▒▒▒▒░ 6/10 ✔ 6 · ○ 1 · ◌ 3 Shaped after the audit finding bar on purpose, and drawn by the same renderer: met is the done band, everything settled-but-not-met is the dropped band, and still-open criteria are the empty track. Criteria have no in-progress state, so the active band is always zero. Absent entirely on a task with no criteria, which is most of them. The glyphs are the FINDING glyphs. theme.CriterionState delegates every shared word to theme.FindingStatus rather than restating it, so `◌ deferred` is the same mark on a criterion as on a finding by construction — and TestCriterionStateReusesFindingGlyphs fails if the two ever diverge. `met` and `not met` borrow the marks of `fixed` and `open`; `n/a` is the one state findings have no word for, so it takes its own mark, and the test asserts it collides with none of theirs. `task ac` gets the same treatment. It had been rendering every state in one warn yellow — identical for deferred, wontfix, and n/a — which is precisely the second visual language this criterion rules out. --- internal/cli/render/render.go | 2 +- internal/cli/render/style.go | 16 +++++++ internal/domain/body.go | 24 +++++++++++ internal/domain/body_test.go | 34 +++++++++++++++ internal/theme/theme.go | 25 +++++++++++ internal/theme/theme_test.go | 30 +++++++++++++ internal/tui/detail.go | 42 ++++++++++++++++++- internal/tui/detail_test.go | 35 ++++++++++++++++ internal/wire/schema_comments.json | 1 + ...riterion-say-more-than-done-or-not-done.md | 8 ++-- 10 files changed, 210 insertions(+), 7 deletions(-) create mode 100644 internal/tui/detail_test.go diff --git a/internal/cli/render/render.go b/internal/cli/render/render.go index ae63049..8c406a9 100644 --- a/internal/cli/render/render.go +++ b/internal/cli/render/render.go @@ -182,7 +182,7 @@ func AcceptanceHuman(w io.Writer, st Style, cs []domain.Criterion) { } line := fmt.Sprintf("%s %s %s", mark, st.Dim(fmt.Sprintf("%2d.", c.Index)), c.Text) if c.State.NeedsReason() { - line += " " + st.Warn(string(c.State)+":") + " " + st.Dim(c.Reason) + line += " " + st.CriterionState(string(c.State)) + st.Dim(": "+c.Reason) } fmt.Fprintln(w, line) } diff --git a/internal/cli/render/style.go b/internal/cli/render/style.go index 174ca45..853476d 100644 --- a/internal/cli/render/style.go +++ b/internal/cli/render/style.go @@ -160,6 +160,22 @@ func (s Style) FindingStatus(status string) string { return s.colorSeq(tok.Color) + tok.Glyph + " " + status + ansiReset } +// CriterionState renders an acceptance criterion's state the way FindingStatus renders a +// finding's — glyph then word, in the state's own colour. The tokens come from +// theme.CriterionState, which delegates the shared words to the finding glyphs, so +// `◌ deferred` is the same mark on a criterion as on a finding. +// +// It replaced a single warn-coloured label used for every state: identical yellow for +// deferred, wontfix, and n/a, which is the second visual language criterion 8 of +// let-an-acceptance-criterion-say-more-than-done-or-not-done rules out. +func (s Style) CriterionState(state string) string { + if !s.on || state == "" { + return state + } + tok := theme.CriterionState(state) + return s.colorSeq(tok.Color) + tok.Glyph + " " + state + ansiReset +} + // Priority colors a priority label. func (s Style) Priority(p string) string { return s.wrap(s.colorSeq(theme.Priority(p)), p) diff --git a/internal/domain/body.go b/internal/domain/body.go index a76badf..e45c3de 100644 --- a/internal/domain/body.go +++ b/internal/domain/body.go @@ -231,6 +231,30 @@ func CountAcceptanceCriteria(body string) ACCount { return c } +// CriterionCount is one state's share of a task's acceptance criteria. +type CriterionCount struct { + State CriterionState + N int +} + +// TallyCriteria counts a body's acceptance criteria by state, in the vocabulary's own +// order, omitting states with no members. It is the roll-up's source: a task with a +// criterion that is deferred rather than merely unticked has made a DECISION, and a bare +// "3/8" cannot say so — it reads as five things still to do. +func TallyCriteria(body string) []CriterionCount { + byState := map[CriterionState]int{} + for _, c := range ListAcceptanceCriteria(body) { + byState[c.State]++ + } + out := make([]CriterionCount, 0, len(byState)) + for _, st := range CriterionStates() { + if n := byState[st]; n > 0 { + out = append(out, CriterionCount{State: st, N: n}) + } + } + return out +} + // ListAcceptanceCriteria returns the acceptance criteria in body order, 1-based — // the `task ac --list` view an agent then flips by index. func ListAcceptanceCriteria(body string) []Criterion { diff --git a/internal/domain/body_test.go b/internal/domain/body_test.go index a10d11b..42d2c23 100644 --- a/internal/domain/body_test.go +++ b/internal/domain/body_test.go @@ -518,3 +518,37 @@ func TestListAcceptanceCriteria_ContinuationBounds(t *testing.T) { } } } + +// The roll-up's source. A bare "3/8" reads as five things still to do; the tally is what +// lets a surface say that three of those five were DECIDED rather than forgotten. +func TestTallyCriteria(t *testing.T) { + body := "## Acceptance criteria\n\n" + + "- [x] done one\n" + + "- [x] done two\n" + + "- [ ] still open\n" + + "- [ ] parked · **deferred:** waiting on the ADR\n" + + "- [ ] abandoned · **wontfix:** superseded\n" + + "- [ ] moved · **tracked:** carried by 6g3ag8py12y9\n" + + "- [ ] moot · **n/a:** the tile grid was dropped\n" + got := TallyCriteria(body) + want := []CriterionCount{ + {CriterionMet, 2}, {CriterionUnmet, 1}, {CriterionDeferred, 1}, + {CriterionWontFix, 1}, {CriterionTracked, 1}, {CriterionNA, 1}, + } + if len(got) != len(want) { + t.Fatalf("got %+v, want %+v", got, want) + } + for i := range want { + if got[i] != want[i] { + t.Errorf("position %d = %+v, want %+v", i, got[i], want[i]) + } + } + // States with no members are omitted rather than rendered as zeroes. + if n := len(TallyCriteria("## Acceptance criteria\n\n- [x] only one\n")); n != 1 { + t.Errorf("a single-state body should tally one entry, got %d", n) + } + // No section, no tally — which is most tasks. + if n := len(TallyCriteria("# Title\n\nprose\n")); n != 0 { + t.Errorf("a body with no criteria should tally nothing, got %d", n) + } +} diff --git a/internal/theme/theme.go b/internal/theme/theme.go index 3e229a9..a271f7a 100644 --- a/internal/theme/theme.go +++ b/internal/theme/theme.go @@ -105,6 +105,31 @@ func FindingStatus(s string) Token { } } +// CriterionState maps an acceptance criterion's state to its glyph + colour. +// +// The words the two vocabularies SHARE delegate to FindingStatus rather than restating its +// glyphs. That is the point of sharing a word: a reader learns one mark for `deferred` and +// it means the same thing wherever it appears, and a parallel table here would be free to +// drift the way the finding-status docs did (M3 of 2026-08-17-finding-status-surface). +// TestCriterionStateReusesFindingGlyphs holds the delegation. +func CriterionState(s string) Token { + switch strings.ToLower(strings.TrimSpace(s)) { + case "met": + return FindingStatus("fixed") // resolved, here — the same tick a fixed finding gets + case "not met": + return FindingStatus("open") // still outstanding + case "n/a": + // The one state findings have no word for. A criterion that stopped applying is + // neither done nor dropped, so it gets its own mark rather than borrowing one that + // would overstate it. + return Token{"–", ColorGray} + default: + // deferred · wontfix · tracked are the SAME words as finding statuses and take the + // same marks by construction, not by coincidence. + return FindingStatus(s) + } +} + // Liveness maps an epic's derived activity band (core.EpicSummary.Liveness, passed // as its string value so theme stays domain-only) to a glyph + color. The shape // carries the state through a mono terminal: ● working (live work, like an active diff --git a/internal/theme/theme_test.go b/internal/theme/theme_test.go index de43bc7..0cd87ba 100644 --- a/internal/theme/theme_test.go +++ b/internal/theme/theme_test.go @@ -209,3 +209,33 @@ func TestStartedDatePrefersStartedAtOverLastTouched(t *testing.T) { t.Errorf("StartedDate fallback = %q, want the ordinary date", got) } } + +// Criterion 8 of let-an-acceptance-criterion-say-more-than-done-or-not-done: criterion +// states must REUSE the finding glyph vocabulary, not run a parallel one beside it. This +// is the test that makes that structural rather than aspirational — every word the two +// vocabularies share must render identically, so a reader learns one mark per word. +// +// A second table here would be free to drift exactly the way the finding-status docs did. +func TestCriterionStateReusesFindingGlyphs(t *testing.T) { + for _, w := range domain.SharedResolutionWords() { + if got, want := CriterionState(w), FindingStatus(w); got != want { + t.Errorf("shared word %q renders %v as a criterion and %v as a finding", w, got, want) + } + } + // The two that are not shared words but are deliberately borrowed anyway: a met + // criterion is resolved here like a fixed finding, an unmet one is outstanding like an + // open one. + for _, tc := range []struct{ criterion, finding string }{{"met", "fixed"}, {"not met", "open"}} { + if got, want := CriterionState(tc.criterion), FindingStatus(tc.finding); got != want { + t.Errorf("criterion %q renders %v; want the %q finding's %v", tc.criterion, got, tc.finding, want) + } + } + // `n/a` is the one criterion state findings have no word for, so it must NOT collide + // with a borrowed mark — a reader seeing ◌ must not have to ask which it means. + na := CriterionState("n/a") + for _, s := range domain.FindingStatuses() { + if na == FindingStatus(s) { + t.Errorf("n/a reuses the %q finding's mark %v, which overstates it", s, na) + } + } +} diff --git a/internal/tui/detail.go b/internal/tui/detail.go index 688f122..d405d18 100644 --- a/internal/tui/detail.go +++ b/internal/tui/detail.go @@ -397,11 +397,11 @@ type taskDetail struct { func (d taskDetail) Title() string { return d.t.Slug } func (d taskDetail) Path() string { return d.t.Path } func (d taskDetail) rawBody() string { return d.body } -func (d taskDetail) meta(w int, s *styles) string { return renderTaskMeta(d.t, w, s) } +func (d taskDetail) meta(w int, s *styles) string { return renderTaskMeta(d.t, d.body, w, s) } // renderTaskMeta formats a task's frontmatter field block (no body), wrapped to // width. The body is rendered separately by the pane (raw or glamour). -func renderTaskMeta(t domain.Task, width int, s *styles) string { +func renderTaskMeta(t domain.Task, body string, width int, s *styles) string { var b strings.Builder detailField(&b, "status", s.statusText(t.Status), s) detailField(&b, "epic", t.Epic, s) @@ -415,9 +415,47 @@ func renderTaskMeta(t domain.Task, width int, s *styles) string { if t.Updated != "" { detailField(&b, "updated", fmt.Sprintf("%s (%s)", t.Updated, theme.RelativeDate(t.Updated)), s) } + if roll := criterionRollup(body, s); roll != "" { + detailField(&b, "acceptance", roll, s) + } return wrap(strings.TrimRight(b.String(), "\n"), width) } +// criterionRollup is the acceptance-criteria summary in the detail HEADER, so a criterion +// that says `deferred` is visible without scrolling into the body. Once a criterion can +// carry a decision, that decision belongs where decisions are read. +// +// Shaped after the audit finding bar deliberately — same renderer, same bands: met is the +// done band, everything settled-but-not-met is the dropped band, and still-unmet criteria +// are the empty track. Criteria have no in-progress state, so the active band is always +// zero. The glyph tally beside it uses theme.CriterionState, which delegates the shared +// words to the finding glyphs, so `◌ deferred` is the same mark in both places. +// +// Empty when the task has no acceptance criteria, which is most tasks. +func criterionRollup(body string, s *styles) string { + counts := domain.TallyCriteria(body) + if len(counts) == 0 { + return "" + } + met, settled, total := 0, 0, 0 + var marks []string + for _, c := range counts { + total += c.N + switch { + case c.State.Met(): + met += c.N + case c.State != domain.CriterionUnmet: + settled += c.N + } + tok := theme.CriterionState(string(c.State)) + marks = append(marks, fmt.Sprintf("%s %d", s.fg(tok.Color, tok.Glyph), c.N)) + } + return fmt.Sprintf("%s %s %s", + s.segBar(met, 0, settled, total, 12), + theme.Counts(met, total), + strings.Join(marks, s.dim(" · "))) +} + // --- epic detail --- type epicDetail struct { diff --git a/internal/tui/detail_test.go b/internal/tui/detail_test.go new file mode 100644 index 0000000..6abee15 --- /dev/null +++ b/internal/tui/detail_test.go @@ -0,0 +1,35 @@ +package tui + +import ( + "strings" + "testing" + + "github.com/charmbracelet/x/ansi" + + "github.com/andy-esch/taskflow/internal/domain" + "github.com/andy-esch/taskflow/internal/theme" +) + +// Criterion 9: the roll-up is in the detail HEADER, so a criterion carrying a decision is +// visible without scrolling into the body. Before this, a task could say "deferred: waiting +// on the schema ADR" and the reader would only find it by scrolling. +func TestRenderTaskMeta_CriterionRollup(t *testing.T) { + body := "## Acceptance criteria\n\n" + + "- [x] done\n- [ ] open\n- [ ] parked · **deferred:** waiting on the ADR\n" + st := testStyles + got := renderTaskMeta(domain.Task{Slug: "t", Status: domain.StatusInProgress}, body, 120, &st) + if !strings.Contains(got, "acceptance") { + t.Fatalf("the header must carry an acceptance roll-up:\n%s", got) + } + plain := ansi.Strip(got) + for _, want := range []string{"1/3", theme.CriterionState("met").Glyph, theme.CriterionState("deferred").Glyph} { + if !strings.Contains(plain, want) { + t.Errorf("roll-up missing %q:\n%s", want, plain) + } + } + // A task with no criteria — most of them — gets no row at all rather than an empty one. + bare := renderTaskMeta(domain.Task{Slug: "t", Status: domain.StatusInProgress}, "# Title\n\nprose\n", 120, &st) + if strings.Contains(ansi.Strip(bare), "acceptance") { + t.Errorf("a task with no criteria should have no acceptance row:\n%s", bare) + } +} diff --git a/internal/wire/schema_comments.json b/internal/wire/schema_comments.json index 5432071..ad6802b 100644 --- a/internal/wire/schema_comments.json +++ b/internal/wire/schema_comments.json @@ -17,6 +17,7 @@ "github.com/andy-esch/taskflow/internal/domain.Criterion.Reason": "Reason is the explanation carried by a non-binary state, required for those and\nempty otherwise.", "github.com/andy-esch/taskflow/internal/domain.Criterion.State": "State is the criterion's disposition. The bracket supplies met/not-met; a\n`· **deferred:** why` suffix refines the not-met case. Every criterion written before\nthis vocabulary existed parses exactly as it always did, which is why there is\nnothing to migrate.", "github.com/andy-esch/taskflow/internal/domain.Criterion.Suffix": "Suffix is the state as the author WROTE it, empty when none was written. State is the\nresolved disposition; keeping both lets lint quote what was typed when the two\ndisagree — a checked criterion that also claims to be deferred resolves to met, and a\nmessage naming \"met\" would tell the author nothing about their own edit.", + "github.com/andy-esch/taskflow/internal/domain.CriterionCount": "CriterionCount is one state's share of a task's acceptance criteria.", "github.com/andy-esch/taskflow/internal/domain.CriterionState": "CriterionState is an acceptance criterion's disposition.", "github.com/andy-esch/taskflow/internal/domain.Descriptor": "Descriptor is the per-entity metadata the tool would otherwise hand-enumerate in a `switch kind` at every layer.", "github.com/andy-esch/taskflow/internal/domain.Descriptor.AuthoringFields": "frontmatter a drafter fills in (not tool-managed stamps)", diff --git a/planning/tasks/6g31g9f8x4cv-let-an-acceptance-criterion-say-more-than-done-or-not-done.md b/planning/tasks/6g31g9f8x4cv-let-an-acceptance-criterion-say-more-than-done-or-not-done.md index ca826fb..f63b13a 100644 --- a/planning/tasks/6g31g9f8x4cv-let-an-acceptance-criterion-say-more-than-done-or-not-done.md +++ b/planning/tasks/6g31g9f8x4cv-let-an-acceptance-criterion-say-more-than-done-or-not-done.md @@ -117,10 +117,10 @@ the surface that shows unmet criteria is the surface that would explain a refusa - [x] The state vocabulary is defined once in `domain` and shared with finding status — either the same set or a declared subset — with a test that fails if the two drift apart. -- [ ] Criterion states reuse the finding glyph/colour vocabulary rather than introducing a - parallel one. · **deferred:** no glyph vocabulary rendered yet; falls out of the TUI roll-up in criterion 9 -- [ ] A task's acceptance-criteria roll-up is visible near the top of its TUI detail view, - not only by scrolling into the body. · **deferred:** TUI detail-header roll-up not built yet; the CLI tally landed first +- [x] Criterion states reuse the finding glyph/colour vocabulary rather than introducing a + parallel one. +- [x] A task's acceptance-criteria roll-up is visible near the top of its TUI detail view, + not only by scrolling into the body. - [ ] A decision is recorded on whether a `tracked` CRITERION must name its destination the way a `tracked` FINDING does. `SetFindingStatus` refuses a bare `tracked` and lint flags one, but `task ac --tracked --reason "just because"` is accepted — the same From ef63027ceb439c2c53c941d9227f2551af4ed756 Mon Sep 17 00:00:00 2001 From: Andy Eschbacher Date: Mon, 24 Aug 2026 18:54:10 -0400 Subject: [PATCH 4/4] feat(task): gate `task complete` on unexplained acceptance criteria MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `task complete` accepted a task with every criterion unticked. It now refuses when a criterion is unmet AND carries no state, and completes when each one is either met or explained. --force overrides. error: task "…" has 7 acceptance criterion/criteria still unmet with no reason (#1, #2, #3, #4, #5, #6, #7); tick them, give each a state (`task ac --defer|--wontfix|--tracked|--na`), or pass --force This is the task counterpart of a rule the tool already had: MoveAudit refuses to close an audit with open findings. The guard sits in the same place — the store, before the dry-run return — so a preview fails identically to the real write instead of passing and then failing. What makes the gate tolerable is the vocabulary. Before it, "refuse on unmet criteria" meant "tick every box or never finish", because an unticked box was the only thing a criterion could say. Now it can say `wontfix`, `deferred`, `tracked`, or `n/a`, and each is a DECISION — so the gate blocks silence, not disagreement. A task with three explicitly abandoned criteria completes; one with three nobody looked at does not. force is threaded through Store.Move rather than added as a separate Complete path, so the TUI's lifecycle transition is held to the same rule as the CLI's and surfaces the refusal as its error flash. Only completion is gated: deferring a task with unmet criteria is the normal case. The smoke test now proves the refusal at the exit-code boundary through a real process, then ticks and completes — it had been silently relying on completion accepting anything. Closes M5 of 2026-07-24-ai-agent-cli-ergonomics and criterion 6 of let-an-acceptance-criterion-say-more-than-done-or-not-done, which is now complete. --- cmd/tskflwctl/main_test.go | 9 +++ docs/cli/tskflwctl_task_complete.md | 12 +++- internal/cli/task.go | 22 +++++-- internal/core/service_epic_test.go | 2 +- internal/core/service_task.go | 7 +- internal/core/store.go | 4 +- internal/domain/body.go | 15 +++++ internal/store/fsstore.go | 32 +++++++-- internal/store/harden_test.go | 4 +- internal/store/move_test.go | 66 +++++++++++++++++-- internal/store/occ_test.go | 2 +- internal/store/resolve_test.go | 2 +- internal/store/schema_version_test.go | 2 +- internal/tui/dashboard_test.go | 2 +- internal/tui/entity.go | 4 +- ...f7es-2026-07-24-ai-agent-cli-ergonomics.md | 9 ++- ...riterion-say-more-than-done-or-not-done.md | 48 +++++++++++++- 17 files changed, 210 insertions(+), 32 deletions(-) diff --git a/cmd/tskflwctl/main_test.go b/cmd/tskflwctl/main_test.go index 718c609..94a4bb7 100644 --- a/cmd/tskflwctl/main_test.go +++ b/cmd/tskflwctl/main_test.go @@ -90,6 +90,15 @@ func TestSmoke_LifecycleAndExitCodes(t *testing.T) { if out, code := run(t, root, "task", "start", slug); code != 0 { t.Fatalf("task start: exit %d\n%s", code, out) } + // The scaffold's acceptance criterion is unticked and unexplained, so completing is + // refused (exit 11) — the task counterpart of `audit close` refusing while findings + // are open. Through a real process, so the gate is proven at the exit-code boundary. + if out, code := run(t, root, "task", "complete", slug); code != 11 { + t.Fatalf("complete with an unexplained criterion should exit 11, got %d\n%s", code, out) + } + if out, code := run(t, root, "task", "ac", slug, "--check", "1"); code != 0 { + t.Fatalf("task ac --check: exit %d\n%s", code, out) + } if out, code := run(t, root, "task", "complete", slug); code != 0 { t.Fatalf("task complete: exit %d\n%s", code, out) } diff --git a/docs/cli/tskflwctl_task_complete.md b/docs/cli/tskflwctl_task_complete.md index e3e8883..4f44e95 100644 --- a/docs/cli/tskflwctl_task_complete.md +++ b/docs/cli/tskflwctl_task_complete.md @@ -2,6 +2,15 @@ Move task(s) to completed +### Synopsis + +Move task(s) to completed. + +Refuses a task whose acceptance criteria are still unmet with no reason given — +the task counterpart of `audit close` refusing while findings are open. A criterion +carrying a state (`task ac --defer|--wontfix|--tracked|--na`) has been DECIDED and +does not block; only a silently unticked box does. --force completes anyway. + ``` tskflwctl task complete ... [flags] ``` @@ -16,7 +25,8 @@ tskflwctl task complete ... [flags] ### Options ``` - -h, --help help for complete + --force complete even with unmet, unexplained acceptance criteria + -h, --help help for complete ``` ### Options inherited from parent commands diff --git a/internal/cli/task.go b/internal/cli/task.go index c751c23..d4f65b0 100644 --- a/internal/cli/task.go +++ b/internal/cli/task.go @@ -675,13 +675,14 @@ func newTaskMoveCmd(app *App) *cobra.Command { if err != nil { return err // already wraps ErrValidation and lists valid statuses } - return runTransition(app, to, args[:len(args)-1]) + return runTransition(app, to, args[:len(args)-1], false) }, } } func newTransitionCmd(app *App, use, short string, to domain.Status) *cobra.Command { - return &cobra.Command{ + var force bool + cmd := &cobra.Command{ Use: use + " ...", Short: short, Example: " tskflwctl task " + use + " my-task\n tskflwctl task " + use + " task-a task-b", @@ -698,9 +699,20 @@ func newTransitionCmd(app *App, use, short string, to domain.Status) *cobra.Comm } args = []string{slug} } - return runTransition(app, to, args) + return runTransition(app, to, args, force) }, } + // Only `complete` has a gate to override — offering --force on `start` or `ready` + // would advertise a check that does not exist there. + if to == domain.StatusCompleted { + cmd.Long = short + ".\n\n" + + "Refuses a task whose acceptance criteria are still unmet with no reason given —\n" + + "the task counterpart of `audit close` refusing while findings are open. A criterion\n" + + "carrying a state (`task ac --defer|--wontfix|--tracked|--na`) has been DECIDED and\n" + + "does not block; only a silently unticked box does. --force completes anyway." + cmd.Flags().BoolVar(&force, "force", false, "complete even with unmet, unexplained acceptance criteria") + } + return cmd } // deprecatedTransitionCmd builds a hidden back-compat alias for a renamed verb: @@ -715,9 +727,9 @@ func deprecatedTransitionCmd(app *App, oldVerb, newVerb string, to domain.Status } // runTransition moves each task to status `to`, via the shared runMoves report. -func runTransition(app *App, to domain.Status, slugs []string) error { +func runTransition(app *App, to domain.Status, slugs []string, force bool) error { return runMoves(app, slugs, string(to), - func(slug string) (domain.Task, error) { return app.Svc.Move(slug, to, app.DryRun) }, + func(slug string) (domain.Task, error) { return app.Svc.Move(slug, to, app.DryRun, force) }, func(t domain.Task) string { return t.Slug }) } diff --git a/internal/core/service_epic_test.go b/internal/core/service_epic_test.go index 089078f..95607a7 100644 --- a/internal/core/service_epic_test.go +++ b/internal/core/service_epic_test.go @@ -26,7 +26,7 @@ func (nopStore) ListTasksWithBodies() ([]TaskWithBody, []domain.FileProblem, err func (nopStore) ResolveTaskPath(string) (string, error) { return "", domain.ErrNotFound } func (nopStore) ResolveEpicPath(string) (string, error) { return "", domain.ErrNotFound } func (nopStore) ResolveAuditPath(string) (string, error) { return "", domain.ErrNotFound } -func (nopStore) Move(string, domain.Status, time.Time, bool) (domain.Task, error) { +func (nopStore) Move(string, domain.Status, time.Time, bool, bool) (domain.Task, error) { return domain.Task{}, nil } func (nopStore) Defer(string, string, time.Time, bool) (domain.Task, error) { diff --git a/internal/core/service_task.go b/internal/core/service_task.go index a46ae73..3110488 100644 --- a/internal/core/service_task.go +++ b/internal/core/service_task.go @@ -169,12 +169,13 @@ func (s *Service) AppendBody(slug, text string, dryRun bool) (domain.Task, strin } // Move transitions a task to the given status (lifecycle engine behind the -// explicit verbs). Moving to the current status is an idempotent no-op. +// explicit verbs). Moving to the current status is an idempotent no-op. force bypasses +// the acceptance-criteria gate on a completion — see Store.Move. // dryRun validates everything and returns the would-be task without writing. -func (s *Service) Move(slug string, to domain.Status, dryRun bool) (domain.Task, error) { +func (s *Service) Move(slug string, to domain.Status, dryRun, force bool) (domain.Task, error) { now := s.now() return retryOnConflict(s, dryRun, func() (domain.Task, error) { - return s.store.Move(slug, to, now, dryRun) + return s.store.Move(slug, to, now, dryRun, force) }) } diff --git a/internal/core/store.go b/internal/core/store.go index c0fe526..000b374 100644 --- a/internal/core/store.go +++ b/internal/core/store.go @@ -27,7 +27,9 @@ type TaskStore interface { // Mutators take dryRun: true runs EVERY validation (resolve, parse-before- // commit, collision/CAS checks) and returns the would-be result, but stops // short of touching disk — so a dry-run that would fail fails identically. - Move(slug string, to domain.Status, now time.Time, dryRun bool) (domain.Task, error) + // force skips the acceptance-criteria gate on a move to completed (the task + // counterpart of MoveAudit's bucket↔state refusal). + Move(slug string, to domain.Status, now time.Time, dryRun, force bool) (domain.Task, error) // Defer moves a task to deferred and, when until is non-empty, records it as // revisit_at ("snooze until") in the SAME atomic write — so a deferred task can // never be left without the snooze date it was deferred with (the lost-second- diff --git a/internal/domain/body.go b/internal/domain/body.go index e45c3de..ff677e1 100644 --- a/internal/domain/body.go +++ b/internal/domain/body.go @@ -231,6 +231,21 @@ func CountAcceptanceCriteria(body string) ACCount { return c } +// UnexplainedCriteria returns the criteria that are unmet AND say nothing about why — +// a bare unticked box. They are the ones that block `task complete`, and the distinction +// is the whole point of the state vocabulary: a criterion marked `wontfix` or `deferred` +// has been DECIDED, and a decision should not stand in the way of finishing a task. Only +// silence should. +func UnexplainedCriteria(body string) []Criterion { + var out []Criterion + for _, c := range ListAcceptanceCriteria(body) { + if c.State == CriterionUnmet { + out = append(out, c) + } + } + return out +} + // CriterionCount is one state's share of a task's acceptance criteria. type CriterionCount struct { State CriterionState diff --git a/internal/store/fsstore.go b/internal/store/fsstore.go index 8ba90d4..54e0982 100644 --- a/internal/store/fsstore.go +++ b/internal/store/fsstore.go @@ -4,6 +4,7 @@ import ( "fmt" "os" "path/filepath" + "strconv" "strings" "time" @@ -117,8 +118,8 @@ func (s *FS) GetTask(slug string) (domain.Task, string, error) { // Move transitions a task to status `to`: it updates frontmatter (status + // dates) and relocates the file to the target status directory. Moving to the // current status is an idempotent no-op. -func (s *FS) Move(slug string, to domain.Status, now time.Time, dryRun bool) (domain.Task, error) { - return s.moveTask(slug, to, now, dryRun, nil) +func (s *FS) Move(slug string, to domain.Status, now time.Time, dryRun, force bool) (domain.Task, error) { + return s.moveTask(slug, to, now, dryRun, force, nil) } // Defer moves a task to deferred and records `until` as revisit_at in the SAME @@ -131,7 +132,7 @@ func (s *FS) Defer(slug, until string, now time.Time, dryRun bool) (domain.Task, if until != "" { extra = map[string]any{"revisit_at": until} } - return s.moveTask(slug, domain.StatusDeferred, now, dryRun, extra) + return s.moveTask(slug, domain.StatusDeferred, now, dryRun, false, extra) } // moveTask is the shared engine behind Move and Defer: it ensures the task ends up @@ -139,7 +140,7 @@ func (s *FS) Defer(slug, until string, now time.Time, dryRun bool) (domain.Task, // in ONE atomic write. A real transition (from != to) relocates the file; an // in-place rewrite (from == to, used by a re-defer that carries a new revisit_at) // overwrites the existing file. When nothing would change it's an idempotent no-op. -func (s *FS) moveTask(slug string, to domain.Status, now time.Time, dryRun bool, extra map[string]any) (domain.Task, error) { +func (s *FS) moveTask(slug string, to domain.Status, now time.Time, dryRun, force bool, extra map[string]any) (domain.Task, error) { if !to.Valid() { return domain.Task{}, fmt.Errorf("%q: %w", to, domain.ErrValidation) } @@ -157,6 +158,19 @@ func (s *FS) moveTask(slug string, to domain.Status, now time.Time, dryRun bool, if err != nil { return domain.Task{}, err } + // Acceptance-criteria invariant, the task counterpart of the bucket↔state rule + // MoveAudit enforces: completing a task whose criteria are silently unticked writes a + // state the reader cannot trust. A criterion carrying deferred/wontfix/tracked/n/a has + // been DECIDED and does not block — only silence does. Runs before the dry-run return + // so a preview fails identically. + if to == domain.StatusCompleted && !force { + _, body := splitFrontmatter(content) + if unmet := domain.UnexplainedCriteria(string(body)); len(unmet) > 0 { + return domain.Task{}, fmt.Errorf( + "%w: task %q has %d acceptance criterion/criteria still unmet with no reason (%s); tick them, give each a state (`task ac --defer|--wontfix|--tracked|--na`), or pass --force", + domain.ErrValidation, slug, len(unmet), criterionIndexes(unmet)) + } + } from := cur.Status date := now.Format("2006-01-02") @@ -396,3 +410,13 @@ func parseTask(content []byte, path string) (domain.Task, error) { t.Path = path return t, nil } + +// criterionIndexes lists criteria by their 1-based index, so the refusal names exactly +// which ones to act on — the numbers `task ac` already shows. +func criterionIndexes(cs []domain.Criterion) string { + out := make([]string, len(cs)) + for i, c := range cs { + out[i] = "#" + strconv.Itoa(c.Index) + } + return strings.Join(out, ", ") +} diff --git a/internal/store/harden_test.go b/internal/store/harden_test.go index c19c437..630c3aa 100644 --- a/internal/store/harden_test.go +++ b/internal/store/harden_test.go @@ -25,7 +25,7 @@ func TestFS_Move_RejectsUnreloadableWithoutMoving(t *testing.T) { path, out := testutil.TaskFixture(root, "ready-to-start", "alpha.md", original) testutil.Write(t, path, out) - _, err := NewFS(root).Move("alpha", domain.StatusInProgress, time.Now(), false) + _, err := NewFS(root).Move("alpha", domain.StatusInProgress, time.Now(), false, false) if err == nil { t.Fatal("want an error for a move that wouldn't reload") } @@ -108,7 +108,7 @@ func TestFS_Move_ConflictsWhenEditedConcurrently(t *testing.T) { } defer func() { testHookBeforeMoveWrite = nil }() - _, err := fs.Move("alpha", domain.StatusInProgress, time.Now(), false) + _, err := fs.Move("alpha", domain.StatusInProgress, time.Now(), false, false) if !errors.Is(err, domain.ErrConflict) { t.Fatalf("want ErrConflict for a concurrently-edited task, got %v", err) } diff --git a/internal/store/move_test.go b/internal/store/move_test.go index 1eebb67..3b522c2 100644 --- a/internal/store/move_test.go +++ b/internal/store/move_test.go @@ -27,7 +27,7 @@ func TestFS_Move(t *testing.T) { path := writeTaskAt(t, root, "ready-to-start", "alpha.md", "---\nstatus: ready-to-start\nepic: 01-x\n---\n# Alpha\n") now := time.Date(2026, 6, 7, 0, 0, 0, 0, time.UTC) - task, err := NewFS(root).Move("alpha", domain.StatusInProgress, now, false) + task, err := NewFS(root).Move("alpha", domain.StatusInProgress, now, false, false) if err != nil { t.Fatal(err) } @@ -50,7 +50,7 @@ func TestFS_Move(t *testing.T) { func TestFS_Move_Idempotent(t *testing.T) { root := t.TempDir() writeTask(t, root, "in-progress", "beta.md", "---\nstatus: in-progress\n---\n# B\n") - task, err := NewFS(root).Move("beta", domain.StatusInProgress, time.Now(), false) + task, err := NewFS(root).Move("beta", domain.StatusInProgress, time.Now(), false, false) if err != nil { t.Fatal(err) } @@ -83,7 +83,7 @@ func TestFS_Move_RevisitAt(t *testing.T) { // Re-defer (deferred -> deferred): idempotent no-op, snooze date untouched. redeferPath := deferred("redefer.md") - task, err := fs.Move("redefer", domain.StatusDeferred, now, false) + task, err := fs.Move("redefer", domain.StatusDeferred, now, false, false) if err != nil { t.Fatal(err) } @@ -106,7 +106,7 @@ func TestFS_Move_RevisitAt(t *testing.T) { } { name := "leave-" + tc.dir + ".md" path := deferred(name) - if _, err := fs.Move(strings.TrimSuffix(name, ".md"), tc.to, now, false); err != nil { + if _, err := fs.Move(strings.TrimSuffix(name, ".md"), tc.to, now, false, false); err != nil { t.Fatalf("move to %s: %v", tc.to, err) } got := read(path) @@ -191,7 +191,7 @@ func TestFS_Defer_BareNoDate(t *testing.T) { } func TestFS_Move_NotFound(t *testing.T) { - _, err := NewFS(t.TempDir()).Move("nope", domain.StatusCompleted, time.Now(), false) + _, err := NewFS(t.TempDir()).Move("nope", domain.StatusCompleted, time.Now(), false, false) if !errors.Is(err, domain.ErrNotFound) { t.Errorf("want ErrNotFound, got %v", err) } @@ -205,7 +205,7 @@ func TestFS_Resolve_Ambiguous(t *testing.T) { idB := testutil.TaskID("dup-b") testutil.Write(t, filepath.Join(root, "tasks", idA+"-dup.md"), "---\nstatus: ready-to-start\n---\n") testutil.Write(t, filepath.Join(root, "tasks", idB+"-dup.md"), "---\nstatus: in-progress\n---\n") - _, err := NewFS(root).Move("dup", domain.StatusCompleted, time.Now(), false) + _, err := NewFS(root).Move("dup", domain.StatusCompleted, time.Now(), false, false) if !errors.Is(err, domain.ErrAmbiguous) { t.Errorf("want ErrAmbiguous, got %v", err) } @@ -217,3 +217,57 @@ func TestFS_Resolve_Ambiguous(t *testing.T) { } } } + +// Criterion 6 of let-an-acceptance-criterion-say-more-than-done-or-not-done, decided +// 2026-08-24: completing a task whose criteria are silently unticked writes a state the +// reader cannot trust, so it is refused — the task counterpart of MoveAudit refusing to +// close an audit with open findings. A criterion carrying a STATE has been decided and +// does not block; only silence does. +func TestMove_CompleteGatesOnUnexplainedCriteria(t *testing.T) { + body := func(criteria string) string { + return "---\nid: 6fjangd7kvh3\nstatus: in-progress\ndescription: d\ntags: [a]\n---\n\n## Acceptance criteria\n\n" + criteria + } + now := time.Date(2026, 8, 24, 0, 0, 0, 0, time.UTC) + + t.Run("a bare unticked box refuses", func(t *testing.T) { + root := t.TempDir() + writeTask(t, root, "in-progress", "6fjangd7kvh3-gated.md", body("- [x] done\n- [ ] silently unticked\n")) + _, err := NewFS(root).Move("gated", domain.StatusCompleted, now, false, false) + if !errors.Is(err, domain.ErrValidation) { + t.Fatalf("want ErrValidation, got %v", err) + } + if !strings.Contains(err.Error(), "#2") { + t.Errorf("the refusal should name which criterion to act on: %v", err) + } + // …and the refusal must be identical under --dry-run, so a preview cannot pass + // where the real write would fail. + if _, err := NewFS(root).Move("gated", domain.StatusCompleted, now, true, false); !errors.Is(err, domain.ErrValidation) { + t.Errorf("dry-run must fail identically, got %v", err) + } + }) + + t.Run("an explained criterion does not block", func(t *testing.T) { + root := t.TempDir() + writeTask(t, root, "in-progress", "6fjangd7kvh3-decided.md", + body("- [x] done\n- [ ] parked · **deferred:** waiting on the ADR\n- [ ] moot · **n/a:** dropped\n")) + if _, err := NewFS(root).Move("decided", domain.StatusCompleted, now, false, false); err != nil { + t.Fatalf("decided criteria must not block completion: %v", err) + } + }) + + t.Run("force completes anyway", func(t *testing.T) { + root := t.TempDir() + writeTask(t, root, "in-progress", "6fjangd7kvh3-forced.md", body("- [ ] silently unticked\n")) + if _, err := NewFS(root).Move("forced", domain.StatusCompleted, now, false, true); err != nil { + t.Fatalf("--force must bypass the gate: %v", err) + } + }) + + t.Run("only completion is gated", func(t *testing.T) { + root := t.TempDir() + writeTask(t, root, "in-progress", "6fjangd7kvh3-parked.md", body("- [ ] silently unticked\n")) + if _, err := NewFS(root).Move("parked", domain.StatusDeferred, now, false, false); err != nil { + t.Fatalf("deferring is not completing and must not be gated: %v", err) + } + }) +} diff --git a/internal/store/occ_test.go b/internal/store/occ_test.go index a76cb3f..7e70c23 100644 --- a/internal/store/occ_test.go +++ b/internal/store/occ_test.go @@ -89,7 +89,7 @@ func TestMove_ConflictsOnConcurrentContentEdit(t *testing.T) { testHookBeforeMoveWrite = orig } - _, err := fs.Move("m", domain.StatusInProgress, time.Date(2026, 6, 20, 0, 0, 0, 0, time.UTC), false) + _, err := fs.Move("m", domain.StatusInProgress, time.Date(2026, 6, 20, 0, 0, 0, 0, time.UTC), false, false) if !errors.Is(err, domain.ErrConflict) { t.Fatalf("a concurrent in-place edit during a move must conflict, got %v", err) } diff --git a/internal/store/resolve_test.go b/internal/store/resolve_test.go index fa1999f..657db20 100644 --- a/internal/store/resolve_test.go +++ b/internal/store/resolve_test.go @@ -83,7 +83,7 @@ func TestResolve_FuzzyTiers(t *testing.T) { // id-led path and only its frontmatter status changes in place. func TestMove_FuzzyKeepsCanonicalSlug(t *testing.T) { fs := fuzzyRepo(t) - task, err := fs.Move("backoff", domain.StatusInProgress, time.Now(), false) + task, err := fs.Move("backoff", domain.StatusInProgress, time.Now(), false, false) if err != nil { t.Fatal(err) } diff --git a/internal/store/schema_version_test.go b/internal/store/schema_version_test.go index 1196388..3ecfb9b 100644 --- a/internal/store/schema_version_test.go +++ b/internal/store/schema_version_test.go @@ -59,7 +59,7 @@ func TestSchemaVersion_ParsesAndSurvivesEdits(t *testing.T) { if got := readFile(t, path); !strings.Contains(got, "schema: 1") { t.Errorf("SetFields dropped the reserved schema key:\n%s", got) } - if _, err := fs.Move("keep", domain.StatusInProgress, bodyNow, false); err != nil { + if _, err := fs.Move("keep", domain.StatusInProgress, bodyNow, false, false); err != nil { t.Fatal(err) } got := readFile(t, path) diff --git a/internal/tui/dashboard_test.go b/internal/tui/dashboard_test.go index cfcfb87..b601259 100644 --- a/internal/tui/dashboard_test.go +++ b/internal/tui/dashboard_test.go @@ -370,7 +370,7 @@ func TestModel_DashboardRefreshesOnMutation(t *testing.T) { } // Move alpha out of the working set behind the dashboard's back (as the CLI or // another process would), then reload. - if _, err := m.svc.Move("alpha", domain.StatusCompleted, false); err != nil { + if _, err := m.svc.Move("alpha", domain.StatusCompleted, false, false); err != nil { t.Fatal(err) } m = drainBatch(t, m, m.reloadAll()) diff --git a/internal/tui/entity.go b/internal/tui/entity.go index 08b7e81..9773964 100644 --- a/internal/tui/entity.go +++ b/internal/tui/entity.go @@ -234,7 +234,9 @@ func (t *entityTab) matches(word string) bool { // (movedMsg → flash + reload) or failure (actionErrMsg → flash, no reload). func moveTask(svc *core.Service, id string, tr transition) tea.Cmd { return func() tea.Msg { - if _, err := svc.Move(id, domain.Status(tr.to), false); err != nil { + // force=false: a TUI completion is held to the same acceptance-criteria gate as + // the CLI's, and the refusal surfaces as the action's error flash. + if _, err := svc.Move(id, domain.Status(tr.to), false, false); err != nil { return actionErrMsg{slug: id, err: err} } return movedMsg{slug: id, to: tr.to} diff --git a/planning/audits/6fsa47r4f7es-2026-07-24-ai-agent-cli-ergonomics.md b/planning/audits/6fsa47r4f7es-2026-07-24-ai-agent-cli-ergonomics.md index 9016990..4fe3628 100644 --- a/planning/audits/6fsa47r4f7es-2026-07-24-ai-agent-cli-ergonomics.md +++ b/planning/audits/6fsa47r4f7es-2026-07-24-ai-agent-cli-ergonomics.md @@ -4,6 +4,7 @@ id: 6fsa47r4f7es bucket: open area: ai-agent-cli-ergonomics date: "2026-07-24" +updated_at: "2026-08-24" --- # Audit: AI-agent CLI ergonomics — 2026-07-24 @@ -248,7 +249,7 @@ equivalent write surface. parser. Do not build a general Markdown editor. The useful abstraction is a small set of domain operations over the conventions the tool already owns. -#### M5. `task complete` does not reconcile unfinished acceptance criteria · **Status:** open +#### M5. `task complete` does not reconcile unfinished acceptance criteria · **Status:** fixed 2026-08-24 **File:** internal/cli/moves.go; internal/core/service_task.go | **Component:** workflow integrity **Effort:** S · **Urgency:** soon @@ -266,6 +267,12 @@ Alternatively make this policy configurable per planning repo, but always surfac unchecked count in the transition receipt and in lint. Never auto-check criteria merely because the status changed. +**Resolution:** task complete now refuses when a criterion is unmet with no +reason, mirroring MoveAudit's refusal to close an audit with open findings, with +--force to override. The gate only became tolerable once a criterion could carry +a state: it blocks silence, not disagreement — a task with three explicitly +abandoned criteria completes, one with three never looked at does not. + #### M6. Multi-document agent workflows have no preflighted, restartable change set · **Status:** open **File:** internal/cli/moves.go | **Component:** orchestration diff --git a/planning/tasks/6g31g9f8x4cv-let-an-acceptance-criterion-say-more-than-done-or-not-done.md b/planning/tasks/6g31g9f8x4cv-let-an-acceptance-criterion-say-more-than-done-or-not-done.md index f63b13a..e34b1bc 100644 --- a/planning/tasks/6g31g9f8x4cv-let-an-acceptance-criterion-say-more-than-done-or-not-done.md +++ b/planning/tasks/6g31g9f8x4cv-let-an-acceptance-criterion-say-more-than-done-or-not-done.md @@ -1,7 +1,7 @@ --- schema: 1 id: 6g31g9f8x4cv -status: in-progress +status: completed epic: 20-cli-ux-and-ergonomics description: Criteria are a binary checkbox while findings carry a seven-state vocabulary; an unchecked box cannot distinguish not-yet from won't-do from deferred. effort: M @@ -12,6 +12,7 @@ tags: [cli, domain, planning-model] created: "2026-08-23" updated_at: "2026-08-24" started_at: "2026-08-24" +completed_at: "2026-08-24" --- # Let an acceptance criterion say more than done or not-done @@ -75,7 +76,7 @@ Design-first; do not start implementing until these are answered. fenced blocks being ignored exactly as today. - [x] `ACCount` and every surface that renders a tally (`task show`, `status`, the TUI) keep reporting something honest when criteria are no longer two-valued. -- [ ] A decision is recorded on whether `task complete` gates on unmet criteria, either way. · **deferred:** workflow change; decide once the vocabulary has been lived with — no rework either way +- [x] A decision is recorded on whether `task complete` gates on unmet criteria, either way. ## Out of scope @@ -121,7 +122,7 @@ the surface that shows unmet criteria is the surface that would explain a refusa parallel one. - [x] A task's acceptance-criteria roll-up is visible near the top of its TUI detail view, not only by scrolling into the body. -- [ ] A decision is recorded on whether a `tracked` CRITERION must name its destination the +- [x] A decision is recorded on whether a `tracked` CRITERION must name its destination the way a `tracked` FINDING does. `SetFindingStatus` refuses a bare `tracked` and lint flags one, but `task ac --tracked --reason "just because"` is accepted — the same word carrying a weaker guarantee on one of the two entities that share it. The @@ -198,3 +199,44 @@ complaint, not repeated here. is stripped before matching, because this repo's own candidate lists use ✅ ⏳ ⛔ and finding M2 shows what happens when that is left to chance. - **Trailing prose tolerated** the way `**Status:** fixed 2026-01-01 (PR #9)` already is. + +### `task complete` gates on unexplained criteria — decided 2026-08-24 + +`task complete` refuses when a criterion is unmet AND carries no state, and completes +when every criterion is either met or explained. `--force` overrides. + +This is the task counterpart of a rule the tool already had: `MoveAudit` refuses to close +an audit with open findings. Same situation, same answer — and putting the guard in the +same place (the store, before the dry-run return) means a `--dry-run` preview fails +identically to the real write rather than passing and then failing. + +What makes the gate tolerable is the vocabulary itself. Before it, "refuse on unmet +criteria" would have meant "tick every box or never finish", because an unticked box was +the only way to say anything. Now a criterion can say `wontfix`, `deferred`, `tracked`, or +`n/a`, and each of those is a DECISION — so the gate blocks silence, not disagreement. You +can complete a task with three criteria you have explicitly abandoned; you cannot complete +one with three you never looked at. + +The refusal names the criteria by the same 1-based index `task ac` prints, and the detail +header roll-up (criterion 9) is where a reader sees the same thing before trying. + +Closes M5 of [2026-07-24-ai-agent-cli-ergonomics](../audits/6fsa47r4f7es-2026-07-24-ai-agent-cli-ergonomics.md). + +### A `tracked` destination is checked for PRESENCE, not shape — decided 2026-08-24 + +Both entities require a non-empty explanation of where the work went, and neither validates +its form. `tracked by 6g3ag8py12y9` and `tracked by the config epic` are both accepted. + +The finding that raised this (M3 of 2026-08-24-finding-note-and-vocabulary-selfreview) +overstated the asymmetry it found. It said findings *enforce* a destination while criteria +do not; in fact `SetFindingStatus` only requires the decoration to be non-empty — `tracked +hmm` passes. The two were already symmetric in strictness. What differed was the wording of +the error, not the rule. + +Shape validation was considered and rejected: a destination is legitimately an epic id, an +ADR, or an external issue, and a Crockford-id regex would reject `tracked by ADR-0003`, +which is a perfectly good handoff. The check that WOULD be worth having is resolution — lint +flagging a `tracked` that names an id which does not exist in the workspace — because that +catches a typo or a deleted destination, which a shape regex never would. It is not built +here; it is a better idea than the one this criterion asked about, and belongs with the +other lint work rather than bolted on.