diff --git a/docs/memory/cli/index.md b/docs/memory/cli/index.md index 0fa5803..eaa7c11 100644 --- a/docs/memory/cli/index.md +++ b/docs/memory/cli/index.md @@ -7,7 +7,7 @@ description: "CLI source structure (cmd/idea + internal/idea + version wiring), | File | Description | Last Updated | |------|-------------|-------------| -| [edit](edit.md) | `idea edit` two-form contract: inline replacement (two-arg form) vs. $EDITOR round-trip (`edit `) — editor resolution chain, temp-file mechanics, edge/exit semantics, and the fake-editor test seam | — | +| [edit](edit.md) | `idea edit` two-form contract: inline replacement (two-arg form) vs. $EDITOR round-trip (`edit `) — editor resolution chain, temp-file mechanics, edge/exit semantics, and the fake-editor test seam | 2026-06-12 | | [prune](prune.md) | Bulk-remove subcommand (`idea prune`): dry-run-by-default/`--force` contract, stdout/stderr channel split, exit codes, the deliberate non-archival design, and the `removeIdeaAt` seam shared with `Rm` | 2026-06-12 | -| [structure](structure.md) | Source tree layout (cmd/idea + internal/idea), root command factory, command aliases vs. the bare-text shorthand, backlog line lifecycle (lenient read / canonical write incl. the escaped-text convention for multiline ideas), help-dump contract, and version stamping | 2026-06-12 | +| [structure](structure.md) | Source tree layout (cmd/idea + internal/idea), root command factory, command aliases vs. the bare-text shorthand, backlog line lifecycle (lenient read / canonical write incl. the escaped-text convention for multiline ideas and the explicit `idea fmt` canonicalizer with bare-checkbox adoption), help-dump contract, and version stamping | 2026-06-12 | | [update](update.md) | Self-update subcommand (`idea update`): Homebrew-backed upgrade flow, non-brew install fallback hint, and the `--skip-brew-update` flag | 2026-06-10 | diff --git a/docs/memory/cli/structure.md b/docs/memory/cli/structure.md index 7e147d8..5ee3505 100644 --- a/docs/memory/cli/structure.md +++ b/docs/memory/cli/structure.md @@ -1,5 +1,5 @@ --- -description: "Source tree layout (cmd/idea + internal/idea), root command factory, command aliases vs. the bare-text shorthand, backlog line lifecycle (lenient read / canonical write incl. the escaped-text convention for multiline ideas), help-dump contract, and version stamping" +description: "Source tree layout (cmd/idea + internal/idea), root command factory, command aliases vs. the bare-text shorthand, backlog line lifecycle (lenient read / canonical write incl. the escaped-text convention for multiline ideas and the explicit `idea fmt` canonicalizer with bare-checkbox adoption), help-dump contract, and version stamping" --- # CLI Source Structure @@ -15,15 +15,17 @@ src/ cmd/ idea/ # cobra entry point; one file per subcommand main.go # newRootCmd() factory + main() - add.go list.go show.go done.go reopen.go edit.go rm.go prune.go resolve.go update.go shell_init.go + add.go list.go show.go done.go reopen.go edit.go rm.go prune.go fmt.go resolve.go update.go shell_init.go help_dump.go # hidden "help-dump" subcommand (CLI tree → JSON) - main_test.go shell_init_test.go help_dump_test.go + main_test.go fmt_test.go shell_init_test.go help_dump_test.go internal/ idea/ # package logic (parsing, formatting, ID gen, file I/O, worktree resolution, self-update, $EDITOR round-trip) idea.go idea_test.go editor.go editor_test.go + fmt.go # idea fmt: Fmt/FmtResult + bare-checkbox adoption + fmt_test.go prune_test.go update.go update_test.go @@ -134,14 +136,38 @@ func formatLineWith(i Idea, text string) string { - `FormatLine(i)` = `formatLineWith(i, EscapeText(i.Text))` — the **persisted/escaped** form; the single source of on-disk output truth. Every write path inherits the one-physical-line guarantee through it: `Add`'s append, `SaveFile`'s rebuild, the confirmations, and `RequireSingle`'s multi-match error listing. - `DisplayLine(i)` = `formatLineWith(i, i.Text)` — the **real-text** form for human-facing display (plain `idea show`). -Output is always canonical: `- ` bullet, no leading whitespace, date present, single-space delimiters, escaped text, LF line endings (`SaveFile` joins on `\n` and ends the file with a single trailing LF). Because `SaveFile` regenerates **every** recognized idea line from `FormatLine`, the first mutating command (`done`/`reopen`/`edit`/`rm`) normalizes the whole file at once: variant bullets → `-`, indentation stripped, CRLF → LF, dateless → dated, legacy lone backslashes → doubled (`\\`). This **normalize-on-write** is a deliberate, accepted trade-off — a single `idea done` can produce a large git diff on a file with many variant/dateless lines. Non-idea lines (headers, blank lines, prose) pass through unchanged (Constitution Principle I). +Output is always canonical: `- ` bullet, no leading whitespace, date present, single-space delimiters, escaped text, LF line endings (`SaveFile` joins on `\n` and ends the file with a single trailing LF). Because `SaveFile` regenerates **every** recognized idea line from `FormatLine`, the first mutating command (`done`/`reopen`/`edit`/`rm`, or `prune --force` when it removes items) normalizes the whole file at once: variant bullets → `-`, indentation stripped, CRLF → LF, dateless → dated, legacy lone backslashes → doubled (`\\`). This **normalize-on-write** is a deliberate, accepted trade-off — a single `idea done` can produce a large git diff on a file with many variant/dateless lines. To land that churn as its own commit — with no semantic change riding along — use the explicit canonicalizer `idea fmt` (see below). Non-idea lines (headers, blank lines, prose) pass through unchanged (Constitution Principle I). -**Date backfill on save.** `SaveFile` stamps `time.Now().Format("2006-01-02")` on any idea whose `Date == ""` *before* serializing, and returns `(count, error)` — the count of backfilled dates. Stamping at the save seam (not in `ParseLine`) keeps `ParseLine` pure and keeps `MarshalJSON` correct, since the in-memory `Idea` has a date by the time it is marshaled after a save. The write is atomic (temp file + rename) so a crash mid-write cannot leave the source-of-truth backlog partially written. +**Date backfill on save.** The stamping lives in `render(f *File, today string) (string, int)` — the date-stamp + rebuild step extracted from `SaveFile` by `260612-4m3a-add-fmt-canonicalizer-adoption` so `Fmt` can build the canonical content without writing it. `render` stamps the caller-supplied `today` on any idea whose `Date == ""` *before* serializing and returns the backfill count; `SaveFile` is now `render(f, time.Now().Format("2006-01-02"))` + atomic write, still returning `(count, error)`, while `Fmt` passes the same `today` it used for counting and adoption so one run stamps one consistent date even across a midnight boundary (PR-review fix). Stamping at the save seam (not in `ParseLine`) keeps `ParseLine` pure and keeps `MarshalJSON` correct, since the in-memory `Idea` has a date by the time it is marshaled after a save. The write is atomic (temp file + rename) so a crash mid-write cannot leave the source-of-truth backlog partially written. -**Backfill stderr notice (Constitution IV split).** The backfill count flows up to the command layer: the mutating internal ops `Done`, `Reopen`, `Edit`, `Rm` return `(Idea, int, error)`. When count > 0, the `cmd/idea` layer prints `note: stamped today's date on N previously-dateless item(s)` to **stderr** via the `printBackfillNotice` helper (`main.go`, using `cmd.ErrOrStderr()`); it is suppressed entirely at count 0. stdout stays the machine-parseable confirmation only (Constitution Principle VI). `internal/idea` writes nothing to stderr — output-channel policy lives in `cmd/` per Principle IV. The backfill notice was the first idea command output deliberately routed to stderr; `prune`'s dry-run confirm hint (`Re-run with --force to confirm.` — see `prune.md`) and `idea edit`'s editor-form no-op note (`note: text unchanged — nothing to do` — see `edit.md`) followed, making advisory-notes-to-stderr the established channel policy rather than a one-off. +**Backfill stderr notice (Constitution IV split).** The backfill count flows up to the command layer: the mutating internal ops `Done`, `Reopen`, `Edit`, `Rm` return `(Idea, int, error)`. When count > 0, the `cmd/idea` layer prints `note: stamped today's date on N previously-dateless item(s)` to **stderr** via the `printBackfillNotice` helper (`main.go`, using `cmd.ErrOrStderr()`); it is suppressed entirely at count 0. stdout stays the machine-parseable confirmation only (Constitution Principle VI). `internal/idea` writes nothing to stderr — output-channel policy lives in `cmd/` per Principle IV. The backfill notice was the first idea command output deliberately routed to stderr; `prune`'s dry-run confirm hint (`Re-run with --force to confirm.` — see `prune.md`), `idea edit`'s editor-form no-op note (`note: text unchanged — nothing to do` — see `edit.md`), and `idea fmt`'s entire report (adoption lines + summary counts — see below) followed, making advisory-notes-to-stderr the established channel policy rather than a one-off. The behavior contract is documented for external consumers in `../../specs/backlog-format.md` and `../../specs/overview.md`. +### Explicit canonicalizer & adoption (`idea fmt`) + +`idea fmt` (added by `260612-4m3a-add-fmt-canonicalizer-adoption`) is the explicit, gofmt-style trigger for the canonical write above: it rewrites the whole backlog into canonical form with no semantic change required, so the normalize-on-write churn can land as its own commit. `fmt` is the **only explicit whole-file write verb** — mutating CRUD commands keep their incidental normalize-on-write; `list`/`show` stay non-mutating. + +**Internal seams (no second serialization path).** `internal/idea/fmt.go` exports `Fmt(path string, check bool) (FmtResult, error)` with `FmtResult{Adopted []Idea, Normalized, Backfilled int, Changed bool}`. To let `Fmt` reuse the single parse walk and single serialization point, the same change split `LoadFile` into a thin wrapper over `parseContent(content string) *File` and `SaveFile` into atomic-write over `render(f *File, today string) (string, int)` (date-stamp + rebuild, no write; the caller supplies the date). `File.lines` now retains each idea line's raw (post-`\r`-strip) text instead of the former `""` placeholder — required for per-line `Normalized` counting (canonical `FormatLine` output vs. raw line) and invisible to every other caller, since `render`'s rebuild overwrites idea slots from `FormatLine`. Public `LoadFile`/`SaveFile` behavior is unchanged. + +**Automatic adoption of bare checkboxes.** `fmt` additionally brings plain markdown task-list lines under management — the only path that does. A line is an adoption candidate iff `ParseLine` rejects it AND it matches `adoptRegex` (`internal/idea/fmt.go`): + +```go +^\s*[-*+] \[([ xX])\] (.+)$ +``` + +with two guards evaluated on the **whitespace-trimmed** captured text — trimmed first so extra spaces between the checkbox and a bracket cannot defeat the guard (`- [ ] [DEV-1011] x` stays inert; tightened by this change's post-review rework): blank text is not adopted, and bracket-led text (`shapeBPrefixRegex` — Shape B remnants, `[DEV-1011]`, `[TODO]`, non-4-char `[ab1]`) is not adopted, erring toward preservation. Each adopted line gets a fresh 4-char ID unique against both the file's parsed IDs and IDs assigned earlier in the same pass (`generateUniqueIDInSet`, an in-memory-set variant mirroring `generateUniqueID`'s retry skeleton — dedup of the two deferred), date = today (counted as *Adopted*, never as *Backfilled* — backfill counts only pre-existing managed dateless lines), checked state preserved (`[x]`/`[X]` adopt as done, canonical `[x]`), and the trimmed text stored as real text (escaped via `FormatLine` on write like every idea). Adopted indented/nested checkboxes flatten to top level — canonical form has no indentation. + +**Whole-file verdict.** `FmtResult.Changed` compares the rebuilt content against the original on-disk bytes; it drives both the write (skipped entirely when byte-identical — an idempotent second run touches nothing, not even mtime; a 0-byte file is left untouched, no trailing LF invented) and the `--check` exit code. Accepted edge: a CRLF-only or EOF-newline-only difference rewrites the file / fails `--check` while the per-line counts may read zero — `Changed` is authoritative, the counts are approximate. + +**cmd layer & output contract** (`cmd/idea/fmt.go`): `fmtCmd()` with a local `--check` flag and `Args: cobra.NoArgs`. stdout stays empty on every path — success is silence + exit 0, the `gofmt -w` precedent (Constitution VI). stderr composition, in order: one `adopted: [id] {escaped text}` line per adopted idea (file order, via `EscapeText`), then `printBackfillNotice` verbatim, then `fmt: N line(s) normalized, M line(s) adopted` printed only when the file changes (or would change). Zero-activity runs print nothing; `internal/idea` writes nothing to stderr (Constitution IV split). `--check` writes nothing, prints the same report, and exits 1 via the `errSilent` sentinel when non-canonical (0 when clean) — one flag serving both the dry-run preview and the scripts/CI gate. Accepted edge: the `--check` report shows freshly generated would-be IDs that are never persisted — a later real run assigns different ones. + +**Namespace claim.** Cobra resolves the `fmt` subcommand name before the root bare-text fallback, so `idea fmt some text` errors instead of adding an idea — the same namespace trade as the `ls` alias decision; "fmt" was judged to plausibly never begin bare idea prose. + +**Governance note (Constitution I).** Adoption narrows the round-trip preservation guarantee: bare checkbox lines lacking the `[id]` anchor were previously guaranteed non-idea pass-through, and `fmt` (and only `fmt`) now claims them. The carve-out is documented in `../../specs/backlog-format.md` (Round-Trip Preservation carve-out + format-contract change note) but the constitution text itself is unamended — judged defensible at review because `fmt` is an explicit migration verb the user invokes, not round-trip parsing; a future constitution amendment could codify the carve-out explicitly. + +**Uniqueness blind spot (consistent, accepted).** Adopted-ID uniqueness checks parsed ideas only — a 4-char bracket inside an unparseable line is invisible to it, exactly as it is to `Add`'s `checkIDCollision`. + ## Command help text (`Short` vs `Long`) Every subcommand sets an enriched cobra `Long` describing what it does, its key flags, the worktree-vs-`--main` resolution (for backlog-touching commands), and a short example. `Short` stays the terse one-liner used by the `Available Commands` sidebar and the `idea -h` root listing — it is a public, byte-stable string; depth goes in `Long` only. The convention was applied repo-wide by `260602-s73u-enrich-command-long-help` (the 8 backlog/update commands; `main.go` / `shell_init.go` already carried `Long`). @@ -155,7 +181,7 @@ This is the single source for the shll.ai command-reference: the `help-dump` sub ```go root.AddCommand( addCmd(), listCmd(), showCmd(), doneCmd(), reopenCmd(), - editCmd(), rmCmd(), pruneCmd(), updateCmd(), newShellInitCmd(), helpDumpCmd(), + editCmd(), rmCmd(), pruneCmd(), fmtCmd(), updateCmd(), newShellInitCmd(), helpDumpCmd(), ) ``` @@ -170,7 +196,7 @@ The factory exists so the live cobra tree can be constructed in two places off t **Routing rule (load-bearing).** Cobra resolves subcommand names **and aliases** before the root `RunE` bare-text fallback fires. Two consequences: 1. Before the alias existed, `idea ls` did not error — it fell through to the bare-text shorthand and silently appended a junk idea with the text "ls". The alias fixed that footgun. -2. Every alias permanently removes a word from the start of bare-text idea capture (`idea ` → `idea add `). Adding an alias is therefore a **namespace decision, not a convenience decision** — any word claimed as an alias can never again begin an idea typed bare. The same holds for **subcommand names** themselves: the `prune` verb (added by `260612-drc1-add-prune-subcommand`) claims `prune` from the bare-text namespace — `idea prune the old cache` now routes to the subcommand (and errors under its `cobra.NoArgs`) instead of capturing an idea beginning with "prune". +2. Every alias permanently removes a word from the start of bare-text idea capture (`idea ` → `idea add `). Adding an alias is therefore a **namespace decision, not a convenience decision** — any word claimed as an alias can never again begin an idea typed bare. The same holds for **subcommand names** themselves: the `prune` verb (added by `260612-drc1-add-prune-subcommand`) claims `prune` from the bare-text namespace — `idea prune the old cache` now routes to the subcommand (and errors under its `cobra.NoArgs`) instead of capturing an idea beginning with "prune"; `fmt` was accepted on the same bar by `260612-4m3a-add-fmt-canonicalizer-adoption` ("fmt" plausibly never begins bare idea prose). **Rejected aliases** (surveyed and rejected in the 04rt intake discussion; future proposals must clear the same bar): `remove`/`delete` (rm), `upgrade` (update), `cat` (show) — each plausibly starts bare-text idea prose; `undo` (reopen) — implies revert-last-action semantics worth reserving. Scope decision: `ls` is the only alias. diff --git a/docs/specs/backlog-format.md b/docs/specs/backlog-format.md index b07e228..b9381bf 100644 --- a/docs/specs/backlog-format.md +++ b/docs/specs/backlog-format.md @@ -70,7 +70,7 @@ A dateless line and its dated counterpart parse to the same `Idea` modulo the `D ## Date Backfill on Write -`idea` does not preserve datelessness. When a recognized idea has no date and the file is saved by a mutating command (`done`, `reopen`, `edit`, `rm`), `idea` backfills **today's date** before writing, so every persisted idea line carries a date. +`idea` does not preserve datelessness. When a recognized idea has no date and the file is saved by a mutating command (`done`, `reopen`, `edit`, `rm`, or `prune --force` when it removes items) or by `idea fmt`, `idea` backfills **today's date** before writing, so every persisted idea line carries a date. Worked example (`idea done rk7t` on a dateless line, run on 2026-06-10): @@ -89,7 +89,7 @@ The notice is suppressed entirely when no dates were backfilled. ## Normalize-on-Write -`idea` rebuilds **every** recognized idea line from the canonical form on save — not just the line you edited. So the first mutating command (`done`/`edit`/`reopen`/`rm`) canonicalizes the whole file at once: +`idea` rebuilds **every** recognized idea line from the canonical form on save — not just the line you edited. So the first mutating command (`done`/`edit`/`reopen`/`rm`, or `prune --force` when it removes items) canonicalizes the whole file at once: - variant bullets (`*`, `+`) → `-` - leading indentation → stripped @@ -97,7 +97,37 @@ The notice is suppressed entirely when no dates were backfilled. - dateless → dated (today) - legacy lone backslashes in description text → doubled (`a\b` → `a\\b` on disk; the decoded content is unchanged) -This is a deliberate, accepted trade-off. A single `idea done` on one item can therefore produce a larger git diff if the file had many variant or dateless lines. **Non-mutating** commands (`list`, `show`) never rewrite the file, so pure reads are diff-free. +This is a deliberate, accepted trade-off. A single `idea done` on one item can therefore produce a larger git diff if the file had many variant or dateless lines. **Non-mutating** commands (`list`, `show`) never rewrite the file, so pure reads are diff-free. To land the canonicalization churn as its own diff — without a semantic change riding along — use `idea fmt` (next section). + +## Explicit Canonicalization & Adoption (`idea fmt`) + +`idea fmt` is the explicit, gofmt-style canonicalizer: it rewrites the whole backlog into the canonical form using the same rule set as [normalize-on-write](#normalize-on-write), with no semantic change required to trigger it. Run it once on a legacy file to commit the formatting churn separately; subsequent mutating commands then produce minimal semantic diffs. + +- `fmt` is the **only explicit whole-file write verb**. Mutating CRUD commands keep their incidental normalize-on-write; `list`/`show` remain non-mutating. +- `fmt` is **idempotent**: a second run is byte-stable. When the file is already canonical, `fmt` skips the write entirely (not even the mtime changes). + +**Automatic adoption of bare checkboxes.** `fmt` additionally brings plain markdown task-list lines under management. A line is an adoption candidate iff it does *not* parse as an idea AND matches the bare-checkbox shape: + +``` +^\s*[-*+] \[([ xX])\] (.+)$ +``` + +with one precision guard: the captured text — evaluated **after trimming surrounding whitespace**, so extra spaces between the checkbox and a bracket cannot defeat it (`- [ ] [DEV-1011] x` stays pass-through) — must NOT begin with a `[...]` bracket. Each adopted line receives a fresh unique 4-char ID and today's date; its checked state is preserved (`[x]`/`[X]` adopt as done, `[ ]` as open); its whitespace-trimmed text is treated as real text and escaped on write like any other idea. Worked example (run on 2026-06-12): + +``` +in: * [ ] buy milk + - [X] ship the release + - [ ] [DEV-1011] external item +out: - [ ] [k3v9] 2026-06-12: buy milk + - [x] [p2m4] 2026-06-12: ship the release + - [ ] [DEV-1011] external item ← untouched (bracket guard) +``` + +The bracket guard keeps inert: Shape B lines (the `[issue_ids]` slot — see the next section) and bracket-metadata lines such as `- [ ] [TODO] buy milk` or `- [ ] [ab1] text` — external-looking metadata errs toward preservation. Inert lines keep their **text** verbatim; as for all preserved content, line endings canonicalize when the file is rewritten (CRLF → LF, single trailing LF), so the round-trip is byte-identical on LF files specifically. Text-less and whitespace-only checkboxes (`- [ ]` with no text) are not adopted. Headers, prose, and blank lines pass through with the same text-verbatim guarantee. + +**Output contract.** stdout stays empty — success is silence plus exit 0 (the `gofmt -w` precedent). All reporting goes to **stderr**: one `adopted: [id] {escaped text}` line per adopted idea, the dateless-backfill advisory, and a summary count line. A run that changes nothing prints nothing. + +**`--check` mode.** `idea fmt --check` writes nothing, prints the same report (what *would* be normalized / adopted / backfilled), and exits **1** when the file is non-canonical, **0** when it is already canonical — one flag serving both the dry-run preview and the scripts/CI gate. ## Shape B Pass-Through (legacy second-bracket lines) @@ -108,7 +138,7 @@ A line with a second bracket immediately following the ID — e.g. an `[{issue_i ``` - Shape B lines are **not** parsed by `idea`, even under the relaxed regex. They do not appear in `idea list`, cannot be addressed by `idea show `, and are unaffected by `done`/`reopen`/`edit`/`rm`. -- Shape B lines are preserved **byte-for-byte** through any `idea` operation that round-trips the file (per Constitution principle I). Normalize-on-write does not touch them. +- Shape B lines are preserved **verbatim** through any `idea` operation that round-trips the file (per Constitution principle I) — their text is never parsed or rewritten. As for all preserved content, a whole-file rewrite emits canonical LF line endings, so the round-trip is byte-identical on LF files (a CRLF or missing-EOF-newline file canonicalizes its endings even on inert lines). Normalize-on-write does not otherwise touch them. - The `[{issue_ids}]` slot — and Shape B semantics generally — are owned by **external consumers**. For example, fab-kit's `/fab-new` writes Linear-style issue IDs (e.g. `[DEV-1011]`) into the slot and parses them back independently of `idea`. This lets `idea` and external tooling share the same backlog file without either tool needing to understand the other's metadata. @@ -120,6 +150,8 @@ Per Constitution principle I (Plain-Text Backlog as Source of Truth), parsing an 1. **Between lines** — section headers, blank lines, and prose between items pass through unchanged. 2. **Whole lines that look like idea entries but aren't** — any line that does not parse as an idea (including Shape B lines, and lines missing the checkbox/ID anchors) is preserved verbatim. +> **Carve-out (`idea fmt` adoption).** Bare checkbox lines lacking the 4-char `[id]` anchor (e.g. `- [ ] buy milk`) were previously guaranteed non-idea pass-through under rule 2. As of the fmt change, `idea fmt` — and **only** `fmt` — claims them via [automatic adoption](#explicit-canonicalization--adoption-idea-fmt). Every other command still preserves them verbatim, and bracket-led lines (including Shape B) remain inert pass-through everywhere. + The file is meant to be hand-edited; `idea` exists to reduce friction over hand-editing, not to take ownership of the file or its embedded metadata. ## Stability Commitment & Contract Change @@ -129,6 +161,8 @@ The file is meant to be hand-edited; `idea` exists to reduce friction over hand- > **Format-contract change note.** Earlier versions of this spec declared the date a mandatory part of the line and described an idea-managed "Shape A" that *required* the date. As of the resilient-parser change (`260610-wtmn-resilient-backlog-parser`), the date is **optional on input** and **canonical (always present) on output**, and `idea` additionally accepts `*`/`+` bullets, leading whitespace, and CRLF endings on input. This widens the input contract and was a deliberate, documented change to fix silent-failure on dateless backlogs (e.g. the shll.ai backlog). The **output** contract is unchanged: `idea` still emits exactly one canonical, machine-parseable form. Shape B second-bracket lines remain inert pass-through, exactly as before. +> **Format-contract change note.** As of the fmt change (`260612-4m3a-add-fmt-canonicalizer-adoption`), adoption **widens which lines `idea` may rewrite**: bare checkbox lines without the 4-char `[id]` anchor were previously guaranteed non-idea pass-through; `idea fmt` (and only `fmt`) now adopts them as managed canonical idea lines (fresh ID, today's date, checked state preserved). No other command's rewrite scope changed, the canonical output form is unchanged, and Shape B second-bracket lines remain inert pass-through with their text untouched, exactly as before (as with all preserved content, a rewrite emits canonical LF endings, so only LF files round-trip byte-identically). + > **Format-contract change note.** As of the multiline-escape change (`260610-49mw-escape-multiline-idea-text`), the `{description}` field is **escaped** in canonical output: backslash persists as `\\` and newline as `\n`, with CRLF/lone CR normalized to LF before escaping (see [Escaped Text in the Description](#escaped-text-in-the-description)). Structurally nothing changed — still exactly one physical line per idea in the same canonical shape — so line-by-line consumers are unaffected; consumers that display raw description text will show escape sequences for multiline ideas (that *is* the canonical form). Descriptions written before this convention decode leniently (unrecognized escapes pass through verbatim) and canonicalize on the next mutating save: lone backslashes double (`a\b` → `a\\b` on disk, decoded content unchanged), and a second save is byte-stable. One rare, accepted consequence: a legacy description containing the literal two-character sequence `\n` (e.g. `C:\new`) is reinterpreted as a real newline on read. See also [Constitution principle I](../../fab/project/constitution.md) on plain-text backlog preservation. diff --git a/docs/specs/overview.md b/docs/specs/overview.md index 4330147..2338762 100644 --- a/docs/specs/overview.md +++ b/docs/specs/overview.md @@ -40,6 +40,7 @@ The backlog file path can also be overridden globally by `--file ` (relati | `idea edit "text"` | Replace an idea's text inline | | `idea rm --force` | Delete an idea (requires `--force` to confirm) | | `idea prune [--force]` | Bulk-remove all done ideas (dry run by default; `--force` to delete) | +| `idea fmt` | Rewrite the backlog into canonical form, adopting bare checkbox lines (`--check` reports without writing) | **Editor form contract** (`idea edit `, no text argument): an unchanged buffer is a no-op — the backlog is untouched, a `note: text unchanged — nothing to do` advisory goes to stderr, and the exit code is 0. An emptied buffer is refused: no change, non-zero exit. A non-zero editor exit aborts: the backlog is untouched, non-zero exit. Passing `--id`/`--date` with the no-text form still opens the editor, applies the metadata at save, and suppresses the unchanged no-op — a metadata-only change lands without mutating the text. @@ -54,8 +55,9 @@ Queries (the `` argument on `show`, `done`, `reopen`, `edit`, `rm`) match `idea` is **liberal in what it accepts and strict in what it emits**: - **Lenient on read.** The `YYYY-MM-DD:` date segment is **optional** on input, and `idea` also accepts `*`/`+` bullets (in addition to `-`), arbitrary leading whitespace, and CRLF or LF line endings. A line is recognized as an idea by its `[ ]`/`[x]` checkbox plus 4-char `[id]` anchors. This means a hand-edited or externally-authored backlog of dateless `- [ ] [id] text` lines is read correctly rather than silently ignored. -- **Canonical on write.** Every idea line `idea` writes uses one canonical form — `- ` bullet, no indentation, LF endings, and a date that is **always present** (today's date is backfilled when the input had none). A mutating command (`done`/`reopen`/`edit`/`rm`) normalizes all recognized idea lines in the file at once; non-mutating commands (`list`/`show`) never rewrite the file. +- **Canonical on write.** Every idea line `idea` writes uses one canonical form — `- ` bullet, no indentation, LF endings, and a date that is **always present** (today's date is backfilled when the input had none). A mutating command (`done`/`reopen`/`edit`/`rm`, or `prune --force` when it removes items) normalizes all recognized idea lines in the file at once; non-mutating commands (`list`/`show`) never rewrite the file. - **Backfill notice.** When a mutating save stamps today's date on one or more previously-dateless items, `idea` prints a brief advisory notice to **stderr** (`note: stamped today's date on N previously-dateless item(s)`), keeping stdout machine-parseable. The notice is suppressed when nothing was backfilled. +- **Explicit canonicalizer.** `idea fmt` rewrites the whole file into canonical form on demand — no semantic change required — and additionally **adopts** bare checkbox lines lacking the `[id]` anchor (fresh unique ID, today's date, checked state preserved), turning an existing markdown task list into a managed backlog in one command. It is idempotent (a second run is byte-stable and skips the write), reports to stderr while stdout stays empty, and `idea fmt --check` writes nothing, prints the would-be report, and exits non-zero when the file is not canonical. For the full backlog line format — accepted input variants, the canonical output form, date backfill, Shape B pass-through, and the format-contract change note — see [`backlog-format.md`](backlog-format.md). diff --git a/fab/changes/260612-4m3a-add-fmt-canonicalizer-adoption/.history.jsonl b/fab/changes/260612-4m3a-add-fmt-canonicalizer-adoption/.history.jsonl new file mode 100644 index 0000000..612059c --- /dev/null +++ b/fab/changes/260612-4m3a-add-fmt-canonicalizer-adoption/.history.jsonl @@ -0,0 +1,31 @@ +{"action":"enter","driver":"fab-new","event":"stage-transition","stage":"intake","ts":"2026-06-12T03:31:29Z"} +{"args":"4m3a","cmd":"fab-new","event":"command","ts":"2026-06-12T03:31:29Z"} +{"delta":"+2.5","event":"confidence","score":2.5,"trigger":"calc-score","ts":"2026-06-12T03:33:30Z"} +{"delta":"+0.0","event":"confidence","score":2.5,"trigger":"calc-score","ts":"2026-06-12T03:34:00Z"} +{"cmd":"fab-clarify","event":"command","ts":"2026-06-12T03:37:57Z"} +{"delta":"+0.0","event":"confidence","score":2.5,"trigger":"calc-score","ts":"2026-06-12T04:22:58Z"} +{"delta":"+1.0","event":"confidence","score":3.5,"trigger":"calc-score","ts":"2026-06-12T04:23:02Z"} +{"delta":"+0.0","event":"confidence","score":3.5,"trigger":"calc-score","ts":"2026-06-12T04:23:03Z"} +{"delta":"+0.0","event":"confidence","score":3.5,"trigger":"calc-score","ts":"2026-06-12T04:23:06Z"} +{"delta":"+0.0","event":"confidence","score":3.5,"trigger":"calc-score","ts":"2026-06-12T04:23:07Z"} +{"delta":"+0.0","event":"confidence","score":3.5,"trigger":"calc-score","ts":"2026-06-12T04:23:16Z"} +{"delta":"+0.3","event":"confidence","score":3.8,"trigger":"calc-score","ts":"2026-06-12T05:01:12Z"} +{"delta":"+0.3","event":"confidence","score":4.1,"trigger":"calc-score","ts":"2026-06-12T05:01:15Z"} +{"delta":"+0.3","event":"confidence","score":4.4,"trigger":"calc-score","ts":"2026-06-12T05:01:18Z"} +{"delta":"+0.3","event":"confidence","score":4.7,"trigger":"calc-score","ts":"2026-06-12T05:01:21Z"} +{"delta":"+0.3","event":"confidence","score":5,"trigger":"calc-score","ts":"2026-06-12T05:01:23Z"} +{"delta":"+0.0","event":"confidence","score":5,"trigger":"calc-score","ts":"2026-06-12T05:01:25Z"} +{"delta":"+0.0","event":"confidence","score":5,"trigger":"calc-score","ts":"2026-06-12T05:01:29Z"} +{"delta":"+0.0","event":"confidence","score":5,"trigger":"calc-score","ts":"2026-06-12T05:01:36Z"} +{"cmd":"fab-fff","event":"command","ts":"2026-06-12T05:03:48Z"} +{"action":"enter","driver":"fab-fff","event":"stage-transition","stage":"apply","ts":"2026-06-12T05:04:00Z"} +{"action":"enter","driver":"fab-fff","event":"stage-transition","stage":"review","ts":"2026-06-12T05:20:20Z"} +{"action":"enter","driver":"fab-fff","event":"stage-transition","stage":"hydrate","ts":"2026-06-12T05:43:06Z"} +{"event":"review","result":"passed","ts":"2026-06-12T05:43:06Z"} +{"cmd":"fab-continue","event":"command","ts":"2026-06-12T05:44:06Z"} +{"action":"enter","driver":"fab-fff","event":"stage-transition","stage":"ship","ts":"2026-06-12T05:47:49Z"} +{"cmd":"git-pr","event":"command","ts":"2026-06-12T05:48:33Z"} +{"action":"enter","driver":"fab-fff","event":"stage-transition","stage":"review-pr","ts":"2026-06-12T05:50:01Z"} +{"cmd":"git-pr-review","event":"command","ts":"2026-06-12T05:51:27Z"} +{"cmd":"fab-continue","event":"command","ts":"2026-06-12T06:10:14Z"} +{"event":"review","result":"passed","ts":"2026-06-12T06:14:24Z"} diff --git a/fab/changes/260612-4m3a-add-fmt-canonicalizer-adoption/.status.yaml b/fab/changes/260612-4m3a-add-fmt-canonicalizer-adoption/.status.yaml new file mode 100644 index 0000000..6a54006 --- /dev/null +++ b/fab/changes/260612-4m3a-add-fmt-canonicalizer-adoption/.status.yaml @@ -0,0 +1,51 @@ +id: 4m3a +name: 260612-4m3a-add-fmt-canonicalizer-adoption +created: 2026-06-12T03:31:29Z +created_by: sahil-noon +change_type: feat +issues: [] +progress: + intake: done + apply: done + review: done + hydrate: done + ship: done + review-pr: done +plan: + generated: true + task_count: 10 + acceptance_count: 19 + acceptance_completed: 19 +confidence: + certain: 9 + confident: 0 + tentative: 0 + unresolved: 0 + score: 5.0 + fuzzy: true + dimensions: + signal: 95.6 + reversibility: 80.6 + competence: 83.3 + disambiguation: 77.8 +stage_metrics: + intake: {started_at: "2026-06-12T03:31:29Z", driver: fab-new, iterations: 1, completed_at: "2026-06-12T05:04:00Z"} + apply: {started_at: "2026-06-12T05:04:00Z", driver: fab-fff, iterations: 1, completed_at: "2026-06-12T05:20:20Z"} + review: {started_at: "2026-06-12T05:20:20Z", driver: fab-fff, iterations: 1, completed_at: "2026-06-12T05:43:06Z"} + hydrate: {started_at: "2026-06-12T05:43:06Z", driver: fab-fff, iterations: 1, completed_at: "2026-06-12T05:47:49Z"} + ship: {started_at: "2026-06-12T05:47:49Z", driver: fab-fff, iterations: 1, completed_at: "2026-06-12T05:50:01Z"} + review-pr: {started_at: "2026-06-12T05:50:01Z", driver: fab-fff, iterations: 1, completed_at: "2026-06-12T06:14:24Z"} +prs: + - https://github.com/sahil87/idea/pull/18 +true_impact: + added: 0 + deleted: 0 + net: 0 + excluding: + added: 0 + deleted: 0 + net: 0 + computed_at: "2026-06-12T05:47:49Z" + computed_at_stage: hydrate +# true_impact: lazily created on first apply-finish (no placeholder here). +last_updated: 2026-06-12T06:14:24Z diff --git a/fab/changes/260612-4m3a-add-fmt-canonicalizer-adoption/intake.md b/fab/changes/260612-4m3a-add-fmt-canonicalizer-adoption/intake.md new file mode 100644 index 0000000..0fb521b --- /dev/null +++ b/fab/changes/260612-4m3a-add-fmt-canonicalizer-adoption/intake.md @@ -0,0 +1,158 @@ +# Intake: Add `idea fmt` — Explicit Canonicalizer with Automatic Checkbox Adoption + +**Change**: 260612-4m3a-add-fmt-canonicalizer-adoption +**Created**: 2026-06-12 + +## Origin + +Initiated via `/fab-new 4m3a` (one-shot, backlog ID input). Backlog item `[4m3a]` (2026-06-12), decoded text: + +> Add `idea fmt` — explicit gofmt-style canonicalizer with automatic adoption of bare checkboxes. +> +> PROBLEM: Canonicalization currently happens only as a side-effect of the first mutating command (normalize-on-write rewrites the whole file), so a single `idea done` on a legacy file mixes formatting churn into a semantic diff. Separately, plain `- [ ]` checkbox lines without a 4-char `[id]` anchor are invisible to idea (non-idea pass-through) — there is no path to adopt an existing markdown task list into idea management. +> +> BEHAVIOR: +> - `idea fmt` rewrites the backlog to canonical form: `-` bullet, no indentation, LF endings, date backfill (today, with the existing stderr advisory notice), escape normalization (legacy lone backslashes doubled). Reuses the existing single canonical writer in internal/idea — fmt should be a thin verb over the existing parse/format/save machinery, extended with adoption. No second serialization path. +> - ADOPTION IS AUTOMATIC (decided): lines matching a bare checkbox (`- [ ] text` / `- [x] text`, also `*`/`+` bullets and leading indentation) that LACK an `[id]` anchor get a fresh unique 4-char ID assigned and become managed canonical idea lines (date backfilled to today). +> - Idempotent: second run is byte-stable. +> - Non-idea, non-checkbox content (headers, prose, blank lines) preserved verbatim per Constitution principle I. Shape B second-bracket lines (e.g. `[ni3o] [DEV-1011]`) remain inert byte-for-byte pass-through — fmt must not touch them. +> - Respects --file / --main / IDEAS_FILE resolution like every other command. list/show stay non-mutating; fmt is the only explicit whole-file write verb. +> +> OPEN QUESTIONS (solutioning): false-positive risk on auto-adopt (per-line stderr report at minimum; consider --dry-run); adopt completed `[x]` too? (leaning both); `--check` mode in or out; output contract (counts, stdout/stderr split); update backlog-format.md + overview.md. + +The item's "decided" markers are encoded as Certain assumptions; its open questions are resolved below as graded assumptions (one Tentative — see Assumptions #5). + +## Why + +1. **Diff hygiene (the pain point)**: `SaveFile` regenerates every recognized idea line on any mutating command, so the *first* `idea done` on a legacy file (variant bullets, indentation, dateless lines, legacy backslashes) produces a large mixed diff — formatting churn entangled with the one semantic change. Reviewers cannot tell what actually changed. An explicit `idea fmt` lets the user land the formatting churn as its own commit, after which mutating commands produce minimal semantic diffs. +2. **Adoption gap (the missing capability)**: a pre-existing markdown task list (`- [ ] buy milk`) is invisible to `idea` — the 4-char `[id]` anchor is required for a line to parse. There is today *no command* that brings such lines under management; the user must hand-assign IDs. `fmt` with automatic adoption converts an existing task list into a managed backlog in one command. +3. **Why this approach**: gofmt's model (one explicit canonicalizer verb, one canonical form, idempotent) is the proven pattern, and the canonical writer already exists (`FormatLine`/`SaveFile`). Making `fmt` a thin verb over that machinery — rather than a second serialization path — is both explicitly decided in the backlog item and mandated by Constitution IV (logic in `internal/idea`) and the single-formatter arrangement (`formatLineWith` is the sole home of the format string). Alternatives rejected: an `--adopt` opt-in flag (rejected in the backlog item — adoption is automatic), and canonicalize-on-`list` (violates the non-mutating-reads contract). + +## What Changes + +### 1. New subcommand: `idea fmt` (`src/cmd/idea/fmt.go`) + +``` +idea fmt [--check] # plus inherited persistent flags --file, --main (and IDEAS_FILE) +``` + +- New `fmtCmd() *cobra.Command` factory registered in `newRootCmd()` (`src/cmd/idea/main.go`), per Constitution III. The cobra file contains only flag wiring and output formatting; all logic lives in `internal/idea` (Constitution IV). +- Enriched `Long` help per the repo convention (terse `Short`, depth in `Long`, inline example) — the help-dump node appears automatically; no schema change. +- **Namespace note**: cobra resolves subcommand names before the root bare-text fallback, so `idea fmt some text` stops routing to the add-shorthand once this command exists. Same namespace trade as the `ls` alias decision; "fmt" plausibly never begins bare idea prose — acceptable. + +### 2. Canonicalization — explicit trigger over the existing writer + +New exported entry point in `internal/idea` (e.g. `Fmt(path string, check bool) (FmtResult, error)` — exact name/shape is a plan decision) that composes the existing machinery: `LoadFile` → adoption pass (below) → `SaveFile`. No second serialization path; every rewrite rule is the existing normalize-on-write set: + +- variant bullets (`*`, `+`) → `-`; leading indentation stripped; CRLF → LF +- dateless → today's date, with the **existing** stderr advisory notice mechanism (backfill count flows up; `printBackfillNotice` wording reused) +- legacy lone backslashes in text doubled (`a\b` → `a\\b` on disk; decoded content unchanged) + +**Idempotency**: a second run is byte-stable. When the rebuilt content is byte-identical to the file, `fmt` skips the write entirely (no mtime churn, no atomic-rename of identical bytes). + +**Mechanism note for plan**: counting "normalized" lines requires comparing each regenerated line against its original raw text, which `LoadFile` currently discards for idea lines (placeholder `""`). The plan must either retain raw lines in `File` or compare whole-file before/after bytes; either is internal-only and must not change `LoadFile`'s public behavior. + +### 3. Automatic adoption of bare checkboxes + +A line is an **adoption candidate** iff it does *not* parse as an idea (`ParseLine` returns false) AND matches the bare-checkbox shape: + +``` +^\s*[-*+] \[([ xX])\] (.+)$ # adoption-candidate regex (text group must be non-empty) +``` + +with one **precision guard**: the text group must NOT begin with a `[...]` bracket. This keeps inert, byte-for-byte: + +- Shape B lines — `- [ ] [ni3o] [DEV-1011] 2026-02-12: text` (the `[issue_ids]` slot is owned by external consumers; `ParseLine` already rejects these, and the adoption guard must too) +- bracket-metadata lines — `- [ ] [TODO] buy milk`, `- [ ] [ab1] text` (non-4-char bracket): external-looking metadata, err toward preservation + +Each adopted line gets: + +- a **fresh unique 4-char ID** — unique against both the IDs already in the file and IDs assigned earlier in the same fmt pass (the existing `checkIDCollision` reads from disk only; in-memory uniqueness within one run is a plan-level extension) +- **date = today** (counted as *adopted*, not double-counted as *date-backfilled* — backfill counts only pre-existing managed dateless lines) +- **checked state preserved**: `[x]` and `[X]` adopt as done (`[x]` canonical), `[ ]` as open +- text treated as **real text** (CR-normalized, escaped on write via `FormatLine` like every other write — a literal `\` doubles on disk, consistent with the legacy backslash policy) + +Worked example (run on 2026-06-12): + +``` +in: * [ ] buy milk + - [X] ship the release + - [ ] [DEV-1011] external item +out: - [ ] [k3v9] 2026-06-12: buy milk + - [x] [p2m4] 2026-06-12: ship the release + - [ ] [DEV-1011] external item ← untouched (bracket guard) +``` + +Headers, prose, blank lines, and empty checkboxes (`- [ ]` with no text — regex requires non-empty text) pass through verbatim per Constitution I. + +### 4. `--check` mode — unified preview + CI gate + +`idea fmt --check`: writes nothing, prints the same report (what *would* be normalized / adopted / backfilled), and exits non-zero when the file is non-canonical (any normalization, adoption, or backfill would occur), zero when already canonical. One flag serves both needs raised in the backlog item: the dry-run preview before destructive-ish adoption *and* the scripts/CI gate. + + +### 5. Output contract + +- **stdout**: empty. Success is silence + exit 0 (`gofmt -w` precedent); stdout stays machine-parseable per Constitution VI. +- **stderr**: all human-facing reporting — + - per-line adoption report, one line per adopted idea (e.g. `adopted: [k3v9] buy milk`) — mitigates the false-positive risk by making every claimed line visible + - summary counts: lines normalized / adopted / dates backfilled (exact wording is a plan decision; zero-activity runs print nothing) + - the existing dateless-backfill advisory (`note: stamped today's date on N previously-dateless item(s)`) is retained/subsumed by the counts — plan decides composition, but the stderr channel and advisory tone are fixed +- `internal/idea` writes nothing to stderr — counts flow up in the result value and `cmd/idea` prints, per the established Constitution IV split. + +### 6. File resolution & command surface + +`fmt` inherits `--file` / `--main` persistent flags and `IDEAS_FILE` resolution like every command. `list`/`show` remain non-mutating; **fmt becomes the only explicit whole-file write verb** (mutating CRUD commands keep their incidental normalize-on-write). + +### 7. Spec updates (contract change) + +- `docs/specs/backlog-format.md`: new format-contract change note — adoption **widens which lines `idea` may rewrite**: bare checkbox lines were previously guaranteed non-idea pass-through; after this change, `idea fmt` (and only `fmt`) claims them. Round-Trip Preservation section gets the carve-out; Shape B guarantees unchanged. +- `docs/specs/overview.md`: command table row for `idea fmt`; parse/format section gains a sentence on the explicit canonicalizer. + +## Affected Memory + +- `cli/structure`: (modify) backlog line lifecycle gains the explicit `fmt` verb (canonicalize + adopt), the adoption-candidate regex and bracket precision guard, the `--check` contract, and the fmt subcommand note (namespace claim, stderr report convention) + +## Impact + +- `src/internal/idea/` — adoption + fmt logic (new `fmt.go` or extension of `idea.go`; keep files small per config directive), possible internal-only `File`/`LoadFile` extension for change detection; table-driven tests with `t.TempDir()` (Constitution V) +- `src/cmd/idea/fmt.go` (new) + `src/cmd/idea/main.go` (one-line registration); e2e coverage in `cmd/idea` tests for routing, `--check` exit codes, stderr/stdout split +- `docs/specs/backlog-format.md`, `docs/specs/overview.md` — contract documentation +- No new dependencies (stdlib + cobra only). help-dump JSON schema unchanged (new node appears via the existing walk). Release/CI machinery untouched. + +## Open Questions + +None — the backlog item's open questions are resolved as graded assumptions below (#4, #5, #6, #7). The former Tentative (#5, `--check` unification) was confirmed by the user in the 2026-06-12 clarify session. + +## Clarifications + +### Session 2026-06-12 + +| Q | Answer | +|---|--------| +| `--check` mode shape: single unified flag, separate `--dry-run`/`--check`, or no flag at all? | Single `--check` flag — writes nothing, prints the would-be per-line report + counts to stderr, exits 1 when the file is non-canonical, 0 when clean. No separate `--dry-run`. | + +### Session 2026-06-12 (bulk confirm) + +| # | Action | Detail | +|---|--------|--------| +| 4 | Confirmed | — | +| 6 | Confirmed | — | +| 7 | Confirmed | — | +| 8 | Confirmed | — | +| 9 | Confirmed | — | + +## Assumptions + +| # | Grade | Decision | Rationale | Scores | +|---|-------|----------|-----------|--------| +| 1 | Certain | `fmt` is a thin verb over the existing `LoadFile`/`SaveFile`/`FormatLine` machinery; no second serialization path | Explicitly decided in backlog item; mandated by Constitution IV and the single-formatter arrangement | S:95 R:80 A:95 D:95 | +| 2 | Certain | Adoption is automatic — no opt-in flag | Explicitly marked "(decided)" in the backlog item | S:100 R:70 A:90 D:95 | +| 3 | Certain | Date backfill stamps today via the existing `SaveFile` seam with the existing stderr advisory mechanism | Explicit in item; mechanism already exists and is documented | S:95 R:85 A:95 D:95 | +| 4 | Certain | Completed bare checkboxes (`[x]`/`[X]`) are adopted too, preserving checked state | Clarified — user confirmed | S:95 R:80 A:75 D:80 | +| 5 | Certain | Single `--check` flag unifies dry-run preview and CI gate (write nothing, print report, exit 1 if non-canonical); no separate `--dry-run` | Clarified — user confirmed | S:95 R:85 A:60 D:40 | +| 6 | Certain | Output channels: per-line adoption report + summary counts to stderr; stdout empty (silence + exit code = success) | Clarified — user confirmed | S:95 R:80 A:85 D:70 | +| 7 | Certain | Adoption precision guard: candidates whose text begins with any `[...]` bracket are skipped (stay verbatim pass-through) | Clarified — user confirmed | S:95 R:70 A:85 D:75 | +| 8 | Certain | Adoption candidates accept `*`/`+` bullets, leading indentation (explicit in item) and uppercase `[X]` (lenient-read posture); text-less checkboxes are not adopted | Clarified — user confirmed | S:95 R:85 A:80 D:70 | +| 9 | Certain | `fmt` skips the write when the rebuilt content is byte-identical (idempotent run touches nothing, not even mtime) | Clarified — user confirmed | S:95 R:90 A:85 D:80 | + +9 assumptions (9 certain, 0 confident, 0 tentative, 0 unresolved). diff --git a/fab/changes/260612-4m3a-add-fmt-canonicalizer-adoption/plan.md b/fab/changes/260612-4m3a-add-fmt-canonicalizer-adoption/plan.md new file mode 100644 index 0000000..abe54c8 --- /dev/null +++ b/fab/changes/260612-4m3a-add-fmt-canonicalizer-adoption/plan.md @@ -0,0 +1,182 @@ +# Plan: Add `idea fmt` — Explicit Canonicalizer with Automatic Checkbox Adoption + +**Change**: 260612-4m3a-add-fmt-canonicalizer-adoption +**Intake**: `intake.md` + +## Requirements + +### CLI: `idea fmt` command surface + +#### R1: Subcommand registration and flag surface +A new `fmtCmd() *cobra.Command` factory in `src/cmd/idea/fmt.go` MUST be registered in `newRootCmd()` (`src/cmd/idea/main.go`). The command SHALL take no positional args (`cobra.NoArgs`), define a local `--check` flag, inherit the persistent `--file`/`--main` flags and `IDEAS_FILE` resolution via `resolveFile()`, and carry an enriched `Long` help (terse `Short`, depth in `Long`, inline example) per the repo convention. The cobra file SHALL contain only flag wiring and output formatting (Constitution III/IV). + +- **GIVEN** a git repo with a backlog +- **WHEN** `idea fmt` runs +- **THEN** cobra routes to the fmt subcommand (never the root bare-text add shorthand) +- **AND** `idea fmt some text` errors without adding an idea or touching the backlog (namespace claim accepted in the intake) + +- **GIVEN** a linked worktree +- **WHEN** `idea fmt --main` runs +- **THEN** the main worktree's backlog is formatted, mirroring every other command's resolution + +#### R2: Canonicalization as a thin verb over the existing writer +`internal/idea` SHALL gain an exported `Fmt(path string, check bool) (FmtResult, error)` entry point that composes the **existing** parse/format/save machinery (the same parse walk as `LoadFile`, the same render/serialization used by `SaveFile`/`FormatLine`). There MUST NOT be a second serialization path. A run rewrites recognized idea lines to canonical form: variant bullets (`*`, `+`) → `-`, leading indentation stripped, CRLF → LF, dateless → today's date (counted as backfilled, surfaced via the existing advisory mechanism), legacy lone backslashes in text doubled on disk (decoded content unchanged). + +- **GIVEN** a legacy backlog containing ` * [x] [e5f6] fix bug` and `- [ ] [rk7t] a\b` (dateless, CRLF endings) +- **WHEN** `idea fmt` runs on 2026-06-12 +- **THEN** the file holds `- [x] [e5f6] 2026-06-12: fix bug` and `- [ ] [rk7t] 2026-06-12: a\\b` with LF endings +- **AND** the result reports the normalized-line and backfilled-date counts + +- **GIVEN** a missing backlog file +- **WHEN** `idea fmt` runs +- **THEN** the read error propagates (non-zero exit), consistent with `done`/`edit` + +#### R3: Automatic adoption of bare checkboxes +A line is an adoption candidate iff it does NOT parse as an idea (`ParseLine` returns false) AND matches `^\s*[-*+] \[([ xX])\] (.+)$` with non-blank text. Each adopted line MUST receive: a fresh 4-char `[a-z0-9]{4}` ID unique against both the IDs already in the file and IDs assigned earlier in the same fmt pass; date = today (counted as *adopted*, never double-counted as *backfilled*); preserved checked state (`[x]`/`[X]` adopt as done, canonical `[x]`; `[ ]` as open); and its whitespace-trimmed matched text treated as real text (escaped on write via `FormatLine` — a literal `\` doubles on disk). + +- **GIVEN** a backlog containing `* [ ] buy milk` and `- [X] ship the release` +- **WHEN** `idea fmt` runs on 2026-06-12 +- **THEN** the lines become `- [ ] [xxxx] 2026-06-12: buy milk` and `- [x] [yyyy] 2026-06-12: ship the release` with distinct fresh IDs that collide with no existing ID +- **AND** the result lists both adopted ideas in file order + +#### R4: Precision guards and verbatim preservation +A candidate whose text group — evaluated after trimming surrounding whitespace, so extra spaces between the checkbox and the bracket cannot defeat the guard — begins with a `[...]` bracket MUST NOT be adopted — Shape B lines (`- [ ] [ni3o] [DEV-1011] 2026-02-12: text`), bracket-metadata lines (`- [ ] [DEV-1011] external item`, `- [ ] [TODO] buy milk`, `- [ ] [ab1] text`), and their extra-space variants (`- [ ] [DEV-1011] x`, `- [ ] [ab12] 2026-01-01: x`) stay byte-for-byte pass-through. Text-less and whitespace-only checkboxes (`- [ ]`, `- [ ] `) MUST NOT be adopted. Headers, prose, and blank lines pass through verbatim (Constitution I). + +- **GIVEN** a backlog with `- [ ] [DEV-1011] external item`, `# Backlog`, a blank line, and prose +- **WHEN** `idea fmt` runs +- **THEN** every one of those lines is byte-identical afterward + +#### R5: Idempotency and skip-write +A second `idea fmt` run MUST be byte-stable. When the rebuilt content is byte-identical to the on-disk file, fmt MUST skip the write entirely (no mtime churn, no atomic rename of identical bytes). A 0-byte file is left untouched (no trailing LF invented). + +- **GIVEN** a backlog just written by `idea fmt` +- **WHEN** `idea fmt` runs again +- **THEN** the file bytes and mtime are unchanged and the result reports no changes + +#### R6: `--check` mode — unified preview and CI gate +`idea fmt --check` MUST write nothing, print the same stderr report as a real run (what *would* be normalized / adopted / backfilled), and exit non-zero (1) when the file is non-canonical — any normalization, adoption, or backfill would occur — and zero when already canonical. The non-canonical exit uses the `errSilent` sentinel so no extra `ERROR:` line is printed. + +- **GIVEN** a non-canonical backlog +- **WHEN** `idea fmt --check` runs +- **THEN** the exit code is 1, the would-be report is on stderr, and the file bytes are unchanged + +- **GIVEN** a fully canonical backlog +- **WHEN** `idea fmt --check` runs +- **THEN** the exit code is 0 and nothing is printed + +#### R7: Output contract — silent stdout, advisory stderr +stdout MUST stay empty on every fmt path (success is silence + exit 0, the `gofmt -w` precedent; Constitution VI). All human-facing reporting goes to stderr: one `adopted: [id] {escaped text}` line per adopted idea (file order), the existing backfill advisory (`note: stamped today's date on N previously-dateless item(s)` via `printBackfillNotice`, suppressed at 0), and a summary line with the normalized/adopted counts printed only when the run changes (or would change) the file. Zero-activity runs print nothing. `internal/idea` MUST write nothing to stderr — counts flow up in `FmtResult` and `cmd/idea` prints (Constitution IV split). + +- **GIVEN** a backlog where fmt adopts 1 line and backfills 1 date +- **WHEN** `idea fmt` runs +- **THEN** stdout is empty and stderr carries the adoption line, the backfill note, and the summary counts + +### Specs: contract documentation + +#### R8: Spec updates for the widened rewrite contract +`docs/specs/backlog-format.md` MUST document the explicit canonicalizer: a section describing `idea fmt` (canonical rewrite, adoption-candidate shape and bracket precision guard, `--check` contract), a carve-out in Round-Trip Preservation (bare checkbox lines were previously guaranteed non-idea pass-through; after this change `idea fmt` — and only `fmt` — claims them), and a new format-contract change note. Shape B guarantees stay unchanged. `docs/specs/overview.md` MUST gain a command-table row for `idea fmt` and a sentence on the explicit canonicalizer in the parse/format section. + +- **GIVEN** an external consumer reading the specs +- **WHEN** they consult Round-Trip Preservation and the command table +- **THEN** the `fmt` adoption carve-out and the new command are documented, and Shape B pass-through guarantees read unchanged + +### Non-Goals + +- No separate `--dry-run` flag — the user-confirmed single `--check` flag serves both preview and CI gate. +- No adoption by any other command — mutating CRUD commands keep only their incidental normalize-on-write; `list`/`show` stay non-mutating. +- No help-dump JSON schema change — the new node appears via the existing walk. +- No `docs/memory/` edits — `cli/structure.md` is hydrate's responsibility, not apply's. +- No new dependencies (stdlib + cobra only). + +### Design Decisions + +1. **Internal seams**: split `LoadFile` into `parseContent(content) *File` + thin file-reading wrapper, and `SaveFile` into `render(f) (string, int)` + atomic write — *Why*: `Fmt` needs the original bytes (byte-stability/skip-write) and the rebuilt content without writing (`--check`), while keeping one parse walk and one serialization point — *Rejected*: re-reading the file twice in `Fmt` (racy, double I/O) and duplicating the join/stamp logic (second serialization path, forbidden). +2. **Raw-line retention**: `File.lines` stores each idea line's raw (post-`\r`-strip) text instead of the `""` placeholder — *Why*: per-line "normalized" counting needs the original raw line, which `SaveFile`'s copy-then-overwrite rebuild never reads, so the change is invisible to all existing callers — *Rejected*: a parallel `rawIdea []string` field (must be maintained by `Rm`'s splicing — a drift landmine). +3. **Whole-file `Changed`**: the write/exit verdict compares rebuilt content against the original bytes — *Why*: catches CRLF-on-prose and EOF-newline differences that per-line idea comparison cannot — *Rejected*: deriving `Changed` from the counts (misses non-idea-line byte changes). + +## Tasks + +### Phase 1: Internal refactors (no behavior change) + +- [x] T001 Refactor `src/internal/idea/idea.go`: extract `parseContent(content string) *File` from `LoadFile` (thin wrapper remains), extract `render(f *File) (string, int)` from `SaveFile` (date-stamp + rebuild, no write), and retain each idea line's raw text in its `f.lines` slot (replacing the `""` placeholder; update comments). Run existing `internal/idea` tests to confirm zero behavior change. + +### Phase 2: Core implementation (`internal/idea`) + +- [x] T002 Create `src/internal/idea/fmt.go`: `adoptRegex` (`^\s*[-*+] \[([ xX])\] (.+)$`), candidate guards (fails `ParseLine`, text not bracket-led via `shapeBPrefixRegex`, text non-blank), `generateUniqueIDInSet` helper (in-memory uniqueness, retry-capped), and `adoptBareCheckboxes(f *File, today string) []Idea` that merges adopted ideas into `f.ideas`/`f.ideaIndices` in file order. +- [x] T003 Add `FmtResult{Adopted []Idea; Normalized int; Backfilled int; Changed bool}` and `Fmt(path string, check bool) (FmtResult, error)` to `src/internal/idea/fmt.go`: read bytes once → `parseContent` → early-return on empty file → count backfill + per-line normalized (canonical vs raw, pre-existing ideas only) → adoption pass → `render` → `Changed` = rebuilt != original → atomic write only when `!check && Changed`. +- [x] T004 Create `src/internal/idea/fmt_test.go`: table-driven tests with `t.TempDir()` covering canonicalization (bullets/indent/CRLF/dateless/legacy backslashes), adoption (bullet variants, indentation, `[X]`→done, fresh in-run-unique IDs, today's date, backslash text escaping), guards (Shape B, `[DEV-1011]`/`[TODO]`/`[ab1]`, text-less and whitespace-only checkboxes), verbatim preservation, count separation (adopted vs backfilled), idempotency + skip-write (mtime unchanged), check mode (no write, `Changed` reported), empty file no-op, missing file error, and the intake worked example. + +### Phase 3: CLI integration & e2e + +- [x] T005 Create `src/cmd/idea/fmt.go` (`fmtCmd()` with `--check`, stderr report: `adopted: [id] {escaped text}` lines → `printBackfillNotice` → summary counts when changed; `errSilent` on non-canonical `--check`) and register `fmtCmd()` in `newRootCmd()` in `src/cmd/idea/main.go`. +- [x] T006 Create `src/cmd/idea/fmt_test.go` e2e tests (reusing `buildBinary`/`setupGitRepo`/`writeRepoBacklog`/`runSplit`/`readRepoBacklog`): routing (`idea fmt` never falls through to add; `idea fmt some text` errors, backlog untouched), stdout-empty/stderr-report split, `--check` exit codes (1 non-canonical + file unchanged, 0 clean + silent), adoption worked example end-to-end, idempotent second run. Add the `fmt` row to `TestHelpDump_RealSubcommandsPresent` in `src/cmd/idea/help_dump_test.go`. + +### Phase 4: Docs & verification + +- [x] T007 Update `docs/specs/backlog-format.md`: new "Explicit Canonicalization & Adoption (`idea fmt`)" section, Round-Trip Preservation carve-out, format-contract change note (Shape B unchanged). +- [x] T008 Update `docs/specs/overview.md`: `idea fmt` command-table row + explicit-canonicalizer sentence in the parse/format section. +- [x] T009 Verification: `cd src && go test ./...` (full suite), `gofmt -l` on all changed Go files, `go vet ./...` — all clean. + +### Rework (post-review) + +- [x] T010 Evaluate the bracket precision guard against the whitespace-trimmed capture and store adopted text trimmed in `src/internal/idea/fmt.go`; add guard-bypass rows (`- [ ] [DEV-1011] x`, `- [ ] [ab12] 2026-01-01: x`) and an adopted-text-trim row to `src/internal/idea/fmt_test.go`; mirror the trimmed-guard wording in `docs/specs/backlog-format.md`. + +## Acceptance + +### Functional Completeness + +- [x] A-001 R1: `idea fmt` exists as a registered `fmtCmd()` factory with `--check`, inherits `--file`/`--main`/`IDEAS_FILE`, takes no positional args, and carries enriched `Long` help; stray args error without touching the backlog +- [x] A-002 R2: fmt canonicalizes variant bullets, indentation, CRLF, dateless lines, and legacy backslashes through the existing render machinery — no second serialization path exists in the diff +- [x] A-003 R3: bare checkboxes (`-`/`*`/`+` bullets, indentation, `[ ]`/`[x]`/`[X]`) are adopted with fresh 4-char IDs unique against the file and within the run, today's date, and preserved checked state +- [x] A-004 R4: bracket-led candidates (Shape B, `[DEV-1011]`, `[TODO]`, `[ab1]`) and text-less checkboxes remain byte-for-byte; headers/prose/blank lines verbatim +- [x] A-005 R5: a second fmt run is byte-stable and skips the write entirely (mtime unchanged); a 0-byte file is untouched +- [x] A-006 R6: `--check` writes nothing, prints the would-be report to stderr, exits 1 when non-canonical and 0 when clean +- [x] A-007 R7: stdout is empty on every fmt path; stderr carries the per-line adoption report, the existing backfill advisory wording, and summary counts; zero-activity runs print nothing; `internal/idea` performs no stderr writes +- [x] A-008 R8: both spec files document the new command and the widened-rewrite carve-out; Shape B guarantees read unchanged + +### Scenario Coverage + +- [x] A-009 R3: the intake worked example (mixed `* [ ]`/`- [X]`/`- [ ] [DEV-1011]` input) is pinned by a test asserting the exact output shape +- [x] A-010 R3: tests assert adopted lines count as adopted but never as backfilled, and pre-existing dateless managed lines count as backfilled — no cross-counting between the two (a backfilled line also counting as normalized is correct: its bytes change) +- [x] A-011 R5: an idempotency test asserts the second run reports `Changed == false` with identical bytes + +### Edge Cases & Error Handling + +- [x] A-012 R4: whitespace-only checkbox text (`- [ ] `) is not adopted +- [x] A-013 R3: adopted text containing a literal backslash persists escaped (`a\b` → `a\\b` on disk) with decoded content unchanged +- [x] A-014 R2: fmt on a missing backlog file exits non-zero with the read error + +### Code Quality + +- [x] A-015 Pattern consistency: new code follows the factory/`RunE`/`resolveFile` and `internal/idea` naming + comment conventions of surrounding code +- [x] A-016 No unnecessary duplication: reuses `ParseLine`, `FormatLine`, `EscapeText`, `shapeBPrefixRegex`, `generateRandomID`, `atomicWriteFile`, and `printBackfillNotice` rather than reimplementing +- [x] A-017 No god functions: `Fmt` and `adoptBareCheckboxes` stay focused (< 50 lines each without clear reason) +- [x] A-018 No magic strings: regexes and report formats live in named package-level vars/clearly-scoped literals consistent with the codebase +- [x] A-019 Tests are table-driven against real temp dirs (`t.TempDir()`), no mocks (Constitution V); gofmt and go vet clean + +## Notes + +- Check items as you review: `- [x]` +- All acceptance items must pass before `/fab-continue` (hydrate) +- If an item is not applicable, mark checked and prefix with **N/A**: `- [x] A-NNN **N/A**: {reason}` + +## Deletion Candidates + +- None — this change adds new functionality without making existing code redundant. The only superseded convention (the `""` placeholder for idea slots in `File.lines`, `src/internal/idea/idea.go`) was replaced in place by T001, leaving nothing behind; `generateUniqueID` and `checkIDCollision` retain their call sites in `Add`. + +## Assumptions + +| # | Grade | Decision | Rationale | Scores | +|---|-------|----------|-----------|--------| +| 1 | Confident | API shape `Fmt(path string, check bool) (FmtResult, error)` with `FmtResult{Adopted []Idea, Normalized, Backfilled int, Changed bool}` | Intake sketched this exact signature ("exact name/shape is a plan decision"); the fields are precisely what the cmd layer's report needs | S:80 R:85 A:85 D:75 | +| 2 | Confident | Internal refactor: `parseContent`/`render` extraction + raw idea lines retained in the `File.lines` placeholder slots | Intake offered "retain raw lines in File or compare whole-file bytes"; both seams are used (per-line counts + byte-stability) with `LoadFile`/`SaveFile` public behavior unchanged | S:75 R:80 A:90 D:70 | +| 3 | Confident | stderr composition: adoption lines, then the existing `printBackfillNotice` line verbatim, then `fmt: N line(s) normalized, M line(s) adopted` printed only when the file changes | Intake fixes channel + advisory tone and delegates composition; reusing `printBackfillNotice` satisfies "wording reused" while the summary carries the remaining two counts | S:70 R:90 A:80 D:60 | +| 4 | Confident | Adoption report line format `adopted: [id] {escaped text}` using `EscapeText` | Intake gives the format as an example; escaping keeps one physical report line per adopted idea, matching the `Added:` confirmation precedent | S:80 R:90 A:85 D:70 | +| 5 | Confident | `--check` prints the identical report including freshly generated would-be IDs (not persisted; a later real run assigns different IDs) | Clarified wording says "prints the same report"; one shared code path is the simplest honest implementation of that contract | S:75 R:90 A:75 D:60 | +| 6 | Confident | Whitespace-only checkbox text is not adopted (TrimSpace guard beyond the regex's `(.+)`) | Extends the intake's non-empty-text rule to its evident intent; errs toward preservation per Constitution I | S:65 R:90 A:85 D:75 | +| 7 | Confident | Missing backlog file → error propagates (like `done`/`edit`); 0-byte file → silent no-op (no trailing LF invented) | Consistent with existing mutating-command behavior; Constitution I forbids inventing content in a file fmt was asked to preserve | S:60 R:90 A:85 D:70 | +| 8 | Confident | `Normalized` counts pre-existing managed lines whose canonical form differs from the raw (post-`\r`-strip) line; CRLF-only/EOF-newline-only differences still trigger the rewrite and non-zero `--check` via whole-file `Changed` but may not increment per-line counts | Counting mechanism explicitly delegated to plan; the whole-file compare guarantees the write/exit contract is exact even where the count is approximate | S:60 R:85 A:75 D:65 | +| 9 | Certain | `fmtCmd()` joins the existing `AddCommand` list, the help-dump node appears via the existing walk (no schema change), and `Args: cobra.NoArgs` makes stray args error rather than fall through | Intake states the namespace note and schema invariance explicitly; cobra mechanics are deterministic | S:90 R:90 A:90 D:90 | +| 10 | Confident | Adopted text is whitespace-trimmed before storage, and the bracket precision guard evaluates the trimmed capture | Rework decision (review should-fix): leading/trailing spaces are checkbox surface formatting, not content — canonical lines use single-space delimiters; trimming both closes the guard bypass (`- [ ] [DEV-1011] x`) and keeps adopted canonical lines free of leading-space noise | S:65 R:90 A:85 D:75 | + +10 assumptions (1 certain, 9 confident, 0 tentative). diff --git a/src/cmd/idea/fmt.go b/src/cmd/idea/fmt.go new file mode 100644 index 0000000..4dac898 --- /dev/null +++ b/src/cmd/idea/fmt.go @@ -0,0 +1,69 @@ +package main + +import ( + "fmt" + + "github.com/sahil87/idea/internal/idea" + "github.com/spf13/cobra" +) + +func fmtCmd() *cobra.Command { + var check bool + + cmd := &cobra.Command{ + Use: "fmt", + Short: "Rewrite the backlog in canonical form, adopting bare checkboxes", + Long: `Rewrite the current worktree's backlog into canonical form, gofmt-style. + +Every recognized idea line is regenerated canonically: "-" bullet, no +indentation, LF endings, today's date stamped on dateless items, and legacy +lone backslashes doubled. Bare checkbox lines without a 4-char [id] anchor +(e.g. "- [ ] buy milk", also */+ bullets and [x]/[X]) are adopted as managed +ideas: each gets a fresh unique ID and today's date, with its checked state +preserved. Lines whose text starts with a bracket (e.g. "- [ ] [DEV-1011] ...") +and all other non-idea content keep their text verbatim; line endings +canonicalize file-wide (CRLF becomes LF, single trailing LF). A second run is +byte-stable, and an already-canonical file is not rewritten at all. + +stdout stays empty; the report (one "adopted:" line per adopted idea plus +summary counts) goes to stderr. --check writes nothing, prints the same report, +and exits 1 when the file would change, 0 when it is already canonical. --main +targets the main worktree's backlog and --file / IDEAS_FILE point elsewhere +(see "idea --help"). + + idea fmt + idea fmt --check`, + Args: cobra.NoArgs, + RunE: func(cmd *cobra.Command, args []string) error { + path, err := resolveFile() + if err != nil { + return err + } + + res, err := idea.Fmt(path, check) + if err != nil { + return err + } + + for _, a := range res.Adopted { + fmt.Fprintf(cmd.ErrOrStderr(), "adopted: [%s] %s\n", a.ID, idea.EscapeText(a.Text)) + } + printBackfillNotice(cmd, res.Backfilled) + if res.Changed { + fmt.Fprintf(cmd.ErrOrStderr(), "fmt: %d line(s) normalized, %d line(s) adopted\n", + res.Normalized, len(res.Adopted)) + } + + if check && res.Changed { + // Non-canonical under --check: the report above is the + // message; exit 1 without an extra ERROR line. + return errSilent + } + return nil + }, + } + + cmd.Flags().BoolVar(&check, "check", false, "Write nothing; report what would change and exit 1 if the file is not canonical") + + return cmd +} diff --git a/src/cmd/idea/fmt_test.go b/src/cmd/idea/fmt_test.go new file mode 100644 index 0000000..2da3dc6 --- /dev/null +++ b/src/cmd/idea/fmt_test.go @@ -0,0 +1,179 @@ +package main + +import ( + "os/exec" + "strings" + "testing" + "time" +) + +// TestFmt_Routing verifies "idea fmt" resolves to the fmt subcommand (never +// the root bare-text add shorthand) and that stray args error instead of +// falling through to add — the namespace claim accepted in the intake. +func TestFmt_Routing(t *testing.T) { + bin := buildBinary(t) + + tests := []struct { + name string + args []string + wantErr bool + }{ + {"fmt routes to subcommand", []string{"fmt"}, false}, + {"stray args error, no add fallthrough", []string{"fmt", "some", "text"}, true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + repo := setupGitRepo(t) + writeRepoBacklog(t, repo, "# Backlog\n\n- [ ] [ab12] 2026-06-01: seeded idea\n") + before := readRepoBacklog(t, repo) + + stdout, stderr, err := runSplit(t, bin, repo, tt.args...) + if tt.wantErr && err == nil { + t.Fatalf("%v succeeded, want error\nstdout=%q stderr=%q", tt.args, stdout, stderr) + } + if !tt.wantErr && err != nil { + t.Fatalf("%v failed: %v\nstdout=%q stderr=%q", tt.args, err, stdout, stderr) + } + if stdout != "" { + t.Errorf("stdout = %q, want empty", stdout) + } + if after := readRepoBacklog(t, repo); after != before { + t.Errorf("backlog changed by %v:\nbefore:\n%s\nafter:\n%s", tt.args, before, after) + } + }) + } +} + +// TestFmt_RewritesAndReportsOnStderr runs the intake's worked example end to +// end: candidates adopted, bracket-guarded line untouched, stdout silent, and +// the full report (adoption lines, backfill note, summary) on stderr. +func TestFmt_RewritesAndReportsOnStderr(t *testing.T) { + bin := buildBinary(t) + repo := setupGitRepo(t) + today := time.Now().Format("2006-01-02") + + writeRepoBacklog(t, repo, "# Backlog\n\n"+ + "* [ ] buy milk\n"+ + "- [X] ship the release\n"+ + "- [ ] [DEV-1011] external item\n"+ + "- [ ] [rk7t] dateless managed\n") + + stdout, stderr, err := runSplit(t, bin, repo, "fmt") + if err != nil { + t.Fatalf("fmt failed: %v\nstdout=%q stderr=%q", err, stdout, stderr) + } + + if stdout != "" { + t.Errorf("stdout = %q, want empty (success is silence)", stdout) + } + if got := strings.Count(stderr, "adopted: ["); got != 2 { + t.Errorf("stderr has %d adoption lines, want 2:\n%s", got, stderr) + } + if !strings.Contains(stderr, "buy milk") || !strings.Contains(stderr, "ship the release") { + t.Errorf("adoption report missing adopted texts:\n%s", stderr) + } + if !strings.Contains(stderr, "note: stamped today's date on 1 previously-dateless item(s)") { + t.Errorf("stderr missing backfill advisory:\n%s", stderr) + } + if !strings.Contains(stderr, "fmt: 1 line(s) normalized, 2 line(s) adopted") { + t.Errorf("stderr missing summary counts:\n%s", stderr) + } + + after := readRepoBacklog(t, repo) + if !strings.Contains(after, "- [ ] [DEV-1011] external item\n") { + t.Errorf("bracket-guarded line was touched:\n%s", after) + } + if !strings.Contains(after, ": buy milk\n") || !strings.Contains(after, today+": ship the release\n") { + t.Errorf("adopted lines not canonical:\n%s", after) + } + if strings.Contains(after, "* [ ]") || strings.Contains(after, "[X]") { + t.Errorf("variant forms survived fmt:\n%s", after) + } +} + +// TestFmt_Idempotent verifies the second run is byte-stable and silent. +func TestFmt_Idempotent(t *testing.T) { + bin := buildBinary(t) + repo := setupGitRepo(t) + writeRepoBacklog(t, repo, "* [ ] buy milk\n- [ ] [rk7t] dateless\n") + + if _, _, err := runSplit(t, bin, repo, "fmt"); err != nil { + t.Fatalf("first fmt failed: %v", err) + } + first := readRepoBacklog(t, repo) + + stdout, stderr, err := runSplit(t, bin, repo, "fmt") + if err != nil { + t.Fatalf("second fmt failed: %v\nstdout=%q stderr=%q", err, stdout, stderr) + } + if stdout != "" || stderr != "" { + t.Errorf("second run not silent: stdout=%q stderr=%q", stdout, stderr) + } + if second := readRepoBacklog(t, repo); second != first { + t.Errorf("second run changed bytes:\nfirst:\n%s\nsecond:\n%s", first, second) + } +} + +// TestFmt_CheckMode verifies the --check contract: exit 1 + would-be report + +// untouched file when non-canonical; exit 0 + silence when canonical. +func TestFmt_CheckMode(t *testing.T) { + bin := buildBinary(t) + + tests := []struct { + name string + backlog string + wantExit int + wantReport bool + }{ + { + name: "non-canonical exits 1 and reports", + backlog: "* [ ] buy milk\n- [ ] [rk7t] dateless\n", + wantExit: 1, + wantReport: true, + }, + { + name: "canonical exits 0 silently", + backlog: "# Backlog\n\n- [ ] [ab12] 2026-06-01: canonical idea\n", + wantExit: 0, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + repo := setupGitRepo(t) + writeRepoBacklog(t, repo, tt.backlog) + + stdout, stderr, err := runSplit(t, bin, repo, "fmt", "--check") + + exit := 0 + if err != nil { + ee, ok := err.(*exec.ExitError) + if !ok { + t.Fatalf("fmt --check failed to run: %v", err) + } + exit = ee.ExitCode() + } + if exit != tt.wantExit { + t.Errorf("exit code = %d, want %d\nstderr=%q", exit, tt.wantExit, stderr) + } + if stdout != "" { + t.Errorf("stdout = %q, want empty", stdout) + } + if tt.wantReport { + if !strings.Contains(stderr, "adopted: [") || !strings.Contains(stderr, "fmt: ") { + t.Errorf("expected would-be report on stderr, got %q", stderr) + } + if strings.Contains(stderr, "ERROR:") { + t.Errorf("non-canonical --check must exit silently (no ERROR line), got %q", stderr) + } + } else if stderr != "" { + t.Errorf("stderr = %q, want empty on a canonical file", stderr) + } + + if after := readRepoBacklog(t, repo); after != tt.backlog { + t.Errorf("--check modified the file:\nbefore:\n%s\nafter:\n%s", tt.backlog, after) + } + }) + } +} diff --git a/src/cmd/idea/help_dump_test.go b/src/cmd/idea/help_dump_test.go index aad8e01..29bef3d 100644 --- a/src/cmd/idea/help_dump_test.go +++ b/src/cmd/idea/help_dump_test.go @@ -97,6 +97,7 @@ func TestHelpDump_RealSubcommandsPresent(t *testing.T) { {"edit", "idea edit"}, {"rm", "idea rm"}, {"prune", "idea prune"}, + {"fmt", "idea fmt"}, {"update", "idea update"}, {"shell-init", "idea shell-init"}, } diff --git a/src/cmd/idea/main.go b/src/cmd/idea/main.go index d17e20f..fa5c05f 100644 --- a/src/cmd/idea/main.go +++ b/src/cmd/idea/main.go @@ -60,6 +60,7 @@ Shorthand: "idea " is equivalent to "idea add ".`, editCmd(), rmCmd(), pruneCmd(), + fmtCmd(), updateCmd(), newShellInitCmd(), helpDumpCmd(), diff --git a/src/internal/idea/fmt.go b/src/internal/idea/fmt.go new file mode 100644 index 0000000..fc8b842 --- /dev/null +++ b/src/internal/idea/fmt.go @@ -0,0 +1,188 @@ +package idea + +import ( + "fmt" + "os" + "regexp" + "strings" + "time" +) + +// adoptRegex matches a bare markdown checkbox line lacking the 4-char [id] +// anchor — the adoption-candidate shape for "idea fmt". It mirrors lineRegex's +// lenient surface (-/*/+ bullets, leading whitespace) and additionally accepts +// an uppercase [X] checkbox. A line is only a candidate when ParseLine has +// already rejected it, so managed idea lines never reach this regex. +var adoptRegex = regexp.MustCompile(`^\s*[-*+] \[([ xX])\] (.+)$`) + +// FmtResult reports what Fmt changed — or, in check mode, would change. +type FmtResult struct { + // Adopted holds the newly adopted ideas, in file order. + Adopted []Idea + // Normalized counts pre-existing managed idea lines whose canonical form + // differs from their raw on-disk text (adopted lines are not counted). + Normalized int + // Backfilled counts pre-existing managed dateless lines stamped with + // today's date (adopted lines carry today's date but count as Adopted). + Backfilled int + // Changed reports whether the rebuilt content differs from the on-disk + // bytes. It drives both the write (skipped when false) and the --check + // exit code. + Changed bool +} + +// Fmt rewrites the backlog at path into canonical form: every recognized idea +// line is regenerated via the existing render machinery (variant bullets, +// indentation, CRLF endings, dateless lines, and legacy lone backslashes all +// canonicalize), and bare checkbox lines without an [id] anchor are adopted as +// managed ideas (see adoptBareCheckboxes). Non-idea, non-candidate content +// passes through verbatim. +// +// Fmt is idempotent: when the rebuilt content is byte-identical to the file, +// the write is skipped entirely (no mtime churn). With check set, Fmt never +// writes — it only reports what a real run would do. Fmt writes nothing to +// stderr; counts flow up in FmtResult and the cmd layer prints (Constitution +// IV split). +func Fmt(path string, check bool) (FmtResult, error) { + var res FmtResult + + data, err := os.ReadFile(path) + if err != nil { + return res, err + } + f := parseContent(string(data)) + if len(f.lines) == 0 { + // Empty (0-byte) file: nothing to canonicalize, and inventing a + // trailing newline would violate verbatim preservation. + return res, nil + } + + // Count what canonicalization will touch among the pre-existing managed + // lines, comparing each regenerated line against its raw on-disk text. + today := time.Now().Format("2006-01-02") + for i, lineIdx := range f.ideaIndices { + stamped := f.ideas[i] + if stamped.Date == "" { + stamped.Date = today + res.Backfilled++ + } + if FormatLine(stamped) != f.lines[lineIdx] { + res.Normalized++ + } + } + + res.Adopted, err = adoptBareCheckboxes(f, today) + if err != nil { + return res, err + } + + content, _ := render(f, today) + res.Changed = content != string(data) + if check || !res.Changed { + return res, nil + } + if err := atomicWriteFile(path, []byte(content), 0644); err != nil { + return res, err + } + return res, nil +} + +// adoptBareCheckboxes converts bare checkbox lines (adoption candidates) into +// managed ideas, merging them into f.ideas/f.ideaIndices in file order, and +// returns the adopted ideas. +// +// A line is an adoption candidate iff it did not parse as an idea (it sits in +// a non-idea slot) AND it matches adoptRegex AND its whitespace-trimmed text +// neither is blank nor begins with a [...] bracket (the precision guard +// keeping Shape B and bracket-metadata lines like `- [ ] [DEV-1011] ...` +// inert — trimming first so extra spaces cannot defeat the guard). Each +// adopted idea gets a fresh 4-char ID unique against both the IDs already in +// the file and IDs assigned earlier in the same pass, today's date, its +// checked state preserved ([x]/[X] -> done), and its trimmed text taken as +// real text (it is escaped on write via FormatLine like every other idea). +func adoptBareCheckboxes(f *File, today string) ([]Idea, error) { + used := make(map[string]bool, len(f.ideas)) + for _, i := range f.ideas { + used[i.ID] = true + } + ideaAt := make(map[int]bool, len(f.ideaIndices)) + for _, lineIdx := range f.ideaIndices { + ideaAt[lineIdx] = true + } + + var adopted []Idea + var adoptedIndices []int + for lineIdx, line := range f.lines { + if ideaAt[lineIdx] { + continue + } + m := adoptRegex.FindStringSubmatch(line) + if m == nil { + continue + } + // The guard and the adopted text both use the whitespace-trimmed + // capture: extra spaces between the checkbox and a bracket must not + // defeat the guard (e.g. `- [ ] [DEV-1011] x`), and leading/trailing + // spaces are checkbox surface formatting, not content. + text := strings.TrimSpace(m[2]) + // Precision guard: bracket-led text ([DEV-1011], [TODO], Shape B + // remnants) stays verbatim pass-through — err toward preservation. + // Blank text (whitespace-only checkbox) is likewise not adopted. + if text == "" || shapeBPrefixRegex.MatchString(text) { + continue + } + id, err := generateUniqueIDInSet(used, 10) + if err != nil { + return nil, err + } + used[id] = true + adopted = append(adopted, Idea{ + ID: id, + Date: today, + Text: text, + Done: m[1] == "x" || m[1] == "X", + }) + adoptedIndices = append(adoptedIndices, lineIdx) + } + if len(adopted) == 0 { + return nil, nil + } + + // Merge the adopted ideas into the existing (sorted-by-line) slices so + // f.ideas stays in file order for render's rebuild. + mergedIdeas := make([]Idea, 0, len(f.ideas)+len(adopted)) + mergedIndices := make([]int, 0, len(f.ideaIndices)+len(adoptedIndices)) + e, a := 0, 0 + for e < len(f.ideas) || a < len(adopted) { + if a >= len(adopted) || (e < len(f.ideas) && f.ideaIndices[e] < adoptedIndices[a]) { + mergedIdeas = append(mergedIdeas, f.ideas[e]) + mergedIndices = append(mergedIndices, f.ideaIndices[e]) + e++ + } else { + mergedIdeas = append(mergedIdeas, adopted[a]) + mergedIndices = append(mergedIndices, adoptedIndices[a]) + a++ + } + } + f.ideas = mergedIdeas + f.ideaIndices = mergedIndices + + return adopted, nil +} + +// generateUniqueIDInSet generates a random 4-char ID not present in used. +// Unlike generateUniqueID (which checks the on-disk file), uniqueness is +// checked against an in-memory set so IDs assigned earlier in the same fmt +// pass cannot collide before anything is written. +func generateUniqueIDInSet(used map[string]bool, maxRetries int) (string, error) { + for i := 0; i < maxRetries; i++ { + id, err := generateRandomID() + if err != nil { + return "", err + } + if !used[id] { + return id, nil + } + } + return "", fmt.Errorf("failed to generate unique ID after %d attempts", maxRetries) +} diff --git a/src/internal/idea/fmt_test.go b/src/internal/idea/fmt_test.go new file mode 100644 index 0000000..e4ec685 --- /dev/null +++ b/src/internal/idea/fmt_test.go @@ -0,0 +1,433 @@ +package idea + +import ( + "os" + "path/filepath" + "regexp" + "strings" + "testing" + "time" +) + +// writeTempBacklog writes content to a fresh temp backlog file via the shared +// writeBacklog helper (idea_test.go) and returns its path. +func writeTempBacklog(t *testing.T, content string) string { + t.Helper() + return writeBacklog(t, t.TempDir(), content) +} + +func readBacklog(t *testing.T, path string) string { + t.Helper() + data, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + return string(data) +} + +// --- Canonicalization (existing normalize-on-write rule set, explicit trigger) --- + +func TestFmt_Canonicalize(t *testing.T) { + today := time.Now().Format("2006-01-02") + + tests := []struct { + name string + in string + want string + normalized int + backfilled int + changed bool + }{ + { + name: "variant bullet and indentation", + in: " * [x] [e5f6] 2025-06-08: fix bug\n+ [ ] [a7k2] 2025-06-15: add dark mode\n", + want: "- [x] [e5f6] 2025-06-08: fix bug\n- [ ] [a7k2] 2025-06-15: add dark mode\n", + normalized: 2, + changed: true, + }, + { + name: "dateless line backfills today", + in: "- [ ] [rk7t] tune the reporter\n", + want: "- [ ] [rk7t] " + today + ": tune the reporter\n", + normalized: 1, + backfilled: 1, + changed: true, + }, + { + name: "legacy lone backslash doubles on disk", + in: "- [ ] [ab12] 2026-01-01: a\\b\n", + want: "- [ ] [ab12] 2026-01-01: a\\\\b\n", + normalized: 1, + changed: true, + }, + { + name: "crlf endings normalize to lf", + in: "- [ ] [ab12] 2026-01-01: text\r\nprose line\r\n", + want: "- [ ] [ab12] 2026-01-01: text\nprose line\n", + // The raw line is stored post-\r-strip, so the per-line count + // stays 0; the whole-file comparison still flags the change. + normalized: 0, + changed: true, + }, + { + name: "missing trailing newline gains one", + in: "- [ ] [ab12] 2026-01-01: text", + want: "- [ ] [ab12] 2026-01-01: text\n", + changed: true, + }, + { + name: "already canonical is untouched", + in: "# Backlog\n\n- [ ] [ab12] 2026-01-01: text\n- [x] [e5f6] 2025-06-08: done item\n", + want: "# Backlog\n\n- [ ] [ab12] 2026-01-01: text\n- [x] [e5f6] 2025-06-08: done item\n", + changed: false, + }, + { + name: "non-idea content verbatim", + in: "# Backlog\n\nsome prose between items\n\t indented prose\n", + want: "# Backlog\n\nsome prose between items\n\t indented prose\n", + changed: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + path := writeTempBacklog(t, tt.in) + res, err := Fmt(path, false) + if err != nil { + t.Fatalf("Fmt failed: %v", err) + } + if got := readBacklog(t, path); got != tt.want { + t.Errorf("file content:\ngot: %q\nwant: %q", got, tt.want) + } + if res.Normalized != tt.normalized { + t.Errorf("Normalized = %d, want %d", res.Normalized, tt.normalized) + } + if res.Backfilled != tt.backfilled { + t.Errorf("Backfilled = %d, want %d", res.Backfilled, tt.backfilled) + } + if res.Changed != tt.changed { + t.Errorf("Changed = %v, want %v", res.Changed, tt.changed) + } + if len(res.Adopted) != 0 { + t.Errorf("Adopted = %v, want none", res.Adopted) + } + }) + } +} + +// --- Adoption of bare checkboxes --- + +func TestFmt_AdoptsBareCheckboxes(t *testing.T) { + today := time.Now().Format("2006-01-02") + + tests := []struct { + name string + in string // the single candidate line (plus trailing newline) + wantText string + wantDone bool + }{ + {"dash bullet open", "- [ ] buy milk\n", "buy milk", false}, + {"star bullet open", "* [ ] buy milk\n", "buy milk", false}, + {"plus bullet done", "+ [x] ship the release\n", "ship the release", true}, + {"uppercase X adopts as done", "- [X] ship the release\n", "ship the release", true}, + {"indented candidate", " - [ ] indented task\n", "indented task", false}, + {"surrounding whitespace trims from adopted text", "- [ ] padded task \n", "padded task", false}, + } + + lineShape := regexp.MustCompile(`^- \[([ x])\] \[([a-z0-9]{4})\] (\d{4}-\d{2}-\d{2}): (.+)$`) + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + path := writeTempBacklog(t, tt.in) + res, err := Fmt(path, false) + if err != nil { + t.Fatalf("Fmt failed: %v", err) + } + if len(res.Adopted) != 1 { + t.Fatalf("Adopted = %d ideas, want 1", len(res.Adopted)) + } + a := res.Adopted[0] + if a.Text != tt.wantText { + t.Errorf("adopted Text = %q, want %q", a.Text, tt.wantText) + } + if a.Done != tt.wantDone { + t.Errorf("adopted Done = %v, want %v", a.Done, tt.wantDone) + } + if a.Date != today { + t.Errorf("adopted Date = %q, want today %q", a.Date, today) + } + if res.Backfilled != 0 { + t.Errorf("Backfilled = %d, want 0 (adoption is not backfill)", res.Backfilled) + } + + line := strings.TrimSuffix(readBacklog(t, path), "\n") + m := lineShape.FindStringSubmatch(line) + if m == nil { + t.Fatalf("adopted line %q is not canonical", line) + } + if m[2] != a.ID { + t.Errorf("on-disk ID = %q, want %q (from result)", m[2], a.ID) + } + wantCheck := " " + if tt.wantDone { + wantCheck = "x" + } + if m[1] != wantCheck { + t.Errorf("on-disk checkbox = %q, want %q", m[1], wantCheck) + } + }) + } +} + +func TestFmt_AdoptionGuards(t *testing.T) { + tests := []struct { + name string + line string + }{ + {"shape B second bracket", "- [ ] [ni3o] [DEV-1011] 2026-02-12: capture more metrics"}, + {"bracket metadata external id", "- [ ] [DEV-1011] external item"}, + {"bracket metadata word", "- [ ] [TODO] buy milk"}, + {"non-4-char bracket", "- [ ] [ab1] text"}, + {"extra space cannot defeat bracket guard", "- [ ] [DEV-1011] external item"}, + {"extra space before managed-looking line", "- [ ] [ab12] 2026-01-01: text"}, + {"textless checkbox", "- [ ]"}, + {"whitespace-only text", "- [ ] "}, + {"no space after checkbox", "- [ ]glued text"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + in := tt.line + "\n" + path := writeTempBacklog(t, in) + res, err := Fmt(path, false) + if err != nil { + t.Fatalf("Fmt failed: %v", err) + } + if len(res.Adopted) != 0 { + t.Errorf("Adopted = %v, want none", res.Adopted) + } + if res.Changed { + t.Error("Changed = true, want false") + } + if got := readBacklog(t, path); got != in { + t.Errorf("line not preserved byte-for-byte:\ngot: %q\nwant: %q", got, in) + } + }) + } +} + +// TestFmt_WorkedExample pins the intake's worked example: mixed candidates are +// adopted in file order, the bracket-guarded line stays untouched, and +// surrounding non-idea content is verbatim. +func TestFmt_WorkedExample(t *testing.T) { + today := time.Now().Format("2006-01-02") + in := "# Backlog\n\n* [ ] buy milk\n- [X] ship the release\n- [ ] [DEV-1011] external item\n" + + path := writeTempBacklog(t, in) + res, err := Fmt(path, false) + if err != nil { + t.Fatalf("Fmt failed: %v", err) + } + if len(res.Adopted) != 2 { + t.Fatalf("Adopted = %d ideas, want 2", len(res.Adopted)) + } + if res.Adopted[0].Text != "buy milk" || res.Adopted[1].Text != "ship the release" { + t.Errorf("Adopted order/text = [%q, %q], want file order", res.Adopted[0].Text, res.Adopted[1].Text) + } + + want := "# Backlog\n\n" + + "- [ ] [" + res.Adopted[0].ID + "] " + today + ": buy milk\n" + + "- [x] [" + res.Adopted[1].ID + "] " + today + ": ship the release\n" + + "- [ ] [DEV-1011] external item\n" + if got := readBacklog(t, path); got != want { + t.Errorf("file content:\ngot: %q\nwant: %q", got, want) + } +} + +// TestFmt_AdoptedIDsUnique asserts fresh IDs are unique against both the IDs +// already in the file and IDs assigned earlier in the same pass. +func TestFmt_AdoptedIDsUnique(t *testing.T) { + var b strings.Builder + b.WriteString("- [ ] [ab12] 2026-01-01: existing managed idea\n") + for i := 0; i < 30; i++ { + b.WriteString("- [ ] candidate task\n") + } + + path := writeTempBacklog(t, b.String()) + res, err := Fmt(path, false) + if err != nil { + t.Fatalf("Fmt failed: %v", err) + } + if len(res.Adopted) != 30 { + t.Fatalf("Adopted = %d ideas, want 30", len(res.Adopted)) + } + + seen := map[string]bool{"ab12": true} + for _, a := range res.Adopted { + if err := ValidateID(a.ID); err != nil { + t.Errorf("adopted ID %q invalid: %v", a.ID, err) + } + if seen[a.ID] { + t.Errorf("duplicate ID %q assigned", a.ID) + } + seen[a.ID] = true + } +} + +// TestFmt_AdoptedBackslashText verifies adopted text is treated as real text: +// a literal backslash doubles on disk and decodes back unchanged. +func TestFmt_AdoptedBackslashText(t *testing.T) { + path := writeTempBacklog(t, "- [ ] open C:\\new in editor\n") + + res, err := Fmt(path, false) + if err != nil { + t.Fatalf("Fmt failed: %v", err) + } + if len(res.Adopted) != 1 { + t.Fatalf("Adopted = %d ideas, want 1", len(res.Adopted)) + } + if res.Adopted[0].Text != "open C:\\new in editor" { + t.Errorf("adopted Text = %q, want the raw text", res.Adopted[0].Text) + } + if got := readBacklog(t, path); !strings.Contains(got, "open C:\\\\new in editor") { + t.Errorf("on-disk text should be escaped (doubled backslash), got %q", got) + } + + // Round-trip: the persisted line decodes back to the original text. + f, err := LoadFile(path) + if err != nil { + t.Fatal(err) + } + if len(f.ideas) != 1 || f.ideas[0].Text != "open C:\\new in editor" { + t.Errorf("round-trip Text = %+v, want original text", f.ideas) + } +} + +// --- Counting separation --- + +func TestFmt_CountsSeparation(t *testing.T) { + in := "- [ ] [ab12] dateless managed\n" + // backfilled (and normalized: bytes change) + "- [ ] bare candidate\n" + // adopted only + "- [x] [e5f6] 2025-06-08: canonical\n" // untouched + + path := writeTempBacklog(t, in) + res, err := Fmt(path, false) + if err != nil { + t.Fatalf("Fmt failed: %v", err) + } + if res.Backfilled != 1 { + t.Errorf("Backfilled = %d, want 1 (adopted line must not count)", res.Backfilled) + } + if len(res.Adopted) != 1 { + t.Errorf("Adopted = %d, want 1", len(res.Adopted)) + } + if res.Normalized != 1 { + t.Errorf("Normalized = %d, want 1 (the backfilled line; not the canonical or adopted ones)", res.Normalized) + } + if !res.Changed { + t.Error("Changed = false, want true") + } +} + +// --- Idempotency & skip-write --- + +func TestFmt_IdempotentSecondRun(t *testing.T) { + path := writeTempBacklog(t, " * [ ] [ab12] dateless variant\n- [ ] adopt me\n") + + res1, err := Fmt(path, false) + if err != nil { + t.Fatalf("first Fmt failed: %v", err) + } + if !res1.Changed { + t.Fatal("first run should report Changed") + } + after1 := readBacklog(t, path) + + // Push mtime into the past so an (incorrect) rewrite would be detectable. + past := time.Now().Add(-time.Hour).Truncate(time.Second) + if err := os.Chtimes(path, past, past); err != nil { + t.Fatal(err) + } + + res2, err := Fmt(path, false) + if err != nil { + t.Fatalf("second Fmt failed: %v", err) + } + if res2.Changed { + t.Error("second run Changed = true, want false (byte-stable)") + } + if res2.Normalized != 0 || res2.Backfilled != 0 || len(res2.Adopted) != 0 { + t.Errorf("second run counts = %+v, want all zero", res2) + } + if after2 := readBacklog(t, path); after2 != after1 { + t.Errorf("second run changed bytes:\nfirst: %q\nsecond: %q", after1, after2) + } + fi, err := os.Stat(path) + if err != nil { + t.Fatal(err) + } + if !fi.ModTime().Equal(past) { + t.Errorf("mtime churned on a byte-stable run: %v, want %v (write must be skipped)", fi.ModTime(), past) + } +} + +// --- Check mode --- + +func TestFmt_CheckWritesNothing(t *testing.T) { + in := "* [ ] [ab12] dateless variant\n- [ ] adopt me\n" + path := writeTempBacklog(t, in) + + res, err := Fmt(path, true) + if err != nil { + t.Fatalf("Fmt --check failed: %v", err) + } + if !res.Changed { + t.Error("Changed = false, want true (file is non-canonical)") + } + if res.Normalized != 1 || res.Backfilled != 1 || len(res.Adopted) != 1 { + t.Errorf("check-mode counts = %+v, want the same report as a real run", res) + } + if got := readBacklog(t, path); got != in { + t.Errorf("check mode modified the file:\ngot: %q\nwant: %q", got, in) + } +} + +func TestFmt_CheckCleanFile(t *testing.T) { + in := "# Backlog\n\n- [ ] [ab12] 2026-01-01: canonical\n" + path := writeTempBacklog(t, in) + + res, err := Fmt(path, true) + if err != nil { + t.Fatalf("Fmt --check failed: %v", err) + } + if res.Changed { + t.Error("Changed = true, want false on a canonical file") + } + if got := readBacklog(t, path); got != in { + t.Error("check mode modified a canonical file") + } +} + +// --- Edge cases --- + +func TestFmt_EmptyFileUntouched(t *testing.T) { + path := writeTempBacklog(t, "") + + res, err := Fmt(path, false) + if err != nil { + t.Fatalf("Fmt failed: %v", err) + } + if res.Changed { + t.Error("Changed = true, want false on an empty file") + } + if got := readBacklog(t, path); got != "" { + t.Errorf("empty file gained content %q, want untouched", got) + } +} + +func TestFmt_MissingFileErrors(t *testing.T) { + path := filepath.Join(t.TempDir(), "missing.md") + if _, err := Fmt(path, false); err == nil { + t.Error("expected an error for a missing file") + } +} diff --git a/src/internal/idea/idea.go b/src/internal/idea/idea.go index 2c5375c..11439f6 100644 --- a/src/internal/idea/idea.go +++ b/src/internal/idea/idea.go @@ -229,7 +229,10 @@ func WorktreeRoot() (string, error) { // File represents a loaded backlog file, preserving non-idea lines. type File struct { // lines stores every line in order. Non-idea lines are stored as-is. - // Idea lines are stored as empty strings (their content comes from ideas). + // Idea lines store their raw on-disk text (post-\r-strip); render's + // rebuild overwrites those slots from FormatLine, so the raw value is + // never serialized — it exists so Fmt can compare each regenerated line + // against the original (per-line "normalized" counting). lines []string // ideaIndices maps from ideas slice index to lines slice index. ideaIndices []int @@ -243,11 +246,16 @@ func LoadFile(path string) (*File, error) { if err != nil { return nil, err } + return parseContent(string(data)), nil +} +// parseContent parses backlog file content into a File. It is the single +// parse walk shared by LoadFile and Fmt (which needs the original bytes for +// byte-stability detection and so reads the file itself). +func parseContent(content string) *File { f := &File{} - content := string(data) if content == "" { - return f, nil + return f } // Trim a single trailing newline (the canonical EOF newline) so that a @@ -266,7 +274,7 @@ func LoadFile(path string) (*File, error) { // *content* is otherwise preserved verbatim per Constitution I). line = strings.TrimSuffix(line, "\r") if idea, ok := ParseLine(line); ok { - f.lines = append(f.lines, "") // placeholder + f.lines = append(f.lines, line) // raw text; rebuilt from FormatLine on save f.ideaIndices = append(f.ideaIndices, i) f.ideas = append(f.ideas, idea) } else { @@ -274,7 +282,7 @@ func LoadFile(path string) (*File, error) { } } - return f, nil + return f } // SaveFile writes the backlog file, reconstructing from preserved lines and ideas. @@ -292,8 +300,22 @@ func LoadFile(path string) (*File, error) { // and then renamed over the target path, so a crash mid-write cannot leave the // backlog (the source of truth) partially written or empty. func SaveFile(f *File, path string) (int, error) { + content, backfilled := render(f, time.Now().Format("2006-01-02")) + if err := atomicWriteFile(path, []byte(content), 0644); err != nil { + return 0, err + } + return backfilled, nil +} + +// render stamps the given date on dateless ideas (returning the backfill count) +// and rebuilds the canonical file content without writing it. The caller +// supplies today so one logical operation stamps a single consistent date — +// Fmt's counting pass, its adoption dates, and the rendered bytes must not +// disagree across a midnight boundary. It is the single serialization point: +// SaveFile writes its output, and Fmt compares it against the on-disk bytes to +// decide whether a write (or a --check failure) is needed. +func render(f *File, today string) (string, int) { backfilled := 0 - today := time.Now().Format("2006-01-02") for i := range f.ideas { if f.ideas[i].Date == "" { f.ideas[i].Date = today @@ -309,11 +331,7 @@ func SaveFile(f *File, path string) (int, error) { result[idx] = FormatLine(f.ideas[i]) } - content := strings.Join(result, "\n") + "\n" - if err := atomicWriteFile(path, []byte(content), 0644); err != nil { - return 0, err - } - return backfilled, nil + return strings.Join(result, "\n") + "\n", backfilled } // atomicWriteFile writes data to path atomically by writing to a temp file