diff --git a/.abcd/development/intents/shipped/itd-80-intent-lifecycle-automation.md b/.abcd/development/intents/shipped/itd-80-intent-lifecycle-automation.md index 6388586e..f0568cb7 100644 --- a/.abcd/development/intents/shipped/itd-80-intent-lifecycle-automation.md +++ b/.abcd/development/intents/shipped/itd-80-intent-lifecycle-automation.md @@ -105,9 +105,6 @@ closing two gaps (cited evidence per criterion; pinned judge/prompt/rubric hashe ## Audit Notes - - - Fidelity review — receipt rcp-1c213fa02f85 (verifier intent-fidelity-reviewer claude-opus-4-8). diff --git a/.abcd/work/issues/open/iss-67-intent-lifecycle-fidelity-gaps.md b/.abcd/work/issues/resolved/iss-67-intent-lifecycle-fidelity-gaps.md similarity index 76% rename from .abcd/work/issues/open/iss-67-intent-lifecycle-fidelity-gaps.md rename to .abcd/work/issues/resolved/iss-67-intent-lifecycle-fidelity-gaps.md index 85cf5a53..56abb4c0 100644 --- a/.abcd/work/issues/open/iss-67-intent-lifecycle-fidelity-gaps.md +++ b/.abcd/work/issues/resolved/iss-67-intent-lifecycle-fidelity-gaps.md @@ -7,6 +7,7 @@ category: "bug" source: "agent-finding" found_during: "clean-slate-sweep" found_at: "internal/core/intent/review.go" +resolution: "Plan/ingest AC gate aligned (require top-level bullet, no perpetual dead-letter); Audit Notes placeholder cleared on first review block (both delimiter styles; itd-80 backfilled); ReEmitReview doc corrected. seed9 (byte-identical DEAD_LETTER re-write) deferred as cosmetic. security PASS + ruthless SHIP." --- intent lifecycle fidelity gaps (itd-80): Plan accepts an Acceptance-Criteria section with no top-level bullet (hasAcceptanceCriteria only checks non-blank) so the intent plans then perpetually dead-letters at ingest (countAcceptanceCriteria==0) — Plan and ingest disagree on has-criteria (intent.go:108, C9/seed8); review ingest appends the audit below the template Empty-until placeholder instead of clearing it on first verdict (seed3); ReEmitReview documents a re-park but silently no-ops on an already-INGESTED/DEAD_LETTER receipt (review.go:188, C10/seed10); DEAD_LETTER re-ingest rewrites byte-identical content instead of a no-op (seed9). Detector: Plan and ingest share countAcceptanceCriteria; first-verdict-clears-placeholder; terminal-receipt re-emit; idempotent deadletter. Corpus: C9, seed3, C10, seed9. \ No newline at end of file diff --git a/CHANGELOG.md b/CHANGELOG.md index e4691be8..e35567fc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -137,6 +137,12 @@ called out in a **Breaking** section. - `abcd` status now reports `IsGitRepo` correctly in a linked git worktree or a submodule, where `.git` is a regular gitfile rather than a directory (iss-72). +- `abcd intent plan` now refuses an `## Acceptance Criteria` section with no + top-level `-`/`*` bullet, matching the ingest gate — an intent can no longer be + planned into a state where every fidelity verdict dead-letters for having zero + positional criteria. The intent template's Audit Notes placeholder is cleared + when the first review block lands, so a populated audit carries no stale "Empty" + claim (iss-67). ### Security diff --git a/internal/core/intent/intent.go b/internal/core/intent/intent.go index d9910198..3056b911 100644 --- a/internal/core/intent/intent.go +++ b/internal/core/intent/intent.go @@ -101,28 +101,15 @@ func Validate(it Intent) error { 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). +// hasAcceptanceCriteria reports whether content carries a `## Acceptance Criteria` +// section with at least one top-level -/* bullet — the itd-1 discipline Plan +// enforces. It requires a BULLET (not merely non-blank prose) so the Plan gate +// agrees with the ingest gate (countAcceptanceCriteria): an intent Plan accepts +// is one whose criteria the fidelity review can actually enumerate and judge, +// never a prose-only or numbered section that would perpetually dead-letter every +// verdict for having zero positional criteria. 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 + return countAcceptanceCriteria(content) > 0 } // setFrontmatterFields returns content with the given frontmatter keys set to diff --git a/internal/core/intent/intent_test.go b/internal/core/intent/intent_test.go index 549e5089..d0b49c7d 100644 --- a/internal/core/intent/intent_test.go +++ b/internal/core/intent/intent_test.go @@ -179,6 +179,23 @@ func TestPlanRefusesEmptyAcceptanceCriteria(t *testing.T) { } } +// TestPlanRefusesBulletlessAcceptanceCriteria (iss-67 C9) proves Plan refuses an +// Acceptance Criteria section that has prose but no top-level -/* bullet. The old +// gate accepted any non-blank line, but the ingest gate counts only bullets — so +// such an intent planned, then dead-lettered every fidelity verdict forever +// (zero criteria to map). Plan and ingest must agree on "has criteria". +func TestPlanRefusesBulletlessAcceptanceCriteria(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\nThe system should just work well.\n") + if _, err := Plan(root, "itd-10"); err == nil { + t.Fatal("Plan must refuse an Acceptance Criteria section with no top-level bullet") + } + if _, err := os.Stat(filepath.Join(root, draftsDir, "itd-10-alpha.md")); err != nil { + t.Fatal("draft must remain in place after refusal") + } +} + func TestPlanRefusesNonDraft(t *testing.T) { root := t.TempDir() writeFile(t, root, plannedDir+"/itd-10-alpha.md", diff --git a/internal/core/intent/review.go b/internal/core/intent/review.go index 42fd18f5..812c8a0f 100644 --- a/internal/core/intent/review.go +++ b/internal/core/intent/review.go @@ -64,6 +64,14 @@ var ( bulletRe = regexp.MustCompile(`^[-*]\s+\S`) // markerRe matches a parked review marker line inside the Audit Notes. markerRe = regexp.MustCompile(``) + // auditPlaceholderRe matches an intent template's Audit Notes placeholder, + // dropped when the first real review block lands so a populated audit carries no + // stale "Empty" claim. It tolerates both delimiter styles the templates have + // used — italic `_Empty. Populated by ..._` and angle-bracket `` + // — so template wording drift does not silently leave the placeholder behind. A + // real audit line never starts with `_Empty`/`]\s*$`) // criterionIDRe validates a criterion id shape before it is positionally bounded. criterionIDRe = regexp.MustCompile(`^ac-([0-9]+)$`) ) @@ -220,9 +228,14 @@ func emitReviewForIntent(repoRoot string, it Intent) (ReviewEmitResult, error) { return res, nil } -// ReEmitReview re-parks the OWED stub and request for a shipped intent (the -// manual `abcd intent review ` verb). It resolves the intent, refuses one -// not in shipped/, and delegates to the shared emit. +// ReEmitReview handles the manual `abcd intent review ` verb for a shipped +// intent. It resolves the intent, refuses one not in shipped/, and delegates to +// emitReviewForIntent. Behaviour depends on the intent's current review state: an +// OWED receipt (or none) (re-)parks the OWED stub and rewrites its ephemeral +// request; a TERMINAL receipt is not re-reviewed — an already-INGESTED or +// already-DEAD_LETTER receipt returns that status unchanged (re-reviewing would +// discard the recorded audit), so the caller learns the review is already +// resolved rather than silently receiving a fresh stub. func ReEmitReview(repoRoot, intentID string) (ReviewEmitResult, error) { if !intentIDRe.MatchString(intentID) { return ReviewEmitResult{}, fmt.Errorf("intent: id %q must match ^itd-[0-9]+$", intentID) @@ -581,7 +594,16 @@ func appendToAuditNotes(content, block string) string { break } } - section := lines[head+1 : end] + // Copy the section out (never alias the backing array) and drop the template + // placeholder line, so the first real review block replaces the "Empty" claim + // rather than sitting beneath it. + section := make([]string, 0, end-head) + for _, ln := range lines[head+1 : end] { + if auditPlaceholderRe.MatchString(strings.TrimRight(ln, "\r")) { + continue + } + section = append(section, ln) + } // Drop trailing blank lines inside the section, then re-add one separator. for len(section) > 0 && strings.TrimSpace(section[len(section)-1]) == "" { section = section[:len(section)-1] diff --git a/internal/core/intent/review_test.go b/internal/core/intent/review_test.go index 78b2bbaf..10d970dc 100644 --- a/internal/core/intent/review_test.go +++ b/internal/core/intent/review_test.go @@ -458,3 +458,25 @@ func TestFullReviewCycle(t *testing.T) { } } } + +// TestFirstReviewBlockClearsPlaceholder (iss-67 seed3) proves the intent +// template's Audit Notes placeholder ("_Empty. Populated by ..._") is dropped when +// the first review block lands, so a populated audit carries no stale "Empty" +// claim above the real verdict. +func TestFirstReviewBlockClearsPlaceholder(t *testing.T) { + // Both template placeholder styles the record has used must be cleared. + for _, placeholder := range []string{ + "_Empty. Populated by intent-fidelity-reviewer when intent moves to shipped/._", + "", + } { + content := "---\nid: itd-9\n---\n# alpha\n\n## Acceptance Criteria\n\n- one\n\n" + + "## Audit Notes\n\n" + placeholder + "\n" + out := upsertReviewBlock(content, "rcp-abc123", owedBlock("rcp-abc123")) + if strings.Contains(out, "Empty until") || strings.Contains(out, "_Empty. Populated by") { + t.Fatalf("the placeholder %q must be cleared once a review block lands:\n%s", placeholder, out) + } + if !strings.Contains(out, "OWED receipt=rcp-abc123") { + t.Fatalf("the OWED block must be present:\n%s", out) + } + } +}