diff --git a/.abcd/record-lint.json b/.abcd/record-lint.json index 84648583..6787dc74 100644 --- a/.abcd/record-lint.json +++ b/.abcd/record-lint.json @@ -21,6 +21,7 @@ "no_brittle_line_refs": {"enabled": true, "severity": "warn"}, "directory_coverage": {"enabled": true, "severity": "warn", "exempt": []}, "intent_lifecycle": {"enabled": true, "severity": "blocker", "intents_dir": "intents"}, + "spec_lifecycle": {"enabled": true, "severity": "blocker", "specs_dir": "specs", "intents_dir": "intents"}, "persona_registry": {"enabled": true, "severity": "blocker", "registry": ".abcd/development/personas.json"}, "context_status_free": {"enabled": true, "severity": "blocker", "target": ".abcd/work/CONTEXT.md"}, "surface_coverage": {"enabled": true, "severity": "blocker", "commands_dir": "commands/abcd", "skills_dir": "skills", "registry": ".abcd/development/brief/04-surfaces/README.md"}, diff --git a/CHANGELOG.md b/CHANGELOG.md index 9987cea6..3c0f46e6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -98,6 +98,16 @@ called out in a **Breaking** section. ### Changed +- `abcd spec close ` now reconciles the linked intent (itd-80): it moves + the intent `planned/ → shipped/` and then closes the spec, so one command + completes the lifecycle transition. It is fail-closed (a missing/empty intent + link, a non-existent or ambiguously-linked intent, bidirectional drift, or an + intent in an unexpected bucket refuses with no partial move) and idempotent (a + re-run on an already-shipped intent / already-closed spec is a clean no-op). + The intent's `## Audit Notes` are left untouched. A new `spec_lifecycle` + record-lint rule mirrors `intent_lifecycle` on the spec side: every spec under + `specs/{open,closed}/` must carry a well-formed `id`/`slug`/`intent` link whose + named intent EXISTS and points back at this spec (bidirectional agreement). - The issue ledger moved from `.abcd/development/activity/issues` to `.abcd/work/issues` (the committed shared-working tier). diff --git a/internal/core/intent/intent.go b/internal/core/intent/intent.go index 7df6384c..574a8256 100644 --- a/internal/core/intent/intent.go +++ b/internal/core/intent/intent.go @@ -204,6 +204,18 @@ type LinkResult struct { Spec spec.Spec `json:"spec"` } +// ReconcileResult reports a completed Reconcile (the deterministic half of +// `abcd spec close`): the closed spec, the linked intent in its post-reconcile +// state, whether the intent moved this call (false on an idempotent re-run), and +// the intent's bucket transition (From → To). +type ReconcileResult struct { + Spec spec.Spec `json:"spec"` + Intent Intent `json:"intent"` + IntentMoved bool `json:"intent_moved"` + From string `json:"from"` + To string `json:"to"` +} + // LinkedPair is one intent↔spec link in the lifecycle summary. type LinkedPair struct { Intent string `json:"intent"` diff --git a/internal/core/intent/intent_test.go b/internal/core/intent/intent_test.go index 474ef5f7..b93e43dd 100644 --- a/internal/core/intent/intent_test.go +++ b/internal/core/intent/intent_test.go @@ -336,3 +336,215 @@ func TestStatusCounts(t *testing.T) { t.Fatalf("linked pairs = %+v", v.Linked) } } + +// plannedLinked is a planned intent already carrying both link sides (the shape +// Plan leaves): kind + spec_id set, ready to ship. +func plannedLinked(id, slug, specID string) string { + return "---\nid: " + id + "\nslug: " + slug + "\nspec_id: " + specID + "\nkind: standalone\n---\n" + + "# " + slug + "\n\n## Acceptance Criteria\n\n- ok\n\n## Audit Notes\n" +} + +// specNaming is an open spec file whose intent: link names the given intent. +func specNaming(id, slug, intentID string) string { + return "---\nid: " + id + "\nslug: " + slug + "\nintent: " + intentID + "\n---\n# " + slug + "\n" +} + +func TestReconcileHappyPath(t *testing.T) { + root := t.TempDir() + writeFile(t, root, plannedDir+"/itd-10-alpha.md", plannedLinked("itd-10", "alpha", "spc-1")) + writeFile(t, root, specsOpen+"/spc-1-alpha.md", specNaming("spc-1", "alpha", "itd-10")) + + res, err := Reconcile(root, "spc-1") + if err != nil { + t.Fatal(err) + } + if !res.IntentMoved || res.From != BucketPlanned || res.To != BucketShipped { + t.Fatalf("Reconcile result = %+v", res) + } + if res.Intent.Bucket != BucketShipped || res.Intent.ID != "itd-10" { + t.Fatalf("Reconcile intent = %+v", res.Intent) + } + if res.Spec.Status != spec.StatusClosed { + t.Fatalf("Reconcile spec status = %q, want closed", res.Spec.Status) + } + // Intent moved planned -> shipped; spec moved open -> closed. + if _, err := os.Stat(filepath.Join(root, plannedDir, "itd-10-alpha.md")); !os.IsNotExist(err) { + t.Fatal("planned intent should be gone after reconcile") + } + if _, err := os.Stat(filepath.Join(root, shippedDir, "itd-10-alpha.md")); err != nil { + t.Fatalf("shipped intent should exist: %v", err) + } + if _, err := os.Stat(filepath.Join(root, specsOpen, "spc-1-alpha.md")); !os.IsNotExist(err) { + t.Fatal("open spec should be gone after reconcile") + } + if _, err := os.Stat(filepath.Join(root, specsClosed, "spc-1-alpha.md")); err != nil { + t.Fatalf("closed spec should exist: %v", err) + } + // Audit Notes are left empty/untouched (Phase 4 fills them). + body, err := os.ReadFile(filepath.Join(root, shippedDir, "itd-10-alpha.md")) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(body), "## Audit Notes") { + t.Fatalf("Audit Notes heading should survive verbatim:\n%s", body) + } +} + +// TestReconcileIdempotent proves a re-run on an already-shipped intent whose +// spec is already closed is a clean no-op/complete, not an error. +func TestReconcileIdempotent(t *testing.T) { + root := t.TempDir() + writeFile(t, root, plannedDir+"/itd-10-alpha.md", plannedLinked("itd-10", "alpha", "spc-1")) + writeFile(t, root, specsOpen+"/spc-1-alpha.md", specNaming("spc-1", "alpha", "itd-10")) + + if _, err := Reconcile(root, "spc-1"); err != nil { + t.Fatalf("first reconcile: %v", err) + } + res, err := Reconcile(root, "spc-1") + if err != nil { + t.Fatalf("second reconcile must be a clean no-op: %v", err) + } + if res.IntentMoved { + t.Fatalf("second reconcile must not move the intent: %+v", res) + } + if res.Intent.Bucket != BucketShipped || res.Spec.Status != spec.StatusClosed { + t.Fatalf("idempotent reconcile state = %+v", res) + } +} + +// TestReconcileClosesSpecWhenIntentAlreadyShipped covers the partial-failure +// recovery path: the intent already shipped but the spec is still open (a prior +// run moved the intent then failed before the close). Re-running just closes the +// spec. +func TestReconcileClosesSpecWhenIntentAlreadyShipped(t *testing.T) { + root := t.TempDir() + writeFile(t, root, shippedDir+"/itd-10-alpha.md", plannedLinked("itd-10", "alpha", "spc-1")) + writeFile(t, root, specsOpen+"/spc-1-alpha.md", specNaming("spc-1", "alpha", "itd-10")) + + res, err := Reconcile(root, "spc-1") + if err != nil { + t.Fatal(err) + } + if res.IntentMoved { + t.Fatalf("intent already shipped; must not move: %+v", res) + } + if res.Spec.Status != spec.StatusClosed { + t.Fatalf("spec should be closed: %+v", res.Spec) + } +} + +func TestReconcileFailsNoIntentLink(t *testing.T) { + root := t.TempDir() + // A spec whose intent link is malformed cannot be minted by Create, so write a + // spec whose intent names a non-existent intent to exercise the missing-intent path. + writeFile(t, root, specsOpen+"/spc-1-alpha.md", specNaming("spc-1", "alpha", "itd-99")) + if _, err := Reconcile(root, "spc-1"); err == nil { + t.Fatal("Reconcile must fail closed when the named intent does not exist") + } + // No partial move: the spec is untouched (still open). + if _, err := os.Stat(filepath.Join(root, specsOpen, "spc-1-alpha.md")); err != nil { + t.Fatal("spec must stay open after a fail-closed reconcile") + } +} + +// TestReconcileFailsWrongBucket refuses an intent that was never planned (still +// in drafts), with no partial move. +func TestReconcileFailsWrongBucket(t *testing.T) { + root := t.TempDir() + writeFile(t, root, draftsDir+"/itd-10-alpha.md", draftWithAC("itd-10", "alpha")) + writeFile(t, root, specsOpen+"/spc-1-alpha.md", specNaming("spc-1", "alpha", "itd-10")) + if _, err := Reconcile(root, "spc-1"); err == nil { + t.Fatal("Reconcile must refuse an intent still in drafts") + } + if _, err := os.Stat(filepath.Join(root, draftsDir, "itd-10-alpha.md")); err != nil { + t.Fatal("drafts intent must not move on a fail-closed reconcile") + } + if _, err := os.Stat(filepath.Join(root, specsOpen, "spc-1-alpha.md")); err != nil { + t.Fatal("spec must stay open on a fail-closed reconcile") + } +} + +// TestReconcileFailsBidirectionalDrift refuses when the intent the spec names +// points back at a different spec (a one-sided link). +func TestReconcileFailsBidirectionalDrift(t *testing.T) { + root := t.TempDir() + writeFile(t, root, plannedDir+"/itd-10-alpha.md", plannedLinked("itd-10", "alpha", "spc-2")) + writeFile(t, root, specsOpen+"/spc-1-alpha.md", specNaming("spc-1", "alpha", "itd-10")) + if _, err := Reconcile(root, "spc-1"); err == nil { + t.Fatal("Reconcile must refuse when the intent's spec_id disagrees with the spec") + } + if _, err := os.Stat(filepath.Join(root, plannedDir, "itd-10-alpha.md")); err != nil { + t.Fatal("planned intent must not move on drift") + } +} + +// TestReconcileFailsAmbiguousLink refuses when two specs realise the same intent. +func TestReconcileFailsAmbiguousLink(t *testing.T) { + root := t.TempDir() + writeFile(t, root, plannedDir+"/itd-10-alpha.md", plannedLinked("itd-10", "alpha", "spc-1")) + writeFile(t, root, specsOpen+"/spc-1-alpha.md", specNaming("spc-1", "alpha", "itd-10")) + writeFile(t, root, specsOpen+"/spc-2-alpha.md", specNaming("spc-2", "alpha", "itd-10")) + if _, err := Reconcile(root, "spc-1"); err == nil { + t.Fatal("Reconcile must refuse when more than one spec realises the intent") + } +} + +func TestReconcileRejectsBadSpecID(t *testing.T) { + if _, err := Reconcile(t.TempDir(), "spc-../../etc"); err == nil { + t.Fatal("Reconcile must reject a traversal spec id") + } +} + +func TestReconcileFailsMissingSpec(t *testing.T) { + if _, err := Reconcile(t.TempDir(), "spc-9"); err == nil { + t.Fatal("Reconcile must fail when the spec does not exist") + } +} + +// TestFullCycle drives the real lifecycle: draft -> Plan -> Reconcile (spec +// close), asserting the intent ends in shipped/, the spec in closed/, BOTH +// lifecycle lint rules find zero issues, and a second reconcile is idempotent. +func TestFullCycle(t *testing.T) { + root := t.TempDir() + writeFile(t, root, draftsDir+"/itd-10-alpha.md", draftWithAC("itd-10", "alpha")) + + pr, err := Plan(root, "itd-10") + if err != nil { + t.Fatalf("Plan: %v", err) + } + rr, err := Reconcile(root, pr.Spec.ID) + if err != nil { + t.Fatalf("Reconcile: %v", err) + } + if rr.Intent.Bucket != BucketShipped || rr.Spec.Status != spec.StatusClosed { + t.Fatalf("cycle end state = %+v", rr) + } + if _, err := os.Stat(filepath.Join(root, shippedDir, "itd-10-alpha.md")); err != nil { + t.Fatalf("intent must be shipped: %v", err) + } + if _, err := os.Stat(filepath.Join(root, specsClosed, "spc-1-alpha.md")); err != nil { + t.Fatalf("spec must be closed: %v", err) + } + + cfg := lint.Config{ + Roots: []string{".abcd/development"}, + Rules: map[string]lint.RuleConfig{ + "intent_lifecycle": {Enabled: true, Severity: "blocker", IntentsDir: "intents"}, + "spec_lifecycle": {Enabled: true, Severity: "blocker", SpecsDir: "specs", IntentsDir: "intents"}, + }, + } + findings, err := lint.Lint(cfg, root) + if err != nil { + t.Fatal(err) + } + for _, f := range findings { + if f.RuleID == "intent_lifecycle" || f.RuleID == "spec_lifecycle" { + t.Fatalf("post-cycle lifecycle finding: %s:%d %s %s", f.File, f.Line, f.RuleID, f.Message) + } + } + + // Second reconcile is a clean no-op. + if _, err := Reconcile(root, pr.Spec.ID); err != nil { + t.Fatalf("second reconcile must be idempotent: %v", err) + } +} diff --git a/internal/core/intent/lifecycle.go b/internal/core/intent/lifecycle.go index 71e60d85..c47c5a43 100644 --- a/internal/core/intent/lifecycle.go +++ b/internal/core/intent/lifecycle.go @@ -160,21 +160,14 @@ func Plan(repoRoot, intentID string) (PlanResult, error) { 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 { + // 3. Move drafts/ → planned/ via the shared, trust-guarded move. 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. + plannedRel, err := moveIntentToBucket(repoRoot, draftRel, BucketPlanned) + if 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) - } + plannedAbs := filepath.Join(repoRoot, plannedRel) // 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 @@ -247,6 +240,132 @@ func Link(repoRoot, intentID, specID string) (LinkResult, error) { return LinkResult{Intent: it, Spec: sp}, nil } +// Reconcile is the deterministic half of `abcd spec close`: it advances the +// intent a spec realises, then closes the spec, so one command marks the spec +// done AND ships its linked intent. +// +// Ordering is intent-first, spec-last, so a partial failure is recoverable by +// re-running: the intent moves planned/ → shipped/ before spec.Close runs, so a +// failure at the move leaves the spec OPEN (retry-safe), never a closed spec with +// a still-planned intent. It is idempotent: an already-shipped intent is not +// re-moved, and a re-run on an already-closed spec is a clean no-op/complete +// rather than an error. +// +// It fails closed with NO partial move when: the spec has no/empty intent link; +// the named intent does not exist; the link is ambiguous (more than one spec +// realises the intent); the intent's spec_id disagrees with this spec +// (bidirectional drift); or the intent is in an unexpected bucket (e.g. still in +// drafts — it was never planned). Every id is validated against the ^spc-/^itd- +// regexes before any path is built. The intent's `## Audit Notes` are left +// untouched (the fidelity audit is a later phase; the intent ships with them empty). +func Reconcile(repoRoot, specID string) (ReconcileResult, error) { + if !specIDRe.MatchString(specID) { + return ReconcileResult{}, fmt.Errorf("intent: spec id %q must match ^spc-[0-9]+$", specID) + } + store, err := spec.Load(repoRoot) + if err != nil { + return ReconcileResult{}, err + } + sp, ok := store.Lookup(specID) + if !ok { + return ReconcileResult{}, fmt.Errorf("intent: spec %s not found", specID) + } + + // Resolve the linked intent from the spec's intent: field, validated before it + // is ever used to build a path. + intentID := sp.Intent + if !intentIDRe.MatchString(intentID) { + return ReconcileResult{}, fmt.Errorf("intent: spec %s has no well-formed intent link (got %q); refusing to reconcile", specID, intentID) + } + // Ambiguity guard: cross-check the spec's link against the whole store. If more + // than one spec claims this intent, the link is ambiguous and we refuse rather + // than ship an intent whose realising spec is undetermined. + var claimers []string + for _, s := range store.Specs { + if s.Intent == intentID { + claimers = append(claimers, s.ID) + } + } + if len(claimers) > 1 { + return ReconcileResult{}, fmt.Errorf("intent: link ambiguous — %d specs realise %s (%s); refusing to reconcile", len(claimers), intentID, strings.Join(claimers, ", ")) + } + + corpus, err := Load(repoRoot) + if err != nil { + return ReconcileResult{}, err + } + it, ok := corpus.Lookup(intentID) + if !ok { + return ReconcileResult{}, fmt.Errorf("intent: %s (linked by spec %s) not found in any bucket; refusing to reconcile", intentID, specID) + } + // Bidirectional agreement: the intent must point back at THIS spec. A null or + // mismatched spec_id is drift (a one-sided link) — fail closed rather than ship + // an intent that names a different, or no, spec. + if it.SpecID != specID { + return ReconcileResult{}, fmt.Errorf("intent: %s spec_id is %q but spec %s claims it (bidirectional link disagrees); refusing to reconcile", intentID, it.SpecID, specID) + } + // Bucket guard runs BEFORE any move, so an unexpected bucket (drafts, + // disciplines, superseded) yields no partial move. + switch it.Bucket { + case BucketPlanned, BucketShipped: + // planned → advance; shipped → idempotent (already advanced). + default: + return ReconcileResult{}, fmt.Errorf("intent: %s is in %s (linked by spec %s); expected planned or shipped — refusing to reconcile", intentID, it.Bucket, specID) + } + + res := ReconcileResult{Spec: sp, Intent: it, From: it.Bucket, To: it.Bucket} + // 1. Advance the intent planned/ → shipped/ FIRST. Its (kind, spec_id) are + // already set (Plan wrote them), so the shipped record is lint-valid without + // touching frontmatter. If this fails, the spec stays open — the whole + // operation retries cleanly. + if it.Bucket == BucketPlanned { + dstRel, err := moveIntentToBucket(repoRoot, it.Path, BucketShipped) + if err != nil { + return ReconcileResult{}, err + } + it.Bucket = BucketShipped + it.Path = dstRel + res.Intent = it + res.IntentMoved = true + res.To = BucketShipped + } + + // 2. Close the spec, but only if still open — a re-run on an already-closed + // spec is a clean completion, not the "already closed" error spec.Close raises. + if sp.Status == spec.StatusOpen { + closed, err := spec.Close(repoRoot, specID) + if err != nil { + return ReconcileResult{}, err + } + res.Spec = closed + } + return res, nil +} + +// moveIntentToBucket moves the intent file at srcRel into dstBucket via os.Rename +// (atomic on one filesystem), behind the store's trust guards: it refuses to +// follow a symlinked destination directory and refuses to clobber an existing +// destination file. It returns the new repo-relative path. This is the single +// canonical intent move, shared by Plan (drafts → planned) and Reconcile +// (planned → shipped). +func moveIntentToBucket(repoRoot, srcRel, dstBucket string) (string, error) { + name := filepath.Base(srcRel) + dstRelDir := filepath.Join(IntentsRelDir, dstBucket) + dstRel := filepath.Join(dstRelDir, name) + dstAbs := filepath.Join(repoRoot, dstRel) + if err := ensureRealDir(filepath.Join(repoRoot, dstRelDir), dstRelDir); err != nil { + return "", err + } + if _, err := os.Lstat(dstAbs); err == nil { + return "", fmt.Errorf("intent: refusing to overwrite existing %s", dstRel) + } + srcBucket := filepath.Base(filepath.Dir(srcRel)) + if err := os.Rename(filepath.Join(repoRoot, srcRel), dstAbs); err != nil { + return "", fmt.Errorf("intent: moving %s %s->%s: %w", name, srcBucket, dstBucket, err) + } + return dstRel, 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 @@ -306,7 +425,10 @@ func readRepoFile(abs, rel string) ([]byte, error) { return data, nil } -// ensureRealDir creates dir if absent, refusing to follow a symlinked directory. +// ensureRealDir creates dir if absent, refusing a symlinked leaf directory. +// NOTE: a symlinked ANCESTOR (e.g. a symlinked intents/) is not caught here — a +// low-severity follow-up under the trusted-worktree model (planting one needs +// write access equal to editing the record directly). 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) diff --git a/internal/core/lint/config.go b/internal/core/lint/config.go index 56c0f998..d18ef4a9 100644 --- a/internal/core/lint/config.go +++ b/internal/core/lint/config.go @@ -55,7 +55,11 @@ type RuleConfig struct { // Exempt is the directory_coverage glob allowlist. Exempt []string `json:"exempt"` // IntentsDir is the intent_lifecycle intents subdirectory (relative to a root). + // spec_lifecycle also reads it to resolve the intent corpus its specs link to. IntentsDir string `json:"intents_dir"` + // SpecsDir is the spec_lifecycle specs subdirectory (relative to a root), + // mirroring IntentsDir. Default "specs". + SpecsDir string `json:"specs_dir"` // Allowlist is the stray_root_docs permitted basename-stem list (upper-cased, // extension-stripped) for top-level markdown files. Allowlist []string `json:"allowlist"` diff --git a/internal/core/lint/lint.go b/internal/core/lint/lint.go index 4977a632..99e938a6 100644 --- a/internal/core/lint/lint.go +++ b/internal/core/lint/lint.go @@ -62,6 +62,10 @@ var ( stepNameKeyRe = regexp.MustCompile(`^\s+name:\s*(.+?)\s*$`) specIDRe = regexp.MustCompile(`^spc-`) supersededRe = regexp.MustCompile(`^itd-\d+`) + // spec_lifecycle: anchored id/link/filename matchers for the spec store. + specFileRe = regexp.MustCompile(`^spc-\d+.*\.md$`) + specIDFullRe = regexp.MustCompile(`^spc-\d+$`) + intentIDFullRe = regexp.MustCompile(`^itd-\d+$`) intentBuckets = map[string]bool{ "drafts": true, "planned": true, "shipped": true, "disciplines": true, "superseded": true, @@ -148,6 +152,14 @@ func Lint(cfg Config, repoRoot string) ([]Finding, error) { } findings = append(findings, il...) } + + if specCfg, ok := cfg.Rules["spec_lifecycle"]; ok && specCfg.Enabled { + sl, err := checkSpecLifecycle(repoRoot, rootAbs, specCfg, cfg) + if err != nil { + return nil, err + } + findings = append(findings, sl...) + } } // stray_root_docs is repo-root scoped and non-recursive — independent of @@ -1152,6 +1164,148 @@ func validateIntent(rel, bucket string, fields map[string]fmField, known map[str return out } +// checkSpecLifecycle is the spec-side mirror of checkIntentLifecycle (check +// family G). It discovers spec files under specs/{open,closed}/ and validates +// each has a well-formed id/slug/intent link, that the named intent EXISTS in the +// corpus, and — the load-bearing cross-check — that the intent points back at +// this spec (bidirectional agreement). A missing specs/ directory is soft. +func checkSpecLifecycle(repoRoot, rootAbs string, cfg RuleConfig, top Config) ([]Finding, error) { + specsDir := cfg.SpecsDir + if specsDir == "" { + specsDir = "specs" + } + specsRoot := filepath.Join(rootAbs, specsDir) + if _, err := os.Stat(specsRoot); err != nil { + return nil, nil // missing specs/ is soft, mirroring intent_lifecycle + } + + // Index the intent corpus: which ids exist, and each intent's spec_id value. + // This is what lets a spec's link be checked for existence and back-agreement. + intentsDir := cfg.IntentsDir + if intentsDir == "" { + intentsDir = "intents" + } + knownIntent := map[string]bool{} + intentSpecID := map[string]string{} + _ = filepath.WalkDir(filepath.Join(rootAbs, intentsDir), func(path string, d os.DirEntry, err error) error { + if err != nil || d.IsDir() || !intentFileRe.MatchString(d.Name()) { + return nil + } + content, rerr := os.ReadFile(path) + if rerr != nil { + return nil + } + fields := frontmatterFields(strings.Split(string(content), "\n")) + id := fields["id"].value + if !intentIDFullRe.MatchString(id) { + id = intentIDRe.FindString(d.Name()) + } + if id != "" { + knownIntent[id] = true + intentSpecID[id] = fields["spec_id"].value + } + return nil + }) + + var out []Finding + for _, bucket := range []string{"open", "closed"} { + bucketDir := filepath.Join(specsRoot, bucket) + entries, err := os.ReadDir(bucketDir) + if err != nil { + if os.IsNotExist(err) { + continue + } + return nil, err + } + for _, e := range entries { + if e.IsDir() || !specFileRe.MatchString(e.Name()) { + continue + } + fileAbs := filepath.Join(bucketDir, e.Name()) + content, err := os.ReadFile(fileAbs) + if err != nil { + return nil, err + } + rel := repoRel(repoRoot, fileAbs) + fields := frontmatterFields(strings.Split(string(content), "\n")) + if contentExempt(rel, fields, top) { + continue + } + out = append(out, validateSpec(rel, fields, knownIntent, intentSpecID, cfg.Severity)...) + } + } + return out, nil +} + +func validateSpec(rel string, fields map[string]fmField, knownIntent map[string]bool, intentSpecID map[string]string, severity string) []Finding { + var out []Finding + add := func(line int, msg string) { + if line == 0 { + line = 1 + } + out = append(out, Finding{ + File: rel, Line: line, RuleID: "spec_lifecycle", + Severity: severity, Message: msg, + }) + } + + // Status is the bucket directory (open/closed), never a field. + if st, ok := fields["status"]; ok { + add(st.line, "status: key forbidden; spec status is the bucket directory (open/closed), not a field") + } + + id := fields["id"] + idValid := specIDFullRe.MatchString(id.value) + if !idValid { + add(id.line, "spec id must be present and match ^spc-\\d+$ (got '"+id.value+"')") + } + if slug, ok := fields["slug"]; !ok || isNull(slug.value) { + add(slug.line, "spec slug must be present") + } + + intent := fields["intent"] + if !intentIDFullRe.MatchString(intent.value) { + add(intent.line, "spec intent link must be present and match ^itd-\\d+$ (got '"+intent.value+"')") + return out // no existence/agreement check possible without a well-formed link + } + if !knownIntent[intent.value] { + add(intent.line, "spec intent '"+intent.value+"' does not exist in any bucket") + return out + } + // Bidirectional agreement: the named intent must carry spec_id == this spec's + // id. Drift either way (the intent points elsewhere, or at null) is flagged. + if idValid { + back := intentSpecID[intent.value] + if specNum(back) != specNum(id.value) { + add(intent.line, "bidirectional drift: spec '"+id.value+"' names intent '"+intent.value+"' but that intent's spec_id is '"+back+"'") + } + } + return out +} + +// specNum extracts the numeric N from a spec id or spec_id value (which may carry +// a trailing slug, e.g. spc-1-thing), or -1 when there is no spc- id at all. It +// lets the bidirectional check compare spc-1 against a reserved spec_id: spc-1 +// written with or without a slug suffix. +func specNum(v string) int { + if !strings.HasPrefix(v, "spc-") { + return -1 + } + rest := v[len("spc-"):] + end := 0 + for end < len(rest) && rest[end] >= '0' && rest[end] <= '9' { + end++ + } + if end == 0 { + return -1 + } + n, err := strconv.Atoi(rest[:end]) + if err != nil { + return -1 + } + return n +} + // fmField is a frontmatter key's value and 1-based source line. type fmField struct { value string diff --git a/internal/core/lint/lint_test.go b/internal/core/lint/lint_test.go index c8892aa0..20ced8e3 100644 --- a/internal/core/lint/lint_test.go +++ b/internal/core/lint/lint_test.go @@ -1272,3 +1272,79 @@ func TestPersonaRegistryConfigGuards(t *testing.T) { t.Fatalf("disabled rule produced findings: %+v", fs) } } + +func TestSpecLifecycle(t *testing.T) { + root := t.TempDir() + base := "rec/specs" + ibase := "rec/intents" + + // The intent corpus the specs cross-check against. + writeFile(t, root, ibase+"/shipped/itd-10-alpha.md", "---\nid: itd-10\nkind: standalone\nspec_id: spc-1\n---\n# ok\n") + writeFile(t, root, ibase+"/shipped/itd-20-beta.md", "---\nid: itd-20\nkind: standalone\nspec_id: spc-9\n---\n# ok\n") // points at a DIFFERENT spec (drift target) + writeFile(t, root, ibase+"/planned/itd-30-gamma.md", "---\nid: itd-30\nkind: standalone\nspec_id: null\n---\n# ok\n") // never linked back + + // Good spec: names itd-10, which points back at spc-1. Agreement passes. + writeFile(t, root, base+"/closed/spc-1-alpha.md", "---\nid: spc-1\nslug: alpha\nintent: itd-10\n---\n# ok\n") + + // Bad: names an intent that does not exist in the corpus. + writeFile(t, root, base+"/open/spc-2-nope.md", "---\nid: spc-2\nslug: nope\nintent: itd-99\n---\n# bad\n") + // Bad: bidirectional drift — names itd-20, but itd-20's spec_id is spc-9, not spc-3. + writeFile(t, root, base+"/open/spc-3-drift.md", "---\nid: spc-3\nslug: drift\nintent: itd-20\n---\n# bad\n") + // Bad: malformed spec id. + writeFile(t, root, base+"/open/spc-4-badid.md", "---\nid: spec-4\nslug: badid\nintent: itd-10\n---\n# bad\n") + // Bad: names itd-30, whose spec_id is null (drift: intent points at nothing). + writeFile(t, root, base+"/open/spc-5-null.md", "---\nid: spc-5\nslug: null\nintent: itd-30\n---\n# bad\n") + + cfg := Config{ + Roots: []string{"rec"}, + Rules: map[string]RuleConfig{ + "spec_lifecycle": {Enabled: true, Severity: "blocker", SpecsDir: "specs", IntentsDir: "intents"}, + }, + } + fs, err := Lint(cfg, root) + if err != nil { + t.Fatal(err) + } + + badFiles := []string{ + filepath.Join(base, "open", "spc-2-nope.md"), + filepath.Join(base, "open", "spc-3-drift.md"), + filepath.Join(base, "open", "spc-4-badid.md"), + filepath.Join(base, "open", "spc-5-null.md"), + } + for _, bf := range badFiles { + found := false + for _, f := range fs { + if f.File == bf && f.RuleID == "spec_lifecycle" { + found = true + } + } + if !found { + t.Errorf("expected spec_lifecycle finding on %s; got %+v", bf, fs) + } + } + // The good spec produces no finding. + for _, f := range fs { + if filepath.Base(f.File) == "spc-1-alpha.md" && f.RuleID == "spec_lifecycle" { + t.Errorf("unexpected finding on clean spec: %+v", f) + } + } +} + +func TestSpecLifecycleMissingDirIsSoft(t *testing.T) { + root := t.TempDir() + writeFile(t, root, "rec/README.md", "# rec\n") + cfg := Config{ + Roots: []string{"rec"}, + Rules: map[string]RuleConfig{ + "spec_lifecycle": {Enabled: true, Severity: "blocker", SpecsDir: "specs", IntentsDir: "intents"}, + }, + } + fs, err := Lint(cfg, root) + if err != nil { + t.Fatal(err) + } + if n := countRule(fs, "spec_lifecycle"); n != 0 { + t.Fatalf("missing specs/ dir must be soft; got %+v", fs) + } +} diff --git a/internal/core/spec/store.go b/internal/core/spec/store.go index c40ae669..624c666b 100644 --- a/internal/core/spec/store.go +++ b/internal/core/spec/store.go @@ -243,7 +243,10 @@ func readRepoFile(abs, rel string) ([]byte, error) { return data, nil } -// ensureDir creates dir if absent, refusing to follow a symlinked directory. +// ensureDir creates dir if absent, refusing a symlinked leaf directory. +// NOTE: a symlinked ANCESTOR (e.g. a symlinked specs/) is not caught here — a +// low-severity follow-up under the trusted-worktree model (planting one needs +// write access equal to editing the record directly). 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) diff --git a/internal/surface/cli/cli.go b/internal/surface/cli/cli.go index 8b96e453..6b877b02 100644 --- a/internal/surface/cli/cli.go +++ b/internal/surface/cli/cli.go @@ -468,9 +468,8 @@ type specStatusView struct { // 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. +// sub-verb closes a spec AND reconciles its linked intent (planned -> shipped) +// via intent.Reconcile, so one command completes the lifecycle transition. func newSpecCommand(asJSON *bool) *cobra.Command { specCmd := &cobra.Command{ Use: "spec", @@ -502,22 +501,28 @@ func newSpecCommand(asJSON *bool) *cobra.Command { }, } - // close — open/ -> closed/. No lifecycle reconcile yet (next phase). + // close — closes the spec AND reconciles the linked intent + // (planned -> shipped). Fail-closed and idempotent (see intent.Reconcile). specCmd.AddCommand(&cobra.Command{ Use: "close ", - Short: "Close a spec (open/ -> closed/); lifecycle reconcile lands in the next phase", + Short: "Close a spec (open/ -> closed/) and ship its linked intent (planned/ -> shipped/)", 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]) + res, err := intent.Reconcile(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 render(cmd.OutOrStdout(), *asJSON, res, func(w io.Writer) { + fmt.Fprintf(w, "abcd spec close — %s open -> closed\n %s\n", res.Spec.ID, res.Spec.Path) + if res.IntentMoved { + fmt.Fprintf(w, " reconciled intent %s: %s -> %s\n", res.Intent.ID, res.From, res.To) + } else { + fmt.Fprintf(w, " intent %s already %s (no move)\n", res.Intent.ID, res.To) + } }) }, }) diff --git a/internal/surface/cli/intent_cli_test.go b/internal/surface/cli/intent_cli_test.go index 507e99e0..9c2f91a2 100644 --- a/internal/surface/cli/intent_cli_test.go +++ b/internal/surface/cli/intent_cli_test.go @@ -160,23 +160,57 @@ func TestSpecBareText(t *testing.T) { func TestSpecCloseHappy(t *testing.T) { repo := t.TempDir() t.Chdir(repo) + // spec close now reconciles the linked intent, so the intent must exist and + // be planned+linked back to this spec. + writeRepoFile(t, repo, cliPlanned+"/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") 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"` + Spec struct { + Status string `json:"status"` + Path string `json:"path"` + } `json:"spec"` + Intent struct { + Bucket string `json:"bucket"` + } `json:"intent"` + IntentMoved bool `json:"intent_moved"` + From string `json:"from"` + To string `json:"to"` } 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 got.Spec.Status != "closed" { + t.Fatalf("spec close status = %q, want closed", got.Spec.Status) + } + if !got.IntentMoved || got.From != "planned" || got.To != "shipped" || got.Intent.Bucket != "shipped" { + t.Fatalf("reconcile envelope = %+v", got) } 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) } + if _, err := os.Stat(filepath.Join(repo, ".abcd/development/intents/shipped", "itd-10-alpha.md")); err != nil { + t.Fatalf("shipped intent file missing: %v", err) + } +} + +// TestSpecCloseReconcileText checks the human-readable close render names the +// intent that moved and its from->to. +func TestSpecCloseReconcileText(t *testing.T) { + repo := t.TempDir() + t.Chdir(repo) + writeRepoFile(t, repo, cliPlanned+"/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") + 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", "close", "spc-1")) + if !strings.Contains(out, "itd-10") || !strings.Contains(out, "planned") || !strings.Contains(out, "shipped") { + t.Fatalf("close text missing reconcile detail:\n%s", out) + } } func TestSpecCloseMissingErrors(t *testing.T) {