diff --git a/cmd/planctl/main.go b/cmd/planctl/main.go index a58afda..b49873e 100644 --- a/cmd/planctl/main.go +++ b/cmd/planctl/main.go @@ -10,6 +10,9 @@ import ( "fmt" "io" "os" + "path/filepath" + "regexp" + "sort" "strings" ) @@ -67,15 +70,411 @@ func run(args []string, stdout, stderr io.Writer) int { } } -// spec:planctl/R6.2+D§3.6 -// runLint is the stub for the lint subcommand. Task 5 replaces this with -// plan-dir discovery + the full lint pipeline; for now it accepts any args -// and exits 0 so the skeleton is self-contained. R6.2 (read-only) is -// trivially upheld here — the stub performs no filesystem writes. -func runLint(_ []string, _ io.Writer, _ io.Writer) int { +// spec:planctl/D§3.7 +// PlanResult is the per-plan lint output consumed by the emitter. Until +// task 6.1 creates emit.go this type lives here; task 6 will move it. +type PlanResult struct { + PlanDir string + Diagnostics []Diagnostic + TaskCount int + ReqCount int + DesCount int +} + +// spec:planctl/R3.1+R3.2+R5.3+R5.4+R5.5+D§1+D§3.5+D§3.7 +// runLint drives the lint subcommand end-to-end: parse flags, resolve +// plan directories, load each plan, run the rule pipeline, emit the +// results, and return the process exit code. +// +// v1 scope per PRD R6.2: strictly read-only — no writes to any plan +// file. The output path is a placeholder text emitter; task 6 replaces +// it with the text / json emitters from design §3.7. +func runLint(args []string, stdout, stderr io.Writer) int { + flags, err := parseLintFlags(args) + if err != nil { + fmt.Fprintln(stderr, err.Error()) + return 2 + } + cwd, err := os.Getwd() + if err != nil { + fmt.Fprintf(stderr, "planctl: getwd: %v\n", err) + return 2 + } + plans, err := resolvePlans(flags.planDir, cwd) + if err != nil { + fmt.Fprintln(stderr, err.Error()) + return 2 + } + results := make([]PlanResult, 0, len(plans)) + for _, dir := range plans { + p, err := loadPlan(dir) + if err != nil { + fmt.Fprintln(stderr, err.Error()) + return 2 + } + results = append(results, lintPlan(p, flags)) + } + placeholderEmit(stdout, results) + return computeExit(results, flags.strict) +} + +// spec:planctl/R5.3+D§3.5 +// loadPlan reads the three canonical markdown files (prd.md, design.md, +// tasks.md) from dir and records the two close-out markers +// (codex-sessions.md, any handoff*.md). Missing markdown files yield +// nil ScanResult pointers rather than errors — R2.8 (no design), R3.1 +// (no prd fatal), and R3.4 (zero-task tasks.md) are downstream-rule +// concerns, handled in lintPlan / the rule checkers. +// +// Scan paths are rewritten to the plan-relative form ("prd.md", not the +// absolute path) so diagnostics quote compact file names per R5.2. +func loadPlan(dir string) (*Plan, error) { + absDir, err := filepath.Abs(dir) + if err != nil { + return nil, err + } + p := &Plan{Dir: absDir} + files := []struct { + name string + dst **ScanResult + }{ + {"prd.md", &p.PRD}, + {"design.md", &p.Design}, + {"tasks.md", &p.Tasks}, + } + for _, f := range files { + path := filepath.Join(absDir, f.name) + if _, err := os.Stat(path); err != nil { + continue + } + r, err := Scan(path) + if err != nil { + return nil, fmt.Errorf("planctl: scan %s: %w", path, err) + } + r.Path = f.name + *f.dst = &r + } + if _, err := os.Stat(filepath.Join(absDir, "codex-sessions.md")); err == nil { + p.CodexLog = true + } + if matches, _ := filepath.Glob(filepath.Join(absDir, "handoff*.md")); len(matches) > 0 { + p.HandoffFound = true + } + return p, nil +} + +// spec:planctl/R3.1+R3.2+R5.3+D§1+D§3.7 +// lintPlan runs BuildIndex + the four rule checkers on p and returns a +// PlanResult whose PlanDir is the plan-dir basename (so multi-plan +// output can attribute diagnostics without the full path). +// +// PRD-absent short-circuit (R3.1 / R3.2): emit ONLY the missing-prd +// fatal plus checkCloseoutFiles (which is PRD-independent — it only +// needs task lines and the two close-out booleans). R2 / R4 checks are +// skipped because they have no basis without declared R-ids. +func lintPlan(p *Plan, flags lintFlags) PlanResult { + planDirName := filepath.Base(p.Dir) + if p.PRD == nil { + idx := BuildIndex(p) + diags := newMissingPRDResult(planDirName) + diags = append(diags, checkCloseoutFiles(p, idx)...) + backfillPlanDir(diags, planDirName) + sortDiagnostics(diags) + return PlanResult{ + PlanDir: planDirName, + Diagnostics: diags, + TaskCount: len(idx.TaskLines), + } + } + idx := BuildIndex(p) + hasDesign := p.Design != nil + var diags []Diagnostic + diags = append(diags, checkTagSyntax(idx)...) + diags = append(diags, checkCrossRef(idx, hasDesign)...) + diags = append(diags, checkCloseoutFiles(p, idx)...) + diags = append(diags, checkEars(idx, flags.noEars)...) + backfillPlanDir(diags, planDirName) + sortDiagnostics(diags) + return PlanResult{ + PlanDir: planDirName, + Diagnostics: diags, + TaskCount: len(idx.TaskLines), + ReqCount: len(idx.PRDRequirements), + DesCount: len(idx.DesignSections), + } +} + +// backfillPlanDir stamps every diag's PlanDir with planDir when unset. +// Rule checkers don't know the plan-dir basename; the orchestrator does. +func backfillPlanDir(diags []Diagnostic, planDir string) { + for i := range diags { + if diags[i].PlanDir == "" { + diags[i].PlanDir = planDir + } + } +} + +// spec:planctl/R5.4+R5.5+D§3.7 +// computeExit returns the process exit code for a set of linted plans +// per PRD R5.4 / R5.5: +// +// - 0 — no error-severity diagnostics (warnings alone are informational) +// - 1 — any error, or any warning when strict=true +// +// Exit code 2 is reserved for discovery / I/O failures at the outer +// layer (runLint returns 2 directly in those paths) and is never +// produced here. +func computeExit(plans []PlanResult, strict bool) int { + for _, pr := range plans { + for _, d := range pr.Diagnostics { + if d.Severity == SevError { + return 1 + } + if d.Severity == SevWarning && strict { + return 1 + } + } + } return 0 } +// spec:planctl/R5.2+R5.3 +// placeholderEmit is a minimal text emitter used until task 6.1 adds the +// full emitText / emitJSON from design §3.7. One line per diagnostic in +// the R5.2 canonical form; clean plans emit the R5.3 summary line. No +// multi-plan headers / aggregate summary — those arrive in task 6. +func placeholderEmit(w io.Writer, plans []PlanResult) { + for _, p := range plans { + if len(p.Diagnostics) == 0 { + fmt.Fprintf(w, "%s: clean (%d tasks, %d requirements, %d design sections)\n", + p.PlanDir, p.TaskCount, p.ReqCount, p.DesCount) + continue + } + for _, d := range p.Diagnostics { + path := d.Path + if path == "" { + path = p.PlanDir + } + sev := "error" + if d.Severity == SevWarning { + sev = "warning" + } + fmt.Fprintf(w, "%s:%d: [%s] %s: %s\n", path, d.Line, sev, d.Code, d.Message) + } + } +} + +// spec:planctl/R5.1+D§3.5 +// planDirPattern matches a plan-dir basename: five digits (YYWWD), a +// dash, and a lowercase kebab slug starting with a letter. Used both +// for Case B (is `d` itself a plan dir?) and Case C (which `dev/plans/*` +// entries are plans?). +var planDirPattern = regexp.MustCompile(`^\d{5}-[a-z][a-z0-9-]+$`) + +// spec:planctl/R5.6+R5.8+D§3.6+D§4 +// lintFlags captures the parsed `planctl lint` invocation — the +// accepted flag surface is part of the stable public CLI contract per +// design §4. +type lintFlags struct { + format string // "text" | "json", default "text" + strict bool + noEars bool + color string // "auto" | "always" | "never", accepted but a no-op in v1 per D-1 + planDir string // optional positional argument +} + +// spec:planctl/R5.6+R5.8+D§3.6+D§4 +// parseLintFlags parses the `lint` subcommand's args (subcommand name +// already stripped). Multiple invocations of a flag use the last value +// (natural consequence of sequential assignment). Unknown flags return +// an error that the caller translates to exit 2 per R5.8. +// +// Both `--flag value` and `--flag=value` forms are accepted for the two +// value-taking flags (--format, --color) to stay friendly with common +// CLI invocation styles. +func parseLintFlags(args []string) (lintFlags, error) { + f := lintFlags{format: "text", color: "auto"} + for i := 0; i < len(args); i++ { + a := args[i] + switch { + case a == "--strict": + f.strict = true + case a == "--no-ears": + f.noEars = true + case strings.HasPrefix(a, "--format="): + v, err := validateFormat(strings.TrimPrefix(a, "--format=")) + if err != nil { + return lintFlags{}, err + } + f.format = v + case a == "--format": + i++ + if i >= len(args) { + return lintFlags{}, fmt.Errorf("planctl: --format requires a value") + } + v, err := validateFormat(args[i]) + if err != nil { + return lintFlags{}, err + } + f.format = v + case strings.HasPrefix(a, "--color="): + v, err := validateColor(strings.TrimPrefix(a, "--color=")) + if err != nil { + return lintFlags{}, err + } + f.color = v + case a == "--color": + i++ + if i >= len(args) { + return lintFlags{}, fmt.Errorf("planctl: --color requires a value") + } + v, err := validateColor(args[i]) + if err != nil { + return lintFlags{}, err + } + f.color = v + case strings.HasPrefix(a, "-"): + return lintFlags{}, fmt.Errorf("planctl: unknown flag %q", a) + default: + if f.planDir != "" { + return lintFlags{}, fmt.Errorf("planctl: unexpected extra argument %q", a) + } + f.planDir = a + } + } + return f, nil +} + +// validateFormat gates --format=. +func validateFormat(v string) (string, error) { + if v == "text" || v == "json" { + return v, nil + } + return "", fmt.Errorf("planctl: invalid --format value %q (want text or json)", v) +} + +// validateColor gates --color=. All three values are accepted; the +// flag itself is a no-op in v1 per D-1 (TTY detection deferred to v2). +func validateColor(v string) (string, error) { + if v == "auto" || v == "always" || v == "never" { + return v, nil + } + return "", fmt.Errorf("planctl: invalid --color value %q (want auto, always, or never)", v) +} + +// spec:planctl/R5.1+D§3.5 +// resolvePlans implements the R5.1 discovery decision tree (design §3.5): +// +// - Case A: explicit != "". Stat the path; return the single absolute +// path, or an error that the caller translates to exit 2. +// - Case B/C: a unified upward walk from cwd. At each ancestor `d` we +// check Case B first (is d itself a plan dir?) then Case C (does +// d/dev/plans/ exist with at least one plan child?). First hit wins; +// empty Case C enumeration falls through to continue walking. +// - Case D: walk reached filesystem root with no hit → 3-option error. +// +// Case C excludes anything named `archive` and never recurses into it. +// Results are returned sorted lexicographically, which also sorts +// chronologically thanks to the YYWWD prefix. +// +// NOTE: this is strictly an upward walk; we do NOT `filepath.WalkDir` +// downward looking for dev/plans/ (by design §3.5 — recursion would be +// slow on large trees and would pick up accidentally-nested plan dirs). +func resolvePlans(explicit string, cwd string) ([]string, error) { + if explicit != "" { + abs, err := filepath.Abs(explicit) + if err != nil { + return nil, fmt.Errorf("planctl: resolve %q: %w", explicit, err) + } + info, err := os.Stat(abs) + if err != nil { + return nil, fmt.Errorf("planctl: plan directory %q does not exist", explicit) + } + if !info.IsDir() { + return nil, fmt.Errorf("planctl: path %q is not a directory", explicit) + } + return []string{abs}, nil + } + + absCwd, err := filepath.Abs(cwd) + if err != nil { + return nil, fmt.Errorf("planctl: resolve cwd %q: %w", cwd, err) + } + + for d := absCwd; ; { + if plan := planDirAt(d); plan != "" { + return []string{plan}, nil + } + plansDir := filepath.Join(d, "dev", "plans") + if info, err := os.Stat(plansDir); err == nil && info.IsDir() { + planDirs, err := enumeratePlansChildren(plansDir) + if err != nil { + return nil, fmt.Errorf("planctl: enumerate %q: %w", plansDir, err) + } + if len(planDirs) > 0 { + return planDirs, nil + } + } + parent := filepath.Dir(d) + if parent == d { + break + } + d = parent + } + return nil, fmt.Errorf("planctl: no plan directory found; pass a path explicitly, cd into a plan directory, or cd to a repo containing dev/plans/") +} + +// spec:planctl/R5.1+D§3.5 +// planDirAt returns d if d is itself a plan directory: basename matches +// planDirPattern, d's parent's basename is "plans", and the grandparent's +// basename is "dev". Returns "" otherwise. +// +// The triple-check is how we distinguish a real plan dir (`repo/dev/plans/ +// 26172-planctl/`) from a dir that just happens to have a plan-dir-like +// name elsewhere in the tree. +func planDirAt(d string) string { + if !planDirPattern.MatchString(filepath.Base(d)) { + return "" + } + parent := filepath.Dir(d) + if filepath.Base(parent) != "plans" { + return "" + } + if filepath.Base(filepath.Dir(parent)) != "dev" { + return "" + } + return d +} + +// spec:planctl/R5.1+D§3.5 +// enumeratePlansChildren reads plansDir and returns direct-child plan +// directories as absolute paths in lexicographic order. Entries named +// "archive" are excluded (and NOT recursed into, satisfying R5.1's +// "excluding dev/plans/archive/" clause). Non-directories and entries +// not matching planDirPattern are skipped. +func enumeratePlansChildren(plansDir string) ([]string, error) { + entries, err := os.ReadDir(plansDir) + if err != nil { + return nil, err + } + var out []string + for _, e := range entries { + if !e.IsDir() { + continue + } + name := e.Name() + if name == "archive" { + continue + } + if !planDirPattern.MatchString(name) { + continue + } + out = append(out, filepath.Join(plansDir, name)) + } + sort.Strings(out) + return out, nil +} + // spec:planctl/R5.7 // printUsage writes the --help output: synopsis, flags, classification codes. // Classification codes are part of the stable public contract per design §4, diff --git a/cmd/planctl/main_test.go b/cmd/planctl/main_test.go index 6d260ea..04b6145 100644 --- a/cmd/planctl/main_test.go +++ b/cmd/planctl/main_test.go @@ -2,6 +2,9 @@ package main import ( "bytes" + "os" + "path/filepath" + "reflect" "strings" "testing" ) @@ -124,13 +127,188 @@ func TestRun_NoArgs(t *testing.T) { } } -// spec:planctl/R6.2+D§3.6 -// TestRun_LintStub asserts the lint subcommand is reachable (stub returns 0 -// until task 5 wires the real pipeline). Fuller tests land with task 5/6/7. -func TestRun_LintStub(t *testing.T) { +// spec:planctl/R3.1+R3.2+R5.3+R5.4+D§1 +// TestRun_LintMissingPRD verifies the lint subcommand dispatches end-to- +// end and produces the missing-prd fatal path when prd.md is absent. +// Uses a temp dir so the test is isolated from the actual working- +// directory contents (otherwise Case C would discover this repo's own +// plan dirs). Parent 5.5 adds the full resolvePlans coverage. +func TestRun_LintMissingPRD(t *testing.T) { + dir := t.TempDir() var stdout, stderr bytes.Buffer - exit := run([]string{"lint"}, &stdout, &stderr) - if exit != 0 { - t.Fatalf("exit = %d, want 0 (stub)", exit) + exit := run([]string{"lint", dir}, &stdout, &stderr) + if exit != 1 { + t.Fatalf("exit = %d, want 1 (missing-prd); stdout=%q stderr=%q", exit, stdout.String(), stderr.String()) + } + if !strings.Contains(stdout.String(), "missing-prd") { + t.Errorf("stdout lacks 'missing-prd': %q", stdout.String()) } } + +// spec:planctl/R5.1+D§3.5 +// TestResolvePlans exercises the four R5.1 cases A/B/C/D using +// t.TempDir()-staged directory shapes. `/tmp` isn't used directly for +// Case D because the tempdir's ancestors are the actual test-subject +// for "no dev/plans/ ever" — any stray `dev/plans/` under /tmp would +// break the test, so we explicitly verify by filepath. +func TestResolvePlans(t *testing.T) { + t.Run("Case A: explicit valid path", func(t *testing.T) { + dir := t.TempDir() + got, err := resolvePlans(dir, "/irrelevant") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(got) != 1 { + t.Fatalf("want 1 result, got %d: %v", len(got), got) + } + abs, _ := filepath.Abs(dir) + if got[0] != abs { + t.Errorf("got %q, want %q", got[0], abs) + } + }) + t.Run("Case A: explicit invalid path", func(t *testing.T) { + _, err := resolvePlans("/nonexistent/nope/never/1234567", "/irrelevant") + if err == nil { + t.Fatalf("want error for missing path, got nil") + } + }) + t.Run("Case B: deeply-nested cwd inside plan dir", func(t *testing.T) { + root := t.TempDir() + planDir := filepath.Join(root, "dev", "plans", "26172-planctl") + deep := filepath.Join(planDir, "sub", "deeper", "nested") + if err := os.MkdirAll(deep, 0o755); err != nil { + t.Fatalf("mkdir: %v", err) + } + got, err := resolvePlans("", deep) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(got) != 1 { + t.Fatalf("want 1 result, got %d: %v", len(got), got) + } + absPlan, _ := filepath.Abs(planDir) + if got[0] != absPlan { + t.Errorf("got %q, want %q", got[0], absPlan) + } + }) + t.Run("Case C: repo root with plans + archive filter + lex sort", func(t *testing.T) { + root := t.TempDir() + // Three plans + an archive and an unrelated dir; only the two + // pattern-matching non-archive dirs should be returned, sorted. + for _, d := range []string{ + "dev/plans/26170-alpha", + "dev/plans/26172-beta", + "dev/plans/archive/26100-retired", + "dev/plans/not-a-plan", + } { + if err := os.MkdirAll(filepath.Join(root, d), 0o755); err != nil { + t.Fatalf("mkdir %s: %v", d, err) + } + } + got, err := resolvePlans("", root) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + absRoot, _ := filepath.Abs(root) + want := []string{ + filepath.Join(absRoot, "dev", "plans", "26170-alpha"), + filepath.Join(absRoot, "dev", "plans", "26172-beta"), + } + if !reflect.DeepEqual(got, want) { + t.Errorf("got %v, want %v", got, want) + } + }) + t.Run("Case D: no plans ancestor returns error", func(t *testing.T) { + // Walk up from a tempdir whose ancestry we've verified has no + // dev/plans/ layer. If /tmp happens to contain dev/plans/ this + // test would be fragile; sanity-check by scanning ancestors. + root := t.TempDir() + for d := root; ; { + if _, err := os.Stat(filepath.Join(d, "dev", "plans")); err == nil { + t.Skipf("skipping: dev/plans/ exists at ancestor %s of tempdir", d) + } + parent := filepath.Dir(d) + if parent == d { + break + } + d = parent + } + _, err := resolvePlans("", root) + if err == nil { + t.Fatalf("want error for Case D, got nil") + } + if !strings.Contains(err.Error(), "no plan directory found") { + t.Errorf("error message should mention 'no plan directory found': %v", err) + } + }) +} + +// spec:planctl/R5.6+R5.8+D§3.6 +// TestParseLintFlags covers the accepted flag surface (R5.6 / D§3.6) +// plus the unknown-flag rejection path (R5.8). --color values are +// validated but not acted on — v1 keeps the flag a no-op per D-1. +func TestParseLintFlags(t *testing.T) { + t.Run("defaults", func(t *testing.T) { + f, err := parseLintFlags(nil) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if f.format != "text" || f.color != "auto" || f.strict || f.noEars || f.planDir != "" { + t.Errorf("unexpected default flags: %+v", f) + } + }) + t.Run("--format=json", func(t *testing.T) { + f, err := parseLintFlags([]string{"--format=json"}) + if err != nil || f.format != "json" { + t.Errorf("want format=json, got %+v err=%v", f, err) + } + }) + t.Run("--format json (space form)", func(t *testing.T) { + f, err := parseLintFlags([]string{"--format", "json"}) + if err != nil || f.format != "json" { + t.Errorf("want format=json, got %+v err=%v", f, err) + } + }) + t.Run("--strict", func(t *testing.T) { + f, _ := parseLintFlags([]string{"--strict"}) + if !f.strict { + t.Errorf("strict not set") + } + }) + t.Run("--no-ears", func(t *testing.T) { + f, _ := parseLintFlags([]string{"--no-ears"}) + if !f.noEars { + t.Errorf("noEars not set") + } + }) + t.Run("--color=never", func(t *testing.T) { + f, err := parseLintFlags([]string{"--color=never"}) + if err != nil || f.color != "never" { + t.Errorf("want color=never, got %+v err=%v", f, err) + } + }) + t.Run("unknown flag is an error", func(t *testing.T) { + _, err := parseLintFlags([]string{"--bogus"}) + if err == nil { + t.Errorf("unknown flag should error") + } + }) + t.Run("invalid --format value is an error", func(t *testing.T) { + _, err := parseLintFlags([]string{"--format=xml"}) + if err == nil { + t.Errorf("invalid format should error") + } + }) + t.Run("positional plan-dir", func(t *testing.T) { + f, err := parseLintFlags([]string{"--strict", "my-plan"}) + if err != nil || f.planDir != "my-plan" || !f.strict { + t.Errorf("want planDir=my-plan strict=true, got %+v err=%v", f, err) + } + }) + t.Run("last-value-wins on repeated flag", func(t *testing.T) { + f, _ := parseLintFlags([]string{"--format=text", "--format=json"}) + if f.format != "json" { + t.Errorf("last value should win: got %q", f.format) + } + }) +} diff --git a/dev/plans/26172-planctl/codex-sessions.md b/dev/plans/26172-planctl/codex-sessions.md index 724007e..b51432e 100644 --- a/dev/plans/26172-planctl/codex-sessions.md +++ b/dev/plans/26172-planctl/codex-sessions.md @@ -9,3 +9,4 @@ Append-only traceability log of codex review sessions for this plan. Each entry - 2026-04-21 code-review-parent-2 019db23c-6c44-7d00-b3a4-6653882f0964 _(2 rounds; R1 flagged missing `// spec:planctl/...` tags on `splitLines` / `buildNewlineIndex` / `offsetToLineCol` and missing test coverage for "nested emphasis around code" + "mixed" cases from task 2.7; did not reproduce the `offsetToLineCol` off-by-one I'd mentioned fixing. R2 approved after adding the three tags, three new test functions (NestedEmphasisAroundCode, MixedFeatures, InlineCodeMask_ExactRanges), and TestScanBytes_FencedBlockWithoutTrailingNewline for the residual EOF edge concern. Also fixed: `.gitignore` pattern `planctl` → `/planctl` so scan.go / scan_test.go weren't silently ignored as "any path matching planctl".)_ - 2026-04-21 code-review-parent-3 019db26e-629f-7f51-afe2-28dbb1d14ba6 _(3 rounds; R1 flagged under-asserted diagnostic tests — `assertDiagSubset` ignored extras so the recovery + in-fence + inline-code-span cases could hide stray diagnostics; plus light traceability on several helpers/patterns. R1 explicitly endorsed the widened `designHeadingPrefix` (`^(\d+(?:\.\d+)*)\.? `) over the task's literal regex because the in-repo design.md uses `## N. Title` period-space numbering for level-2 headings — strict-per-spec would have flagged every D§0 / D§1 / D§8 task citation as orphan-design. R2 approved the `assertDiagsExact` fix but flagged remaining untagged helpers. R3 approved after adding `// spec:planctl/*` anchors on `scanTagAfterOpener`, `openerAt`, `tagUnclosedDiag`, `annotationPattern`, `kindLabel`, `fallbackScanPRDReqs`, `firstTaskCheckBox`, `fallbackScanTaskLines`, `leadingSpaces`, plus the opener-const block and the three pattern vars (`prdReqPrefix`/`prdReqFallbackPattern`/`designHeadingPrefix`/`taskCheckboxPattern`) — 39 total anchors in index.go.)_ - 2026-04-21 code-review-parent-4 019db2b8-21c8-7d61-85d6-1da5c18d8033 _(2 rounds; R1 flagged high-severity sort stability — `sort.Slice` is not stable, so two Line=0 `missing-closeout-file` diagnostics (or two malformed-tag diags on the same line) could flip order between runs, violating PRD §6. Also flagged medium-severity missing traceability on `lineText`, `earsBody`, `earsBodyPrefix`, `matchesEars`. R1 endorsed the `Position.Text` extension as a clean way to keep `checkEars(idx Index, skip bool)` signature literal, and approved the unmatched-`**` EARS-exemption test. R2 approved after switching to `sort.SliceStable` + adding `Message` as a 4th-level tie-breaker, plus regression test `TestSortDiagnostics_StableOnEqualCode`, plus the four missing spec anchors.)_ +- 2026-04-21 code-review-parent-5 019db2ca-721d-7b11-8760-639b2588b114 _(1 round; approved on R1. No substantive issues on resolvePlans (4-case decision tree), parseLintFlags (both `--flag=value` and `--flag value` forms, last-value-wins, unknown-flag error), runLint / lintPlan (missing-PRD short-circuit still runs close-out check; rule-checker skip on PRD=nil; PlanDir backfill), computeExit (returns only 0/1 — 2 reserved for outer layer), placeholderEmit format, or test coverage. One residual not-a-defect: `--format --strict` gets treated as an invalid format value rather than missing-value; PRD doesn't define the case, left alone.)_ diff --git a/dev/plans/26172-planctl/tasks.md b/dev/plans/26172-planctl/tasks.md index 7689d21..43dd6fa 100644 --- a/dev/plans/26172-planctl/tasks.md +++ b/dev/plans/26172-planctl/tasks.md @@ -101,13 +101,13 @@ As you complete each task, flip `[ ]` to `[x]` in this file. Update after each s - [x] 4.8 Create `cmd/planctl/lint_test.go` with one subtest per checker. Each feeds a manually-constructed `Index` / `Plan` and asserts the `[]Diagnostic` matches expected `Code` + `Line` + `Severity`. Coverage: orphan + uncovered happy paths, `infra` sentinel skip, design-absent skip (R2.8), zero-task tasks.md does not trigger missing-closeout (R3.4), missing-prd fatal, EARS happy + each of the 5 patterns + bold-prefix exemption + `--no-ears` skip _Requirements: R1.3, R1.4, R2.4–R2.9, R3.1–R3.8, R4.1–R4.4_ _Design: D§3.4, D§7.1_ - [x] 4.9 Verify `go test ./cmd/planctl/ -run Lint` passes _Requirements: infra_ -- [ ] 5.0 Plan-dir discovery + CLI dispatch wiring _Requirements: R5.1, R5.3, R5.4, R5.5, R5.6, R5.8, R6.1, R6.2_ _Design: D§3.5, D§3.6_ - - [ ] 5.1 Implement `resolvePlans(explicit string, cwd string) ([]string, error)` per design §3.5 decision tree: Case A (`explicit != ""` → stat; exit 2 on failure), unified upward walk for Cases B and C (single ancestor loop checking plan-dir shape and `/dev/plans/` in order, first hit wins), Case D (loop reached root → error with 3-option message). On Case C, exclude `archive/` directory and any entries under it; sort results lexicographically. **Do NOT** `filepath.WalkDir` downward — upward walk only per design §3.5 _Requirements: R5.1_ _Design: D§3.5_ - - [ ] 5.2 Add flag parsing in `main.go` (hand-rolled, no 3rd-party flag library): `--format={text,json}` (default `text`), `--strict`, `--no-ears`, `--color={auto,always,never}` (accepted, no-op per D-1), `--help`, `--version`. Unknown flag → exit 2. Multiple invocations of a flag use the last value. The accepted flag surface is part of the stable public CLI contract per design §4 _Requirements: R5.6, R5.7, R5.8_ _Design: D§0 (D-1), D§3.6, D§4_ - - [ ] 5.3 Wire the lint subcommand: `resolvePlans` → for each plan dir, `loadPlan` (reads prd.md + design.md + tasks.md via `Scan`, checks for codex-sessions.md and handoff*.md via `os.Stat`), then `BuildIndex`, then run all 4 rule checkers (passing `--no-ears` through to `checkEars`), collect `[]Diagnostic`, call `sortDiagnostics`, build a `PlanResult{PlanDir, Diagnostics, TaskCount, ReqCount, DesCount}`. Handle missing-prd (task 4.5) early — skip R2 / R4 when PRD absent. Output via a placeholder text emitter that writes one line per diagnostic; task 6 replaces it _Requirements: R3.1, R3.2, R5.3, R5.4, R5.5_ _Design: D§1, D§3.5_ - - [ ] 5.4 Implement exit-code computation helper `computeExit(plans []PlanResult, strict bool) int` returning 0 / 1 / 2. With `--strict`, warnings are promoted to errors for exit purposes (R5.5). `2` is only returned from discovery / I/O failures at the outer layer, not from lint results _Requirements: R5.4, R5.5_ _Design: D§3.7_ - - [ ] 5.5 Extend `main_test.go` to cover `resolvePlans`: Case A with valid / invalid path, Case B from a deeply-nested CWD inside a plan-dir, Case C from a repo root containing `dev/plans/{a,b,archive/c}/`, Case D from `/tmp` (no plans ancestor). Use `t.TempDir()` to stage directory shapes. Also cover flag parsing: `--format=json`, `--strict`, `--no-ears`, `--color=never`, unknown flag _Requirements: R5.1, R5.6, R5.8_ _Design: D§3.5, D§3.6_ - - [ ] 5.6 Verify `go test ./cmd/planctl/` passes (full package) against the placeholder emitter from 5.3. Tests that assert exact output format may need updating when task 6 lands — note that in the commit message so task 6.6 knows to adjust any output-format assertions _Requirements: infra_ +- [x] 5.0 Plan-dir discovery + CLI dispatch wiring _Requirements: R5.1, R5.3, R5.4, R5.5, R5.6, R5.8, R6.1, R6.2_ _Design: D§3.5, D§3.6_ + - [x] 5.1 Implement `resolvePlans(explicit string, cwd string) ([]string, error)` per design §3.5 decision tree: Case A (`explicit != ""` → stat; exit 2 on failure), unified upward walk for Cases B and C (single ancestor loop checking plan-dir shape and `/dev/plans/` in order, first hit wins), Case D (loop reached root → error with 3-option message). On Case C, exclude `archive/` directory and any entries under it; sort results lexicographically. **Do NOT** `filepath.WalkDir` downward — upward walk only per design §3.5 _Requirements: R5.1_ _Design: D§3.5_ + - [x] 5.2 Add flag parsing in `main.go` (hand-rolled, no 3rd-party flag library): `--format={text,json}` (default `text`), `--strict`, `--no-ears`, `--color={auto,always,never}` (accepted, no-op per D-1), `--help`, `--version`. Unknown flag → exit 2. Multiple invocations of a flag use the last value. The accepted flag surface is part of the stable public CLI contract per design §4 _Requirements: R5.6, R5.7, R5.8_ _Design: D§0 (D-1), D§3.6, D§4_ + - [x] 5.3 Wire the lint subcommand: `resolvePlans` → for each plan dir, `loadPlan` (reads prd.md + design.md + tasks.md via `Scan`, checks for codex-sessions.md and handoff*.md via `os.Stat`), then `BuildIndex`, then run all 4 rule checkers (passing `--no-ears` through to `checkEars`), collect `[]Diagnostic`, call `sortDiagnostics`, build a `PlanResult{PlanDir, Diagnostics, TaskCount, ReqCount, DesCount}`. Handle missing-prd (task 4.5) early — skip R2 / R4 when PRD absent. Output via a placeholder text emitter that writes one line per diagnostic; task 6 replaces it _Requirements: R3.1, R3.2, R5.3, R5.4, R5.5_ _Design: D§1, D§3.5_ + - [x] 5.4 Implement exit-code computation helper `computeExit(plans []PlanResult, strict bool) int` returning 0 / 1 / 2. With `--strict`, warnings are promoted to errors for exit purposes (R5.5). `2` is only returned from discovery / I/O failures at the outer layer, not from lint results _Requirements: R5.4, R5.5_ _Design: D§3.7_ + - [x] 5.5 Extend `main_test.go` to cover `resolvePlans`: Case A with valid / invalid path, Case B from a deeply-nested CWD inside a plan-dir, Case C from a repo root containing `dev/plans/{a,b,archive/c}/`, Case D from `/tmp` (no plans ancestor). Use `t.TempDir()` to stage directory shapes. Also cover flag parsing: `--format=json`, `--strict`, `--no-ears`, `--color=never`, unknown flag _Requirements: R5.1, R5.6, R5.8_ _Design: D§3.5, D§3.6_ + - [x] 5.6 Verify `go test ./cmd/planctl/` passes (full package) against the placeholder emitter from 5.3. Tests that assert exact output format may need updating when task 6 lands — note that in the commit message so task 6.6 knows to adjust any output-format assertions _Requirements: infra_ - [ ] 6.0 Emit — text and JSON formatters _Requirements: R5.2, R5.3, R5.9, R5.10, R5.11_ _Design: D§3.7_ - [ ] 6.1 Create `cmd/planctl/emit.go` with `PlanResult` struct and `emitText(w io.Writer, plans []PlanResult, strict bool) int` / `emitJSON(w io.Writer, plans []PlanResult, strict bool) int` signatures matching design §3.7. Return value is the final process exit code — delegate to `computeExit` from task 5.4 _Requirements: R5.2, R5.3, R5.9, R5.10, R5.11_ _Design: D§3.7_