diff --git a/cmd/tskflwctl/main_test.go b/cmd/tskflwctl/main_test.go index 718c6092..94a4bb77 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 e3e88835..4f44e956 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/render/render.go b/internal/cli/render/render.go index ae630495..8c406a95 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 174ca45e..853476df 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/cli/task.go b/internal/cli/task.go index c751c23d..d4f65b00 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 089078fe..95607a7e 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 a46ae735..3110488a 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 c0fe5263..000b3745 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 ac810c8f..ff677e19 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 { @@ -201,6 +231,45 @@ 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 + 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 { @@ -248,20 +317,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 37142904..42d2c234 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,117 @@ 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) + } + } +} + +// 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/store/fsstore.go b/internal/store/fsstore.go index 8ba90d42..54e09823 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 c19c437d..630c3aab 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 1eebb67b..3b522c2e 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 a76cb3f0..7e70c23a 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 fa1999f1..657db20b 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 11963888..3ecfb9b4 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/theme/theme.go b/internal/theme/theme.go index 3e229a97..a271f7a8 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 de43bc70..0cd87ba9 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/dashboard_test.go b/internal/tui/dashboard_test.go index cfcfb875..b6012590 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/detail.go b/internal/tui/detail.go index 688f122e..d405d189 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 00000000..6abee15e --- /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/tui/entity.go b/internal/tui/entity.go index 08b7e813..97739646 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/internal/wire/schema_comments.json b/internal/wire/schema_comments.json index 54320714..ad6802b3 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/audits/6fsa47r4f7es-2026-07-24-ai-agent-cli-ergonomics.md b/planning/audits/6fsa47r4f7es-2026-07-24-ai-agent-cli-ergonomics.md index 9016990c..4fe36289 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/audits/6g1397jfke23-2026-08-17-finding-status-surface.md b/planning/audits/6g1397jfke23-2026-08-17-finding-status-surface.md index b3dc1eaf..f01bebc8 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 ebff045e..ba4ccc32 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 4148e524..90e805e6 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 e3d8c0aa..9f9e6c09 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 4f2a1dbd..d3704ddd 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 4f3acdc4..e34b1bc1 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: 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 @@ -11,6 +11,8 @@ autonomy_level: 3 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 @@ -74,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 @@ -116,10 +118,19 @@ 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 +- [x] Criterion states reuse the finding glyph/colour vocabulary rather than introducing a 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 +- [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. +- [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 + 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`. ## Decisions, 2026-08-24 — settled before implementation @@ -188,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.