From fcc4304b31f778891fa3f60dfbf855ed7090d261 Mon Sep 17 00:00:00 2001 From: REPPL <77722411+REPPL@users.noreply.github.com> Date: Sat, 11 Jul 2026 17:21:52 +0100 Subject: [PATCH 1/5] docs: capture itd-80 intent-lifecycle-automation + slice-1 decisions itd-80 is the dogfood payload for the intent-lifecycle pipeline: a standalone intent whose acceptance criteria ARE the steel thread, to be driven drafts->planned->shipped through the machinery it specifies. Records the build sign-off, the option-b dogfood approach, and the spc-N minting rule (max(spec files U intent spec_id)+1 => first mint spc-2). Assisted-by: Claude:claude-opus-4-8 --- .../itd-80-intent-lifecycle-automation.md | 75 +++++++++++++++++++ .abcd/work/DECISIONS.md | 17 +++++ 2 files changed, 92 insertions(+) create mode 100644 .abcd/development/intents/drafts/itd-80-intent-lifecycle-automation.md diff --git a/.abcd/development/intents/drafts/itd-80-intent-lifecycle-automation.md b/.abcd/development/intents/drafts/itd-80-intent-lifecycle-automation.md new file mode 100644 index 00000000..84ffd4ee --- /dev/null +++ b/.abcd/development/intents/drafts/itd-80-intent-lifecycle-automation.md @@ -0,0 +1,75 @@ +--- +id: itd-80 +slug: intent-lifecycle-automation +kind: null +suggested_kind: standalone +bundle: null +spec_id: null +reclassification_history: [] +builds_on: [itd-34, itd-1] +related_adrs: [adr-25, adr-26, adr-27] +severity: major +--- + +# An Intent Ships Itself: `planned → shipped` Follows Its Spec Closing, With A Fidelity Audit Attached + +## Press Release + +> **abcd ships intent-lifecycle automation: an intent moves from `planned/` to `shipped/` as a side-effect of its linked spec closing, and arrives carrying a fidelity audit.** The developer runs `abcd intent plan itd-N` to link an intent to a freshly-minted native spec (`spc-M`) and commit it to `planned/`. When the work is done and they run `abcd spec close spc-M`, a deterministic reconcile detects the change through the intent↔spec link and moves the intent to `shipped/` — no `move` verb, no hand-editing, because the directory *is* the lifecycle state. The move emits a host-delegated fidelity-review request over the intent's Acceptance Criteria and the delivered code; the host's verdict is ingested back into the intent's `## Audit Notes` as per-criterion verdicts (MET / MET_WITH_CONCERNS / NOT_MET / INCONCLUSIVE) plus a three-bucket prose audit (honoured / diverged / missing), every claim carrying a cited `file:line` evidence pointer. +> +> "I stopped babysitting the roadmap," said Maya, an autonomous-development practitioner. "I plan an intent, I close its spec, and the intent ships itself — and the audit that lands with it tells me, criterion by criterion with evidence, whether what I built actually matches what I promised. The lifecycle stopped being a thing I maintained by hand and became a thing that maintains itself." + +## Why This Matters + +Until now, an intent's journey through `drafts → planned → shipped` was a set of manual file moves and hand-written audits — the very kind of undisciplined, drift-prone bookkeeping abcd exists to eliminate. itd-3 was driven through the lifecycle entirely by hand (its `## Audit Notes` were authored manually) precisely because no machinery existed. That manual precedent is the reference the automation must reproduce. + +This intent builds the automation. Directory location stays the single source of truth (adr-26 directory-as-truth): there is no `status:` field to drift and no `abcd intent move` to invite manual state-setting. The lifecycle advances only as a *consequence* of a real event — a spec closing — detected through an ID-anchored link that survives file moves. Because abcd cannot run an LLM itself (adr-25 host-delegated default), the fidelity review is asynchronous: abcd emits a request and parks a receipt; the host runs the reviewer; the verdict is ingested through a validated transport. The result is a lifecycle that advances itself and a shipped record that carries its own evidence. + +## What's In Scope + +- **A minimal native spec store** (`internal/core/spec`): directory-as-truth spec records, `spc-N` id minting that does not collide with reserved ids, an ID-anchored bidirectional link to an intent (`intent: itd-N`), and status-by-directory (open vs closed). +- **`abcd intent` verb family**: bare `abcd intent` renders lifecycle status (never mutates); `abcd intent plan ` mints a spec, writes the bidirectional link, sets `kind` (default `standalone`), and moves the intent `drafts → planned`; `abcd intent link ` retroactively links a pre-existing spec; `abcd intent review ingest` ingests a verdict. +- **`abcd spec` verb family**: bare render; `abcd spec close ` marks a spec done and runs the reconcile. +- **A deterministic reconcile**: on spec close, detect the linked intent via the immutable link, move it `planned → shipped` with an atomic same-filesystem rename, and repair any drifted derived `spec_id`. Fail closed on a missing or ambiguous link — never a silent or partial move. Keeps the `intent_lifecycle` record-lint contract green (the `shipped/` `spec_id ^spc-` requirement satisfied, not relaxed). +- **A host-delegated `intent-fidelity-reviewer` markdown agent** (new `agents/` directory): a single-document (Role 1) fidelity reviewer that reads an intent's Acceptance Criteria + the delivered diff and returns a VSA-shaped verdict JSON with per-criterion verdicts and a honoured/diverged/missing audit, each entry citing evidence. +- **An async verdict-ingest transport**: emit a review-request + park an `OWED` receipt at the ship-move; ingest schema-gates the verdict, semantically checks it (every `criterion_id` exists in the intent, every verdict in-enum), and appends idempotently (keyed on the receipt id) to `## Audit Notes`. A malformed/absent verdict after retry → receipt `DEAD_LETTER`, affected criteria recorded `INCONCLUSIVE`, raw payload retained. +- **Dogfooding**: this intent (itd-80) is the pipeline's first real payload — it is driven `drafts → planned → shipped` through the machinery it specifies, and its `## Audit Notes` are produced by the automation, not by hand. + +## What's Out of Scope + +- **The inter-spec dependency graph** and richer spec-store features (adr-26's full vision) — the store here is the minimal floor: link, status-by-directory, reconcile. +- **Bundle-member and discipline lifecycles** (itd-34) — slice 1 handles `standalone` only. +- **Reviewer Roles 2 and 3** (cross-document consistency, shape-classification; itd-48) and **loop-to-acceptance** (itd-50) — the reviewer is Role 1, one-shot, report-only. +- **Multi-judge juries / calibration harness** — a single host verdict; a calibration corpus is added only once real verdicts exist. +- **Auto-generation or auto-planning of the spec body** — `plan` mints the spec record and link; authoring the spec's content is not automated here. +- **A vendor/event "hook"** — the reconcile is a deterministic Go step inside `abcd spec close`, never a harness event. + +## Acceptance Criteria + +> _BDD format, per the [itd-1 discipline](../disciplines/itd-1-acceptance-gates.md). These gates are checked by `intent-fidelity-reviewer`'s single-document role when this intent moves to `shipped/` — this intent dogfoods that path._ + +- **Given** a `standalone` intent in `drafts/` with a valid `## Acceptance Criteria` section and `spec_id: null`, **when** the developer runs `abcd intent plan itd-N`, **then** a native spec `spc-M` is minted (with `intent: itd-N`), the intent gains `spec_id: spc-M` and a binding `kind: standalone`, the file moves `drafts → planned`, and `make record-lint` stays green. +- **Given** a planned intent linked to an open spec `spc-M`, **when** the developer runs `abcd spec close spc-M`, **then** a deterministic reconcile detects the linked intent through the immutable link and moves it `planned → shipped` with an atomic rename, satisfying the `intent_lifecycle` `shipped/` contract, and no unrelated file is modified. +- **Given** the ship-move, **when** reconcile runs, **then** exactly one VSA-shaped fidelity-review request (over the intent's Acceptance Criteria + the delivered diff) is emitted and exactly one `OWED` receipt carrying an idempotency key is parked. +- **Given** a host-produced verdict JSON that matches a parked receipt, **when** the developer runs `abcd intent review ingest`, **then** the payload is schema-gated and semantically checked (every `criterion_id` exists in the intent; every verdict is one of MET / MET_WITH_CONCERNS / NOT_MET / INCONCLUSIVE), and on success the per-criterion verdicts plus a honoured/diverged/missing audit — each entry carrying a cited `file:line` evidence pointer — are appended to the intent's `## Audit Notes`, and the receipt flips `OWED → INGESTED`. +- **Given** a verdict already ingested for a receipt, **when** ingest runs again with the same `receipt_id`, **then** it is a no-op: `## Audit Notes` is not duplicated (idempotent append keyed on the receipt id). +- **Given** a malformed/missing intent↔spec link at reconcile time, or a verdict referencing an unknown receipt at ingest time, **when** the operation runs, **then** it fails closed — no partial move, no partial append — with a clear error; and a verdict still invalid after retry marks the receipt `DEAD_LETTER`, records the affected criteria `INCONCLUSIVE`, and retains the raw payload. +- **Given** a crafted `itd-`/`spc-` id containing path-traversal or unexpected characters, **when** any verb resolves it to a file, **then** the id is validated against `^itd-[0-9]+$` / `^spc-[0-9]+$` and rejected otherwise — no file outside the intent/spec directories is read, written, or moved. + +## Prior Art + +- **[itd-34](../planned/itd-34-three-intent-kinds.md)** (three intent kinds) — defines the `kind` field and the standalone/bundle/discipline lifecycle paths; slice 1 implements the `standalone` path only. This intent is the lifecycle *automation* itd-34's ACs assume exists. +- **[itd-46](../planned/itd-46-abcd-intent-quoted-text-create-symmetric.md)** — the `abcd intent` create ergonomics (markdown surface); complementary, not overlapping (that is the create path; this is plan→ship→audit). +- **[itd-48](../planned/itd-48-intent-fidelity-reviewer-roles-2-3.md)** — the reviewer's Roles 2/3; this intent delivers Role 1 that they extend. +- **itd-3** (shipped, manual precedent) — its hand-authored `## Audit Notes` are the golden reference the automated audit must reproduce in shape. +- **adr-26** (native spec store — directory-as-truth), **adr-25** (host-delegated LLM default), **adr-27** (autonomous-run receipt gating) — the load-bearing decisions this intent instantiates. +- **`.abcd/development/plans/2026-07-11-intent-lifecycle.md`** — the SOTA-researched design plan this intent builds to. + +## Open Questions + +- **`spc-N` minting under reserved ids.** The corpus pre-references `spc-` ids in planning docs and itd-3 reserved `spc-1`. Slice 1 mints `max(spec-store files ∪ intent `spec_id`) + 1` to avoid live collisions; reconciling the store's sequential minting with the brief's aspirational spec numbering is deferred to the richer spec-store slice. +- **Where the review-request + receipt are parked.** A local ephemeral queue vs a committed slot — leaning local-ephemeral under `.abcd/.work.local/` for the request, with the receipt as the correlation record; settle at plan. + +## Audit Notes + + diff --git a/.abcd/work/DECISIONS.md b/.abcd/work/DECISIONS.md index a57555f9..54786ce4 100644 --- a/.abcd/work/DECISIONS.md +++ b/.abcd/work/DECISIONS.md @@ -192,3 +192,20 @@ parallel-agent merge contention bites. NOT_MET; every divergence is a signed-off D1–D4 delta, the one gap is the AC6 legacy-harvest completeness. Inbound links repointed planned→shipped by hand — the link-drift-on-move the future reconcile pass automates. +- 2026-07-11 — Intent-lifecycle slice 1 (build sign-off given): the pipeline is + **dogfooded** — itd-3 stays shipped as the reference fixture (option b), and a + new tightly-scoped intent **itd-80-intent-lifecycle-automation** (ACs = the + steel thread) is the pipeline's first real payload, driven drafts→planned→ + shipped through the machinery it specifies. Slice scope: minimal native spec + store (`internal/core/spec`, directory-as-truth open/closed, `intent:` link), + `abcd intent` (plan/link/review-ingest + bare render) and `abcd spec` + (close + bare render) verbs, deterministic reconcile inside `spec close` + (no vendor event), host-delegated `intent-fidelity-reviewer` markdown agent + (Role 1 only) + async outbox/inbox verdict ingest to `## Audit Notes`. +- 2026-07-11 — `spc-N` minting rule for slice 1: `max(N over spec-store files ∪ + N over every intent's spec_id) + 1`, so the first mint is spc-2 (itd-3's + reserved spc-1 is respected without a backing spec file). Reconciling the + store's sequential minting with the brief's aspirational spc-numbering is + deferred to the richer spec-store slice. Reviewer roles 2/3 (itd-48), + loop-to-acceptance (itd-50), bundle/discipline lifecycles, and the spec + dependency graph are all explicitly deferred. From 1809522f607c9f161756192ed3caf6963a6946a2 Mon Sep 17 00:00:00 2001 From: REPPL <77722411+REPPL@users.noreply.github.com> Date: Sat, 11 Jul 2026 17:21:53 +0100 Subject: [PATCH 2/5] =?UTF-8?q?feat:=20itd-80=20phase=201=20=E2=80=94=20na?= =?UTF-8?q?tive=20spec=20store=20(internal/core/spec)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Minimal directory-as-truth spec store: specs live under .abcd/development/specs/{open,closed}/spc-N-.md, the bucket IS the status (no status: field). Exposes Load / NextID / Create / Close / Lookup / ByIntent with the ID-anchored intent: link. Trust-boundary guards: spc-/itd- id regexes validated before any path is built, symlink refusal, regular-file + size cap; missing specs/ is soft, malformed frontmatter is a hard error. Dependency-free (private copy of the lint frontmatter line-scanner; no YAML lib). NOT wired yet — Phase 2 adds the abcd intent / abcd spec verbs that reach it. Part of itd-80. Assisted-by: Claude:claude-opus-4-8 --- internal/core/spec/spec.go | 182 +++++++++++++++++++++ internal/core/spec/spec_test.go | 64 ++++++++ internal/core/spec/store.go | 265 +++++++++++++++++++++++++++++++ internal/core/spec/store_test.go | 187 ++++++++++++++++++++++ 4 files changed, 698 insertions(+) create mode 100644 internal/core/spec/spec.go create mode 100644 internal/core/spec/spec_test.go create mode 100644 internal/core/spec/store.go create mode 100644 internal/core/spec/store_test.go diff --git a/internal/core/spec/spec.go b/internal/core/spec/spec.go new file mode 100644 index 00000000..a46da2a5 --- /dev/null +++ b/internal/core/spec/spec.go @@ -0,0 +1,182 @@ +// Package spec is abcd's transport-agnostic native spec store (intent +// lifecycle, itd-64). It owns the in-memory model of spec records and the +// disk operations that mint, load, and transition them. Every function takes a +// structured request and returns a structured result; nothing here writes to +// stdout or knows about a CLI, MCP, or hook surface — the front doors under +// internal/surface/* marshal these results for their transport. +// +// A spec record is a markdown file under +// /.abcd/development/specs/{open,closed}/spc-N-.md. The bucket +// directory IS the status (directory-as-truth: open/ vs closed/, with no +// status: frontmatter field), mirroring how intents encode their lifecycle by +// bucket. The load-bearing field is intent: itd-N, the link a spec declares to +// the intent it realises. +// +// Frontmatter is read by a private line scanner (frontmatterFields), not a YAML +// parser — the package pulls in zero new dependencies. Ids are validated against +// strict regexes before any path is built, so a hostile id can never traverse +// out of the spec store. +package spec + +import ( + "fmt" + "regexp" + "strconv" + "strings" +) + +// Bucket directory names. The directory a spec file lives in is its status. +const ( + StatusOpen = "open" + StatusClosed = "closed" +) + +// Repo-relative store locations. +const ( + // SpecsRelDir is the spec-store root, relative to the repo worktree. + SpecsRelDir = ".abcd/development/specs" + // IntentsRelDir is where intents live; NextID scans it for reserved spec ids. + IntentsRelDir = ".abcd/development/intents" +) + +// maxSpecFileBytes caps any spec/intent markdown file read (trust boundary). +const maxSpecFileBytes = 256 * 1024 + +// intentBuckets are the intent lifecycle directories NextID scans for reserved +// spec_id values. +var intentBuckets = []string{"drafts", "planned", "shipped", "disciplines", "superseded"} + +var ( + // specIDRe constrains a spec id so it can never be used to build a path that + // escapes the store (path-traversal defence). + specIDRe = regexp.MustCompile(`^spc-[0-9]+$`) + // intentIDRe constrains the load-bearing intent link the same way. + intentIDRe = regexp.MustCompile(`^itd-[0-9]+$`) + // slugRe constrains a slug to kebab-case, since a slug becomes a filename. + slugRe = regexp.MustCompile(`^[a-z0-9]+(?:-[a-z0-9]+)*$`) + // specNumRe extracts the numeric N from a spec id or a spec_id value that may + // carry a trailing slug (spc-1, spc-2-thing). + specNumRe = regexp.MustCompile(`^spc-([0-9]+)`) + // specFileRe matches a spec-store filename and captures its id. + specFileRe = regexp.MustCompile(`^(spc-[0-9]+)-.*\.md$`) + // intentFileRe matches an intent filename. + intentFileRe = regexp.MustCompile(`^itd-[0-9]+.*\.md$`) +) + +// Spec is one spec record. Status is the bucket it was found in; Path is +// repo-relative (never an absolute local path). +type Spec struct { + ID string `json:"id"` // spc-N + Slug string `json:"slug"` // kebab-case + Intent string `json:"intent"` // itd-N, the load-bearing link + Status string `json:"status"` // open | closed (directory-as-truth) + Path string `json:"path"` // repo-relative markdown path +} + +// Store is the in-memory set of spec records discovered under both buckets. +type Store struct { + Specs []Spec `json:"specs"` +} + +// Lookup returns the spec with the given id; ok is false when absent. +func (s Store) Lookup(specID string) (Spec, bool) { + for _, sp := range s.Specs { + if sp.ID == specID { + return sp, true + } + } + return Spec{}, false +} + +// ByIntent returns the spec linked to the given intent id; ok is false when no +// spec realises that intent. +func (s Store) ByIntent(intentID string) (Spec, bool) { + for _, sp := range s.Specs { + if sp.Intent == intentID { + return sp, true + } + } + return Spec{}, false +} + +// Validate enforces the id regexes and that intent is a well-formed itd-N. It +// is the fail-closed guard both Load and the minting path run before trusting a +// record's id in a filesystem path. +func Validate(s Spec) error { + if !specIDRe.MatchString(s.ID) { + return fmt.Errorf("spec: id %q must match ^spc-[0-9]+$", s.ID) + } + if !intentIDRe.MatchString(s.Intent) { + return fmt.Errorf("spec %s: intent %q must match ^itd-[0-9]+$", s.ID, s.Intent) + } + return nil +} + +// specNum extracts the numeric N from a spec id or spec_id value, or 0 if none. +func specNum(id string) int { + m := specNumRe.FindStringSubmatch(id) + if m == nil { + return 0 + } + n, _ := strconv.Atoi(m[1]) + return n +} + +// renderSpec is the minimal spec-file body: frontmatter carrying the id, slug, +// and the load-bearing intent link, plus a title and a Summary placeholder. +func renderSpec(id, slug, intentID string) string { + var b strings.Builder + b.WriteString("---\n") + fmt.Fprintf(&b, "id: %s\n", id) + fmt.Fprintf(&b, "slug: %s\n", slug) + fmt.Fprintf(&b, "intent: %s\n", intentID) + b.WriteString("---\n") + fmt.Fprintf(&b, "# %s\n\n", slug) + b.WriteString("## Summary\n\nTODO\n") + return b.String() +} + +// --- frontmatter line scanner --- +// +// Replicated privately (not imported) from internal/core/lint so this package +// stays dependency-free: it is a line scanner, not a YAML parser. It reads only +// the block between the first two `---` lines, top-level keys only, first key +// wins. + +// fmKeyRe matches a top-level frontmatter key (column 0, no indentation). +var fmKeyRe = regexp.MustCompile(`^([A-Za-z0-9_]+):(.*)$`) + +// fmField is a frontmatter key's value and 1-based source line. +type fmField struct { + value string + line int +} + +// frontmatterFields returns the top-level keys of the leading frontmatter block +// (between the first two `---` lines). Nested keys and list items are ignored. +func frontmatterFields(lines []string) map[string]fmField { + fields := map[string]fmField{} + if len(lines) == 0 || strings.TrimRight(lines[0], "\r") != "---" { + return fields + } + for i := 1; i < len(lines); i++ { + line := strings.TrimRight(lines[i], "\r") + if line == "---" { + break + } + m := fmKeyRe.FindStringSubmatch(line) + if m == nil { + continue + } + key := m[1] + if _, exists := fields[key]; !exists { + fields[key] = fmField{value: strings.TrimSpace(m[2]), line: i + 1} + } + } + return fields +} + +// isNull treats an empty value and the YAML nulls ""/"null"/"~" as null. +func isNull(v string) bool { + return v == "" || v == "null" || v == "~" +} diff --git a/internal/core/spec/spec_test.go b/internal/core/spec/spec_test.go new file mode 100644 index 00000000..8f69e741 --- /dev/null +++ b/internal/core/spec/spec_test.go @@ -0,0 +1,64 @@ +package spec + +import ( + "os" + "path/filepath" + "testing" +) + +// writeFile writes content to root/rel, creating parent directories. Shared by +// both test files in this package. +func writeFile(t *testing.T, root, rel, content string) { + t.Helper() + abs := filepath.Join(root, rel) + if err := os.MkdirAll(filepath.Dir(abs), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(abs, []byte(content), 0o644); err != nil { + t.Fatal(err) + } +} + +func TestValidate(t *testing.T) { + tests := []struct { + name string + spec Spec + wantErr bool + }{ + {"good", Spec{ID: "spc-1", Slug: "thing", Intent: "itd-9"}, false}, + {"bad id", Spec{ID: "spec-1", Intent: "itd-9"}, true}, + {"empty id", Spec{ID: "", Intent: "itd-9"}, true}, + {"bad intent", Spec{ID: "spc-1", Intent: "itd-x"}, true}, + {"empty intent", Spec{ID: "spc-1", Intent: ""}, true}, + {"traversal id", Spec{ID: "spc-../../etc", Intent: "itd-9"}, true}, + {"traversal intent", Spec{ID: "spc-1", Intent: "itd-../../etc"}, true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := Validate(tt.spec) + if (err != nil) != tt.wantErr { + t.Fatalf("Validate(%+v) err = %v, wantErr %v", tt.spec, err, tt.wantErr) + } + }) + } +} + +func TestStoreLookupAndByIntent(t *testing.T) { + store := Store{Specs: []Spec{ + {ID: "spc-1", Slug: "a", Intent: "itd-9", Status: StatusOpen}, + {ID: "spc-2", Slug: "b", Intent: "itd-12", Status: StatusClosed}, + }} + + if s, ok := store.Lookup("spc-2"); !ok || s.Intent != "itd-12" { + t.Fatalf("Lookup(spc-2) = %+v, %v", s, ok) + } + if _, ok := store.Lookup("spc-99"); ok { + t.Fatal("Lookup(spc-99) unexpectedly found") + } + if s, ok := store.ByIntent("itd-9"); !ok || s.ID != "spc-1" { + t.Fatalf("ByIntent(itd-9) = %+v, %v", s, ok) + } + if _, ok := store.ByIntent("itd-77"); ok { + t.Fatal("ByIntent(itd-77) unexpectedly found") + } +} diff --git a/internal/core/spec/store.go b/internal/core/spec/store.go new file mode 100644 index 00000000..748f7e8a --- /dev/null +++ b/internal/core/spec/store.go @@ -0,0 +1,265 @@ +package spec + +import ( + "fmt" + "os" + "path/filepath" + "strings" +) + +// Load discovers spec files under both buckets, parses their frontmatter, and +// returns the in-memory Store. A missing specs/ directory yields an empty store +// (soft, mirroring lint's missing-dir behaviour). A present-but-malformed spec +// file is a hard, loud error. +func Load(repoRoot string) (Store, error) { + var store Store + for _, bucket := range []string{StatusOpen, StatusClosed} { + specs, err := loadBucket(repoRoot, bucket) + if err != nil { + return Store{}, err + } + store.Specs = append(store.Specs, specs...) + } + return store, nil +} + +// loadBucket reads one bucket directory. A missing directory is soft (nil, nil). +func loadBucket(repoRoot, bucket string) ([]Spec, error) { + dir := filepath.Join(repoRoot, SpecsRelDir, bucket) + di, err := os.Lstat(dir) + if os.IsNotExist(err) { + return nil, nil + } + if err != nil { + return nil, fmt.Errorf("spec: stat %s: %w", filepath.Join(SpecsRelDir, bucket), err) + } + if di.Mode()&os.ModeSymlink != 0 { + return nil, fmt.Errorf("spec: %s is a symlink (refusing to follow)", filepath.Join(SpecsRelDir, bucket)) + } + entries, err := os.ReadDir(dir) + if err != nil { + return nil, fmt.Errorf("spec: reading %s: %w", filepath.Join(SpecsRelDir, bucket), err) + } + var specs []Spec + for _, e := range entries { + if e.IsDir() || !specFileRe.MatchString(e.Name()) { + continue + } + rel := filepath.Join(SpecsRelDir, bucket, e.Name()) + data, err := readRepoFile(filepath.Join(dir, e.Name()), rel) + if err != nil { + return nil, err + } + sp, err := parseSpec(rel, string(data), bucket) + if err != nil { + return nil, err + } + specs = append(specs, sp) + } + return specs, nil +} + +// parseSpec builds a Spec from a file's content and validates it. A file whose +// frontmatter lacks a well-formed id or intent is malformed and rejected. +func parseSpec(relPath, content, bucket string) (Spec, error) { + fields := frontmatterFields(strings.Split(content, "\n")) + sp := Spec{ + ID: fields["id"].value, + Slug: fields["slug"].value, + Intent: fields["intent"].value, + Status: bucket, + Path: relPath, + } + if err := Validate(sp); err != nil { + return Spec{}, fmt.Errorf("spec: malformed %s: %w", relPath, err) + } + return sp, nil +} + +// NextID mints the next spec id. The rule is: +// +// max(N over existing spec-store files ∪ N over every intent's spec_id +// frontmatter across .abcd/development/intents/**) + 1 +// +// Scanning the intents is what keeps a freshly minted spec from colliding with +// a reservation: itd-3 shipped with spec_id: spc-1 but has no spec-store file, +// so a spec-only scan would hand out spc-1 again. Folding intent reservations in +// means the first minted id is spc-2 while that reservation stands. +func NextID(repoRoot string) (string, error) { + max := 0 + store, err := Load(repoRoot) + if err != nil { + return "", err + } + for _, sp := range store.Specs { + if n := specNum(sp.ID); n > max { + max = n + } + } + reserved, err := maxIntentSpecNum(repoRoot) + if err != nil { + return "", err + } + if reserved > max { + max = reserved + } + return fmt.Sprintf("spc-%d", max+1), nil +} + +// maxIntentSpecNum returns the highest N across every intent's spec_id +// frontmatter value in the intent lifecycle buckets, or 0 if none. +func maxIntentSpecNum(repoRoot string) (int, error) { + max := 0 + for _, bucket := range intentBuckets { + dir := filepath.Join(repoRoot, IntentsRelDir, bucket) + entries, err := os.ReadDir(dir) + if os.IsNotExist(err) { + continue + } + if err != nil { + return 0, fmt.Errorf("spec: reading %s: %w", filepath.Join(IntentsRelDir, bucket), err) + } + for _, e := range entries { + if e.IsDir() || !intentFileRe.MatchString(e.Name()) { + continue + } + rel := filepath.Join(IntentsRelDir, bucket, e.Name()) + data, err := readRepoFile(filepath.Join(dir, e.Name()), rel) + if err != nil { + return 0, err + } + fields := frontmatterFields(strings.Split(string(data), "\n")) + v := fields["spec_id"].value + if isNull(v) { + continue + } + if n := specNum(v); n > max { + max = n + } + } + } + return max, nil +} + +// Create mints an id via NextID and writes specs/open/spc-N-.md with the +// intent link in frontmatter. Both the intent id and the slug are validated +// before any path is built (the slug becomes a filename). The write is atomic. +func Create(repoRoot, intentID, slug string) (Spec, error) { + if !intentIDRe.MatchString(intentID) { + return Spec{}, fmt.Errorf("spec: intent id %q must match ^itd-[0-9]+$", intentID) + } + if !slugRe.MatchString(slug) { + return Spec{}, fmt.Errorf("spec: slug %q must be kebab-case", slug) + } + id, err := NextID(repoRoot) + if err != nil { + return Spec{}, err + } + openDir := filepath.Join(repoRoot, SpecsRelDir, StatusOpen) + if err := ensureDir(openDir, filepath.Join(SpecsRelDir, StatusOpen)); err != nil { + return Spec{}, err + } + name := fmt.Sprintf("%s-%s.md", id, slug) + if err := atomicWrite(filepath.Join(openDir, name), renderSpec(id, slug, intentID)); err != nil { + return Spec{}, err + } + sp := Spec{ + ID: id, + Slug: slug, + Intent: intentID, + Status: StatusOpen, + Path: filepath.Join(SpecsRelDir, StatusOpen, name), + } + return sp, Validate(sp) +} + +// Close moves a spec file open/ -> closed/ via os.Rename (atomic on one +// filesystem) and returns the updated Spec. It fails closed if the spec is +// missing or already closed. The linked intent is deliberately left untouched: +// moving it is a later reconcile concern that consumes Spec.Intent. +func Close(repoRoot, specID string) (Spec, error) { + if !specIDRe.MatchString(specID) { + return Spec{}, fmt.Errorf("spec: id %q must match ^spc-[0-9]+$", specID) + } + store, err := Load(repoRoot) + if err != nil { + return Spec{}, err + } + sp, ok := store.Lookup(specID) + if !ok { + return Spec{}, fmt.Errorf("spec: %s not found", specID) + } + if sp.Status == StatusClosed { + return Spec{}, fmt.Errorf("spec: %s is already closed", specID) + } + name := filepath.Base(sp.Path) + closedDir := filepath.Join(repoRoot, SpecsRelDir, StatusClosed) + if err := ensureDir(closedDir, filepath.Join(SpecsRelDir, StatusClosed)); err != nil { + return Spec{}, err + } + if err := os.Rename(filepath.Join(repoRoot, sp.Path), filepath.Join(closedDir, name)); err != nil { + return Spec{}, fmt.Errorf("spec: closing %s: %w", specID, err) + } + sp.Status = StatusClosed + sp.Path = filepath.Join(SpecsRelDir, StatusClosed, name) + return sp, nil +} + +// readRepoFile reads a repo file behind the trust-boundary guards: refuse a +// symlinked leaf, require a regular file, and cap the size. +func readRepoFile(abs, rel string) ([]byte, error) { + fi, err := os.Lstat(abs) + if err != nil { + return nil, fmt.Errorf("spec: stat %s: %w", rel, err) + } + if fi.Mode()&os.ModeSymlink != 0 { + return nil, fmt.Errorf("spec: %s is a symlink (refusing to follow)", rel) + } + if !fi.Mode().IsRegular() { + return nil, fmt.Errorf("spec: %s is not a regular file", rel) + } + if fi.Size() > maxSpecFileBytes { + return nil, fmt.Errorf("spec: %s exceeds the %d-byte cap", rel, maxSpecFileBytes) + } + data, err := os.ReadFile(abs) + if err != nil { + return nil, fmt.Errorf("spec: reading %s: %w", rel, err) + } + return data, nil +} + +// ensureDir creates dir if absent, refusing to follow a symlinked directory. +func ensureDir(dir, rel string) error { + if di, err := os.Lstat(dir); err == nil && di.Mode()&os.ModeSymlink != 0 { + return fmt.Errorf("spec: %s is a symlink (refusing to follow)", rel) + } + if err := os.MkdirAll(dir, 0o755); err != nil { + return fmt.Errorf("spec: creating %s: %w", rel, err) + } + return nil +} + +// atomicWrite writes content to a temp file in the destination directory, then +// renames it into place so a reader never sees a partial file. +func atomicWrite(path, content string) error { + tmp, err := os.CreateTemp(filepath.Dir(path), ".spec-*.tmp") + if err != nil { + return fmt.Errorf("spec: creating temp file: %w", err) + } + tmpName := tmp.Name() + _, werr := tmp.WriteString(content) + cerr := tmp.Close() + if werr != nil { + _ = os.Remove(tmpName) + return fmt.Errorf("spec: writing temp file: %w", werr) + } + if cerr != nil { + _ = os.Remove(tmpName) + return fmt.Errorf("spec: closing temp file: %w", cerr) + } + if err := os.Rename(tmpName, path); err != nil { + _ = os.Remove(tmpName) + return fmt.Errorf("spec: finalising %s: %w", path, err) + } + return nil +} diff --git a/internal/core/spec/store_test.go b/internal/core/spec/store_test.go new file mode 100644 index 00000000..eaf5c39d --- /dev/null +++ b/internal/core/spec/store_test.go @@ -0,0 +1,187 @@ +package spec + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +const ( + specsOpen = ".abcd/development/specs/open" + specsClosed = ".abcd/development/specs/closed" + intentsBase = ".abcd/development/intents" +) + +func TestNextIDEmptyRepo(t *testing.T) { + root := t.TempDir() + got, err := NextID(root) + if err != nil { + t.Fatal(err) + } + if got != "spc-1" { + t.Fatalf("NextID(empty) = %q, want spc-1", got) + } +} + +// itd-3 is shipped with spec_id: spc-1 but has no spec-store file. NextID must +// still skip spc-1 so a freshly minted spec never collides with that reservation. +func TestNextIDReservedByIntent(t *testing.T) { + root := t.TempDir() + writeFile(t, root, intentsBase+"/shipped/itd-3-rules-loader.md", + "---\nid: itd-3\nslug: rules-loader\nspec_id: spc-1\nkind: standalone\n---\n# ok\n") + + got, err := NextID(root) + if err != nil { + t.Fatal(err) + } + if got != "spc-2" { + t.Fatalf("NextID = %q, want spc-2 (spc-1 reserved by itd-3)", got) + } +} + +func TestNextIDMaxAcrossSpecsAndIntents(t *testing.T) { + root := t.TempDir() + // A spec-store file at spc-5 (higher than any intent reservation). + writeFile(t, root, specsOpen+"/spc-5-existing.md", + "---\nid: spc-5\nslug: existing\nintent: itd-9\n---\n# ok\n") + // An intent reserving spc-2 (lower; must not lower the max). + writeFile(t, root, intentsBase+"/planned/itd-20-x.md", + "---\nid: itd-20\nslug: x\nspec_id: spc-2-thing\nkind: standalone\n---\n# ok\n") + + got, err := NextID(root) + if err != nil { + t.Fatal(err) + } + if got != "spc-6" { + t.Fatalf("NextID = %q, want spc-6", got) + } +} + +func TestCreateRoundTrip(t *testing.T) { + root := t.TempDir() + sp, err := Create(root, "itd-9", "my-feature") + if err != nil { + t.Fatal(err) + } + if sp.ID != "spc-1" || sp.Intent != "itd-9" || sp.Status != StatusOpen { + t.Fatalf("Create returned %+v", sp) + } + + abs := filepath.Join(root, specsOpen, "spc-1-my-feature.md") + data, err := os.ReadFile(abs) + if err != nil { + t.Fatalf("expected spec file on disk: %v", err) + } + if !strings.Contains(string(data), "intent: itd-9") { + t.Fatalf("spec file missing intent link:\n%s", data) + } + + // Round-trips through Load. + store, err := Load(root) + if err != nil { + t.Fatal(err) + } + if s, ok := store.Lookup("spc-1"); !ok || s.Intent != "itd-9" { + t.Fatalf("Load/Lookup after Create = %+v, %v", s, ok) + } + if s, ok := store.ByIntent("itd-9"); !ok || s.ID != "spc-1" { + t.Fatalf("Load/ByIntent after Create = %+v, %v", s, ok) + } +} + +func TestCreateRejectsBadIntent(t *testing.T) { + root := t.TempDir() + if _, err := Create(root, "itd-../../etc", "slug"); err == nil { + t.Fatal("Create with traversal intent id must fail") + } + if _, err := Create(root, "spc-1", "slug"); err == nil { + t.Fatal("Create with non-itd intent id must fail") + } +} + +func TestCreateRejectsBadSlug(t *testing.T) { + root := t.TempDir() + if _, err := Create(root, "itd-9", "../../etc"); err == nil { + t.Fatal("Create with traversal slug must fail") + } + if _, err := Create(root, "itd-9", "Bad Slug"); err == nil { + t.Fatal("Create with non-kebab slug must fail") + } +} + +func TestLoadMissingDirIsEmpty(t *testing.T) { + root := t.TempDir() + store, err := Load(root) + if err != nil { + t.Fatalf("Load on missing specs dir must be soft: %v", err) + } + if len(store.Specs) != 0 { + t.Fatalf("expected empty store, got %+v", store.Specs) + } +} + +func TestLoadMalformedIsHardError(t *testing.T) { + root := t.TempDir() + // No frontmatter at all -> no id -> hard error. + writeFile(t, root, specsOpen+"/spc-1-broken.md", "# just a title, no frontmatter\n") + if _, err := Load(root); err == nil { + t.Fatal("Load must hard-error on a malformed spec file") + } +} + +func TestLoadRejectsTraversalID(t *testing.T) { + root := t.TempDir() + writeFile(t, root, specsOpen+"/spc-1-evil.md", + "---\nid: spc-../../etc\nslug: evil\nintent: itd-9\n---\n# evil\n") + if _, err := Load(root); err == nil { + t.Fatal("Load must reject a path-traversal id in frontmatter") + } +} + +func TestCloseMovesOpenToClosed(t *testing.T) { + root := t.TempDir() + if _, err := Create(root, "itd-9", "my-feature"); err != nil { + t.Fatal(err) + } + + sp, err := Close(root, "spc-1") + if err != nil { + t.Fatal(err) + } + if sp.Status != StatusClosed || sp.Intent != "itd-9" { + t.Fatalf("Close returned %+v", sp) + } + + if _, err := os.Stat(filepath.Join(root, specsOpen, "spc-1-my-feature.md")); !os.IsNotExist(err) { + t.Fatal("open file should be gone after Close") + } + if _, err := os.Stat(filepath.Join(root, specsClosed, "spc-1-my-feature.md")); err != nil { + t.Fatalf("closed file should exist after Close: %v", err) + } + + // The store now reports it closed. + store, err := Load(root) + if err != nil { + t.Fatal(err) + } + if s, ok := store.Lookup("spc-1"); !ok || s.Status != StatusClosed { + t.Fatalf("after Close, Lookup = %+v, %v", s, ok) + } +} + +func TestCloseMissingFails(t *testing.T) { + root := t.TempDir() + if _, err := Close(root, "spc-99"); err == nil { + t.Fatal("Close on a missing spec must fail") + } +} + +func TestCloseAlreadyClosedFails(t *testing.T) { + root := t.TempDir() + writeFile(t, root, specsClosed+"/spc-1-done.md", + "---\nid: spc-1\nslug: done\nintent: itd-9\n---\n# done\n") + if _, err := Close(root, "spc-1"); err == nil { + t.Fatal("Close on an already-closed spec must fail") + } +} From ac8f08bb2ce3dd4c9fbe08cb30b634913213e253 Mon Sep 17 00:00:00 2001 From: REPPL <77722411+REPPL@users.noreply.github.com> Date: Sat, 11 Jul 2026 17:37:06 +0100 Subject: [PATCH 3/5] refactor: extract shared frontmatter parser to internal/core/frontmatter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Consolidates the frontmatter line-scanner (leading ---…--- block, top-level keys, first-key-wins, 1-based lines, IsNull) into one home and migrates internal/core/spec onto it, deleting spec's private copy. Behaviour- preserving: spec's tests pass unchanged. lint's and memory's copies remain (pre-existing) and are flagged for a later migration — one-canonical-primitive. Assisted-by: Claude:claude-opus-4-8 --- internal/core/frontmatter/frontmatter.go | 55 ++++++++++++++ internal/core/frontmatter/frontmatter_test.go | 72 +++++++++++++++++++ internal/core/spec/spec.go | 53 ++------------ internal/core/spec/store.go | 16 +++-- 4 files changed, 140 insertions(+), 56 deletions(-) create mode 100644 internal/core/frontmatter/frontmatter.go create mode 100644 internal/core/frontmatter/frontmatter_test.go diff --git a/internal/core/frontmatter/frontmatter.go b/internal/core/frontmatter/frontmatter.go new file mode 100644 index 00000000..6ff8ed82 --- /dev/null +++ b/internal/core/frontmatter/frontmatter.go @@ -0,0 +1,55 @@ +// Package frontmatter is abcd's shared markdown-frontmatter line scanner. It is +// deliberately a line scanner, not a YAML parser: it reads only the top-level +// keys of the leading `---`…`---` block, first key wins, and pulls in zero +// dependencies. It exists so the native record stores (internal/core/spec, +// internal/core/intent) share ONE copy of this primitive rather than each +// keeping a private replica. +// +// It is transport-agnostic: no stdout, no os.Exit, no filesystem access — the +// caller supplies the file's lines and decides what the fields mean. +package frontmatter + +import ( + "regexp" + "strings" +) + +// keyRe matches a top-level frontmatter key (column 0, no indentation). +var keyRe = regexp.MustCompile(`^([A-Za-z0-9_]+):(.*)$`) + +// Field is a frontmatter key's value and its 1-based source line. +type Field struct { + Value string + Line int +} + +// Fields returns the top-level keys of the leading frontmatter block (the block +// between the first two `---` lines). Nested keys and list items are ignored, +// and the first occurrence of a key wins. An input whose first line is not `---` +// (or is empty) yields no fields. +func Fields(lines []string) map[string]Field { + fields := map[string]Field{} + if len(lines) == 0 || strings.TrimRight(lines[0], "\r") != "---" { + return fields + } + for i := 1; i < len(lines); i++ { + line := strings.TrimRight(lines[i], "\r") + if line == "---" { + break + } + m := keyRe.FindStringSubmatch(line) + if m == nil { + continue + } + key := m[1] + if _, exists := fields[key]; !exists { + fields[key] = Field{Value: strings.TrimSpace(m[2]), Line: i + 1} + } + } + return fields +} + +// IsNull treats an empty value and the YAML nulls ""/"null"/"~" as null. +func IsNull(v string) bool { + return v == "" || v == "null" || v == "~" +} diff --git a/internal/core/frontmatter/frontmatter_test.go b/internal/core/frontmatter/frontmatter_test.go new file mode 100644 index 00000000..ea023156 --- /dev/null +++ b/internal/core/frontmatter/frontmatter_test.go @@ -0,0 +1,72 @@ +package frontmatter + +import "testing" + +func TestFieldsReadsLeadingBlock(t *testing.T) { + lines := []string{ + "---", + "id: itd-9", + "slug: my-thing", + "spec_id: null", + "---", + "# Title", + "key: not-frontmatter", + } + fields := Fields(lines) + if got := fields["id"]; got.Value != "itd-9" || got.Line != 2 { + t.Fatalf("id = %+v, want {itd-9 2}", got) + } + if got := fields["slug"]; got.Value != "my-thing" || got.Line != 3 { + t.Fatalf("slug = %+v, want {my-thing 3}", got) + } + if _, ok := fields["key"]; ok { + t.Fatal("a key past the closing --- must not be read") + } +} + +func TestFieldsFirstKeyWins(t *testing.T) { + lines := []string{"---", "kind: standalone", "kind: discipline", "---"} + if got := Fields(lines)["kind"]; got.Value != "standalone" || got.Line != 2 { + t.Fatalf("kind = %+v, want first-key-wins {standalone 2}", got) + } +} + +func TestFieldsNoFrontmatter(t *testing.T) { + if got := Fields([]string{"# Title", "id: itd-9"}); len(got) != 0 { + t.Fatalf("no leading --- must yield no fields, got %+v", got) + } + if got := Fields(nil); len(got) != 0 { + t.Fatalf("empty input must yield no fields, got %+v", got) + } +} + +func TestFieldsIgnoresNested(t *testing.T) { + lines := []string{"---", "top: v", " nested: v", "- item", "---"} + fields := Fields(lines) + if _, ok := fields["nested"]; ok { + t.Fatal("indented key must be ignored (top-level only)") + } + if got := fields["top"]; got.Value != "v" { + t.Fatalf("top = %+v, want v", got) + } +} + +func TestFieldsTrimsCarriageReturn(t *testing.T) { + lines := []string{"---\r", "id: itd-9\r", "---\r"} + if got := Fields(lines)["id"]; got.Value != "itd-9" { + t.Fatalf("CRLF id = %+v, want itd-9", got) + } +} + +func TestIsNull(t *testing.T) { + for _, v := range []string{"", "null", "~"} { + if !IsNull(v) { + t.Errorf("IsNull(%q) = false, want true", v) + } + } + for _, v := range []string{"itd-9", "spc-1", "standalone"} { + if IsNull(v) { + t.Errorf("IsNull(%q) = true, want false", v) + } + } +} diff --git a/internal/core/spec/spec.go b/internal/core/spec/spec.go index a46da2a5..1f98fc0f 100644 --- a/internal/core/spec/spec.go +++ b/internal/core/spec/spec.go @@ -12,10 +12,10 @@ // bucket. The load-bearing field is intent: itd-N, the link a spec declares to // the intent it realises. // -// Frontmatter is read by a private line scanner (frontmatterFields), not a YAML -// parser — the package pulls in zero new dependencies. Ids are validated against -// strict regexes before any path is built, so a hostile id can never traverse -// out of the spec store. +// Frontmatter is read by the shared internal/core/frontmatter line scanner, not +// a YAML parser — the package pulls in zero new dependencies. Ids are validated +// against strict regexes before any path is built, so a hostile id can never +// traverse out of the spec store. package spec import ( @@ -135,48 +135,3 @@ func renderSpec(id, slug, intentID string) string { b.WriteString("## Summary\n\nTODO\n") return b.String() } - -// --- frontmatter line scanner --- -// -// Replicated privately (not imported) from internal/core/lint so this package -// stays dependency-free: it is a line scanner, not a YAML parser. It reads only -// the block between the first two `---` lines, top-level keys only, first key -// wins. - -// fmKeyRe matches a top-level frontmatter key (column 0, no indentation). -var fmKeyRe = regexp.MustCompile(`^([A-Za-z0-9_]+):(.*)$`) - -// fmField is a frontmatter key's value and 1-based source line. -type fmField struct { - value string - line int -} - -// frontmatterFields returns the top-level keys of the leading frontmatter block -// (between the first two `---` lines). Nested keys and list items are ignored. -func frontmatterFields(lines []string) map[string]fmField { - fields := map[string]fmField{} - if len(lines) == 0 || strings.TrimRight(lines[0], "\r") != "---" { - return fields - } - for i := 1; i < len(lines); i++ { - line := strings.TrimRight(lines[i], "\r") - if line == "---" { - break - } - m := fmKeyRe.FindStringSubmatch(line) - if m == nil { - continue - } - key := m[1] - if _, exists := fields[key]; !exists { - fields[key] = fmField{value: strings.TrimSpace(m[2]), line: i + 1} - } - } - return fields -} - -// isNull treats an empty value and the YAML nulls ""/"null"/"~" as null. -func isNull(v string) bool { - return v == "" || v == "null" || v == "~" -} diff --git a/internal/core/spec/store.go b/internal/core/spec/store.go index 748f7e8a..ab609eb4 100644 --- a/internal/core/spec/store.go +++ b/internal/core/spec/store.go @@ -5,6 +5,8 @@ import ( "os" "path/filepath" "strings" + + "github.com/REPPL/abcd-cli/internal/core/frontmatter" ) // Load discovers spec files under both buckets, parses their frontmatter, and @@ -62,11 +64,11 @@ func loadBucket(repoRoot, bucket string) ([]Spec, error) { // parseSpec builds a Spec from a file's content and validates it. A file whose // frontmatter lacks a well-formed id or intent is malformed and rejected. func parseSpec(relPath, content, bucket string) (Spec, error) { - fields := frontmatterFields(strings.Split(content, "\n")) + fields := frontmatter.Fields(strings.Split(content, "\n")) sp := Spec{ - ID: fields["id"].value, - Slug: fields["slug"].value, - Intent: fields["intent"].value, + ID: fields["id"].Value, + Slug: fields["slug"].Value, + Intent: fields["intent"].Value, Status: bucket, Path: relPath, } @@ -128,9 +130,9 @@ func maxIntentSpecNum(repoRoot string) (int, error) { if err != nil { return 0, err } - fields := frontmatterFields(strings.Split(string(data), "\n")) - v := fields["spec_id"].value - if isNull(v) { + fields := frontmatter.Fields(strings.Split(string(data), "\n")) + v := fields["spec_id"].Value + if frontmatter.IsNull(v) { continue } if n := specNum(v); n > max { From ddd077216984dafcd5ed0a0a8d2b7ab6640ef465 Mon Sep 17 00:00:00 2001 From: REPPL <77722411+REPPL@users.noreply.github.com> Date: Sat, 11 Jul 2026 17:37:07 +0100 Subject: [PATCH 4/5] =?UTF-8?q?feat:=20itd-80=20phase=202=20=E2=80=94=20ab?= =?UTF-8?q?cd=20intent/spec=20verbs=20(planned=20link=20+=20spec=20close)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit New internal/core/intent (Plan/Link/Status) wired via internal/surface/cli: - abcd intent bare render of lifecycle status (read-only) - abcd intent plan validate ## Acceptance Criteria, mint a native spec, write the bidirectional link + kind:standalone, move drafts/->planned/ (fail-closed: the only transient frontmatter is lint-valid in both buckets) - abcd intent link retroactively link an existing spec - abcd spec bare render of spec status - abcd spec close marks a spec done (open->closed); the linked-intent reconcile lands in phase 3 record-lint stays green across a plan (TestPlanResidualPassesRecordLint runs the real lint engine). Part of itd-80. Assisted-by: Claude:claude-opus-4-8 --- CHANGELOG.md | 16 ++ internal/core/intent/intent.go | 220 +++++++++++++++++ internal/core/intent/intent_test.go | 265 +++++++++++++++++++++ internal/core/intent/lifecycle.go | 300 ++++++++++++++++++++++++ internal/surface/cli/cli.go | 146 ++++++++++++ internal/surface/cli/intent_cli_test.go | 188 +++++++++++++++ 6 files changed, 1135 insertions(+) create mode 100644 internal/core/intent/intent.go create mode 100644 internal/core/intent/intent_test.go create mode 100644 internal/core/intent/lifecycle.go create mode 100644 internal/surface/cli/intent_cli_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 1869cd25..9987cea6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,22 @@ called out in a **Breaking** section. ### Added +- The **intent lifecycle** verbs `abcd intent` and `abcd spec` (itd-80), the + front doors onto the native intent store (`internal/core/intent`). Bare + `abcd intent` renders a read-only lifecycle summary (intent counts by bucket, + spec counts by status, and the linked intent↔spec pairs); `abcd intent plan + ` mints a native spec for a draft intent that carries a non-empty + `## Acceptance Criteria` section (the itd-1 gate), writes both sides of the + bidirectional link (the spec's `intent: itd-N` and the intent's + `spec_id: spc-N` plus a default `kind: standalone`), and moves the intent + `drafts/ → planned/` — fail-closed, so every intermediate on-disk state stays + valid under the `intent_lifecycle` record-lint rule. `abcd intent link + ` retroactively links a planned intent to an existing spec, refusing a + spec that realises a different intent. Bare `abcd spec` renders the spec-store + status; `abcd spec close ` moves a spec `open/ → closed/` (the + lifecycle reconcile that trails a close lands in a later phase). The + frontmatter line-scanner shared by these stores now lives in + `internal/core/frontmatter`. - The **modular rules loader** core and its `abcd rules [domain]` verb (itd-3, phases 1 + 3). `internal/core/rules` holds binary-bundled default rule domains (COMMITTING, DOCUMENTATION, ROADMAP, ISSUES, INTENTS, LIFEBOAT, PII, and diff --git a/internal/core/intent/intent.go b/internal/core/intent/intent.go new file mode 100644 index 00000000..7df6384c --- /dev/null +++ b/internal/core/intent/intent.go @@ -0,0 +1,220 @@ +// Package intent is abcd's transport-agnostic native intent store (intent +// lifecycle, itd-80). It owns the in-memory model of intent records and the disk +// operations that plan, link, and summarise them. Every function takes a +// structured request and returns a structured result; nothing here writes to +// stdout or knows about a CLI, MCP, or hook surface — the front doors under +// internal/surface/* marshal these results for their transport. +// +// An intent record is a markdown file under +// /.abcd/development/intents/{drafts,planned,shipped,disciplines, +// superseded}/itd-N-.md. The bucket directory IS the lifecycle state +// (directory-as-truth: there is no status: frontmatter field), mirroring the +// native spec store. The load-bearing field is spec_id: spc-N, the intent's +// derived side of the bidirectional link to the spec that realises it (the +// spec's reciprocal side is intent: itd-N). +// +// Frontmatter is read by the shared internal/core/frontmatter line scanner, not +// a YAML parser — the package pulls in zero new dependencies. Ids are validated +// against strict regexes before any path is built, so a hostile id can never +// traverse out of the intent store. +package intent + +import ( + "fmt" + "regexp" + "strings" + + "github.com/REPPL/abcd-cli/internal/core/spec" +) + +// IntentsRelDir is the intent-store root, relative to the repo worktree. +const IntentsRelDir = ".abcd/development/intents" + +// Lifecycle buckets. The directory an intent file lives in is its state. +const ( + BucketDrafts = "drafts" + BucketPlanned = "planned" + BucketShipped = "shipped" + BucketDisciplines = "disciplines" + BucketSuperseded = "superseded" +) + +// KindStandalone is the default binding kind Plan writes (a 1:1 intent↔spec). +const KindStandalone = "standalone" + +// Buckets is the fixed lifecycle order used for loading and rendering. +var Buckets = []string{BucketDrafts, BucketPlanned, BucketShipped, BucketDisciplines, BucketSuperseded} + +// maxIntentFileBytes caps any intent markdown file read (trust boundary). +const maxIntentFileBytes = 256 * 1024 + +var ( + // intentIDRe constrains an intent id so it can never build a path that + // escapes the store (path-traversal defence). + intentIDRe = regexp.MustCompile(`^itd-[0-9]+$`) + // specIDRe constrains a spec id the same way. + specIDRe = regexp.MustCompile(`^spc-[0-9]+$`) + // slugRe constrains a slug to kebab-case, since a slug becomes a filename. + slugRe = regexp.MustCompile(`^[a-z0-9]+(?:-[a-z0-9]+)*$`) + // intentFileRe matches an intent-store filename. + intentFileRe = regexp.MustCompile(`^itd-[0-9]+.*\.md$`) + // fmKeyRe matches a top-level frontmatter key (column 0) for the writer. + fmKeyRe = regexp.MustCompile(`^([A-Za-z0-9_]+):(.*)$`) + // acHeadingRe matches the `## Acceptance Criteria` heading (any heading depth). + acHeadingRe = regexp.MustCompile(`^#{1,6}\s+Acceptance Criteria\s*$`) + // headingRe matches any markdown ATX heading line. + headingRe = regexp.MustCompile(`^#{1,6}\s`) +) + +// Intent is one intent record. Bucket is the directory it was found in; Path is +// repo-relative (never an absolute local path). +type Intent struct { + ID string `json:"id"` // itd-N + Slug string `json:"slug"` // kebab-case + Kind string `json:"kind"` // standalone | bundle-member | discipline | null + SpecID string `json:"spec_id"` // spc-N, the derived link (may be null) + Bucket string `json:"bucket"` // lifecycle directory (directory-as-truth) + Path string `json:"path"` // repo-relative markdown path +} + +// Corpus is the in-memory set of intent records discovered across every bucket. +type Corpus struct { + Intents []Intent `json:"intents"` +} + +// Lookup returns the intent with the given id; ok is false when absent. +func (c Corpus) Lookup(id string) (Intent, bool) { + for _, it := range c.Intents { + if it.ID == id { + return it, true + } + } + return Intent{}, false +} + +// Validate enforces the id regex — the fail-closed guard Load runs before +// trusting a record's id in a filesystem path. +func Validate(it Intent) error { + if !intentIDRe.MatchString(it.ID) { + return fmt.Errorf("intent: id %q must match ^itd-[0-9]+$", it.ID) + } + return nil +} + +// hasAcceptanceCriteria reports whether content carries a non-empty +// `## Acceptance Criteria` section (the itd-1 discipline Plan enforces). The +// section is non-empty when at least one non-blank line separates its heading +// from the next heading (or end of file). +func hasAcceptanceCriteria(content string) bool { + lines := strings.Split(content, "\n") + for i, line := range lines { + if !acHeadingRe.MatchString(strings.TrimRight(line, "\r")) { + continue + } + for _, body := range lines[i+1:] { + body = strings.TrimRight(body, "\r") + if headingRe.MatchString(body) { + break // next section reached with no content + } + if strings.TrimSpace(body) != "" { + return true + } + } + return false + } + return false +} + +// setFrontmatterFields returns content with the given frontmatter keys set to +// the given values: an existing top-level key line is rewritten in place, and a +// key not yet present is inserted just before the closing `---` (sorted, for a +// deterministic result). Everything outside the leading frontmatter block — the +// body and untouched keys — is preserved verbatim. An input without a well-formed +// leading frontmatter block is an error (fail closed rather than corrupt a file). +func setFrontmatterFields(content string, updates map[string]string) (string, error) { + lines := strings.Split(content, "\n") + if len(lines) == 0 || strings.TrimRight(lines[0], "\r") != "---" { + return "", fmt.Errorf("intent: file has no leading frontmatter block") + } + closing := -1 + for i := 1; i < len(lines); i++ { + if strings.TrimRight(lines[i], "\r") == "---" { + closing = i + break + } + } + if closing < 0 { + return "", fmt.Errorf("intent: frontmatter block is not closed") + } + + remaining := make(map[string]string, len(updates)) + for k, v := range updates { + remaining[k] = v + } + for i := 1; i < closing; i++ { + m := fmKeyRe.FindStringSubmatch(strings.TrimRight(lines[i], "\r")) + if m == nil { + continue + } + if v, ok := remaining[m[1]]; ok { + lines[i] = m[1] + ": " + v + delete(remaining, m[1]) + } + } + if len(remaining) > 0 { + keys := make([]string, 0, len(remaining)) + for k := range remaining { + keys = append(keys, k) + } + sortStrings(keys) + ins := make([]string, 0, len(keys)) + for _, k := range keys { + ins = append(ins, k+": "+remaining[k]) + } + out := make([]string, 0, len(lines)+len(ins)) + out = append(out, lines[:closing]...) + out = append(out, ins...) + out = append(out, lines[closing:]...) + lines = out + } + return strings.Join(lines, "\n"), nil +} + +// sortStrings sorts in place (small local helper to avoid importing sort for one +// call site). +func sortStrings(s []string) { + for i := 1; i < len(s); i++ { + for j := i; j > 0 && s[j-1] > s[j]; j-- { + s[j-1], s[j] = s[j], s[j-1] + } + } +} + +// PlanResult reports a completed Plan: the updated planned intent and the spec +// minted to realise it. +type PlanResult struct { + Intent Intent `json:"intent"` + Spec spec.Spec `json:"spec"` +} + +// LinkResult reports a completed Link: the updated intent and the spec it now +// declares. +type LinkResult struct { + Intent Intent `json:"intent"` + Spec spec.Spec `json:"spec"` +} + +// LinkedPair is one intent↔spec link in the lifecycle summary. +type LinkedPair struct { + Intent string `json:"intent"` + Spec string `json:"spec"` +} + +// StatusView is the read-only lifecycle summary: intent counts by bucket, spec +// counts by status, and the linked intent↔spec pairs. +type StatusView struct { + Buckets map[string]int `json:"buckets"` + SpecsOpen int `json:"specs_open"` + SpecsClosed int `json:"specs_closed"` + Linked []LinkedPair `json:"linked"` +} diff --git a/internal/core/intent/intent_test.go b/internal/core/intent/intent_test.go new file mode 100644 index 00000000..bee897ad --- /dev/null +++ b/internal/core/intent/intent_test.go @@ -0,0 +1,265 @@ +package intent + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/REPPL/abcd-cli/internal/core/frontmatter" + "github.com/REPPL/abcd-cli/internal/core/lint" +) + +const ( + draftsDir = ".abcd/development/intents/drafts" + plannedDir = ".abcd/development/intents/planned" + shippedDir = ".abcd/development/intents/shipped" + specsOpen = ".abcd/development/specs/open" + specsClosed = ".abcd/development/specs/closed" +) + +// writeFile writes content to root/rel, creating parent directories. +func writeFile(t *testing.T, root, rel, content string) { + t.Helper() + abs := filepath.Join(root, rel) + if err := os.MkdirAll(filepath.Dir(abs), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(abs, []byte(content), 0o644); err != nil { + t.Fatal(err) + } +} + +// draftWithAC is a draft intent carrying a non-empty ## Acceptance Criteria +// section (the itd-1 gate Plan enforces). +func draftWithAC(id, slug string) string { + return "---\n" + + "id: " + id + "\n" + + "slug: " + slug + "\n" + + "spec_id: null\n" + + "kind: null\n" + + "---\n" + + "# " + slug + "\n\n" + + "## Acceptance Criteria\n\n" + + "- **Given** a user, **when** they act, **then** it works.\n" +} + +func TestLoadCorpus(t *testing.T) { + root := t.TempDir() + writeFile(t, root, draftsDir+"/itd-10-alpha.md", draftWithAC("itd-10", "alpha")) + writeFile(t, root, plannedDir+"/itd-2-beta.md", + "---\nid: itd-2\nslug: beta\nspec_id: spc-1\nkind: standalone\n---\n# beta\n") + + c, err := Load(root) + if err != nil { + t.Fatal(err) + } + if len(c.Intents) != 2 { + t.Fatalf("expected 2 intents, got %d: %+v", len(c.Intents), c.Intents) + } + it, ok := c.Lookup("itd-2") + if !ok || it.Bucket != "planned" || it.SpecID != "spc-1" || it.Kind != "standalone" { + t.Fatalf("Lookup(itd-2) = %+v, %v", it, ok) + } + if it, ok := c.Lookup("itd-10"); !ok || it.Bucket != "drafts" { + t.Fatalf("Lookup(itd-10) = %+v, %v", it, ok) + } +} + +func TestLoadMissingDirIsEmpty(t *testing.T) { + c, err := Load(t.TempDir()) + if err != nil { + t.Fatalf("Load on missing intents dir must be soft: %v", err) + } + if len(c.Intents) != 0 { + t.Fatalf("expected empty corpus, got %+v", c.Intents) + } +} + +func TestLoadMalformedIsHardError(t *testing.T) { + root := t.TempDir() + writeFile(t, root, draftsDir+"/itd-1-broken.md", "# no frontmatter, no id\n") + if _, err := Load(root); err == nil { + t.Fatal("Load must hard-error on an intent file with no well-formed id") + } +} + +func TestPlanHappyPath(t *testing.T) { + root := t.TempDir() + writeFile(t, root, draftsDir+"/itd-10-alpha.md", draftWithAC("itd-10", "alpha")) + + res, err := Plan(root, "itd-10") + if err != nil { + t.Fatal(err) + } + if res.Spec.ID != "spc-1" || res.Spec.Intent != "itd-10" { + t.Fatalf("Plan spec = %+v", res.Spec) + } + if res.Intent.Bucket != "planned" || res.Intent.SpecID != "spc-1" || res.Intent.Kind != "standalone" { + t.Fatalf("Plan intent = %+v", res.Intent) + } + + // The draft file is gone; the planned file exists with both link sides. + if _, err := os.Stat(filepath.Join(root, draftsDir, "itd-10-alpha.md")); !os.IsNotExist(err) { + t.Fatal("draft file should be gone after Plan") + } + body, err := os.ReadFile(filepath.Join(root, plannedDir, "itd-10-alpha.md")) + if err != nil { + t.Fatalf("planned file should exist: %v", err) + } + f := frontmatter.Fields(strings.Split(string(body), "\n")) + if f["spec_id"].Value != "spc-1" { + t.Fatalf("planned intent spec_id = %q, want spc-1\n%s", f["spec_id"].Value, body) + } + if f["kind"].Value != "standalone" { + t.Fatalf("planned intent kind = %q, want standalone\n%s", f["kind"].Value, body) + } + // The spec file carries the reciprocal intent link. + sbody, err := os.ReadFile(filepath.Join(root, specsOpen, "spc-1-alpha.md")) + if err != nil { + t.Fatalf("spec file should exist: %v", err) + } + if !strings.Contains(string(sbody), "intent: itd-10") { + t.Fatalf("spec file missing reciprocal link:\n%s", sbody) + } +} + +// TestPlanResidualPassesRecordLint proves that a drafts->planned Plan leaves a +// frontmatter/bucket state the intent_lifecycle record-lint rule accepts — the +// DoD's "make record-lint stays green" guarantee, checked through the real lint +// engine over the fixture. +func TestPlanResidualPassesRecordLint(t *testing.T) { + root := t.TempDir() + writeFile(t, root, draftsDir+"/itd-10-alpha.md", draftWithAC("itd-10", "alpha")) + if _, err := Plan(root, "itd-10"); err != nil { + t.Fatal(err) + } + + cfg := lint.Config{ + Roots: []string{".abcd/development"}, + Rules: map[string]lint.RuleConfig{ + "intent_lifecycle": {Enabled: true, Severity: "blocker", IntentsDir: "intents"}, + }, + } + findings, err := lint.Lint(cfg, root) + if err != nil { + t.Fatal(err) + } + for _, fnd := range findings { + if fnd.RuleID == "intent_lifecycle" { + t.Fatalf("planned intent violates intent_lifecycle: %s:%d %s", fnd.File, fnd.Line, fnd.Message) + } + } +} + +func TestPlanRefusesNoAcceptanceCriteria(t *testing.T) { + root := t.TempDir() + writeFile(t, root, draftsDir+"/itd-10-alpha.md", + "---\nid: itd-10\nslug: alpha\nspec_id: null\nkind: null\n---\n# alpha\n\nNo AC section here.\n") + + if _, err := Plan(root, "itd-10"); err == nil { + t.Fatal("Plan must refuse an intent with no Acceptance Criteria") + } + // Nothing moved, no spec minted. + if _, err := os.Stat(filepath.Join(root, draftsDir, "itd-10-alpha.md")); err != nil { + t.Fatal("draft must remain in place after refusal") + } + if _, err := os.Stat(filepath.Join(root, specsOpen)); !os.IsNotExist(err) { + t.Fatal("no spec should be minted on refusal") + } +} + +func TestPlanRefusesEmptyAcceptanceCriteria(t *testing.T) { + root := t.TempDir() + writeFile(t, root, draftsDir+"/itd-10-alpha.md", + "---\nid: itd-10\nslug: alpha\nspec_id: null\nkind: null\n---\n# alpha\n\n## Acceptance Criteria\n\n## Next Section\n\nbody\n") + if _, err := Plan(root, "itd-10"); err == nil { + t.Fatal("Plan must refuse an intent whose Acceptance Criteria section is empty") + } +} + +func TestPlanRefusesNonDraft(t *testing.T) { + root := t.TempDir() + writeFile(t, root, plannedDir+"/itd-10-alpha.md", + "---\nid: itd-10\nslug: alpha\nspec_id: null\nkind: standalone\n---\n# alpha\n\n## Acceptance Criteria\n\n- ok\n") + if _, err := Plan(root, "itd-10"); err == nil { + t.Fatal("Plan must refuse an intent that is not in drafts/") + } +} + +func TestPlanRejectsBadID(t *testing.T) { + root := t.TempDir() + if _, err := Plan(root, "itd-../../etc"); err == nil { + t.Fatal("Plan must reject a traversal id") + } +} + +func TestLinkHappyPath(t *testing.T) { + root := t.TempDir() + // A planned intent with no spec_id yet, and a spec that already realises it. + writeFile(t, root, plannedDir+"/itd-10-alpha.md", + "---\nid: itd-10\nslug: alpha\nspec_id: null\nkind: standalone\n---\n# alpha\n") + writeFile(t, root, specsOpen+"/spc-3-alpha.md", + "---\nid: spc-3\nslug: alpha\nintent: itd-10\n---\n# alpha\n") + + res, err := Link(root, "itd-10", "spc-3") + if err != nil { + t.Fatal(err) + } + if res.Intent.SpecID != "spc-3" { + t.Fatalf("Link intent spec_id = %q, want spc-3", res.Intent.SpecID) + } + body, err := os.ReadFile(filepath.Join(root, plannedDir, "itd-10-alpha.md")) + if err != nil { + t.Fatal(err) + } + if f := frontmatter.Fields(strings.Split(string(body), "\n")); f["spec_id"].Value != "spc-3" { + t.Fatalf("spec_id not written: %q\n%s", f["spec_id"].Value, body) + } +} + +func TestLinkMismatchFails(t *testing.T) { + root := t.TempDir() + writeFile(t, root, plannedDir+"/itd-10-alpha.md", + "---\nid: itd-10\nslug: alpha\nspec_id: null\nkind: standalone\n---\n# alpha\n") + // The spec realises a DIFFERENT intent. + writeFile(t, root, specsOpen+"/spc-3-other.md", + "---\nid: spc-3\nslug: other\nintent: itd-99\n---\n# other\n") + if _, err := Link(root, "itd-10", "spc-3"); err == nil { + t.Fatal("Link must fail closed when the spec realises a different intent") + } +} + +func TestLinkMissingSpecFails(t *testing.T) { + root := t.TempDir() + writeFile(t, root, plannedDir+"/itd-10-alpha.md", + "---\nid: itd-10\nslug: alpha\nspec_id: null\nkind: standalone\n---\n# alpha\n") + if _, err := Link(root, "itd-10", "spc-9"); err == nil { + t.Fatal("Link must fail when the spec does not exist") + } +} + +func TestStatusCounts(t *testing.T) { + root := t.TempDir() + writeFile(t, root, draftsDir+"/itd-10-alpha.md", draftWithAC("itd-10", "alpha")) + writeFile(t, root, plannedDir+"/itd-2-beta.md", + "---\nid: itd-2\nslug: beta\nspec_id: spc-1\nkind: standalone\n---\n# beta\n") + writeFile(t, root, specsOpen+"/spc-1-beta.md", + "---\nid: spc-1\nslug: beta\nintent: itd-2\n---\n# beta\n") + writeFile(t, root, specsClosed+"/spc-2-old.md", + "---\nid: spc-2\nslug: old\nintent: itd-7\n---\n# old\n") + + v, err := Status(root) + if err != nil { + t.Fatal(err) + } + if v.Buckets["drafts"] != 1 || v.Buckets["planned"] != 1 { + t.Fatalf("bucket counts = %+v", v.Buckets) + } + if v.SpecsOpen != 1 || v.SpecsClosed != 1 { + t.Fatalf("spec counts open=%d closed=%d", v.SpecsOpen, v.SpecsClosed) + } + if len(v.Linked) != 1 || v.Linked[0].Intent != "itd-2" || v.Linked[0].Spec != "spc-1" { + t.Fatalf("linked pairs = %+v", v.Linked) + } +} diff --git a/internal/core/intent/lifecycle.go b/internal/core/intent/lifecycle.go new file mode 100644 index 00000000..bd552973 --- /dev/null +++ b/internal/core/intent/lifecycle.go @@ -0,0 +1,300 @@ +package intent + +import ( + "fmt" + "os" + "path/filepath" + "strings" + + "github.com/REPPL/abcd-cli/internal/core/frontmatter" + "github.com/REPPL/abcd-cli/internal/core/spec" + "github.com/REPPL/abcd-cli/internal/fsutil" +) + +// Load discovers intent files across every lifecycle bucket, parses their +// frontmatter, and returns the in-memory Corpus. A missing intents/ directory +// (or a missing individual bucket) yields no records for it (soft, mirroring +// spec.Load). A present-but-malformed intent file — one whose frontmatter lacks +// a well-formed id — is a hard, loud error. +func Load(repoRoot string) (Corpus, error) { + var c Corpus + for _, bucket := range Buckets { + intents, err := loadBucket(repoRoot, bucket) + if err != nil { + return Corpus{}, err + } + c.Intents = append(c.Intents, intents...) + } + return c, nil +} + +// loadBucket reads one bucket directory. A missing directory is soft (nil, nil). +func loadBucket(repoRoot, bucket string) ([]Intent, error) { + dir := filepath.Join(repoRoot, IntentsRelDir, bucket) + relDir := filepath.Join(IntentsRelDir, bucket) + di, err := os.Lstat(dir) + if os.IsNotExist(err) { + return nil, nil + } + if err != nil { + return nil, fmt.Errorf("intent: stat %s: %w", relDir, err) + } + if di.Mode()&os.ModeSymlink != 0 { + return nil, fmt.Errorf("intent: %s is a symlink (refusing to follow)", relDir) + } + entries, err := os.ReadDir(dir) + if err != nil { + return nil, fmt.Errorf("intent: reading %s: %w", relDir, err) + } + var intents []Intent + for _, e := range entries { + if e.IsDir() || !intentFileRe.MatchString(e.Name()) { + continue + } + rel := filepath.Join(relDir, e.Name()) + data, err := readRepoFile(filepath.Join(dir, e.Name()), rel) + if err != nil { + return nil, err + } + it, err := parseIntent(rel, string(data), bucket) + if err != nil { + return nil, err + } + intents = append(intents, it) + } + return intents, nil +} + +// parseIntent builds an Intent from a file's content and validates it. A file +// whose frontmatter lacks a well-formed id is malformed and rejected. +func parseIntent(relPath, content, bucket string) (Intent, error) { + fields := frontmatter.Fields(strings.Split(content, "\n")) + it := Intent{ + ID: fields["id"].Value, + Slug: fields["slug"].Value, + Kind: fields["kind"].Value, + SpecID: fields["spec_id"].Value, + Bucket: bucket, + Path: relPath, + } + if err := Validate(it); err != nil { + return Intent{}, fmt.Errorf("intent: malformed %s: %w", relPath, err) + } + return it, nil +} + +// Plan is the load-bearing verb. For a draft intent carrying a non-empty +// `## Acceptance Criteria` section, it mints a native spec, writes the intent's +// derived side of the bidirectional link (spec_id + a default kind), and moves +// the intent drafts/ → planned/. +// +// It is fail-closed: every intermediate on-disk state satisfies the +// intent_lifecycle record-lint rule, so a failure at any step leaves a +// consistent, lint-valid record rather than a half-written link. The order — +// create spec, set kind while still a draft, move to planned, then write +// spec_id — is chosen so that (kind=standalone, spec_id=null) is the only +// transient frontmatter, and that shape is valid in BOTH drafts and planned. +func Plan(repoRoot, intentID string) (PlanResult, error) { + if !intentIDRe.MatchString(intentID) { + return PlanResult{}, fmt.Errorf("intent: id %q must match ^itd-[0-9]+$", intentID) + } + corpus, err := Load(repoRoot) + if err != nil { + return PlanResult{}, err + } + it, ok := corpus.Lookup(intentID) + if !ok { + return PlanResult{}, fmt.Errorf("intent: %s not found in any bucket", intentID) + } + if it.Bucket != BucketDrafts { + return PlanResult{}, fmt.Errorf("intent: %s is in %s, not drafts; only a draft can be planned", intentID, it.Bucket) + } + if !slugRe.MatchString(it.Slug) { + return PlanResult{}, fmt.Errorf("intent: %s has slug %q which must be kebab-case", intentID, it.Slug) + } + + draftRel := it.Path + draftAbs := filepath.Join(repoRoot, draftRel) + data, err := readRepoFile(draftAbs, draftRel) + if err != nil { + return PlanResult{}, err + } + content := string(data) + if !hasAcceptanceCriteria(content) { + return PlanResult{}, fmt.Errorf("intent: %s has no non-empty '## Acceptance Criteria' section (itd-1 discipline); refusing to plan", intentID) + } + + // 1. Mint + create the native spec (writes the reciprocal intent: itd-N). + sp, err := spec.Create(repoRoot, intentID, it.Slug) + if err != nil { + return PlanResult{}, err + } + + // 2. Set the binding kind (default standalone) while still in drafts. A draft + // with (kind=standalone, spec_id=null) stays lint-valid, so a failure here + // leaves a consistent record (the spec exists but the intent is unlinked). + kind := it.Kind + if frontmatter.IsNull(kind) { + kind = KindStandalone + } + withKind, err := setFrontmatterFields(content, map[string]string{"kind": kind}) + if err != nil { + return PlanResult{}, err + } + if err := fsutil.WriteFileAtomic(draftAbs, []byte(withKind), 0o644); err != nil { + return PlanResult{}, fmt.Errorf("intent: writing kind to %s: %w", draftRel, err) + } + + // 3. Move drafts/ → planned/. The moved file's (kind=standalone, + // spec_id=null) shape is a valid planned intent, so a rename failure leaves a + // consistent state either side. + name := filepath.Base(draftRel) + plannedRel := filepath.Join(IntentsRelDir, BucketPlanned, name) + plannedAbs := filepath.Join(repoRoot, plannedRel) + if err := ensureRealDir(filepath.Join(repoRoot, IntentsRelDir, BucketPlanned), filepath.Join(IntentsRelDir, BucketPlanned)); err != nil { + return PlanResult{}, err + } + if err := os.Rename(draftAbs, plannedAbs); err != nil { + return PlanResult{}, fmt.Errorf("intent: moving %s drafts->planned: %w", intentID, err) + } + + // 4. Write the derived link (spec_id) now that the file is in planned. A + // planned intent with spec_id=null is still lint-valid, so a failure here is + // consistent too. + withSpec, err := setFrontmatterFields(withKind, map[string]string{"spec_id": sp.ID}) + if err != nil { + return PlanResult{}, err + } + if err := fsutil.WriteFileAtomic(plannedAbs, []byte(withSpec), 0o644); err != nil { + return PlanResult{}, fmt.Errorf("intent: writing spec_id to %s: %w", plannedRel, err) + } + + it.Kind = kind + it.SpecID = sp.ID + it.Bucket = BucketPlanned + it.Path = plannedRel + return PlanResult{Intent: it, Spec: sp}, nil +} + +// Link retroactively writes the derived spec_id link on an existing planned +// intent for an existing spec. It validates both ids, that the intent is in +// planned/, and that the spec exists AND already declares this intent (the +// reciprocal intent: itd-N side); a spec that realises a different intent is a +// mismatch and fails closed rather than forging a one-sided link. +func Link(repoRoot, intentID, specID string) (LinkResult, error) { + if !intentIDRe.MatchString(intentID) { + return LinkResult{}, fmt.Errorf("intent: id %q must match ^itd-[0-9]+$", intentID) + } + if !specIDRe.MatchString(specID) { + return LinkResult{}, fmt.Errorf("intent: spec id %q must match ^spc-[0-9]+$", specID) + } + corpus, err := Load(repoRoot) + if err != nil { + return LinkResult{}, err + } + it, ok := corpus.Lookup(intentID) + if !ok { + return LinkResult{}, fmt.Errorf("intent: %s not found in any bucket", intentID) + } + if it.Bucket != BucketPlanned { + return LinkResult{}, fmt.Errorf("intent: %s is in %s, not planned; only a planned intent can be linked", intentID, it.Bucket) + } + store, err := spec.Load(repoRoot) + if err != nil { + return LinkResult{}, err + } + sp, ok := store.Lookup(specID) + if !ok { + return LinkResult{}, fmt.Errorf("intent: spec %s not found", specID) + } + if sp.Intent != intentID { + return LinkResult{}, fmt.Errorf("intent: spec %s realises %s, not %s (mismatch); refusing to link", specID, sp.Intent, intentID) + } + + rel := it.Path + abs := filepath.Join(repoRoot, rel) + data, err := readRepoFile(abs, rel) + if err != nil { + return LinkResult{}, err + } + updated, err := setFrontmatterFields(string(data), map[string]string{"spec_id": specID}) + if err != nil { + return LinkResult{}, err + } + if err := fsutil.WriteFileAtomic(abs, []byte(updated), 0o644); err != nil { + return LinkResult{}, fmt.Errorf("intent: writing spec_id to %s: %w", rel, err) + } + + it.SpecID = specID + return LinkResult{Intent: it, Spec: sp}, nil +} + +// Status builds the read-only lifecycle summary: intent counts by bucket, spec +// counts by status, and the intent↔spec links (every intent whose spec_id is +// non-null). Linked pairs are ordered by the corpus load order (bucket, then +// directory), which is deterministic. +func Status(repoRoot string) (StatusView, error) { + corpus, err := Load(repoRoot) + if err != nil { + return StatusView{}, err + } + store, err := spec.Load(repoRoot) + if err != nil { + return StatusView{}, err + } + + v := StatusView{Buckets: map[string]int{}} + for _, b := range Buckets { + v.Buckets[b] = 0 + } + for _, it := range corpus.Intents { + v.Buckets[it.Bucket]++ + if !frontmatter.IsNull(it.SpecID) { + v.Linked = append(v.Linked, LinkedPair{Intent: it.ID, Spec: it.SpecID}) + } + } + for _, sp := range store.Specs { + if sp.Status == spec.StatusClosed { + v.SpecsClosed++ + } else { + v.SpecsOpen++ + } + } + return v, nil +} + +// readRepoFile reads a repo file behind the trust-boundary guards: refuse a +// symlinked leaf, require a regular file, and cap the size. (Mirrors the spec +// store's private guard; a shared read-guard is a flagged consolidation target +// alongside ensureRealDir.) +func readRepoFile(abs, rel string) ([]byte, error) { + fi, err := os.Lstat(abs) + if err != nil { + return nil, fmt.Errorf("intent: stat %s: %w", rel, err) + } + if fi.Mode()&os.ModeSymlink != 0 { + return nil, fmt.Errorf("intent: %s is a symlink (refusing to follow)", rel) + } + if !fi.Mode().IsRegular() { + return nil, fmt.Errorf("intent: %s is not a regular file", rel) + } + if fi.Size() > maxIntentFileBytes { + return nil, fmt.Errorf("intent: %s exceeds the %d-byte cap", rel, maxIntentFileBytes) + } + data, err := os.ReadFile(abs) + if err != nil { + return nil, fmt.Errorf("intent: reading %s: %w", rel, err) + } + return data, nil +} + +// ensureRealDir creates dir if absent, refusing to follow a symlinked directory. +func ensureRealDir(dir, rel string) error { + if di, err := os.Lstat(dir); err == nil && di.Mode()&os.ModeSymlink != 0 { + return fmt.Errorf("intent: %s is a symlink (refusing to follow)", rel) + } + if err := os.MkdirAll(dir, 0o755); err != nil { + return fmt.Errorf("intent: creating %s: %w", rel, err) + } + return nil +} diff --git a/internal/surface/cli/cli.go b/internal/surface/cli/cli.go index be8a2c96..8b96e453 100644 --- a/internal/surface/cli/cli.go +++ b/internal/surface/cli/cli.go @@ -20,10 +20,12 @@ import ( "github.com/REPPL/abcd-cli/internal/core/capture" "github.com/REPPL/abcd-cli/internal/core/history" "github.com/REPPL/abcd-cli/internal/core/identity" + "github.com/REPPL/abcd-cli/internal/core/intent" "github.com/REPPL/abcd-cli/internal/core/launch" "github.com/REPPL/abcd-cli/internal/core/lint" "github.com/REPPL/abcd-cli/internal/core/memory" "github.com/REPPL/abcd-cli/internal/core/rules" + "github.com/REPPL/abcd-cli/internal/core/spec" "github.com/spf13/cobra" ) @@ -121,6 +123,8 @@ func NewRootCommand() *cobra.Command { root.AddCommand(newHookCommand()) root.AddCommand(newHistoryCommand(&asJSON)) root.AddCommand(newDocsCommand(&asJSON)) + root.AddCommand(newIntentCommand(&asJSON)) + root.AddCommand(newSpecCommand(&asJSON)) return root } @@ -379,6 +383,148 @@ func newRulesCommand(asJSON *bool) *cobra.Command { } } +// newIntentCommand builds the `intent` verb — the front door onto +// internal/core/intent (itd-80). Bare `abcd intent` renders the read-only +// lifecycle status board (never mutates); the `plan` and `link` sub-verbs carry +// the mutations. Usage/lookup failures exit 2. +func newIntentCommand(asJSON *bool) *cobra.Command { + intentCmd := &cobra.Command{ + Use: "intent", + Short: "Intent lifecycle; bare invocation is read-only status", + Args: cobra.NoArgs, + RunE: func(cmd *cobra.Command, _ []string) error { + cwd, err := os.Getwd() + if err != nil { + return err + } + v, err := intent.Status(cwd) + if err != nil { + return err + } + return render(cmd.OutOrStdout(), *asJSON, v, func(w io.Writer) { + fmt.Fprintf(w, "abcd intent — drafts %d · planned %d · shipped %d · disciplines %d · superseded %d\n", + v.Buckets[intent.BucketDrafts], v.Buckets[intent.BucketPlanned], v.Buckets[intent.BucketShipped], + v.Buckets[intent.BucketDisciplines], v.Buckets[intent.BucketSuperseded]) + fmt.Fprintf(w, " specs: open %d · closed %d\n", v.SpecsOpen, v.SpecsClosed) + for _, p := range v.Linked { + fmt.Fprintf(w, " link: %s -> %s\n", p.Intent, p.Spec) + } + }) + }, + } + + // plan — mint the spec, write both link sides, move drafts -> planned. + intentCmd.AddCommand(&cobra.Command{ + Use: "plan ", + Short: "Plan a draft intent: mint its spec, link both sides, move drafts -> planned", + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + cwd, err := os.Getwd() + if err != nil { + return err + } + res, err := intent.Plan(cwd, args[0]) + if err != nil { + return &exitError{Code: 2, Msg: "abcd intent plan: " + err.Error()} + } + return render(cmd.OutOrStdout(), *asJSON, res, func(w io.Writer) { + fmt.Fprintf(w, "abcd intent plan — %s drafts -> planned, linked %s\n", res.Intent.ID, res.Spec.ID) + fmt.Fprintf(w, " intent: %s\n", res.Intent.Path) + fmt.Fprintf(w, " spec: %s\n", res.Spec.Path) + }) + }, + }) + + // link — retroactively set spec_id on a planned intent. + intentCmd.AddCommand(&cobra.Command{ + Use: "link ", + Short: "Link a planned intent to an existing spec (writes the intent's spec_id)", + Args: cobra.ExactArgs(2), + RunE: func(cmd *cobra.Command, args []string) error { + cwd, err := os.Getwd() + if err != nil { + return err + } + res, err := intent.Link(cwd, args[0], args[1]) + if err != nil { + return &exitError{Code: 2, Msg: "abcd intent link: " + err.Error()} + } + return render(cmd.OutOrStdout(), *asJSON, res, func(w io.Writer) { + fmt.Fprintf(w, "abcd intent link — %s -> %s\n intent: %s\n", res.Intent.ID, res.Spec.ID, res.Intent.Path) + }) + }, + }) + + return intentCmd +} + +// specStatusView is the machine-readable envelope for bare `abcd spec`: the +// open/closed counts and every discovered spec record. +type specStatusView struct { + Open int `json:"open"` + Closed int `json:"closed"` + Specs []spec.Spec `json:"specs"` +} + +// newSpecCommand builds the `spec` verb — the front door onto internal/core/spec +// (itd-80). Bare `abcd spec` renders the read-only spec-store status; the `close` +// sub-verb moves a spec open/ -> closed/. The lifecycle reconcile that trails a +// close (moving the linked intent planned -> shipped) lands in the next phase; +// `close` here touches the spec store only. +func newSpecCommand(asJSON *bool) *cobra.Command { + specCmd := &cobra.Command{ + Use: "spec", + Short: "Native spec store; bare invocation is read-only status", + Args: cobra.NoArgs, + RunE: func(cmd *cobra.Command, _ []string) error { + cwd, err := os.Getwd() + if err != nil { + return err + } + store, err := spec.Load(cwd) + if err != nil { + return err + } + view := specStatusView{Specs: store.Specs} + for _, sp := range store.Specs { + if sp.Status == spec.StatusClosed { + view.Closed++ + } else { + view.Open++ + } + } + return render(cmd.OutOrStdout(), *asJSON, view, func(w io.Writer) { + fmt.Fprintf(w, "abcd spec — open %d · closed %d\n", view.Open, view.Closed) + for _, sp := range store.Specs { + fmt.Fprintf(w, " %s %s %s (%s)\n", sp.ID, sp.Status, sp.Slug, sp.Intent) + } + }) + }, + } + + // close — open/ -> closed/. No lifecycle reconcile yet (next phase). + specCmd.AddCommand(&cobra.Command{ + Use: "close ", + Short: "Close a spec (open/ -> closed/); lifecycle reconcile lands in the next phase", + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + cwd, err := os.Getwd() + if err != nil { + return err + } + sp, err := spec.Close(cwd, args[0]) + if err != nil { + return &exitError{Code: 2, Msg: "abcd spec close: " + err.Error()} + } + return render(cmd.OutOrStdout(), *asJSON, sp, func(w io.Writer) { + fmt.Fprintf(w, "abcd spec close — %s open -> closed\n %s\n", sp.ID, sp.Path) + }) + }, + }) + + return specCmd +} + // newAhoyCommand builds the `ahoy` sub-tree. Bare `ahoy` runs the read-only // detection pass (abcd's convention: bare invocation never mutates); the // install/uninstall/doctor/dry-run sub-verbs are thin consumers of the same diff --git a/internal/surface/cli/intent_cli_test.go b/internal/surface/cli/intent_cli_test.go new file mode 100644 index 00000000..507e99e0 --- /dev/null +++ b/internal/surface/cli/intent_cli_test.go @@ -0,0 +1,188 @@ +package cli + +import ( + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" +) + +// writeRepoFile writes content to root/rel, creating parent directories. +func writeRepoFile(t *testing.T, root, rel, content string) { + t.Helper() + abs := filepath.Join(root, rel) + if err := os.MkdirAll(filepath.Dir(abs), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(abs, []byte(content), 0o644); err != nil { + t.Fatal(err) + } +} + +const ( + cliDrafts = ".abcd/development/intents/drafts" + cliPlanned = ".abcd/development/intents/planned" + cliSpecsOpen = ".abcd/development/specs/open" +) + +func cliDraftWithAC(id, slug string) string { + return "---\nid: " + id + "\nslug: " + slug + "\nspec_id: null\nkind: null\n---\n# " + slug + + "\n\n## Acceptance Criteria\n\n- **Given** x, **when** y, **then** z.\n" +} + +func TestIntentBareText(t *testing.T) { + repo := t.TempDir() + t.Chdir(repo) + writeRepoFile(t, repo, cliDrafts+"/itd-10-alpha.md", cliDraftWithAC("itd-10", "alpha")) + writeRepoFile(t, repo, cliPlanned+"/itd-2-beta.md", + "---\nid: itd-2\nslug: beta\nspec_id: spc-1\nkind: standalone\n---\n# beta\n") + + out := string(runCLI(t, "intent")) + if !strings.Contains(out, "drafts 1") || !strings.Contains(out, "planned 1") { + t.Fatalf("bare intent status missing counts:\n%s", out) + } +} + +func TestIntentBareJSON(t *testing.T) { + repo := t.TempDir() + t.Chdir(repo) + writeRepoFile(t, repo, cliDrafts+"/itd-10-alpha.md", cliDraftWithAC("itd-10", "alpha")) + + out := runCLI(t, "intent", "--json") + var got struct { + Buckets map[string]int `json:"buckets"` + } + if err := json.Unmarshal(out, &got); err != nil { + t.Fatalf("intent --json not JSON: %v\n%s", err, out) + } + if got.Buckets["drafts"] != 1 { + t.Fatalf("intent --json drafts = %d, want 1\n%s", got.Buckets["drafts"], out) + } +} + +func TestIntentPlanHappy(t *testing.T) { + repo := t.TempDir() + t.Chdir(repo) + writeRepoFile(t, repo, cliDrafts+"/itd-10-alpha.md", cliDraftWithAC("itd-10", "alpha")) + + out := runCLI(t, "intent", "plan", "itd-10", "--json") + var got struct { + Intent struct { + Bucket string `json:"bucket"` + SpecID string `json:"spec_id"` + } `json:"intent"` + Spec struct { + ID string `json:"id"` + } `json:"spec"` + } + if err := json.Unmarshal(out, &got); err != nil { + t.Fatalf("plan --json not JSON: %v\n%s", err, out) + } + if got.Intent.Bucket != "planned" || got.Spec.ID != "spc-1" || got.Intent.SpecID != "spc-1" { + t.Fatalf("plan result = %+v", got) + } + // The draft moved and the spec landed on disk. + if _, err := os.Stat(filepath.Join(repo, cliPlanned, "itd-10-alpha.md")); err != nil { + t.Fatalf("planned file missing: %v", err) + } + if _, err := os.Stat(filepath.Join(repo, cliSpecsOpen, "spc-1-alpha.md")); err != nil { + t.Fatalf("spec file missing: %v", err) + } +} + +func TestIntentPlanRefusesNoAC(t *testing.T) { + repo := t.TempDir() + t.Chdir(repo) + writeRepoFile(t, repo, cliDrafts+"/itd-10-alpha.md", + "---\nid: itd-10\nslug: alpha\nspec_id: null\nkind: null\n---\n# alpha\n\nno criteria\n") + if _, err := runCLIErr(t, "intent", "plan", "itd-10"); err == nil { + t.Fatal("plan without Acceptance Criteria must exit non-zero") + } +} + +func TestIntentPlanRefusesNonDraft(t *testing.T) { + repo := t.TempDir() + t.Chdir(repo) + writeRepoFile(t, repo, cliPlanned+"/itd-10-alpha.md", + "---\nid: itd-10\nslug: alpha\nspec_id: null\nkind: standalone\n---\n# alpha\n\n## Acceptance Criteria\n\n- ok\n") + if _, err := runCLIErr(t, "intent", "plan", "itd-10"); err == nil { + t.Fatal("plan on a non-draft intent must exit non-zero") + } +} + +func TestIntentLinkHappy(t *testing.T) { + repo := t.TempDir() + t.Chdir(repo) + writeRepoFile(t, repo, cliPlanned+"/itd-10-alpha.md", + "---\nid: itd-10\nslug: alpha\nspec_id: null\nkind: standalone\n---\n# alpha\n") + writeRepoFile(t, repo, cliSpecsOpen+"/spc-3-alpha.md", + "---\nid: spc-3\nslug: alpha\nintent: itd-10\n---\n# alpha\n") + + out := runCLI(t, "intent", "link", "itd-10", "spc-3", "--json") + var got struct { + Intent struct { + SpecID string `json:"spec_id"` + } `json:"intent"` + } + if err := json.Unmarshal(out, &got); err != nil { + t.Fatalf("link --json not JSON: %v\n%s", err, out) + } + if got.Intent.SpecID != "spc-3" { + t.Fatalf("link spec_id = %q, want spc-3", got.Intent.SpecID) + } +} + +func TestIntentLinkMismatchErrors(t *testing.T) { + repo := t.TempDir() + t.Chdir(repo) + writeRepoFile(t, repo, cliPlanned+"/itd-10-alpha.md", + "---\nid: itd-10\nslug: alpha\nspec_id: null\nkind: standalone\n---\n# alpha\n") + writeRepoFile(t, repo, cliSpecsOpen+"/spc-3-other.md", + "---\nid: spc-3\nslug: other\nintent: itd-99\n---\n# other\n") + if _, err := runCLIErr(t, "intent", "link", "itd-10", "spc-3"); err == nil { + t.Fatal("link with a spec realising a different intent must exit non-zero") + } +} + +func TestSpecBareText(t *testing.T) { + repo := t.TempDir() + t.Chdir(repo) + writeRepoFile(t, repo, cliSpecsOpen+"/spc-1-alpha.md", + "---\nid: spc-1\nslug: alpha\nintent: itd-10\n---\n# alpha\n") + + out := string(runCLI(t, "spec")) + if !strings.Contains(out, "open 1") || !strings.Contains(out, "spc-1") { + t.Fatalf("bare spec status missing spec:\n%s", out) + } +} + +func TestSpecCloseHappy(t *testing.T) { + repo := t.TempDir() + t.Chdir(repo) + writeRepoFile(t, repo, cliSpecsOpen+"/spc-1-alpha.md", + "---\nid: spc-1\nslug: alpha\nintent: itd-10\n---\n# alpha\n") + + out := runCLI(t, "spec", "close", "spc-1", "--json") + var got struct { + Status string `json:"status"` + Path string `json:"path"` + } + if err := json.Unmarshal(out, &got); err != nil { + t.Fatalf("spec close --json not JSON: %v\n%s", err, out) + } + if got.Status != "closed" { + t.Fatalf("spec close status = %q, want closed", got.Status) + } + if _, err := os.Stat(filepath.Join(repo, ".abcd/development/specs/closed", "spc-1-alpha.md")); err != nil { + t.Fatalf("closed spec file missing: %v", err) + } +} + +func TestSpecCloseMissingErrors(t *testing.T) { + repo := t.TempDir() + t.Chdir(repo) + if _, err := runCLIErr(t, "spec", "close", "spc-99"); err == nil { + t.Fatal("closing a missing spec must exit non-zero") + } +} From 9ffb6059fade1db4827edd5219830185065fe8e3 Mon Sep 17 00:00:00 2001 From: REPPL <77722411+REPPL@users.noreply.github.com> Date: Sat, 11 Jul 2026 17:48:00 +0100 Subject: [PATCH 5/5] =?UTF-8?q?fix:=20itd-80=20phase=202=20review=20harden?= =?UTF-8?q?ing=20=E2=80=94=20retry-safe=20Plan,=20atomic-write,=20clobber?= =?UTF-8?q?=20guards?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit From a ruthless-review pass (verdict SHIP, no blockers): - Plan is now retry-safe: refuses a draft that already carries a spec_id, and reuses an existing spec for the intent (via store.ByIntent) instead of minting a duplicate when a prior rename failed mid-flight. - spec.Create routes through fsutil.WriteFileAtomic (regains the fsync + parent-dir sync) and the private atomicWrite third copy is deleted (one-canonical-primitive); spec markdown mode aligned to 0o644. - maxIntentSpecNum gains the symlink-dir refusal the other readers enforce. - Plan and spec.Close refuse to overwrite an existing rename destination (fail-closed; no silent data loss). Part of itd-80. Assisted-by: Claude:claude-opus-4-8 --- internal/core/intent/intent_test.go | 73 +++++++++++++++++++++++++++++ internal/core/intent/lifecycle.go | 22 ++++++++- internal/core/spec/store.go | 44 +++++++---------- internal/core/spec/store_test.go | 27 +++++++++++ 4 files changed, 136 insertions(+), 30 deletions(-) diff --git a/internal/core/intent/intent_test.go b/internal/core/intent/intent_test.go index bee897ad..474ef5f7 100644 --- a/internal/core/intent/intent_test.go +++ b/internal/core/intent/intent_test.go @@ -8,6 +8,7 @@ import ( "github.com/REPPL/abcd-cli/internal/core/frontmatter" "github.com/REPPL/abcd-cli/internal/core/lint" + "github.com/REPPL/abcd-cli/internal/core/spec" ) const ( @@ -187,6 +188,78 @@ func TestPlanRefusesNonDraft(t *testing.T) { } } +// TestPlanReusesExistingSpecForIntent proves Plan is retry-safe: when a spec +// already realises the intent (e.g. a prior Plan minted it but the +// drafts->planned rename failed, leaving the intent a null-spec_id draft), +// re-running Plan reuses that spec instead of minting a duplicate. +func TestPlanReusesExistingSpecForIntent(t *testing.T) { + root := t.TempDir() + writeFile(t, root, draftsDir+"/itd-10-alpha.md", draftWithAC("itd-10", "alpha")) + // Pre-create the spec for this intent; the draft is still an unlinked draft. + sp, err := spec.Create(root, "itd-10", "alpha") + if err != nil { + t.Fatal(err) + } + + res, err := Plan(root, "itd-10") + if err != nil { + t.Fatal(err) + } + if res.Spec.ID != sp.ID { + t.Fatalf("Plan minted a new spec %q, want reuse of %q", res.Spec.ID, sp.ID) + } + if res.Intent.Bucket != "planned" || res.Intent.SpecID != sp.ID { + t.Fatalf("Plan intent = %+v", res.Intent) + } + + // Exactly one spec realises the intent (no duplicate minted). + store, err := spec.Load(root) + if err != nil { + t.Fatal(err) + } + n := 0 + for _, s := range store.Specs { + if s.Intent == "itd-10" { + n++ + } + } + if n != 1 { + t.Fatalf("expected exactly 1 spec for itd-10, got %d: %+v", n, store.Specs) + } +} + +// TestPlanRefusesDraftWithSpecID rejects a draft whose frontmatter already +// carries a non-null spec_id (half-planned / lint-invalid): Plan must not mint +// another spec for it. +func TestPlanRefusesDraftWithSpecID(t *testing.T) { + root := t.TempDir() + writeFile(t, root, draftsDir+"/itd-10-alpha.md", + "---\nid: itd-10\nslug: alpha\nspec_id: spc-1\nkind: standalone\n---\n# alpha\n\n## Acceptance Criteria\n\n- ok\n") + if _, err := Plan(root, "itd-10"); err == nil { + t.Fatal("Plan must refuse a draft that already has a non-null spec_id") + } +} + +// TestPlanRefusesWhenPlannedTargetExists proves Plan fails closed rather than +// clobbering a same-name file already sitting in planned/. +func TestPlanRefusesWhenPlannedTargetExists(t *testing.T) { + root := t.TempDir() + writeFile(t, root, draftsDir+"/itd-10-alpha.md", draftWithAC("itd-10", "alpha")) + writeFile(t, root, plannedDir+"/itd-10-alpha.md", + "---\nid: itd-10\nslug: alpha\nspec_id: null\nkind: standalone\n---\n# pre-existing\n") + if _, err := Plan(root, "itd-10"); err == nil { + t.Fatal("Plan must refuse to overwrite an existing planned target") + } + // The pre-existing planned file is untouched. + body, err := os.ReadFile(filepath.Join(root, plannedDir, "itd-10-alpha.md")) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(body), "pre-existing") { + t.Fatalf("planned target was clobbered:\n%s", body) + } +} + func TestPlanRejectsBadID(t *testing.T) { root := t.TempDir() if _, err := Plan(root, "itd-../../etc"); err == nil { diff --git a/internal/core/intent/lifecycle.go b/internal/core/intent/lifecycle.go index bd552973..71e60d85 100644 --- a/internal/core/intent/lifecycle.go +++ b/internal/core/intent/lifecycle.go @@ -123,12 +123,27 @@ func Plan(repoRoot, intentID string) (PlanResult, error) { if !hasAcceptanceCriteria(content) { return PlanResult{}, fmt.Errorf("intent: %s has no non-empty '## Acceptance Criteria' section (itd-1 discipline); refusing to plan", intentID) } + // A draft that already carries a non-null spec_id is half-planned (and + // lint-invalid): refuse rather than mint a second spec for it. + if !frontmatter.IsNull(it.SpecID) { + return PlanResult{}, fmt.Errorf("intent: %s is a draft with spec_id %q already set (half-planned); refusing to plan", intentID, it.SpecID) + } - // 1. Mint + create the native spec (writes the reciprocal intent: itd-N). - sp, err := spec.Create(repoRoot, intentID, it.Slug) + // 1. Reuse the spec already realising this intent, or mint one. Reusing makes + // Plan retry-safe: a re-run after a failed drafts->planned rename completes the + // operation instead of duplicating the spec. Both branches write the reciprocal + // intent: itd-N side (Create writes it; a reused spec already carries it). + store, err := spec.Load(repoRoot) if err != nil { return PlanResult{}, err } + sp, ok := store.ByIntent(intentID) + if !ok { + sp, err = spec.Create(repoRoot, intentID, it.Slug) + if err != nil { + return PlanResult{}, err + } + } // 2. Set the binding kind (default standalone) while still in drafts. A draft // with (kind=standalone, spec_id=null) stays lint-valid, so a failure here @@ -154,6 +169,9 @@ func Plan(repoRoot, intentID string) (PlanResult, error) { if err := ensureRealDir(filepath.Join(repoRoot, IntentsRelDir, BucketPlanned), filepath.Join(IntentsRelDir, BucketPlanned)); err != nil { return PlanResult{}, err } + if _, err := os.Lstat(plannedAbs); err == nil { + return PlanResult{}, fmt.Errorf("intent: refusing to overwrite existing %s", plannedRel) + } if err := os.Rename(draftAbs, plannedAbs); err != nil { return PlanResult{}, fmt.Errorf("intent: moving %s drafts->planned: %w", intentID, err) } diff --git a/internal/core/spec/store.go b/internal/core/spec/store.go index ab609eb4..c40ae669 100644 --- a/internal/core/spec/store.go +++ b/internal/core/spec/store.go @@ -7,6 +7,7 @@ import ( "strings" "github.com/REPPL/abcd-cli/internal/core/frontmatter" + "github.com/REPPL/abcd-cli/internal/fsutil" ) // Load discovers spec files under both buckets, parses their frontmatter, and @@ -114,10 +115,17 @@ func maxIntentSpecNum(repoRoot string) (int, error) { max := 0 for _, bucket := range intentBuckets { dir := filepath.Join(repoRoot, IntentsRelDir, bucket) - entries, err := os.ReadDir(dir) + di, err := os.Lstat(dir) if os.IsNotExist(err) { continue } + if err != nil { + return 0, fmt.Errorf("spec: stat %s: %w", filepath.Join(IntentsRelDir, bucket), err) + } + if di.Mode()&os.ModeSymlink != 0 { + return 0, fmt.Errorf("spec: %s is a symlink (refusing to follow)", filepath.Join(IntentsRelDir, bucket)) + } + entries, err := os.ReadDir(dir) if err != nil { return 0, fmt.Errorf("spec: reading %s: %w", filepath.Join(IntentsRelDir, bucket), err) } @@ -162,8 +170,9 @@ func Create(repoRoot, intentID, slug string) (Spec, error) { return Spec{}, err } name := fmt.Sprintf("%s-%s.md", id, slug) - if err := atomicWrite(filepath.Join(openDir, name), renderSpec(id, slug, intentID)); err != nil { - return Spec{}, err + // 0o644 matches the intent-side markdown writer — both write committed design-record files. + if err := fsutil.WriteFileAtomic(filepath.Join(openDir, name), []byte(renderSpec(id, slug, intentID)), 0o644); err != nil { + return Spec{}, fmt.Errorf("spec: writing %s: %w", filepath.Join(SpecsRelDir, StatusOpen, name), err) } sp := Spec{ ID: id, @@ -199,6 +208,10 @@ func Close(repoRoot, specID string) (Spec, error) { if err := ensureDir(closedDir, filepath.Join(SpecsRelDir, StatusClosed)); err != nil { return Spec{}, err } + dstRel := filepath.Join(SpecsRelDir, StatusClosed, name) + if _, err := os.Lstat(filepath.Join(closedDir, name)); err == nil { + return Spec{}, fmt.Errorf("spec: refusing to overwrite existing %s", dstRel) + } if err := os.Rename(filepath.Join(repoRoot, sp.Path), filepath.Join(closedDir, name)); err != nil { return Spec{}, fmt.Errorf("spec: closing %s: %w", specID, err) } @@ -240,28 +253,3 @@ func ensureDir(dir, rel string) error { } return nil } - -// atomicWrite writes content to a temp file in the destination directory, then -// renames it into place so a reader never sees a partial file. -func atomicWrite(path, content string) error { - tmp, err := os.CreateTemp(filepath.Dir(path), ".spec-*.tmp") - if err != nil { - return fmt.Errorf("spec: creating temp file: %w", err) - } - tmpName := tmp.Name() - _, werr := tmp.WriteString(content) - cerr := tmp.Close() - if werr != nil { - _ = os.Remove(tmpName) - return fmt.Errorf("spec: writing temp file: %w", werr) - } - if cerr != nil { - _ = os.Remove(tmpName) - return fmt.Errorf("spec: closing temp file: %w", cerr) - } - if err := os.Rename(tmpName, path); err != nil { - _ = os.Remove(tmpName) - return fmt.Errorf("spec: finalising %s: %w", path, err) - } - return nil -} diff --git a/internal/core/spec/store_test.go b/internal/core/spec/store_test.go index eaf5c39d..fd1679d1 100644 --- a/internal/core/spec/store_test.go +++ b/internal/core/spec/store_test.go @@ -170,6 +170,33 @@ func TestCloseMovesOpenToClosed(t *testing.T) { } } +// TestCloseRefusesWhenClosedTargetExists proves Close fails closed rather than +// clobbering a same-name spec already sitting in closed/. +func TestCloseRefusesWhenClosedTargetExists(t *testing.T) { + root := t.TempDir() + if _, err := Create(root, "itd-9", "my-feature"); err != nil { + t.Fatal(err) + } + // A same-name spec already occupies closed/. + writeFile(t, root, specsClosed+"/spc-1-my-feature.md", + "---\nid: spc-1\nslug: my-feature\nintent: itd-9\n---\n# pre-existing\n") + + if _, err := Close(root, "spc-1"); err == nil { + t.Fatal("Close must refuse to overwrite an existing closed target") + } + // The open file is untouched (still there), the closed one not clobbered. + if _, err := os.Stat(filepath.Join(root, specsOpen, "spc-1-my-feature.md")); err != nil { + t.Fatalf("open file must remain after refusal: %v", err) + } + body, err := os.ReadFile(filepath.Join(root, specsClosed, "spc-1-my-feature.md")) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(body), "pre-existing") { + t.Fatalf("closed target was clobbered:\n%s", body) + } +} + func TestCloseMissingFails(t *testing.T) { root := t.TempDir() if _, err := Close(root, "spc-99"); err == nil {