From 38ced153630856d4da11757a691d471895d5b099 Mon Sep 17 00:00:00 2001 From: REPPL <77722411+REPPL@users.noreply.github.com> Date: Sat, 25 Jul 2026 14:18:44 +0100 Subject: [PATCH 1/3] fix: let capture resolve and intent seed stamp a valid impact MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The issue_impact_valid and intent_impact_valid record-lint blockers require a valid impact on records in the release set (resolved/ issues, shipped/ intents), but the verbs that mint those records had no way to set one. The tool's own path therefore produced records its own gates reject: `capture resolve` moved an issue to resolved/ with no impact, and `intent ""` seeded a draft that carried no impact through planning to shipped/. capture.Resolve now takes a required Impact, validated against the shared changelog enum (no default — empty or invalid is refused, never guessed) and written bare (impact: fix), never YAML-quoted, so the frontmatter line-scanner and the enum agree. intent.CreateFromText takes an optional impact, validated the same way and rejected as internal (an intent is user-facing by definition), stamped onto the draft so it survives the impact-preserving move to shipped/. The --impact flag on `capture resolve` is enforced in the core rather than via cobra MarkFlagRequired, keeping the CLI tree's no-required-flags invariant (TestLiveTreeMarksNoFlagRequired). capture wontfix is untouched: a non-action ships nothing, so wontfix/ carries no impact. Resolves ledger iss-117. Assisted-by: Claude:claude-opus-4-8 --- .abcd/development/release/surface.json | 20 +++- CHANGELOG.md | 15 +++ commands/abcd/capture.md | 11 +- commands/abcd/intent.md | 8 +- docs/reference/cli/commands.md | 16 ++- internal/core/capture/capture.go | 5 + internal/core/capture/impact_resolve_test.go | 110 ++++++++++++++++++ internal/core/capture/serialize.go | 20 ++++ internal/core/capture/workflow.go | 32 ++++- internal/core/capture/workflow_test.go | 12 +- internal/core/intent/create.go | 28 ++++- internal/core/intent/create_test.go | 8 +- internal/core/intent/impact_test.go | 96 +++++++++++++++ internal/surface/cli/cli.go | 33 ++++-- .../cli/error_pathleak_surface_test.go | 2 +- 15 files changed, 380 insertions(+), 36 deletions(-) create mode 100644 internal/core/capture/impact_resolve_test.go create mode 100644 internal/core/intent/impact_test.go diff --git a/.abcd/development/release/surface.json b/.abcd/development/release/surface.json index bef0d09..082c825 100644 --- a/.abcd/development/release/surface.json +++ b/.abcd/development/release/surface.json @@ -199,7 +199,15 @@ { "path": "abcd capture resolve", "hidden": false, - "flags": [] + "flags": [ + { + "name": "impact", + "shorthand": "", + "type": "string", + "required": false, + "hidden": false + } + ] }, { "path": "abcd capture wontfix", @@ -391,7 +399,15 @@ { "path": "abcd intent", "hidden": false, - "flags": [] + "flags": [ + { + "name": "impact", + "shorthand": "", + "type": "string", + "required": false, + "hidden": false + } + ] }, { "path": "abcd intent link", diff --git a/CHANGELOG.md b/CHANGELOG.md index 866231e..d768733 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,21 @@ called out in a **Breaking** section. ## [Unreleased] +### Added + +- **`abcd capture resolve` and `abcd intent ""` can now stamp a product + `impact`** (iss-117). A resolved issue and a shipped intent are in the release + set, so the `issue_impact_valid` and `intent_impact_valid` record-lint blockers + require a valid `impact` on those records — but the verbs that mint them had no + way to set one, so the tool's own path produced records its own gates rejected. + `capture resolve` now takes a mandatory `--impact ` + (there is no default: an absent or misspelled value is refused, not guessed), + and `abcd intent ""` takes an optional `--impact ` + that is stamped onto the seeded draft and travels unchanged through planning to + `shipped/`. `internal` is rejected on an intent (a press-release-first intent is + user-facing by definition). `capture wontfix` is unchanged — a non-action ships + nothing, so `wontfix/` carries no impact. + ## [0.4.0] - 2026-07-22 ### Breaking diff --git a/commands/abcd/capture.md b/commands/abcd/capture.md index 7949db9..7497d3e 100644 --- a/commands/abcd/capture.md +++ b/commands/abcd/capture.md @@ -1,7 +1,7 @@ --- name: capture description: Capture issues to the structured per-repo ledger and query them, by invoking the abcd binary. Bare invocation is a read-only status render; list/resolve/wontfix act on the ledger. -argument-hint: "[text] | list --open|--resolved|--wontfix|--all | resolve | wontfix " +argument-hint: "[text] | list --open|--resolved|--wontfix|--all | resolve --impact | wontfix " --- # `/abcd:capture` — issue ledger @@ -64,13 +64,20 @@ blocked by an open dependency are demoted and annotated `[blocked-by iss-N,…]` ## Resolve / wontfix ```bash -abcd capture resolve "" --json +abcd capture resolve "" --impact --json abcd capture wontfix "" --json ``` Each moves the issue out of `open/` and records the note; report the `id` and the `from_status -> to_status` transition from the JSON. +`resolve` requires `--impact`: a resolved issue is in the release set, so it +carries the product judgement the version derivation reads (`additive`, +`breaking`, `fix`, or `internal` — plumbing invisible to users). There is no +default; an absent or misspelled impact is refused rather than guessed, so the +record always satisfies the `issue_impact_valid` gate. `wontfix` takes no impact +(a non-action ships nothing). + Promoting an issue to an intent (`/abcd:capture promote `) is skill-orchestrated, not a binary sub-verb. It hands the issue body to the intent create path — `abcd intent ""` — which files a new draft under diff --git a/commands/abcd/intent.md b/commands/abcd/intent.md index 2288f8d..a24981d 100644 --- a/commands/abcd/intent.md +++ b/commands/abcd/intent.md @@ -27,7 +27,7 @@ and the intent↔spec links. Nothing is created or moved by this invocation. ## Create a draft ```bash -abcd intent "" --json +abcd intent "" [--impact ] --json ``` Files `drafts/itd-N-.md` seeded from the text. Report the new `id` and @@ -35,6 +35,12 @@ Files `drafts/itd-N-.md` seeded from the text. Report the new `id` and placeholder that must be replaced with real Given-When-Then bullets — via the planning interview below — before the draft can be planned. +`--impact` is optional: a draft is "not judged yet", so an unset impact writes +no field. When you do set it, the value is validated (one of `additive`, +`breaking`, `fix` — never `internal`, since an intent is user-facing by +definition) and stamped onto the draft, where it travels unchanged through +planning to `shipped/`, which the `intent_impact_valid` gate requires. + ## THE RULE: no implementation without a planned, specced intent Before implementing ANY `itd-N` — or whenever the user asks you to "build", diff --git a/docs/reference/cli/commands.md b/docs/reference/cli/commands.md index aec15f8..9473f9c 100644 --- a/docs/reference/cli/commands.md +++ b/docs/reference/cli/commands.md @@ -118,7 +118,13 @@ List issues by state (one of --open/--resolved/--wontfix/--all required) Mark an open issue resolved (open/ -> resolved/) -**Usage:** `abcd capture resolve ` +**Usage:** `abcd capture resolve --impact [flags]` + +**Flags:** + +``` + --impact string product impact: additive|breaking|fix|internal (required) +``` #### `abcd capture wontfix` @@ -282,7 +288,13 @@ Show one stored transcript's metadata and redacted body Intent lifecycle; bare invocation is read-only status, quoted text files a draft -**Usage:** `abcd intent [text]` +**Usage:** `abcd intent [text] [flags]` + +**Flags:** + +``` + --impact string stamp the draft's product impact: additive|breaking|fix (optional) +``` #### `abcd intent link` diff --git a/internal/core/capture/capture.go b/internal/core/capture/capture.go index 5f576fe..f4698dc 100644 --- a/internal/core/capture/capture.go +++ b/internal/core/capture/capture.go @@ -133,6 +133,11 @@ type ResolveRequest struct { IssuesRoot string ID string Resolution string + // Impact is the product judgement resolved/ requires (issue_impact_valid): + // one of the shared changelog enum's values (additive|breaking|fix|internal). + // There is no default — an empty or invalid value is refused, never invented, + // so a resolved record the tool mints always satisfies its own blocker. + Impact string } // WontfixRequest moves an open issue to wontfix/. diff --git a/internal/core/capture/impact_resolve_test.go b/internal/core/capture/impact_resolve_test.go new file mode 100644 index 0000000..3b924f9 --- /dev/null +++ b/internal/core/capture/impact_resolve_test.go @@ -0,0 +1,110 @@ +package capture + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/REPPL/abcd-cli/internal/core/lint" +) + +// lintIssueImpact runs only the issue_impact_valid record-lint blocker over the +// ledger under repoRoot and returns its findings. It is the real gate an issue +// the tool resolves must satisfy, so a resolve path that cannot stamp a valid +// impact is caught here rather than at a later hand-lint. +func lintIssueImpact(t *testing.T, repoRoot string) []lint.Finding { + t.Helper() + cfg := lint.Config{ + Rules: map[string]lint.RuleConfig{ + "issue_impact_valid": {Enabled: true, Severity: "blocker", IssuesDir: LedgerRelPath}, + }, + } + fs, err := lint.Lint(cfg, repoRoot) + if err != nil { + t.Fatalf("lint: %v", err) + } + return fs +} + +// TestResolveProducesImpactValidRecord is the iss-117 reproduction: a resolved +// issue lands in resolved/, where issue_impact_valid REQUIRES a valid, non-null +// impact. The tool's own resolve path must therefore be able to stamp one, or the +// record it produces is rejected by the very blocker the tool ships. +func TestResolveProducesImpactValidRecord(t *testing.T) { + repo, ir := ledger(t) + res, err := Capture(CaptureRequest{ + RepoRoot: repo, IssuesRoot: ir, Text: "b", Severity: SeverityMinor, + Category: "bug", Source: "user-observation", FoundDuring: "t", Slug: "note", + }) + if err != nil { + t.Fatal(err) + } + + if _, err := Resolve(ResolveRequest{ + RepoRoot: repo, IssuesRoot: ir, ID: res.ID, Resolution: "fixed the thing", Impact: "fix", + }); err != nil { + t.Fatalf("Resolve: %v", err) + } + + if fs := lintIssueImpact(t, repo); len(fs) != 0 { + t.Fatalf("resolved record must satisfy issue_impact_valid, got %d finding(s): %+v", len(fs), fs) + } +} + +// TestResolveRefusesInvalidImpact proves the resolve boundary fails closed on a +// missing or misspelled impact rather than minting a record the blocker rejects: +// there is no default, so an absent judgement is refused, not invented. +func TestResolveRefusesInvalidImpact(t *testing.T) { + repo, ir := ledger(t) + res, err := Capture(CaptureRequest{ + RepoRoot: repo, IssuesRoot: ir, Text: "b", Severity: SeverityMinor, + Category: "bug", Source: "user-observation", FoundDuring: "t", Slug: "note", + }) + if err != nil { + t.Fatal(err) + } + + for _, bad := range []string{"", "additiv", "Additive", `"fix"`} { + if _, err := Resolve(ResolveRequest{ + RepoRoot: repo, IssuesRoot: ir, ID: res.ID, Resolution: "note", Impact: bad, + }); err == nil { + t.Fatalf("Resolve with impact %q must be refused", bad) + } + } + // The refused transitions must not have moved the issue out of open/. + if _, status, ferr := findIssue(ir, res.ID); ferr != nil || status != StateOpen { + t.Fatalf("a refused resolve must leave the issue in open/: status=%s err=%v", status, ferr) + } +} + +// TestResolveImpactUnquoted guards the wire format: impact is a machine-read enum +// the frontmatter line-scanner compares byte-for-byte, so it must be written bare +// (impact: fix), never YAML-quoted (impact: "fix") the way a prose note is. +func TestResolveImpactUnquoted(t *testing.T) { + repo, ir := ledger(t) + res, err := Capture(CaptureRequest{ + RepoRoot: repo, IssuesRoot: ir, Text: "b", Severity: SeverityMinor, + Category: "bug", Source: "user-observation", FoundDuring: "t", Slug: "note", + }) + if err != nil { + t.Fatal(err) + } + tr, err := Resolve(ResolveRequest{ + RepoRoot: repo, IssuesRoot: ir, ID: res.ID, Resolution: "done", Impact: "internal", + }) + if err != nil { + t.Fatal(err) + } + raw, err := os.ReadFile(filepath.Join(repo, tr.Path)) + if err != nil { + t.Fatal(err) + } + data := string(raw) + if !strings.Contains(data, "\nimpact: internal\n") { + t.Fatalf("impact must be written bare (impact: internal), got:\n%s", data) + } + if strings.Contains(data, `impact: "internal"`) { + t.Fatalf("impact must not be YAML-quoted:\n%s", data) + } +} diff --git a/internal/core/capture/serialize.go b/internal/core/capture/serialize.go index 3f7262e..d42b8f4 100644 --- a/internal/core/capture/serialize.go +++ b/internal/core/capture/serialize.go @@ -12,6 +12,15 @@ type kv struct { val any } +// rawScalar is a frontmatter value written verbatim — no surrounding quotes — +// for a constrained, machine-read enum like `impact`. The frontmatter scanner is +// a line reader that compares the value byte-for-byte, so a quoted `"fix"` reads +// as a different string than the enum member `fix`; the enum fields must be bare. +// Only values already validated against their enum reach here, and yamlScalar +// still rejects any control char, so the verbatim path cannot inject a newline or +// a second frontmatter key. +type rawScalar string + // yamlScalar encodes a scalar value as a safe YAML literal, mirroring // _issue_lib._yaml_scalar. Strings are double-quoted with backslash/dquote // escaping and reject any ASCII control char (< 0x20); ints render bare. @@ -34,6 +43,17 @@ func yamlScalar(value any) (string, error) { esc := strings.ReplaceAll(v, `\`, `\\`) esc = strings.ReplaceAll(esc, `"`, `\"`) return `"` + esc + `"`, nil + case rawScalar: + // Verbatim, but never a value that could break the frontmatter shape: reject + // any control char (a newline would inject a second key) and any character + // outside a bare enum token. The caller has already validated the value + // against its enum; this is defence in depth at the serialise boundary. + for _, r := range v { + if r < 0x20 || !(r == '-' || r == '_' || (r >= 'a' && r <= 'z') || (r >= 'A' && r <= 'Z') || (r >= '0' && r <= '9')) { + return "", fmt.Errorf("%w: unsafe raw scalar %q", ErrMalformedFrontmatter, string(v)) + } + } + return string(v), nil default: return "", fmt.Errorf("%w: unsupported scalar type %T", ErrMalformedFrontmatter, value) } diff --git a/internal/core/capture/workflow.go b/internal/core/capture/workflow.go index 4fc2270..0a8f814 100644 --- a/internal/core/capture/workflow.go +++ b/internal/core/capture/workflow.go @@ -4,11 +4,12 @@ import ( "fmt" "os" "path/filepath" - - "github.com/REPPL/abcd-cli/internal/fsutil" "sort" "strconv" "strings" + + "github.com/REPPL/abcd-cli/internal/core/changelog" + "github.com/REPPL/abcd-cli/internal/fsutil" ) // mutationPreamble runs the idempotent pre-mutation steps: sweep orphan @@ -120,17 +121,30 @@ func commitCapture(req CaptureRequest, issID, slug, placeholder string) (Capture return CaptureResult{ID: issID, Slug: slug, Path: placeholder, Status: StateOpen}, nil } -// Resolve moves an open issue to resolved/, writing the resolution note. +// Resolve moves an open issue to resolved/, writing the resolution note and the +// product impact. resolved/ is gated by issue_impact_valid, so the impact is +// validated against the shared changelog enum up front (empty or invalid is +// refused, never defaulted) and stamped bare alongside the note — the tool's own +// resolve path can never mint a record its own blocker rejects. func Resolve(req ResolveRequest) (TransitionResult, error) { - return transition(req.RepoRoot, req.IssuesRoot, req.ID, "resolution", req.Resolution, StateResolved) + impact, err := changelog.ParseImpact(req.Impact) + if err != nil { + return TransitionResult{}, fmt.Errorf("resolve: %w", err) + } + return transition(req.RepoRoot, req.IssuesRoot, req.ID, "resolution", req.Resolution, + []kv{{"impact", rawScalar(string(impact))}}, StateResolved) } // Wontfix moves an open issue to wontfix/, writing the wontfix_reason note. +// wontfix/ carries no impact (issue_impact_valid gates resolved/ only), so no +// judgement is stamped. func Wontfix(req WontfixRequest) (TransitionResult, error) { - return transition(req.RepoRoot, req.IssuesRoot, req.ID, "wontfix_reason", req.Reason, StateWontfix) + return transition(req.RepoRoot, req.IssuesRoot, req.ID, "wontfix_reason", req.Reason, nil, StateWontfix) } -func transition(repoRoot, issuesRoot, issID, field, note string, target State) (TransitionResult, error) { +// transition moves an open issue to target, setting the defining note field and +// any extra frontmatter fields (e.g. resolved/'s impact) in one atomic write. +func transition(repoRoot, issuesRoot, issID, field, note string, extra []kv, target State) (TransitionResult, error) { rr, ir, err := resolveRoots(repoRoot, issuesRoot) if err != nil { return TransitionResult{}, err @@ -168,6 +182,12 @@ func transition(repoRoot, issuesRoot, issID, field, note string, target State) ( if err != nil { return err } + for _, f := range extra { + newContent, err = setScalarField(newContent, f.key, f.val) + if err != nil { + return err + } + } dst := filepath.Join(ir, statusDirName[target], filepath.Base(src)) fm, _, err := parseFrontmatterAndBody(newContent) diff --git a/internal/core/capture/workflow_test.go b/internal/core/capture/workflow_test.go index 41cd827..f372b6b 100644 --- a/internal/core/capture/workflow_test.go +++ b/internal/core/capture/workflow_test.go @@ -42,7 +42,7 @@ func TestTransitionSerializesOnLedgerLock(t *testing.T) { lockTimeout = 200 * time.Millisecond defer func() { lockTimeout = orig }() - _, err = Resolve(ResolveRequest{RepoRoot: repo, IssuesRoot: ir, ID: res.ID, Resolution: "x"}) + _, err = Resolve(ResolveRequest{RepoRoot: repo, IssuesRoot: ir, ID: res.ID, Resolution: "x", Impact: "fix"}) if !errors.Is(err, ErrAllocatorContention) { t.Fatalf("transition must serialize on the ledger lock (expect contention while held), got err=%v", err) } @@ -259,7 +259,7 @@ func TestResolveTransition(t *testing.T) { if err != nil { t.Fatal(err) } - tr, err := Resolve(ResolveRequest{RepoRoot: repo, IssuesRoot: ir, ID: res.ID, Resolution: "patched in fn-9"}) + tr, err := Resolve(ResolveRequest{RepoRoot: repo, IssuesRoot: ir, ID: res.ID, Resolution: "patched in fn-9", Impact: "fix"}) if err != nil { t.Fatalf("Resolve: %v", err) } @@ -282,11 +282,11 @@ func TestResolveConflictAndUnknown(t *testing.T) { RepoRoot: repo, IssuesRoot: ir, Text: "b", Severity: SeverityMinor, Category: "bug", Source: "manual-test", Slug: "s", FoundDuring: "t", }) - if _, err := Resolve(ResolveRequest{RepoRoot: repo, IssuesRoot: ir, ID: res.ID, Resolution: "done"}); err != nil { + if _, err := Resolve(ResolveRequest{RepoRoot: repo, IssuesRoot: ir, ID: res.ID, Resolution: "done", Impact: "fix"}); err != nil { t.Fatal(err) } // Already resolved -> conflict. - if _, err := Resolve(ResolveRequest{RepoRoot: repo, IssuesRoot: ir, ID: res.ID, Resolution: "again"}); !errors.Is(err, ErrTransitionConflict) { + if _, err := Resolve(ResolveRequest{RepoRoot: repo, IssuesRoot: ir, ID: res.ID, Resolution: "again", Impact: "fix"}); !errors.Is(err, ErrTransitionConflict) { t.Fatalf("want ErrTransitionConflict, got %v", err) } // Unknown id. @@ -381,7 +381,7 @@ func TestStatusCountsAndRecentOpen(t *testing.T) { ids = append(ids, res.ID) } // Resolve the first one. - if _, err := Resolve(ResolveRequest{RepoRoot: repo, IssuesRoot: ir, ID: ids[0], Resolution: "done"}); err != nil { + if _, err := Resolve(ResolveRequest{RepoRoot: repo, IssuesRoot: ir, ID: ids[0], Resolution: "done", Impact: "fix"}); err != nil { t.Fatal(err) } st, err := Status(StatusRequest{RepoRoot: repo, IssuesRoot: ir}) @@ -584,7 +584,7 @@ func TestDerivedPriorityUnblockedFirstThenSeverity(t *testing.T) { // Resolve the blocker: iss-2 becomes unblocked and sorts to the front by its // critical severity. - if _, err := Resolve(ResolveRequest{RepoRoot: repo, IssuesRoot: ir, ID: "iss-1", Resolution: "fixed"}); err != nil { + if _, err := Resolve(ResolveRequest{RepoRoot: repo, IssuesRoot: ir, ID: "iss-1", Resolution: "fixed", Impact: "fix"}); err != nil { t.Fatal(err) } lr2, err := List(ListRequest{RepoRoot: repo, IssuesRoot: ir, State: StateOpen}) diff --git a/internal/core/intent/create.go b/internal/core/intent/create.go index 3f4a40f..91cad29 100644 --- a/internal/core/intent/create.go +++ b/internal/core/intent/create.go @@ -10,6 +10,7 @@ import ( "syscall" "time" + "github.com/REPPL/abcd-cli/internal/core/changelog" "github.com/REPPL/abcd-cli/internal/fsutil" ) @@ -35,11 +36,25 @@ const maxSlugLen = 60 // null and whose spec_id is null) and passes Validate; a human expands it, then // `abcd intent plan` schedules it. This is the quoted-text create path itd-46 // delivers — the create half of what spc-6 AC3 (promote) needs. -func CreateFromText(repoRoot, text string) (Intent, error) { +func CreateFromText(repoRoot, text, impact string) (Intent, error) { trimmed := strings.TrimSpace(text) if trimmed == "" { return Intent{}, fmt.Errorf("intent: refusing to create from empty text") } + // impact is optional on a draft (intent_impact_valid gates the move into + // shipped/, not the seed), but when set it must be a legal, non-internal + // judgement — the same bar the gate applies at shipped/ — so the value the + // tool stamps travels unchanged to shipped/ and passes the blocker there. An + // invalid or internal impact is refused up front, never absorbed. + if impact != "" { + imp, err := changelog.ParseImpact(impact) + if err != nil { + return Intent{}, fmt.Errorf("intent: %w", err) + } + if imp == changelog.ImpactInternal { + return Intent{}, fmt.Errorf("intent: impact must not be internal on an intent — a press-release-first intent is user-facing by definition; declare one of additive|breaking|fix, or record the work as an issue instead") + } + } slug, err := deriveIntentSlug(trimmed) if err != nil { return Intent{}, err @@ -62,7 +77,7 @@ func CreateFromText(repoRoot, text string) (Intent, error) { if _, statErr := os.Lstat(abs); statErr == nil { return fmt.Errorf("intent: refusing to overwrite existing %s", rel) } - content := seedDraft(id, slug, trimmed) + content := seedDraft(id, slug, trimmed, impact) if err := fsutil.WriteFileAtomic(abs, []byte(content), 0o644); err != nil { return fmt.Errorf("intent: writing %s: %w", rel, err) } @@ -140,7 +155,7 @@ func nextIntentID(repoRoot string) (string, error) { // minimal body carrying the seed text under Why This Matters, with the itd-1 // discipline's Acceptance Criteria section left as a placeholder for the human to // fill before planning. -func seedDraft(id, slug, text string) string { +func seedDraft(id, slug, text, impact string) string { var b strings.Builder b.WriteString("---\n") b.WriteString("id: " + id + "\n") @@ -151,6 +166,13 @@ func seedDraft(id, slug, text string) string { b.WriteString("reclassification_history: []\n") b.WriteString("builds_on: []\n") b.WriteString("severity: minor\n") + // impact is written only when the caller declared one (validated in + // CreateFromText). It is bare — the machine-read enum the shipped-intent gate + // compares byte-for-byte — and travels unchanged to shipped/. An unset impact + // writes no line: a draft is "not judged yet", exactly like the null fields. + if impact != "" { + b.WriteString("impact: " + impact + "\n") + } b.WriteString("---\n\n") b.WriteString("# " + titleLine(text) + "\n\n") b.WriteString("## Press Release\n\n") diff --git a/internal/core/intent/create_test.go b/internal/core/intent/create_test.go index 817848c..597ddc7 100644 --- a/internal/core/intent/create_test.go +++ b/internal/core/intent/create_test.go @@ -16,7 +16,7 @@ import ( func TestCreateFromTextSeedsDraft(t *testing.T) { root := t.TempDir() - it, err := CreateFromText(root, "I want users to feel the card respects their time") + it, err := CreateFromText(root, "I want users to feel the card respects their time", "") if err != nil { t.Fatalf("CreateFromText: %v", err) } @@ -59,7 +59,7 @@ func TestCreateFromTextAllocatesNextID(t *testing.T) { writeFile(t, root, plannedDir+"/itd-9-beta.md", "---\nid: itd-9\nslug: beta\nspec_id: spc-1\nkind: standalone\n---\n# beta\n") - it, err := CreateFromText(root, "another product intent") + it, err := CreateFromText(root, "another product intent", "") if err != nil { t.Fatalf("CreateFromText: %v", err) } @@ -73,7 +73,7 @@ func TestCreateFromTextAllocatesNextID(t *testing.T) { func TestCreateFromTextRefusesEmpty(t *testing.T) { root := t.TempDir() for _, in := range []string{"", " ", "\t\n"} { - if _, err := CreateFromText(root, in); err == nil { + if _, err := CreateFromText(root, in, ""); err == nil { t.Fatalf("CreateFromText(%q) must be refused", in) } } @@ -87,7 +87,7 @@ func TestCreateFromTextRefusesEmpty(t *testing.T) { // over a freshly seeded draft — the "abcd audit stays green" guarantee. func TestCreateFromTextPassesRecordLint(t *testing.T) { root := t.TempDir() - if _, err := CreateFromText(root, "seeded from a quoted-text capture"); err != nil { + if _, err := CreateFromText(root, "seeded from a quoted-text capture", ""); err != nil { t.Fatalf("CreateFromText: %v", err) } cfg := lint.Config{ diff --git a/internal/core/intent/impact_test.go b/internal/core/intent/impact_test.go new file mode 100644 index 0000000..c4ff2db --- /dev/null +++ b/internal/core/intent/impact_test.go @@ -0,0 +1,96 @@ +package intent + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/REPPL/abcd-cli/internal/core/lint" +) + +// lintIntentImpact runs only the intent_impact_valid record-lint blocker over the +// intent tree under root and returns its findings. +func lintIntentImpact(t *testing.T, root string) []lint.Finding { + t.Helper() + cfg := lint.Config{ + Roots: []string{".abcd/development"}, + Rules: map[string]lint.RuleConfig{ + "intent_impact_valid": {Enabled: true, Severity: "blocker", IntentsDir: "intents"}, + }, + } + fs, err := lint.Lint(cfg, root) + if err != nil { + t.Fatalf("lint: %v", err) + } + return fs +} + +// TestCreateFromTextStampsImpact is the iss-117 intent half: the seed-draft path +// must be able to stamp a valid impact so the record survives — unchanged by the +// impact-preserving planned->shipped move — into shipped/, where +// intent_impact_valid REQUIRES one. A seed path that cannot set impact produces an +// intent the tool can never ship past its own blocker. +func TestCreateFromTextStampsImpact(t *testing.T) { + root := t.TempDir() + it, err := CreateFromText(root, "a user-facing improvement worth shipping", "additive") + if err != nil { + t.Fatalf("CreateFromText: %v", err) + } + // The seeded draft carries the impact bare (impact: additive), matching the + // machine-read enum the shipped-intent gate compares byte-for-byte. + raw, err := os.ReadFile(filepath.Join(root, it.Path)) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(raw), "\nimpact: additive\n") { + t.Fatalf("seeded draft must carry bare impact, got:\n%s", raw) + } + + // Simulate the planned->shipped move (moveIntentToBucket preserves frontmatter): + // relocate the file into shipped/ and assert intent_impact_valid is satisfied. + shipped := filepath.Join(root, shippedDir, filepath.Base(it.Path)) + if err := os.MkdirAll(filepath.Dir(shipped), 0o755); err != nil { + t.Fatal(err) + } + if err := os.Rename(filepath.Join(root, it.Path), shipped); err != nil { + t.Fatal(err) + } + if fs := lintIntentImpact(t, root); len(fs) != 0 { + t.Fatalf("shipped record must satisfy intent_impact_valid, got %d finding(s): %+v", len(fs), fs) + } +} + +// TestCreateFromTextRejectsBadImpact proves the seed boundary fails closed on an +// impact the intent gate would reject: a misspelling, and — because an intent is +// press-release-first — the otherwise-legal `internal`. +func TestCreateFromTextRejectsBadImpact(t *testing.T) { + root := t.TempDir() + for _, bad := range []string{"additiv", "Additive", "internal", `"fix"`} { + if _, err := CreateFromText(root, "some intent text", bad); err == nil { + t.Fatalf("CreateFromText with impact %q must be refused", bad) + } + } + // No drafts file appeared for any refused create. + if entries, _ := os.ReadDir(filepath.Join(root, draftsDir)); len(entries) != 0 { + t.Fatalf("a refused create wrote %d files, want 0", len(entries)) + } +} + +// TestCreateFromTextImpactOptional keeps the no-impact path intact: an empty +// impact seeds a draft with no impact line (drafts are not gated), exactly as +// before, so the existing capture->intent promotion flow is unchanged. +func TestCreateFromTextImpactOptional(t *testing.T) { + root := t.TempDir() + it, err := CreateFromText(root, "seeded without a judgement yet", "") + if err != nil { + t.Fatalf("CreateFromText: %v", err) + } + raw, err := os.ReadFile(filepath.Join(root, it.Path)) + if err != nil { + t.Fatal(err) + } + if strings.Contains(string(raw), "impact:") { + t.Fatalf("an unset impact must write no impact line, got:\n%s", raw) + } +} diff --git a/internal/surface/cli/cli.go b/internal/surface/cli/cli.go index 90502bb..94c5f97 100644 --- a/internal/surface/cli/cli.go +++ b/internal/surface/cli/cli.go @@ -1072,6 +1072,7 @@ func newRulesCommand(asJSON *bool) *cobra.Command { // 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 { + var intentImpact string intentCmd := &cobra.Command{ Use: "intent [text]", Short: "Intent lifecycle; bare invocation is read-only status, quoted text files a draft", @@ -1092,7 +1093,7 @@ func newIntentCommand(asJSON *bool) *cobra.Command { "unknown intent subcommand %q; did you mean %q? (nothing created — reword the text if you meant to file a draft)", args[0], sug)} } - return createIntentFromText(cmd, cwd, strings.Join(args, " "), *asJSON) + return createIntentFromText(cmd, cwd, strings.Join(args, " "), intentImpact, *asJSON) } v, err := intent.Status(cwd) if err != nil { @@ -1111,6 +1112,11 @@ func newIntentCommand(asJSON *bool) *cobra.Command { }) }, } + // --impact stamps an optional product judgement onto the seeded draft. It is + // optional (a draft is "not judged yet"), but when set it is validated and + // travels unchanged to shipped/, where intent_impact_valid requires it — so the + // tool's own create->plan->ship path can produce a record that clears the gate. + intentCmd.Flags().StringVar(&intentImpact, "impact", "", "stamp the draft's product impact: additive|breaking|fix (optional)") // new "" — backwards-compatible alias for the sub-verb-free create path // (itd-46, lean a): routes to the same create engine and warns on stderr that @@ -1130,7 +1136,7 @@ func newIntentCommand(asJSON *bool) *cobra.Command { } fmt.Fprintln(cmd.ErrOrStderr(), "WARNING: `abcd intent new` is deprecated; use `abcd intent \"\"` (quoted text is the create signal).") - return createIntentFromText(cmd, cwd, strings.Join(args, " "), *asJSON) + return createIntentFromText(cmd, cwd, strings.Join(args, " "), "", *asJSON) }, }) @@ -1234,8 +1240,8 @@ const ledgerDecisionRule = " which ledger? half-formed observation, question, o // files a new draft via intent.CreateFromText and renders the created record. The // engine refuses empty/whitespace text and mints the id under the store lock, so // this surface stays a thin marshaller. -func createIntentFromText(cmd *cobra.Command, cwd, text string, asJSON bool) error { - it, err := intent.CreateFromText(cwd, text) +func createIntentFromText(cmd *cobra.Command, cwd, text, impact string, asJSON bool) error { + it, err := intent.CreateFromText(cwd, text, impact) if err != nil { return &exitError{Code: 2, Msg: "abcd intent: " + err.Error()} } @@ -1786,9 +1792,10 @@ func newCaptureCommand(asJSON *bool) *cobra.Command { listCmd.Flags().BoolVar(&lsAll, "all", false, "issues across all three states") captureCmd.AddCommand(listCmd) - // resolve — open -> resolved with a note. - captureCmd.AddCommand(&cobra.Command{ - Use: "resolve ", + // resolve — open -> resolved with a note and a required product impact. + var resolveImpact string + resolveCmd := &cobra.Command{ + Use: "resolve --impact ", Short: "Mark an open issue resolved (open/ -> resolved/)", Args: cobra.ExactArgs(2), RunE: func(cmd *cobra.Command, args []string) error { @@ -1796,7 +1803,7 @@ func newCaptureCommand(asJSON *bool) *cobra.Command { if err != nil { return err } - res, err := capture.Resolve(capture.ResolveRequest{RepoRoot: cwd, ID: args[0], Resolution: args[1]}) + res, err := capture.Resolve(capture.ResolveRequest{RepoRoot: cwd, ID: args[0], Resolution: args[1], Impact: resolveImpact}) if err != nil { return err } @@ -1804,7 +1811,15 @@ func newCaptureCommand(asJSON *bool) *cobra.Command { fmt.Fprintf(w, "%s %s -> %s — %s\n", res.ID, res.FromStatus, res.ToStatus, res.Path) }) }, - }) + } + // resolved/ is gated by issue_impact_valid: the record carries the product + // judgement the version derivation reads, and there is no default. The flag is + // mandatory in effect — the core (capture.Resolve -> changelog.ParseImpact) + // refuses an empty impact — but it is not marked cobra-required, to keep the + // tree's no-required-flags invariant (TestLiveTreeMarksNoFlagRequired): the + // requirement is enforced semantically in the core, not by a usage annotation. + resolveCmd.Flags().StringVar(&resolveImpact, "impact", "", "product impact: additive|breaking|fix|internal (required)") + captureCmd.AddCommand(resolveCmd) // wontfix — open -> wontfix with a reason. captureCmd.AddCommand(&cobra.Command{ diff --git a/internal/surface/cli/error_pathleak_surface_test.go b/internal/surface/cli/error_pathleak_surface_test.go index fb624ca..13f0968 100644 --- a/internal/surface/cli/error_pathleak_surface_test.go +++ b/internal/surface/cli/error_pathleak_surface_test.go @@ -87,7 +87,7 @@ func TestJSONSuccessEnvelopeNoAbsolutePathLeak(t *testing.T) { t.Fatalf("prep capture failed (code %d): %s", code, se.String()) } }, - args: []string{"capture", "resolve", "iss-1", "handled", "--json"}, + args: []string{"capture", "resolve", "iss-1", "handled", "--impact", "fix", "--json"}, }, { name: "wontfix success path", From 510f63c1bde9f72f75dca7315e056f2088462bb1 Mon Sep 17 00:00:00 2001 From: REPPL <77722411+REPPL@users.noreply.github.com> Date: Sat, 25 Jul 2026 14:19:14 +0100 Subject: [PATCH 2/3] chore: resolve ledger iss-117 via the fixed resolve verb Dogfood proof of the impact write-path fix: `abcd capture resolve iss-117 ... --impact fix` moved the record open/ -> resolved/ and stamped a bare, valid impact, which record-lint's issue_impact_valid accepts (0 blockers). Assisted-by: Claude:claude-opus-4-8 --- ...cd-capture-resolve-and-intent-seeddraft-cannot-set-impact.md | 2 ++ 1 file changed, 2 insertions(+) rename .abcd/work/issues/{open => resolved}/iss-117-abcd-capture-resolve-and-intent-seeddraft-cannot-set-impact.md (67%) diff --git a/.abcd/work/issues/open/iss-117-abcd-capture-resolve-and-intent-seeddraft-cannot-set-impact.md b/.abcd/work/issues/resolved/iss-117-abcd-capture-resolve-and-intent-seeddraft-cannot-set-impact.md similarity index 67% rename from .abcd/work/issues/open/iss-117-abcd-capture-resolve-and-intent-seeddraft-cannot-set-impact.md rename to .abcd/work/issues/resolved/iss-117-abcd-capture-resolve-and-intent-seeddraft-cannot-set-impact.md index 60b1bd0..760bfd9 100644 --- a/.abcd/work/issues/open/iss-117-abcd-capture-resolve-and-intent-seeddraft-cannot-set-impact.md +++ b/.abcd/work/issues/resolved/iss-117-abcd-capture-resolve-and-intent-seeddraft-cannot-set-impact.md @@ -7,6 +7,8 @@ category: "inconsistency" source: "agent-finding" found_during: "itd-73 phase 1 derived versioning" found_at: "internal/core/capture/workflow.go" +resolution: "capture resolve and intent seed now stamp a valid impact; the write paths reach the shared changelog enum so the records they mint satisfy issue_impact_valid and intent_impact_valid" +impact: fix --- abcd capture resolve and intent seedDraft cannot set impact, so the tool's own path produces a record the new issue_impact_valid and intent_impact_valid blockers reject \ No newline at end of file From cdb1c96828900e6d10dd0fb5321a17e53759213f Mon Sep 17 00:00:00 2001 From: REPPL <77722411+REPPL@users.noreply.github.com> Date: Sat, 25 Jul 2026 14:30:20 +0100 Subject: [PATCH 3/3] chore: capture iss-126, the intent ship-path impact gap The iss-117 correctness review confirmed a pre-existing sibling of the same class: Reconcile mints shipped/ records gated by intent_impact_valid but has no way to stamp impact. Recorded first, never fixed ahead of an armed detector. Assisted-by: Claude:claude-fable-5 --- .../iss-126-intent-reconcile-cannot-stamp-impact.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 .abcd/work/issues/open/iss-126-intent-reconcile-cannot-stamp-impact.md diff --git a/.abcd/work/issues/open/iss-126-intent-reconcile-cannot-stamp-impact.md b/.abcd/work/issues/open/iss-126-intent-reconcile-cannot-stamp-impact.md new file mode 100644 index 0000000..3a4185e --- /dev/null +++ b/.abcd/work/issues/open/iss-126-intent-reconcile-cannot-stamp-impact.md @@ -0,0 +1,12 @@ +--- +schema_version: 1 +id: "iss-126" +slug: "intent-reconcile-cannot-stamp-impact" +severity: "major" +category: "inconsistency" +source: "agent-finding" +found_during: "iss-117 review (2026-07-24 run queue, burst 1)" +found_at: "internal/core/intent/lifecycle.go" +--- + +intent Reconcile ships planned intents into shipped/ without stamping impact, so a no-impact seed passes plan and reconcile then trips the intent_impact_valid blocker on a record produced entirely by the tool's own verbs; a plan-or-ship verb must be able to stamp impact (sibling of the iss-117 class, found by its review) \ No newline at end of file