diff --git a/CLAUDE.md b/CLAUDE.md index 92fa972..6495b2e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -40,6 +40,15 @@ These landed after phase 10, and all are documented in `design/orchestration.md` What scc adds over typing `codegraph` directly is the two things it already knows: the workspace root, so `scc graph build` from `specs/` indexes the repo rather than a subtree, and whether the binary is there at all. The graph itself is *not* an scc artifact — not in the manifest, never touched by `scc update`, and `.codegraph/` stays CodeGraph's directory on CodeGraph's schedule. Unlike the launch path, a missing binary in `scc graph` is a hard error: the whole command is the binary. npm is the only installer scc will run. CodeGraph's headline install pipes a remote script into a shell (`curl … | sh`, `irm … | iex`), which is a fine thing for a person to type and not a thing scc executes on their behalf — `InstallHint` names it and leaves the decision where it belongs. +- **`scc map` and `scc patch`.** The artifacts are structured documents that happen to be Markdown, and the cost of treating them as prose is paid on every request rather than once: measured on a real workspace, one plan is 56KB and its 31 specs bring the corpus to ~90k tokens, so an agent answering "what is the next open task?" by reading the file carries the whole plan for the rest of the session. `map` turns a file into addressable pieces — `index | outline | tasks | show | blocks | find | trace` — and `patch` changes one of them — `check | uncheck | task | add | rm | append | prepend | replace | fm`. + + **The addresses are the design.** A task is `1.2`, a requirement `R1.2`, a section `#notes`, a leaf `specs//`, a paragraph `notes:7`; `L120-160` is the escape hatch and the only form that is a line number. That is what lets `patch` write into a file nobody read: a line number stops being true the moment anything above it moves, so an editor addressing by line has to read first — which is the cost the package exists to remove. The guard that reading-first was providing is replaced by three that are stronger for a structured file: an address that does not resolve is an error and never an insert at a guess, the file is re-validated afterwards and **rolled back if the edit introduced a finding** (exit 2), and the displaced and written lines are printed back — elided past a few lines, because a confirmation that echoed 400 lines would put the file in context by the back door. + + Two things measurement decided rather than taste. **`blocks` exists because section addressing bottoms out**: that plan's `## Notes` is 411 lines — half the file — with no headings inside it, but every paragraph opens with a bolded thesis, so the leads alone are an index a twentieth of the size. And **a requirement id is scoped to its spec**: `R2.5` is defined in nine of those 31 specs, so `map trace R2.5` unscoped answers with the list of specs and stops rather than concatenating nine traces. + + **`internal/artifact` owns the grammars, and `internal/validate` consumes them.** The task grammar used to live in the validator; a reader that disagreed with the validator about what a task is would be worse than no reader. The parser now states facts about a line (`Methodologies`, `Loose`, `HasCitation`) and turning a fact into a finding stays in `validate` — which is also what lets `map` read a malformed artifact instead of refusing exactly the file a user most needs to inspect. + + **No search engine.** The obvious reach for `find` is an inverted index; at 352KB and 94 artifacts a linear pass ranks the whole workspace in 55ms, and Tantivy or its kin would cost a CGO surface or a second binary against a stdlib-only `go.mod` and a six-platform cross-compile. What precision needed here was not a better index but a better *unit*: BM25 over addressable regions rather than lines, so a hit comes back as something `show` accepts. The seam is `artifact.Search` — it takes artifacts and returns hits, and nothing outside that file knows how it found them. - **The four seeded `docs/` anchors** (`assets.Seeds`). `init` writes `glossary.md`, `stack.md`, `wiki/index.md`, and `wiki/changelog.md` — the knowledge base's only fixed-name documents, each holding the format its validator checks. A seed is written once and tracked nowhere: not in the manifest, not by `scc update`. `scc` is a redesign of `csdd` (`github.com/protonspy/csdd`), narrowed to spec-driven development and deliberately leaner. When reaching for something from there, port the *decision*, not the file. Already decided against: a TUI, an embedded web dashboard, an MCP server, a devcontainer. @@ -79,7 +88,7 @@ cmd/scc/main.go os.Exit(cli.Run(os.Args[1:])) | scaffold · validate write / check | \ - assets · manifest ears · mdscan templates, hashes, grammars + assets · manifest ears · mdscan · artifact templates, hashes, grammars | paths · workspace · render · textutil · finding | @@ -101,6 +110,7 @@ Three packages sit off to the side of that tree — `rtk`, `headroom`, `codegrap | `internal/assets` | The embedded template set — rules, review agents, skills, slash commands, artifact templates. **Workspace templates are data-free except for the harness profile** (a `(version, harness)` pair still renders byte-identically everywhere, and the manifest records both, so the future three-way merge can still reconstruct the old side); **artifact templates take data** (`spec new` renders them and the user owns the result); **seeds are the `docs/` anchors** — data-free like a workspace file, untracked like an artifact. `Render(h, file)` is the only way to get a workspace file's bytes: it expands paths and synthesizes the per-harness header for agents and commands. `Version` is the template-set version and must be bumped whenever a workspace template changes. | | `internal/scaffold` | Applies the template set to a root (`Apply`) and brings an existing one current (`PlanUpdate`/`ApplyUpdate`). Idempotent, never overwrites without being told to, manifest written last. | | `internal/mdscan` | The only Markdown parser: fence- and HTML-comment-aware headings, checkboxes, links, wikilinks, slugs, plus a small frontmatter reader. `Body` is the comment/fence-stripped text every validator applies its grammar to. | +| `internal/artifact` | The navigable model of one artifact, layered on `mdscan`: sections (two ends — the subtree, and the body before the first child), tasks with their continuation, requirements, decomposition leaves, paragraph blocks. Owns **every grammar** (task, requirement, spec reference), `Find` for address resolution, `Editor` for line splices resolved against the original and applied bottom-up, and `Search`. Knows nothing about findings or exit codes. | | `internal/ears` | EARS requirement parsing, all five patterns plus complex. | | `internal/validate` | The eight validators, one file each, sharing `mdscan` and `finding`. The exception is `stack_manifests.go`: the seven dependency-file readers age on their own schedule, so they sit beside the rule rather than inside it. | | `internal/rtk` | RTK's marker pair and the idempotent splice of its block into the entry file, plus finding or `cargo install`ing the binary. | @@ -126,6 +136,8 @@ Three packages sit off to the side of that tree — `rtk`, `headroom`, `codegrap **Writes are atomic.** Use `workspace.AtomicWrite` for anything a concurrent reader might see. +**A command that edits an artifact verifies it afterwards.** `scc patch` snapshots the file, writes, re-runs the validator that owns it, and restores the snapshot if the edit introduced a finding the file did not already have. Two details are load-bearing: the comparison is on `rule + message` and deliberately **not on line number**, because an insertion moves every finding below it and comparing on line would blame this edit for the whole tail of a pre-existing problem; and an artifact scc has no validator for is written and *reported as unverified* rather than silently claimed clean. + **The marker is the file `/scc-manifest.json`, never the harness directory.** Two reasons, and both are load-bearing: every harness has a global twin in the user's home (`~/.claude`, `~/.codex`, `~/.config/opencode`) that exists on any machine running that tool, so an upward walk accepting the *directory* would resolve the root to `$HOME` for any command run outside a workspace — every command would then read and write the user's global configuration. And those directories exist in every repo that merely *uses* the tool, where scc was never initialized. `workspace.Find` therefore stats a regular file, for each harness in turn. **scc has exactly one file per harness and no config file.** The manifest is it — content hashes, doubling as the marker. scc runs no tests and no linters, so it has nothing to configure; a project's test and lint commands are a rule under `/rules/`, which is Markdown the orchestrator already reads. Resist adding `scc.json`: a JSON schema to version, read by nothing inside the binary, is dead weight. diff --git a/internal/artifact/address.go b/internal/artifact/address.go new file mode 100644 index 0000000..4db1a5d --- /dev/null +++ b/internal/artifact/address.go @@ -0,0 +1,153 @@ +package artifact + +import ( + "fmt" + "regexp" + "strconv" + "strings" + + "github.com/protonspy/spec-claude-code/internal/paths" +) + +// TargetKind says what an address resolved to, so a caller can render it in the +// vocabulary the user already types. +type TargetKind string + +const ( + TargetTask TargetKind = "task" + TargetRequirement TargetKind = "requirement" + TargetSection TargetKind = "section" + TargetLeaf TargetKind = "leaf" + TargetBlock TargetKind = "block" + TargetRange TargetKind = "range" +) + +// Target is one resolved address: what it is, what it is called, and the lines it +// occupies. +type Target struct { + Kind TargetKind `json:"kind"` + Ref string `json:"ref"` + Label string `json:"label"` + Line int `json:"line"` + End int `json:"end"` +} + +// Lines is how many lines the target covers. +func (t Target) Lines() int { return t.End - t.Line + 1 } + +var ( + taskRefRe = regexp.MustCompile(`^\d+(?:\.\d+)*$`) + reqRefRe = regexp.MustCompile(`^(?i:R)\d+(?:\.\d+)+$`) + rangeRefRe = regexp.MustCompile(`^(?i:L)?(\d+)(?:-(\d+))?$`) + blockRefRe = regexp.MustCompile(`^([a-z0-9][a-z0-9-]*):(\d+)$`) +) + +// Find resolves an address against the artifact. +// +// The forms, tried in this order: +// +// 1.2 a task, by its number +// R1.2 a requirement, by its id +// specs/foo/ a decomposition leaf, by the spec it names +// #notes | Notes a section, by anchor slug or by the title as written +// notes:7 the 7th paragraph of that section +// L120-160 an explicit line range, the escape hatch +// +// None of them is a line number except the last, which is why an address survives an +// edit above it. Order matters where the forms could collide: a bare number is a +// task before it is a line, because a caller who means a line writes the L. +func (a *Artifact) Find(ref string) (Target, error) { + ref = strings.TrimSpace(ref) + if ref == "" { + return Target{}, fmt.Errorf("no address given") + } + + if taskRefRe.MatchString(ref) { + if t, ok := a.Task(ref); ok { + return Target{TargetTask, ref, t.Summary(72), t.Line, t.End}, nil + } + return Target{}, a.unknown("task", ref) + } + if reqRefRe.MatchString(ref) { + if r, ok := a.Requirement(ref); ok { + return Target{TargetRequirement, r.ID, clip(r.Text, 72), r.Line, r.End}, nil + } + return Target{}, a.unknown("requirement", ref) + } + if strings.HasPrefix(ref, paths.SpecsSeg+"/") { + if l, ok := a.Leaf(ref); ok { + return Target{TargetLeaf, l.Ref, clip(l.Text, 72), l.Line, l.End}, nil + } + return Target{}, a.unknown("leaf", ref) + } + if m := blockRefRe.FindStringSubmatch(ref); m != nil { + n, _ := strconv.Atoi(m[2]) + for _, b := range a.Blocks() { + if b.Section == m[1] && b.Index == n { + return Target{TargetBlock, ref, clip(b.Lead, 72), b.Line, b.End}, nil + } + } + return Target{}, a.unknown("block", ref) + } + if m := rangeRefRe.FindStringSubmatch(ref); m != nil && strings.ContainsAny(ref, "Ll-") { + from, _ := strconv.Atoi(m[1]) + to := from + if m[2] != "" { + to, _ = strconv.Atoi(m[2]) + } + if from < 1 || from > len(a.Lines) { + return Target{}, fmt.Errorf("line %d is outside %s (%d lines)", from, a.Path, len(a.Lines)) + } + if to > len(a.Lines) { + to = len(a.Lines) + } + return Target{TargetRange, ref, "", from, to}, nil + } + if s, ok := a.Section(ref); ok { + return Target{TargetSection, s.Slug, s.Title, s.Line, s.End}, nil + } + // A paragraph's own slug, which is what the notes index prints and therefore what + // a caller is most likely to paste back. + for _, b := range a.Blocks() { + if b.Slug == ref { + return Target{TargetBlock, fmt.Sprintf("%s:%d", b.Section, b.Index), clip(b.Lead, 72), b.Line, b.End}, nil + } + } + return Target{}, a.unknown("address", ref) +} + +// unknown says what was not found and what the artifact does have, because an +// address that misses is nearly always a near-miss and the caller is an agent that +// cannot see the file. +func (a *Artifact) unknown(kind, ref string) error { + var have []string + switch kind { + case "task": + for _, t := range a.Tasks { + have = append(have, t.Number) + } + case "requirement": + for _, r := range a.Requirements { + have = append(have, r.ID) + } + case "leaf": + for _, l := range a.Leaves { + have = append(have, l.Ref) + } + default: + for _, s := range a.Sections { + have = append(have, "#"+s.Slug) + } + } + if len(have) == 0 { + return fmt.Errorf("no %s %q in %s, which has none", kind, ref, a.Path) + } + return fmt.Errorf("no %s %q in %s — it has %s", kind, ref, a.Path, listOf(have, 12)) +} + +func listOf(items []string, max int) string { + if len(items) <= max { + return strings.Join(items, ", ") + } + return strings.Join(items[:max], ", ") + fmt.Sprintf(", … (%d more)", len(items)-max) +} diff --git a/internal/artifact/artifact.go b/internal/artifact/artifact.go new file mode 100644 index 0000000..f2ac043 --- /dev/null +++ b/internal/artifact/artifact.go @@ -0,0 +1,450 @@ +// Package artifact is the navigable model of one scc artifact — a plan, or one of +// a spec's three files. +// +// It exists for a cost that is paid on every request rather than once: an agent +// that has to answer "what is the next open task?" reads the whole file to find +// out, and a plan that decomposes into twenty-seven specs is tens of kilobytes of +// prose wrapped around thirteen checkboxes. The model turns that file into +// addressable pieces — sections, tasks, requirements, decomposition leaves — each +// with a stable name and a line range, so a caller can ask for exactly the piece it +// needs and read nothing else. +// +// The addresses are the point. A task is `1.2`, a requirement is `R1.2`, a section +// is its GitHub anchor slug, a leaf is `specs//`. None of them is a line +// number, because a line number stops being true the moment anything above it +// moves — and an editor that addresses by line number is an editor that has to read +// the file first, which is the cost this package exists to remove. +// +// Parsing is layered on internal/mdscan, so fenced blocks and HTML comments are +// excluded here exactly as they are for the validators. That is not a convenience: +// scc's own templates carry their instructions in HTML comments, examples included, +// and a model that saw those examples as content would report tasks nobody wrote. +package artifact + +import ( + "fmt" + "os" + "path/filepath" + "sort" + "strings" + + "github.com/protonspy/spec-claude-code/internal/mdscan" + "github.com/protonspy/spec-claude-code/internal/paths" + "github.com/protonspy/spec-claude-code/internal/workspace" +) + +// Kind is which of scc's artifacts a file is. It is derived from the path rather +// than from the content, because the layout is the contract: specs//tasks.md +// is a task list by virtue of where it sits. +type Kind string + +const ( + KindPlan Kind = "plan" + KindRequirements Kind = "requirements" + KindDesign Kind = "design" + KindTasks Kind = "tasks" + KindDoc Kind = "doc" +) + +// Artifact is one parsed file. +// +// Lines is the raw text, kept because every edit in this package is a line splice +// against it and because a caller asking for a range wants what is actually in the +// file — comments and fences included — rather than the blanked-out Body the +// grammars are matched against. +type Artifact struct { + Path string `json:"path"` // workspace-relative, slash-separated + Abs string `json:"-"` // absolute, for reading and writing + Kind Kind `json:"kind"` // plan | requirements | design | tasks | doc + Spec string `json:"spec,omitempty"` + Name string `json:"name"` // the plan's name, or the spec's feature name + Title string `json:"title"` // the H1, or the name when there is none + + Frontmatter map[string]string `json:"frontmatter,omitempty"` + Sections []Section `json:"sections,omitempty"` + Tasks []Task `json:"tasks,omitempty"` + Requirements []Requirement `json:"requirements,omitempty"` + Leaves []Leaf `json:"leaves,omitempty"` + + Lines []string `json:"-"` + Bytes int `json:"bytes"` + + doc *mdscan.Document +} + +// LineCount is the file's length, reported next to Bytes so a caller can see at a +// glance what an outline saved it from reading. +func (a *Artifact) LineCount() int { return len(a.Lines) } + +// Doc exposes the underlying scan for callers that need the blanked-out Body — the +// searcher, which must not match inside a comment or a fence. +func (a *Artifact) Doc() *mdscan.Document { return a.doc } + +// Section is one heading and the region it governs. +// +// Two ends, because "the section" means two different things depending on who is +// asking. End covers the subtree: everything under this heading including its +// children, which is what a reader asking to see `## Decomposition` wants. BodyEnd +// stops at the first child heading, which is where an appended line has to land so +// it does not fall into the last subsection. +type Section struct { + Slug string `json:"slug"` + Level int `json:"level"` + Title string `json:"title"` + Line int `json:"line"` + End int `json:"end"` + BodyEnd int `json:"-"` + Words int `json:"words"` + Tasks int `json:"tasks,omitempty"` + Done int `json:"done,omitempty"` + Leaves int `json:"leaves,omitempty"` +} + +// Leaf is a decomposition item: a plan's reference to a spec that carries the work. +// It has no checkbox on purpose — the state lives in that spec and is read from +// there — so its progress is a roll-up rather than a field. +type Leaf struct { + Ref string `json:"ref"` // "specs//" + Feature string `json:"feature"` + Text string `json:"text"` + Line int `json:"line"` + End int `json:"end"` + Section string `json:"section,omitempty"` +} + +// Load reads and parses one artifact. abs must already be an existing file. +func Load(root, abs string) (*Artifact, error) { + b, err := os.ReadFile(abs) + if err != nil { + return nil, err + } + doc, err := mdscan.Parse(abs, string(b)) + if err != nil && doc == nil { + return nil, err + } + a := &Artifact{ + Abs: abs, + Path: relPath(root, abs), + Lines: doc.Lines, + Bytes: len(b), + doc: doc, + } + a.classify(root) + a.readFrontmatter() + a.buildSections() + a.Tasks = parseTasks(doc) + a.Requirements = parseRequirements(doc) + a.Leaves = parseLeaves(doc) + a.attribute() + return a, nil +} + +// classify decides the artifact's kind and name from where it sits. A file outside +// specs/ and plans/ is a doc: the knowledge base is Markdown too, and a caller that +// points this at docs/wiki/index.md should get a map of it rather than an error. +func (a *Artifact) classify(root string) { + parts := strings.Split(a.Path, "/") + base := parts[len(parts)-1] + switch { + case len(parts) == 2 && parts[0] == paths.PlansSeg: + a.Kind = KindPlan + a.Name = strings.TrimSuffix(base, ".md") + case len(parts) == 3 && parts[0] == paths.SpecsSeg: + a.Spec, a.Name = parts[1], parts[1] + switch base { + case paths.RequirementsSeg: + a.Kind = KindRequirements + case paths.DesignSeg: + a.Kind = KindDesign + case paths.TasksSeg: + a.Kind = KindTasks + default: + a.Kind = KindDoc + } + default: + a.Kind = KindDoc + a.Name = strings.TrimSuffix(base, ".md") + } + a.Title = a.Name + for _, h := range a.doc.Headings { + if h.Level == 1 { + a.Title = h.Text + break + } + } +} + +func (a *Artifact) readFrontmatter() { + if len(a.doc.Frontmatter.Values) == 0 { + return + } + a.Frontmatter = make(map[string]string, len(a.doc.Frontmatter.Values)) + for k, v := range a.doc.Frontmatter.Values { + a.Frontmatter[k] = v + } +} + +// buildSections computes each heading's two ends in one backward pass, then counts +// the words in its body. A heading's subtree ends where the next heading of the same +// or a shallower level begins; its body ends where the next heading of any level +// begins. +func (a *Artifact) buildSections() { + hs := a.doc.Headings + last := len(a.Lines) + a.Sections = make([]Section, 0, len(hs)) + for i, h := range hs { + s := Section{Slug: h.Slug, Level: h.Level, Title: h.Text, Line: h.Line, End: last, BodyEnd: last} + for j := i + 1; j < len(hs); j++ { + if hs[j].Level <= h.Level { + s.End = hs[j].Line - 1 + break + } + } + if i+1 < len(hs) { + s.BodyEnd = hs[i+1].Line - 1 + } + for n := h.Line; n <= s.End && n <= len(a.doc.Body); n++ { + s.Words += len(strings.Fields(a.doc.Body[n-1])) + } + a.Sections = append(a.Sections, s) + } +} + +// attribute assigns every task, requirement and leaf to the deepest heading that +// contains it, and rolls the counts back up into every ancestor. A count that +// stopped at the deepest heading would make `## Tasks` report zero in a plan whose +// tasks all sit under `### 1 · …`. +func (a *Artifact) attribute() { + for i := range a.Tasks { + a.Tasks[i].Section = a.sectionAt(a.Tasks[i].Line) + } + for i := range a.Requirements { + a.Requirements[i].Section = a.sectionAt(a.Requirements[i].Line) + } + for i := range a.Leaves { + a.Leaves[i].Section = a.sectionAt(a.Leaves[i].Line) + } + for i := range a.Sections { + s := &a.Sections[i] + for _, t := range a.Tasks { + if t.Line >= s.Line && t.Line <= s.End { + s.Tasks++ + if t.Checked { + s.Done++ + } + } + } + for _, l := range a.Leaves { + if l.Line >= s.Line && l.Line <= s.End { + s.Leaves++ + } + } + } +} + +// sectionAt is the slug of the deepest heading whose subtree contains line. +func (a *Artifact) sectionAt(line int) string { + slug := "" + for _, s := range a.Sections { + if s.Line <= line && line <= s.End { + slug = s.Slug + } + } + return slug +} + +// Section finds one section by slug, or by a title the caller typed instead. +func (a *Artifact) Section(ref string) (Section, bool) { + want := strings.TrimPrefix(ref, "#") + for _, s := range a.Sections { + if s.Slug == want { + return s, true + } + } + slug := mdscan.Slug(want) + for _, s := range a.Sections { + if s.Slug == slug || strings.EqualFold(s.Title, want) { + return s, true + } + } + return Section{}, false +} + +// Task finds one task by its number. +func (a *Artifact) Task(number string) (Task, bool) { + for _, t := range a.Tasks { + if t.Number == number { + return t, true + } + } + return Task{}, false +} + +// Requirement finds one requirement by its id. +func (a *Artifact) Requirement(id string) (Requirement, bool) { + for _, r := range a.Requirements { + if strings.EqualFold(r.ID, id) { + return r, true + } + } + return Requirement{}, false +} + +// Leaf finds one decomposition leaf by feature name or by the reference as written. +func (a *Artifact) Leaf(ref string) (Leaf, bool) { + feature := strings.Trim(strings.TrimPrefix(ref, paths.SpecsSeg+"/"), "/") + for _, l := range a.Leaves { + if l.Feature == feature { + return l, true + } + } + return Leaf{}, false +} + +// Done reports how many tasks are checked. +func (a *Artifact) Done() (done, total int) { + for _, t := range a.Tasks { + if t.Checked { + done++ + } + } + return done, len(a.Tasks) +} + +// Text returns lines [from, to] inclusive, 1-based and clamped, as they appear in +// the file. +func (a *Artifact) Text(from, to int) string { + if from < 1 { + from = 1 + } + if to > len(a.Lines) { + to = len(a.Lines) + } + if from > to { + return "" + } + return strings.Join(a.Lines[from-1:to], "\n") +} + +// Resolve turns whatever the caller typed into the artifact files it names. +// +// It accepts, in order: a path that exists relative to the workspace root, a path +// that exists relative to the working directory, a bare plan name, and a bare +// feature name — which resolves to all three of that spec's files, because a spec +// is three files and asking to map one is asking to map the spec. +func Resolve(root, arg string) ([]string, error) { + if arg == "" { + return nil, fmt.Errorf("no artifact named") + } + clean := filepath.FromSlash(strings.TrimSuffix(arg, "/")) + for _, cand := range []string{filepath.Join(root, clean), clean} { + if abs, err := filepath.Abs(cand); err == nil { + if info, err := os.Stat(abs); err == nil && info.Mode().IsRegular() { + return []string{abs}, nil + } + } + } + if err := workspace.SafeName(arg, "artifact"); err == nil { + if p := paths.Plan(root, arg); isFile(p) { + return []string{p}, nil + } + if dir := paths.Spec(root, arg); isDir(dir) { + var out []string + for _, seg := range []string{paths.RequirementsSeg, paths.DesignSeg, paths.TasksSeg} { + if p := filepath.Join(dir, seg); isFile(p) { + out = append(out, p) + } + } + if len(out) > 0 { + return out, nil + } + } + } + // A spec directory named the long way: specs// or specs/. + slashed := filepath.ToSlash(clean) + if strings.HasPrefix(slashed, paths.SpecsSeg+"/") { + return Resolve(root, strings.TrimPrefix(slashed, paths.SpecsSeg+"/")) + } + // And one file of a spec named without the specs/ prefix — `/tasks.md`, + // which is what a caller types after `map` printed the feature name at them. + if feature, seg, ok := strings.Cut(slashed, "/"); ok && !strings.Contains(seg, "/") { + if workspace.SafeName(feature, "artifact") == nil && workspace.SafeName(seg, "artifact") == nil { + if p := filepath.Join(paths.Spec(root, feature), seg); isFile(p) { + return []string{p}, nil + } + } + } + return nil, fmt.Errorf("no artifact %q under %s/ or %s/", arg, paths.PlansSeg, paths.SpecsSeg) +} + +// Scan loads every artifact in the workspace: each plan, then each spec's three +// files in phase order. It is the index every other read is a narrowing of. +func Scan(root string) ([]*Artifact, error) { + var out []*Artifact + for _, p := range planFiles(root) { + a, err := Load(root, p) + if err != nil { + return nil, err + } + out = append(out, a) + } + for _, feature := range specNames(root) { + for _, seg := range []string{paths.RequirementsSeg, paths.DesignSeg, paths.TasksSeg} { + p := filepath.Join(paths.Spec(root, feature), seg) + if !isFile(p) { + continue + } + a, err := Load(root, p) + if err != nil { + return nil, err + } + out = append(out, a) + } + } + return out, nil +} + +func planFiles(root string) []string { + entries, err := os.ReadDir(paths.Plans(root)) + if err != nil { + return nil + } + var out []string + for _, e := range entries { + if e.IsDir() || !strings.HasSuffix(e.Name(), ".md") { + continue + } + out = append(out, filepath.Join(paths.Plans(root), e.Name())) + } + sort.Strings(out) + return out +} + +func specNames(root string) []string { + entries, err := os.ReadDir(paths.Specs(root)) + if err != nil { + return nil + } + var out []string + for _, e := range entries { + if e.IsDir() { + out = append(out, e.Name()) + } + } + sort.Strings(out) + return out +} + +func relPath(root, target string) string { + return filepath.ToSlash(workspace.Relative(root, target)) +} + +func isFile(path string) bool { + info, err := os.Stat(path) + return err == nil && info.Mode().IsRegular() +} + +func isDir(path string) bool { + info, err := os.Stat(path) + return err == nil && info.IsDir() +} diff --git a/internal/artifact/artifact_test.go b/internal/artifact/artifact_test.go new file mode 100644 index 0000000..af0eeb5 --- /dev/null +++ b/internal/artifact/artifact_test.go @@ -0,0 +1,319 @@ +package artifact + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +// samplePlan is shaped like the artifacts this package was measured against: tasks +// whose description runs past the checkbox line, a decomposition of leaves that +// carry no boxes, and a Notes section with no headings to navigate by. +const samplePlan = `--- +autonomy: auto +ci: wait +--- + +# Sample — the pipeline + +## Why + +One paragraph about why this exists, and what done means. + +## Decomposition + +- ` + "`specs/job-store/`" + ` — one file per job, atomic writes, and the request id + that recovers an already-paid result after a crash. +- ` + "`specs/model-registry/`" + ` — validation against the model's schema before + anything is submitted. + +## Tasks + +- [x] 1.1 (Unit) Build the thing — write the wrapper, vendor the module, and prove + with a test that it loads +- [ ] 1.2 (TDD) Guard the message before it lands, because HTTP clients embed the + full URL and sometimes a credential with it + +## Notes + +**Order matters.** job-store first, because every other leaf writes through it and +a second writer would race the first. + +**The free path wins.** The expensive command is the one that has to know about the +cheap one, never the other way round. +` + +func writeWorkspace(t *testing.T) (root, planPath string) { + t.Helper() + root = t.TempDir() + if err := os.MkdirAll(filepath.Join(root, "plans"), 0o755); err != nil { + t.Fatal(err) + } + planPath = filepath.Join(root, "plans", "sample.md") + if err := os.WriteFile(planPath, []byte(samplePlan), 0o644); err != nil { + t.Fatal(err) + } + return root, planPath +} + +func load(t *testing.T) (*Artifact, string) { + t.Helper() + root, path := writeWorkspace(t) + a, err := Load(root, path) + if err != nil { + t.Fatalf("Load: %v", err) + } + return a, root +} + +func TestLoadClassifiesAndTitles(t *testing.T) { + a, _ := load(t) + if a.Kind != KindPlan { + t.Errorf("Kind = %q, want %q", a.Kind, KindPlan) + } + if a.Name != "sample" { + t.Errorf("Name = %q, want %q", a.Name, "sample") + } + if a.Title != "Sample — the pipeline" { + t.Errorf("Title = %q", a.Title) + } + if a.Path != "plans/sample.md" { + t.Errorf("Path = %q, want slash-separated and root-relative", a.Path) + } + if a.Frontmatter["autonomy"] != "auto" { + t.Errorf("frontmatter not read: %v", a.Frontmatter) + } +} + +// A task's description runs past its checkbox line in every real artifact. A model +// that ended a task at its first line would address two lines of a five-line item. +func TestTaskCoversItsContinuation(t *testing.T) { + a, _ := load(t) + if len(a.Tasks) != 2 { + t.Fatalf("got %d tasks, want 2", len(a.Tasks)) + } + one := a.Tasks[0] + if one.End <= one.Line { + t.Errorf("task 1.1 ends at %d, started at %d — continuation not captured", one.End, one.Line) + } + if !strings.Contains(one.Detail, "with a test that it loads") { + t.Errorf("Detail missing the continuation: %q", one.Detail) + } + if strings.Contains(one.Text, "with a test") { + t.Errorf("Text swallowed the continuation: %q — a listing prints one line per task", one.Text) + } + if one.Methodology != "Unit" || !one.Checked { + t.Errorf("1.1 parsed as %+v", one) + } + if a.Tasks[1].Methodology != "TDD" || a.Tasks[1].Checked { + t.Errorf("1.2 parsed as %+v", a.Tasks[1]) + } +} + +// A leaf carries no checkbox: its state lives in the spec it names. Counting one as +// a task would double-count the work and report progress nobody made. +func TestLeavesAreNotTasks(t *testing.T) { + a, _ := load(t) + if len(a.Leaves) != 2 { + t.Fatalf("got %d leaves, want 2: %+v", len(a.Leaves), a.Leaves) + } + if a.Leaves[0].Feature != "job-store" { + t.Errorf("Feature = %q", a.Leaves[0].Feature) + } + if !strings.Contains(a.Leaves[0].Text, "one file per job") { + t.Errorf("leaf text = %q", a.Leaves[0].Text) + } + done, total := a.Done() + if done != 1 || total != 2 { + t.Errorf("Done() = %d/%d, want 1/2 — leaves must not count as tasks", done, total) + } +} + +// A section's counts roll up from its children. Without that, `## Tasks` reports +// zero in any plan whose tasks sit under numbered subsections. +func TestSectionCountsRollUp(t *testing.T) { + a, _ := load(t) + s, ok := a.Section("tasks") + if !ok { + t.Fatal("no #tasks section") + } + if s.Tasks != 2 || s.Done != 1 { + t.Errorf("#tasks = %d/%d, want 1/2", s.Done, s.Tasks) + } + if d, ok := a.Section("decomposition"); !ok || d.Leaves != 2 { + t.Errorf("#decomposition leaves = %d, want 2", d.Leaves) + } +} + +func TestBlocksIndexParagraphs(t *testing.T) { + a, _ := load(t) + var notes []Block + for _, b := range a.Blocks() { + if b.Section == "notes" { + notes = append(notes, b) + } + } + if len(notes) != 2 { + t.Fatalf("got %d note paragraphs, want 2: %+v", len(notes), notes) + } + if !strings.HasPrefix(notes[0].Lead, "Order matters.") { + t.Errorf("lead = %q, want the emphasis stripped", notes[0].Lead) + } + if notes[0].End <= notes[0].Line { + t.Errorf("block ends at %d, started at %d", notes[0].End, notes[0].Line) + } +} + +func TestFindResolvesEveryAddressForm(t *testing.T) { + a, _ := load(t) + for _, tc := range []struct { + ref string + kind TargetKind + }{ + {"1.2", TargetTask}, + {"specs/job-store/", TargetLeaf}, + {"#notes", TargetSection}, + {"Notes", TargetSection}, + {"notes:1", TargetBlock}, + {"L1-3", TargetRange}, + } { + got, err := a.Find(tc.ref) + if err != nil { + t.Errorf("Find(%q): %v", tc.ref, err) + continue + } + if got.Kind != tc.kind { + t.Errorf("Find(%q).Kind = %q, want %q", tc.ref, got.Kind, tc.kind) + } + if got.Line < 1 || got.End < got.Line { + t.Errorf("Find(%q) = lines %d-%d", tc.ref, got.Line, got.End) + } + } +} + +// An address that misses must say what the file does have. The caller is an agent +// that cannot see the file, so "not found" alone costs it another round trip. +func TestFindNamesWhatItHasOnAMiss(t *testing.T) { + a, _ := load(t) + _, err := a.Find("9.9") + if err == nil { + t.Fatal("Find(9.9) succeeded") + } + if !strings.Contains(err.Error(), "1.1") || !strings.Contains(err.Error(), "1.2") { + t.Errorf("error does not list the tasks that exist: %v", err) + } +} + +func TestResolveAcceptsPathAndBareName(t *testing.T) { + root, planPath := writeWorkspace(t) + for _, arg := range []string{"plans/sample.md", "sample", planPath} { + got, err := Resolve(root, arg) + if err != nil { + t.Errorf("Resolve(%q): %v", arg, err) + continue + } + if len(got) != 1 { + t.Errorf("Resolve(%q) = %v, want one file", arg, got) + continue + } + same, err := sameFile(got[0], planPath) + if err != nil || !same { + t.Errorf("Resolve(%q) = %q, want %q", arg, got[0], planPath) + } + } +} + +// A name that would escape the workspace must not become a path segment. +func TestResolveRefusesEscape(t *testing.T) { + root, _ := writeWorkspace(t) + if got, err := Resolve(root, ".."); err == nil { + t.Errorf("Resolve(..) = %v, want an error", got) + } +} + +func sameFile(a, b string) (bool, error) { + ai, err := os.Stat(a) + if err != nil { + return false, err + } + bi, err := os.Stat(b) + if err != nil { + return false, err + } + return os.SameFile(ai, bi), nil +} + +func TestSearchRanksAddressableUnits(t *testing.T) { + a, _ := load(t) + hits, err := Search([]*Artifact{a}, "credential URL", SearchOpts{}) + if err != nil { + t.Fatalf("Search: %v", err) + } + if len(hits) == 0 { + t.Fatal("no hits") + } + if hits[0].Ref != "1.2" { + t.Errorf("top hit = %q, want task 1.2", hits[0].Ref) + } + if hits[0].Line < 1 || hits[0].Snippet == "" { + t.Errorf("hit is not usable as an address: %+v", hits[0]) + } +} + +// Terms are ANDed by default: a reader asking for two words means both, and an OR +// over a small corpus returns everything. +func TestSearchDefaultsToAllTerms(t *testing.T) { + a, _ := load(t) + all, _ := Search([]*Artifact{a}, "credential vendor", SearchOpts{}) + any, _ := Search([]*Artifact{a}, "credential vendor", SearchOpts{Any: true}) + if len(all) >= len(any) { + t.Errorf("AND returned %d hits and OR returned %d; AND must be the narrower", len(all), len(any)) + } +} + +func TestSearchByKind(t *testing.T) { + a, _ := load(t) + hits, _ := Search([]*Artifact{a}, "job", SearchOpts{Kind: string(TargetLeaf)}) + for _, h := range hits { + if h.Kind != string(TargetLeaf) { + t.Errorf("--kind leaf returned a %s", h.Kind) + } + } +} + +// The forms a caller actually types after a listing printed the feature name at +// them. `/tasks.md` is the one that is easy to leave out and the one that +// gets typed most. +func TestResolveAcceptsSpecRelativeForms(t *testing.T) { + root := t.TempDir() + dir := filepath.Join(root, "specs", "job-store") + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatal(err) + } + want := filepath.Join(dir, "tasks.md") + if err := os.WriteFile(want, []byte("# T\n"), 0o644); err != nil { + t.Fatal(err) + } + for _, arg := range []string{ + "specs/job-store/tasks.md", + "job-store/tasks.md", + } { + got, err := Resolve(root, arg) + if err != nil { + t.Errorf("Resolve(%q): %v", arg, err) + continue + } + if len(got) != 1 { + t.Errorf("Resolve(%q) = %v, want one file", arg, got) + continue + } + if same, err := sameFile(got[0], want); err != nil || !same { + t.Errorf("Resolve(%q) = %q, want %q", arg, got[0], want) + } + } + // And naming the spec itself is all three of its files, minus the ones absent. + if got, err := Resolve(root, "job-store"); err != nil || len(got) != 1 { + t.Errorf("Resolve(job-store) = %v, %v", got, err) + } +} diff --git a/internal/artifact/edit.go b/internal/artifact/edit.go new file mode 100644 index 0000000..53d6d7d --- /dev/null +++ b/internal/artifact/edit.go @@ -0,0 +1,436 @@ +package artifact + +import ( + "fmt" + "sort" + "strings" +) + +// Editing an artifact without reading it. +// +// This is the half of the package that exists for a policy rather than for a cost: +// an agent harness makes a tool read a file before it may edit it, because a +// string-match edit against a file nobody looked at is a footgun. That guard is +// right for a text editor and wrong for a structured artifact — every edit here is +// addressed by a name the caller already knows (`1.2`, `#notes`, `R1.4`), applies to +// a region the parser found rather than to a string the caller guessed, and reports +// exactly which lines it replaced. The file never has to be in anyone's context. +// +// The safety that replaces reading-first is threefold: an address that does not +// resolve is an error and not an insert; overlapping edits are refused rather than +// merged; and the caller gets the before/after lines back, which is the confirmation +// a read was standing in for. + +// LineWidth is where a rewritten task or paragraph wraps. It matches what the +// artifacts already do, so an edited file does not announce itself by its ragging. +const LineWidth = 88 + +// Change is one applied splice, in the form a caller can print as confirmation. +type Change struct { + Op string `json:"op"` + Ref string `json:"ref"` + Line int `json:"line"` + Before []string `json:"before,omitempty"` + After []string `json:"after,omitempty"` +} + +// splice replaces del lines starting at line at (1-based) with lines. del == 0 is a +// pure insertion before at. +// +// seq is the order the caller asked for, and it matters only where several +// insertions share one position — `patch fm a=1 b=2 c=3` on a file that has none of +// them. Applying bottom-up means those have to go in reverse, or the caller's order +// comes out backwards in the file. +type splice struct { + at int + del int + seq int + lines []string + ch Change +} + +// Editor accumulates edits against one artifact and applies them at the end. +// +// Every address is resolved against the *original* file, and the splices are applied +// from the bottom up, so a batch of edits cannot invalidate its own line numbers +// halfway through. Two edits that touch the same region are a mistake rather than a +// merge, and are refused. +type Editor struct { + a *Artifact + splices []splice + err error +} + +// Edit opens an editor over the artifact. +func (a *Artifact) Edit() *Editor { return &Editor{a: a} } + +// Err is the first failure, if any. Every method is a no-op once one has happened, +// so a caller can chain and check once. +func (e *Editor) Err() error { return e.err } + +func (e *Editor) fail(format string, args ...any) { + if e.err == nil { + e.err = fmt.Errorf(format, args...) + } +} + +// Empty reports whether nothing would change. +func (e *Editor) Empty() bool { return len(e.splices) == 0 } + +// Changes is what the editor did, in file order. +func (e *Editor) Changes() []Change { + out := make([]Change, 0, len(e.splices)) + ordered := append([]splice(nil), e.splices...) + sort.SliceStable(ordered, func(i, j int) bool { return ordered[i].at < ordered[j].at }) + for _, s := range ordered { + out = append(out, s.ch) + } + return out +} + +// Content applies every splice and returns the new file, LF-terminated lines joined +// the way the whole tree writes Markdown. +func (e *Editor) Content() (string, error) { + if e.err != nil { + return "", e.err + } + ordered := append([]splice(nil), e.splices...) + sort.SliceStable(ordered, func(i, j int) bool { + if ordered[i].at != ordered[j].at { + return ordered[i].at > ordered[j].at + } + return ordered[i].seq > ordered[j].seq + }) + for i := 1; i < len(ordered); i++ { + prev, cur := ordered[i-1], ordered[i] + if cur.at+cur.del > prev.at { + return "", fmt.Errorf("two edits touch the same lines (%s and %s); apply them one at a time", + cur.ch.Ref, prev.ch.Ref) + } + } + lines := append([]string(nil), e.a.Lines...) + for _, s := range ordered { + head := append([]string(nil), lines[:s.at-1]...) + tail := lines[s.at-1+s.del:] + lines = append(append(head, s.lines...), tail...) + } + return strings.Join(lines, "\n"), nil +} + +// add records a splice, capturing the lines it displaced for the report. +func (e *Editor) add(op, ref string, at, del int, lines []string) { + if e.err != nil { + return + } + var before []string + if del > 0 { + before = append([]string(nil), e.a.Lines[at-1:at-1+del]...) + } + e.splices = append(e.splices, splice{ + at: at, del: del, seq: len(e.splices), lines: lines, + ch: Change{Op: op, Ref: ref, Line: at, Before: before, After: lines}, + }) +} + +// Check marks a task done or not done. +// +// It rewrites the box and nothing else — not the number, not the description, not +// the wrapping — because the state of a task is the one fact the box holds, and an +// edit that reflowed the line while flipping it would put a diff in front of a +// reviewer that hides what actually changed. +func (e *Editor) Check(number string, done bool) { + if e.err != nil { + return + } + t, ok := e.a.Task(number) + if !ok { + e.fail("%v", e.a.unknown("task", number)) + return + } + if t.Checked == done { + return // already in that state; not an edit, and not an error either + } + line := e.a.Lines[t.Line-1] + from, to := "[ ]", "[x]" + if !done { + from, to = "[x]", "[ ]" + } + i := strings.Index(strings.ToLower(line), strings.ToLower(from)) + if i < 0 { + e.fail("task %s does not carry a checkbox on line %d", number, t.Line) + return + } + op := "uncheck" + if done { + op = "check" + } + e.add(op, number, t.Line, 1, []string{line[:i] + to + line[i+3:]}) +} + +// TaskEdit is what SetTask may change. A nil field is left alone, which is what +// makes "change only the methodology" expressible without restating the description. +type TaskEdit struct { + Text *string + Methodology *string + Requirements *[]string + Number *string + Checked *bool +} + +// SetTask rewrites one task in place, re-rendering it from its parts. +// +// The whole block is replaced, continuation lines included, because a task's +// description is mostly in its continuation — every task in a real plan runs past +// one line — and an edit that touched only the first line would leave the rest +// describing the old task. +func (e *Editor) SetTask(number string, edit TaskEdit) { + if e.err != nil { + return + } + t, ok := e.a.Task(number) + if !ok { + e.fail("%v", e.a.unknown("task", number)) + return + } + next := t + if edit.Number != nil { + next.Number = *edit.Number + } + if edit.Methodology != nil { + next.Methodology = *edit.Methodology + } + if edit.Checked != nil { + next.Checked = *edit.Checked + } + if edit.Requirements != nil { + next.Requirements = *edit.Requirements + } + if edit.Text != nil { + next.Text = strings.TrimSpace(*edit.Text) + next.Detail = "" + } + e.add("set", number, t.Line, t.End-t.Line+1, renderTask(next)) +} + +// NewTask is a task about to be written. Section is where it lands: the slug of the +// heading whose task list it joins. +type NewTask struct { + Section string + Number string + Methodology string + Text string + Requirements []string + Checked bool +} + +// AddTask appends a task to a section's list. +// +// It lands after the last task already in that section — not at the end of the +// section — so a section that ends in prose keeps that prose last, which is where +// every artifact scc writes puts it. +func (e *Editor) AddTask(t NewTask) { + if e.err != nil { + return + } + s, ok := e.a.Section(t.Section) + if !ok { + e.fail("%v", e.a.unknown("section", t.Section)) + return + } + if t.Number == "" { + e.fail("a task needs a number: it is how every later command addresses it") + return + } + if _, taken := e.a.Task(t.Number); taken { + e.fail("task %s already exists in %s", t.Number, e.a.Path) + return + } + at := s.Line + 1 + for _, existing := range e.a.Tasks { + if existing.Line >= s.Line && existing.Line <= s.End && existing.End+1 > at { + at = existing.End + 1 + } + } + if at == s.Line+1 { + // No tasks yet: land after the section's own prose, before its first + // subsection, past the blank lines that trail it. + at = trimBlank(e.a.Lines, s.Line+1, s.BodyEnd) + 1 + } + lines := renderTask(Task{ + Number: t.Number, Methodology: t.Methodology, Text: t.Text, + Requirements: t.Requirements, Checked: t.Checked, + }) + e.add("add", t.Number, at, 0, lines) +} + +// RemoveTask deletes a task and its continuation. +func (e *Editor) RemoveTask(number string) { + if e.err != nil { + return + } + t, ok := e.a.Task(number) + if !ok { + e.fail("%v", e.a.unknown("task", number)) + return + } + e.add("remove", number, t.Line, t.End-t.Line+1, nil) +} + +// Replace swaps the target's lines for text. +func (e *Editor) Replace(ref, text string) { + if e.err != nil { + return + } + target, err := e.a.Find(ref) + if err != nil { + e.fail("%v", err) + return + } + if target.Kind == TargetSection { + // Replacing a section means replacing what it says, not deleting its heading: + // an edit that removed the heading would move every following section up a + // level and silently re-parent the file. + e.add("replace", ref, target.Line+1, target.End-target.Line, blockLines(text)) + return + } + e.add("replace", ref, target.Line, target.Lines(), blockLines(text)) +} + +// Append writes text after the target. +// +// For a section that means after its own prose and before its first subsection, +// never inside the last one — appending to `## Decomposition` must not silently join +// the milestone that happens to be last. +func (e *Editor) Append(ref, text string) { + if e.err != nil { + return + } + target, err := e.a.Find(ref) + if err != nil { + e.fail("%v", err) + return + } + end := target.End + if target.Kind == TargetSection { + if s, ok := e.a.Section(target.Ref); ok { + end = s.BodyEnd + } + } + at := trimBlank(e.a.Lines, target.Line, end) + 1 + e.add("append", ref, at, 0, append([]string{""}, blockLines(text)...)) +} + +// Prepend writes text immediately after the target's first line — under a heading, +// above whatever it already says. +func (e *Editor) Prepend(ref, text string) { + if e.err != nil { + return + } + target, err := e.a.Find(ref) + if err != nil { + e.fail("%v", err) + return + } + at := target.Line + if target.Kind == TargetSection { + at = target.Line + 1 + } + e.add("prepend", ref, at, 0, append(blockLines(text), "")) +} + +// SetFrontmatter writes one key in the leading block, adding the block if the file +// has none. It is how the plan-run loop records its answers — worktree, merge, pr — +// without the skill having to hold the file. +func (e *Editor) SetFrontmatter(key, value string) { + if e.err != nil { + return + } + line := key + ": " + value + fm := e.a.doc.Frontmatter + if !fm.Present { + e.add("frontmatter", key, 1, 0, []string{"---", line, "---", ""}) + return + } + for n := 2; n < fm.Lines; n++ { + k, v, ok := strings.Cut(e.a.Lines[n-1], ":") + if !ok || strings.TrimSpace(k) != key { + continue + } + // Writing the value it already has is not an edit. The plan-run loop restates + // every answer on every resume, and half of them are unchanged; reporting those + // as changes would bury the one that moved. + if strings.TrimSpace(v) == value { + return + } + e.add("frontmatter", key, n, 1, []string{line}) + return + } + e.add("frontmatter", key, fm.Lines, 0, []string{line}) +} + +// renderTask writes a task back out in the grammar, wrapped the way the artifacts +// wrap: the continuation indented to sit under the description rather than under the +// bullet, which is what makes a multi-line task read as one item. +func renderTask(t Task) []string { + head := "- [ ] " + if t.Checked { + head = "- [x] " + } + body := t.Number + if t.Methodology != "" { + body += " (" + t.Methodology + ")" + } + if text := strings.TrimSpace(t.Text); text != "" { + body += " " + text + } + if len(t.Requirements) > 0 { + body += " " + CitationSeparator + " " + strings.Join(t.Requirements, ", ") + } + return wrap(head, strings.Repeat(" ", len(head)), body, LineWidth) +} + +// wrap breaks text into lines that fit width, with prefix on the first and indent on +// the rest. A word longer than the budget goes on its own line rather than being +// broken: the artifacts are full of paths and backticked identifiers, and a broken +// one stops being a name. +func wrap(prefix, indent, text string, width int) []string { + words := strings.Fields(text) + if len(words) == 0 { + return []string{strings.TrimRight(prefix, " ")} + } + var out []string + cur := prefix + words[0] + for _, w := range words[1:] { + if len([]rune(cur))+1+len([]rune(w)) > width { + out = append(out, cur) + cur = indent + w + continue + } + cur += " " + w + } + return append(out, cur) +} + +// blockLines splits caller-supplied text into lines, normalizing the line endings a +// shell on Windows may have handed over. +func blockLines(text string) []string { + text = strings.ReplaceAll(text, "\r\n", "\n") + text = strings.TrimRight(text, "\n") + if text == "" { + return nil + } + return strings.Split(text, "\n") +} + +// trimBlank is the last non-blank line in [from, to], or from-1 when there is none. +func trimBlank(lines []string, from, to int) int { + if to > len(lines) { + to = len(lines) + } + for n := to; n >= from; n-- { + if strings.TrimSpace(lines[n-1]) != "" { + return n + } + } + return from - 1 +} diff --git a/internal/artifact/edit_test.go b/internal/artifact/edit_test.go new file mode 100644 index 0000000..f796276 --- /dev/null +++ b/internal/artifact/edit_test.go @@ -0,0 +1,316 @@ +package artifact + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +func edited(t *testing.T, apply func(*Editor)) (*Artifact, string) { + t.Helper() + a, _ := load(t) + e := a.Edit() + apply(e) + content, err := e.Content() + if err != nil { + t.Fatalf("Content: %v", err) + } + return a, content +} + +// Flipping a box must touch the box and nothing else. An edit that reflowed the line +// while checking it would put a diff in front of a reviewer that hides what changed. +func TestCheckTouchesOnlyTheBox(t *testing.T) { + a, content := edited(t, func(e *Editor) { e.Check("1.2", true) }) + before, after := strings.Split(samplePlan, "\n"), strings.Split(content, "\n") + if len(before) != len(after) { + t.Fatalf("line count changed: %d → %d", len(before), len(after)) + } + changed := 0 + for i := range before { + if before[i] != after[i] { + changed++ + if !strings.Contains(after[i], "[x] 1.2") { + t.Errorf("line %d changed to %q, want the box flipped", i+1, after[i]) + } + } + } + if changed != 1 { + t.Errorf("%d lines changed, want exactly 1", changed) + } + _ = a +} + +// Checking a task that is already checked is not an edit and not an error either. +// A loop that re-runs the same step must be able to say so without failing. +func TestCheckIsIdempotent(t *testing.T) { + a, _ := load(t) + e := a.Edit() + e.Check("1.1", true) + if !e.Empty() { + t.Error("checking an already-checked task produced an edit") + } + if e.Err() != nil { + t.Errorf("and reported an error: %v", e.Err()) + } +} + +// Unchecking and re-checking must restore the file byte for byte. Anything less +// means every no-op pass of a loop leaves a diff behind. +func TestCheckRoundTripsExactly(t *testing.T) { + a, root := load(t) + e := a.Edit() + e.Check("1.1", false) + once, err := e.Content() + if err != nil { + t.Fatal(err) + } + if err := writeAt(a.Abs, once); err != nil { + t.Fatal(err) + } + + b, err := Load(root, a.Abs) + if err != nil { + t.Fatal(err) + } + e2 := b.Edit() + e2.Check("1.1", true) + twice, err := e2.Content() + if err != nil { + t.Fatal(err) + } + if twice != samplePlan { + t.Errorf("round trip did not restore the file:\n%q", twice) + } +} + +// An address that does not resolve is an error, never an insert at a guess. This is +// the property that lets a caller edit a file it has not read. +func TestUnknownAddressIsAnError(t *testing.T) { + a, _ := load(t) + for name, apply := range map[string]func(*Editor){ + "check": func(e *Editor) { e.Check("9.9", true) }, + "set": func(e *Editor) { e.SetTask("9.9", TaskEdit{}) }, + "remove": func(e *Editor) { e.RemoveTask("9.9") }, + "append": func(e *Editor) { e.Append("#nope", "x") }, + "replace": func(e *Editor) { e.Replace("#nope", "x") }, + "add": func(e *Editor) { e.AddTask(NewTask{Section: "nope", Number: "3.1", Text: "x"}) }, + } { + e := a.Edit() + apply(e) + if e.Err() == nil { + t.Errorf("%s against a missing address succeeded", name) + } + if _, err := e.Content(); err == nil { + t.Errorf("%s: Content() succeeded after a failed edit", name) + } + } +} + +// A task's description lives mostly in its continuation, so rewriting one must +// replace the whole block. Leaving the tail would describe the old task. +func TestSetTaskReplacesTheWholeBlock(t *testing.T) { + text := "Rewritten" + _, content := edited(t, func(e *Editor) { e.SetTask("1.1", TaskEdit{Text: &text}) }) + if strings.Contains(content, "vendor the module") { + t.Error("the old continuation survived the rewrite") + } + if !strings.Contains(content, "- [x] 1.1 (Unit) Rewritten") { + t.Errorf("rewritten task not found:\n%s", content) + } +} + +func TestAddTaskLandsAfterTheLastTaskInItsSection(t *testing.T) { + _, content := edited(t, func(e *Editor) { + e.AddTask(NewTask{Section: "tasks", Number: "1.3", Methodology: "Unit", Text: "A third thing"}) + }) + lines := strings.Split(content, "\n") + last, added, notes := -1, -1, -1 + for i, l := range lines { + switch { + case strings.Contains(l, "1.2 (TDD)"): + last = i + case strings.Contains(l, "1.3 (Unit)"): + added = i + case strings.HasPrefix(l, "## Notes"): + notes = i + } + } + if added < 0 { + t.Fatalf("task not added:\n%s", content) + } + if !(added > last && added < notes) { + t.Errorf("1.3 landed at %d; want between task 1.2 (%d) and ## Notes (%d)", added, last, notes) + } +} + +func TestAddTaskRefusesADuplicateNumber(t *testing.T) { + a, _ := load(t) + e := a.Edit() + e.AddTask(NewTask{Section: "tasks", Number: "1.1", Text: "collides"}) + if e.Err() == nil { + t.Error("adding a task with a taken number succeeded") + } +} + +// Replacing a section replaces what it says, never its heading: removing the heading +// would move every following section up a level and silently re-parent the file. +func TestReplaceSectionKeepsTheHeading(t *testing.T) { + _, content := edited(t, func(e *Editor) { e.Replace("#why", "Something else entirely.") }) + if !strings.Contains(content, "## Why") { + t.Error("the heading was removed") + } + if strings.Contains(content, "what done means") { + t.Error("the old body survived") + } + if !strings.Contains(content, "Something else entirely.") { + t.Error("the new body is missing") + } +} + +// Appending to a section with subsections must land in that section's own body, not +// inside whichever subsection happens to be last. +func TestAppendToSectionStopsAtTheFirstSubsection(t *testing.T) { + const nested = `# T + +## Outer + +Intro. + +### Inner + +Inner body. + +## After + +Tail. +` + a := parseString(t, nested) + e := a.Edit() + e.Append("#outer", "Appended.") + content, err := e.Content() + if err != nil { + t.Fatal(err) + } + lines := strings.Split(content, "\n") + appended, inner := -1, -1 + for i, l := range lines { + if strings.HasPrefix(l, "Appended.") { + appended = i + } + if strings.HasPrefix(l, "### Inner") { + inner = i + } + } + if appended < 0 || appended > inner { + t.Errorf("appended at %d, want above ### Inner at %d:\n%s", appended, inner, content) + } +} + +func TestSetFrontmatterReplacesAndAdds(t *testing.T) { + _, content := edited(t, func(e *Editor) { + e.SetFrontmatter("ci", "no-wait") + e.SetFrontmatter("pr", "per-plan") + }) + if !strings.Contains(content, "ci: no-wait") || strings.Contains(content, "ci: wait\n") { + t.Errorf("existing key not replaced:\n%s", firstLines(content, 8)) + } + if !strings.Contains(content, "pr: per-plan") { + t.Errorf("new key not added:\n%s", firstLines(content, 8)) + } +} + +// Two edits to the same region are a mistake rather than a merge. Silently applying +// both would produce a file neither caller asked for. +func TestOverlappingEditsAreRefused(t *testing.T) { + a, _ := load(t) + e := a.Edit() + text := "one" + e.SetTask("1.1", TaskEdit{Text: &text}) + e.RemoveTask("1.1") + if _, err := e.Content(); err == nil { + t.Error("two edits to the same task were applied") + } +} + +// Every address is resolved against the original file, and splices apply bottom-up, +// so a batch cannot invalidate its own line numbers halfway through. +func TestBatchedEditsDoNotShiftEachOther(t *testing.T) { + _, content := edited(t, func(e *Editor) { + e.Check("1.2", true) + e.Append("#why", "A second paragraph.") + e.SetFrontmatter("pr", "per-group") + }) + for _, want := range []string{"[x] 1.2", "A second paragraph.", "pr: per-group"} { + if !strings.Contains(content, want) { + t.Errorf("missing %q from the batch:\n%s", want, content) + } + } + if !strings.Contains(content, "## Decomposition") { + t.Error("the batch corrupted the document structure") + } +} + +func parseString(t *testing.T, content string) *Artifact { + t.Helper() + root := t.TempDir() + path := root + "/plans/x.md" + if err := writeAt(path, content); err != nil { + t.Fatal(err) + } + a, err := Load(root, path) + if err != nil { + t.Fatal(err) + } + return a +} + +func firstLines(s string, n int) string { + lines := strings.Split(s, "\n") + if len(lines) > n { + lines = lines[:n] + } + return strings.Join(lines, "\n") +} + +// writeAt is the test-local file writer; the package's own writes go through +// workspace.AtomicWrite from the CLI, which is not what these tests are exercising. +func writeAt(path, content string) error { + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + return err + } + return os.WriteFile(path, []byte(content), 0o644) +} + +// Several keys written in one call must come out in the order the caller asked for. +// Splices apply bottom-up, so insertions sharing one position have to be reversed on +// the way in or the file reads backwards. +func TestSetFrontmatterKeepsTheOrderAsked(t *testing.T) { + _, content := edited(t, func(e *Editor) { + e.SetFrontmatter("pr", "per-plan") + e.SetFrontmatter("worktree", "per-group") + e.SetFrontmatter("merge", "auto") + }) + pr := strings.Index(content, "pr: per-plan") + wt := strings.Index(content, "worktree: per-group") + mg := strings.Index(content, "merge: auto") + if pr < 0 || wt < 0 || mg < 0 { + t.Fatalf("a key is missing:\n%s", firstLines(content, 10)) + } + if !(pr < wt && wt < mg) { + t.Errorf("keys came out reversed:\n%s", firstLines(content, 10)) + } +} + +// Writing the value a key already has is not an edit. A resumed loop restates every +// answer, and reporting the unchanged ones would bury the one that moved. +func TestSetFrontmatterIsIdempotent(t *testing.T) { + a, _ := load(t) + e := a.Edit() + e.SetFrontmatter("autonomy", "auto") + e.SetFrontmatter("ci", "wait") + if !e.Empty() { + t.Errorf("re-writing the existing values produced %d changes", len(e.Changes())) + } +} diff --git a/internal/artifact/parse.go b/internal/artifact/parse.go new file mode 100644 index 0000000..2c5f36b --- /dev/null +++ b/internal/artifact/parse.go @@ -0,0 +1,348 @@ +package artifact + +import ( + "regexp" + "strings" + + "github.com/protonspy/spec-claude-code/internal/ears" + "github.com/protonspy/spec-claude-code/internal/mdscan" + "github.com/protonspy/spec-claude-code/internal/paths" +) + +// The grammars, in one place. Every consumer reads the same regexes: the validators +// that report on them, and the reader that navigates by them. A reader that +// disagreed with the validator about what a task is would be worse than no reader — +// it would report a task list the validator says does not exist. +var ( + taskNumberRe = regexp.MustCompile(`^(\d+(?:\.\d+)*)\s+(.*)$`) + // MethodologyRe matches the annotation that says how a task gets built. + MethodologyRe = regexp.MustCompile(`\((Unit|TDD)\)`) + // LooseMethodologyRe matches the near-misses, so a task annotated `(test-first)` + // gets told what the vocabulary is instead of being told it has none. + LooseMethodologyRe = regexp.MustCompile(`(?i)\((unit|tdd|test[- ]?first|none)\)`) + // RequirementIDRe matches a citation of a requirement, anywhere. + RequirementIDRe = regexp.MustCompile(`\bR\d+(?:\.\d+)+\b`) + // RequirementRe matches a numbered requirement, and only in that position: a + // bulleted item whose first content is the bolded id. + RequirementRe = regexp.MustCompile(`^\s*[-*+]\s+\*\*(R\d+(?:\.\d+)+)\*\*\s*(?:\(([^)]*)\)\s*)?(.*)$`) + // SpecRefRe matches a reference to a spec from inside a plan. + SpecRefRe = regexp.MustCompile(`\b` + paths.SpecsSeg + `/([a-z0-9][a-z0-9-]*)/?`) + + listItemRe = regexp.MustCompile(`^([ \t]*)[-*+]\s+(.+)$`) +) + +// CitationSeparator is the em dash that opens a task's requirement citations, per +// the grammar in the tasks rule. +const CitationSeparator = "—" + +// Task is one checkbox read against the task grammar: +// +// - [ ] 1.1 (Unit) Parse the manifest file — R1.2, R1.4 +// +// The exported fields describe the task; the four unexported-by-convention grammar +// facts at the bottom describe what the *line* did or did not say, and exist so the +// validator can report a defect without parsing the line a second time. The parser +// states facts; turning a fact into a finding is the validator's job. +// +// The grammar is read from the checkbox's own line only. In practice a task's +// description runs on for several lines — every task in a real plan does — and +// Detail carries that continuation, but a citation wrapped onto the next line is a +// citation the validator already reports, and moving the goalposts here would change +// what eight validators say about files nobody edited. +type Task struct { + Number string `json:"number"` + Checked bool `json:"checked"` + Methodology string `json:"methodology,omitempty"` + Text string `json:"text"` + Detail string `json:"detail,omitempty"` + Requirements []string `json:"requirements,omitempty"` + Line int `json:"line"` + End int `json:"end"` + Indent int `json:"-"` + Section string `json:"section,omitempty"` + + Methodologies int `json:"-"` // how many annotations the line carried + Loose string `json:"-"` // a near-miss annotation, when there is no valid one + HasCitation bool `json:"-"` // the line carried the em dash that opens citations +} + +// Group is the task's number with its last component removed — the heading it +// belongs to under scc's numbering, and what `--group` filters on. +func (t Task) Group() string { + if i := strings.LastIndex(t.Number, "."); i > 0 { + return t.Number[:i] + } + return t.Number +} + +// Summary is the task's description clipped to n runes, for a listing where one +// task is one line. It is the first line only: a 61-line task exists, and printing +// it in a list would defeat the point of the list. +func (t Task) Summary(n int) string { return clip(t.Text, n) } + +// Requirement is one numbered EARS requirement. +type Requirement struct { + ID string `json:"id"` + Text string `json:"text"` + Pattern string `json:"pattern,omitempty"` // the EARS pattern, when the line parses + Delta string `json:"delta,omitempty"` // the parenthesised marker, e.g. (REMOVED) + Line int `json:"line"` + End int `json:"end"` + Section string `json:"section,omitempty"` +} + +// Block is a paragraph: a run of body lines inside a section that is neither a task +// nor a decomposition leaf. +// +// It is here because of what the artifacts actually look like. A real plan's +// `## Notes` was measured at 411 lines — half the file — carrying no headings at +// all, so section addressing bottoms out there and a reader still has to load the +// whole thing. Every one of those paragraphs opened with a bolded lead sentence, +// which is the convention this type exploits: the leads alone are an index, and the +// index is a twentieth of the prose. +type Block struct { + Section string `json:"section"` + Index int `json:"index"` // 1-based within its section + Slug string `json:"slug"` // from the lead, for an address that survives an insert + Lead string `json:"lead"` + Line int `json:"line"` + End int `json:"end"` +} + +// ParseTasks reads every checkbox in doc against the task grammar. +// +// mdscan has already excluded the checkboxes inside fenced blocks and HTML +// comments, which is what keeps a rule file documenting the grammar — or a template +// showing an example — from producing tasks nobody wrote. +func ParseTasks(doc *mdscan.Document) []Task { + return parseTasks(doc) +} + +func parseTasks(doc *mdscan.Document) []Task { + tasks := make([]Task, 0, len(doc.Checkboxes)) + for _, box := range doc.Checkboxes { + t := Task{Line: box.Line, End: box.Line, Checked: box.Checked, Indent: box.Indent} + rest := box.Text + + if m := taskNumberRe.FindStringSubmatch(rest); m != nil { + t.Number, rest = m[1], m[2] + } + + found := MethodologyRe.FindAllStringSubmatch(rest, -1) + t.Methodologies = len(found) + switch len(found) { + case 0: + t.Loose = LooseMethodologyRe.FindString(rest) + case 1: + t.Methodology = found[0][1] + } + + text, tail, hasTail := strings.Cut(rest, CitationSeparator) + t.Text = strings.TrimSpace(MethodologyRe.ReplaceAllString(text, "")) + t.Requirements = RequirementIDRe.FindAllString(tail, -1) + t.HasCitation = hasTail + + t.End = blockEnd(doc, box.Line, box.Indent) + t.Detail = joinLines(doc, box.Line+1, t.End) + tasks = append(tasks, t) + } + return tasks +} + +// parseRequirements reads the numbered requirements out of a requirements.md. +// +// Prose elsewhere in the file is not a requirement and is not reported as one: the +// anchor is the bulleted, bolded id, which is what the template writes and what the +// rule asks for. +func parseRequirements(doc *mdscan.Document) []Requirement { + var out []Requirement + for i, line := range doc.Body { + m := RequirementRe.FindStringSubmatch(line) + if m == nil { + continue + } + r := Requirement{ID: m[1], Delta: m[2], Text: strings.TrimSpace(m[3]), Line: i + 1} + r.End = blockEnd(doc, r.Line, indentOf(line)) + if r.Delta == "" { + if parsed, err := ears.Parse(r.Text); err == nil { + r.Pattern = string(parsed.Pattern) + } + } + out = append(out, r) + } + return out +} + +// parseLeaves reads a plan's decomposition: the list items that reference a spec. +// +// A leaf never carries a checkbox — one source of truth per item, and a leaf's +// state lives in the spec it names — so a list item that has one is a task, and is +// skipped here rather than counted twice. +func parseLeaves(doc *mdscan.Document) []Leaf { + boxes := map[int]bool{} + for _, b := range doc.Checkboxes { + boxes[b.Line] = true + } + var out []Leaf + for i, line := range doc.Body { + num := i + 1 + if boxes[num] { + continue + } + m := listItemRe.FindStringSubmatch(line) + if m == nil { + continue + } + ref := SpecRefRe.FindStringSubmatch(m[2]) + if ref == nil { + continue + } + l := Leaf{ + Ref: paths.SpecsSeg + "/" + ref[1] + "/", + Feature: ref[1], + Line: num, + End: blockEnd(doc, num, indentOf(line)), + } + l.Text = leafText(m[2]) + out = append(out, l) + } + return out +} + +// leafText is the leaf's description: what follows the em dash that separates the +// reference from what it covers, or the whole item when there is none. +func leafText(item string) string { + if _, after, ok := strings.Cut(item, CitationSeparator); ok { + return strings.TrimSpace(after) + } + return strings.TrimSpace(SpecRefRe.ReplaceAllString(item, "")) +} + +// Blocks is the artifact's paragraphs, section by section. +func (a *Artifact) Blocks() []Block { + claimed := map[int]bool{} + for _, t := range a.Tasks { + for n := t.Line; n <= t.End; n++ { + claimed[n] = true + } + } + for _, l := range a.Leaves { + for n := l.Line; n <= l.End; n++ { + claimed[n] = true + } + } + headings := map[int]bool{} + for _, h := range a.doc.Headings { + headings[h.Line] = true + } + + var out []Block + for _, s := range a.Sections { + index := 0 + n := s.Line + 1 + for n <= s.BodyEnd { + if claimed[n] || headings[n] || strings.TrimSpace(a.doc.Body[n-1]) == "" { + n++ + continue + } + start := n + for n <= s.BodyEnd && !claimed[n] && !headings[n] && strings.TrimSpace(a.doc.Body[n-1]) != "" { + n++ + } + index++ + lead := leadText(a.Lines[start-1]) + out = append(out, Block{ + Section: s.Slug, + Index: index, + Slug: mdscan.Slug(clip(lead, 60)), + Lead: lead, + Line: start, + End: n - 1, + }) + } + } + return out +} + +// leadText is a paragraph's opening line with Markdown emphasis stripped, so the +// index reads as sentences rather than as syntax. +func leadText(line string) string { + s := strings.TrimSpace(line) + s = strings.ReplaceAll(s, "**", "") + s = strings.ReplaceAll(s, "`", "") + return strings.TrimSpace(s) +} + +// blockEnd is where a list item's continuation stops: the last line that is +// indented past the item's own marker, blank lines inside the run included, and +// stopping at the next heading or the next item at the same or a shallower indent. +// +// This is what makes a task addressable as a region rather than as a line. Measured +// on a real plan, every task ran to more than one line and the longest ran to +// sixty-one — a reader that returned only the checkbox line would return the number +// and none of the decision. +func blockEnd(doc *mdscan.Document, start, indent int) int { + end := start + for n := start + 1; n <= len(doc.Body); n++ { + line := doc.Body[n-1] + if strings.TrimSpace(line) == "" { + continue + } + if indentOf(line) <= indent || isHeading(doc, n) { + break + } + end = n + } + return end +} + +func isHeading(doc *mdscan.Document, line int) bool { + for _, h := range doc.Headings { + if h.Line == line { + return true + } + } + return false +} + +// indentOf counts leading whitespace with tabs expanded to four, matching how +// mdscan reports a checkbox's indent. +func indentOf(s string) int { + n := 0 + for _, r := range s { + switch r { + case ' ': + n++ + case '\t': + n += 4 + default: + return n + } + } + return n +} + +// joinLines collapses a line range into one whitespace-normalized string. +func joinLines(doc *mdscan.Document, from, to int) string { + if from > to { + return "" + } + var parts []string + for n := from; n <= to && n <= len(doc.Body); n++ { + if f := strings.Fields(doc.Body[n-1]); len(f) > 0 { + parts = append(parts, strings.Join(f, " ")) + } + } + return strings.Join(parts, " ") +} + +// clip shortens s to n runes, marking that it was shortened. It counts runes rather +// than bytes because the artifacts are full of em dashes and box-drawing characters, +// and a byte clip would cut one in half. +func clip(s string, n int) string { + r := []rune(strings.TrimSpace(s)) + if len(r) <= n { + return string(r) + } + return strings.TrimRight(string(r[:n-1]), " ") + "…" +} diff --git a/internal/artifact/search.go b/internal/artifact/search.go new file mode 100644 index 0000000..12c55b1 --- /dev/null +++ b/internal/artifact/search.go @@ -0,0 +1,313 @@ +package artifact + +import ( + "math" + "regexp" + "sort" + "strings" +) + +// Search over the artifacts, and why it is not a search engine. +// +// The obvious reach here is an inverted-index library. It is the wrong tool at this +// size: a plan measured at 56KB and a workspace of twenty-seven specs is under a +// megabyte of Markdown, which a linear pass reads faster than an index can be opened, +// and every real engine is a dependency — a CGO surface, or a second binary to +// install — against a go.mod that is stdlib-only and a build that cross-compiles to +// six platforms. The cost would be paid on every platform to save microseconds. +// +// What precision actually needs here is not a better index but a better unit. Lines +// are the wrong thing to retrieve: a line has no name, so a hit on one still leaves +// the caller reading around it to find out what it belongs to. So the unit indexed +// is the addressable one — a task, a requirement, a leaf, a paragraph — and a hit +// comes back as an address the caller can hand straight to `show`. Ranking is BM25, +// with the terms ANDed by default, because a reader asking for two words means both. +// +// If the corpus ever outgrows this, the seam is here: Search's signature takes +// artifacts and returns hits, and nothing outside this file knows how it found them. + +// Hit is one match, already addressed. +type Hit struct { + Path string `json:"path"` + Ref string `json:"ref"` + Kind string `json:"kind"` + Label string `json:"label"` + Line int `json:"line"` + End int `json:"end"` + Score float64 `json:"score"` + Snippet string `json:"snippet"` +} + +// SearchOpts narrows a search before it ranks anything. +type SearchOpts struct { + Any bool // match any term rather than all of them + Regex bool // treat the query as a regular expression over each unit + Kind string // restrict to one target kind: task, requirement, leaf, block, section + Limit int // 0 means every hit + Fields bool // also match against the file's own path and title +} + +// unit is one indexed thing: an addressable region and the text it holds. +type unit struct { + art *Artifact + ref string + kind TargetKind + label string + line int + end int + lead string + text string + terms map[string]int + len int +} + +// Search ranks every addressable unit in arts against query. +func Search(arts []*Artifact, query string, opts SearchOpts) ([]Hit, error) { + units := index(arts, opts.Kind) + if opts.Regex { + return searchRegex(units, query, opts) + } + terms := tokenize(query) + if len(terms) == 0 { + return nil, nil + } + return rank(units, terms, opts), nil +} + +// index flattens the artifacts into searchable units. A section is indexed by its +// title alone rather than by its whole subtree: its content is already indexed as +// the paragraphs and tasks inside it, and indexing both would make every hit inside +// a long section also a hit on the section. +func index(arts []*Artifact, kind string) []unit { + var units []unit + want := func(k TargetKind) bool { return kind == "" || kind == string(k) } + for _, a := range arts { + if want(TargetSection) { + for _, s := range a.Sections { + units = append(units, mk(a, s.Slug, TargetSection, s.Title, s.Line, s.End, s.Title, s.Title)) + } + } + if want(TargetTask) { + for _, t := range a.Tasks { + body := strings.Join([]string{t.Number, t.Methodology, t.Text, t.Detail, + strings.Join(t.Requirements, " ")}, " ") + units = append(units, mk(a, t.Number, TargetTask, t.Summary(72), t.Line, t.End, t.Text, body)) + } + } + if want(TargetRequirement) { + for _, r := range a.Requirements { + units = append(units, mk(a, r.ID, TargetRequirement, clip(r.Text, 72), r.Line, r.End, + r.Text, r.ID+" "+r.Text+" "+r.Pattern)) + } + } + if want(TargetLeaf) { + for _, l := range a.Leaves { + body := l.Feature + " " + joinRaw(a, l.Line, l.End) + units = append(units, mk(a, l.Ref, TargetLeaf, clip(l.Text, 72), l.Line, l.End, l.Text, body)) + } + } + if want(TargetBlock) { + for _, b := range a.Blocks() { + ref := b.Section + ":" + itoa(b.Index) + units = append(units, mk(a, ref, TargetBlock, clip(b.Lead, 72), b.Line, b.End, + b.Lead, joinRaw(a, b.Line, b.End))) + } + } + } + return units +} + +func mk(a *Artifact, ref string, kind TargetKind, label string, line, end int, lead, text string) unit { + u := unit{art: a, ref: ref, kind: kind, label: label, line: line, end: end, lead: lead, text: text} + u.terms = map[string]int{} + for _, t := range tokenize(text) { + u.terms[t]++ + u.len++ + } + // The lead counts twice. A paragraph's opening sentence is what it is about — + // measured on a real plan, every note in `## Notes` opened with a bolded thesis — + // so a term there is a stronger signal than the same term buried in the argument. + for _, t := range tokenize(lead) { + u.terms[t]++ + u.len++ + } + return u +} + +// rank scores the units that satisfy the term requirement, using BM25 with the +// usual constants. +func rank(units []unit, terms []string, opts SearchOpts) []Hit { + df := map[string]int{} + total := 0 + for _, u := range units { + total += u.len + for t := range u.terms { + df[t]++ + } + } + if len(units) == 0 { + return nil + } + avg := float64(total) / float64(len(units)) + const k1, b = 1.2, 0.75 + + var hits []Hit + for _, u := range units { + matched := 0 + score := 0.0 + for _, t := range terms { + f := float64(u.terms[t]) + if f == 0 { + // A term absent as a whole token may still be a prefix of one, which is + // what makes searching for "pixel" find "pixelart". + if f = float64(prefixCount(u.terms, t)); f == 0 { + continue + } + f *= 0.5 + } + matched++ + idf := math.Log(1 + (float64(len(units))-float64(df[t])+0.5)/(float64(df[t])+0.5)) + score += idf * (f * (k1 + 1)) / (f + k1*(1-b+b*float64(u.len)/avg)) + } + if matched == 0 || (!opts.Any && matched < len(terms)) { + continue + } + hits = append(hits, u.hit(score, terms)) + } + return finish(hits, opts.Limit) +} + +func searchRegex(units []unit, pattern string, opts SearchOpts) ([]Hit, error) { + re, err := regexp.Compile("(?i)" + pattern) + if err != nil { + return nil, err + } + var hits []Hit + for _, u := range units { + if loc := re.FindStringIndex(u.text); loc != nil { + h := u.hit(1, nil) + h.Snippet = around(u.text, loc[0], loc[1]) + hits = append(hits, h) + } + } + return finish(hits, opts.Limit), nil +} + +func (u unit) hit(score float64, terms []string) Hit { + h := Hit{ + Path: u.art.Path, Ref: u.ref, Kind: string(u.kind), Label: u.label, + Line: u.line, End: u.end, Score: score, + } + h.Snippet = snippet(u.text, terms) + return h +} + +// finish sorts by score, then by position, so a run over an unchanged workspace +// returns the same order every time — a ranking that reshuffles ties is a diff +// nobody made. +func finish(hits []Hit, limit int) []Hit { + sort.SliceStable(hits, func(i, j int) bool { + if hits[i].Score != hits[j].Score { + return hits[i].Score > hits[j].Score + } + if hits[i].Path != hits[j].Path { + return hits[i].Path < hits[j].Path + } + return hits[i].Line < hits[j].Line + }) + if limit > 0 && len(hits) > limit { + hits = hits[:limit] + } + return hits +} + +// snippet is the window around the first term that hit, so a caller can tell a real +// match from a coincidence without opening the file. +func snippet(text string, terms []string) string { + lower := strings.ToLower(text) + best := -1 + for _, t := range terms { + if i := strings.Index(lower, t); i >= 0 && (best < 0 || i < best) { + best = i + } + } + if best < 0 { + return clip(text, 120) + } + return around(text, best, best+1) +} + +func around(text string, from, to int) string { + const pad = 60 + start := from - pad + if start < 0 { + start = 0 + } + end := to + pad + if end > len(text) { + end = len(text) + } + out := strings.TrimSpace(text[start:end]) + if start > 0 { + out = "…" + out + } + if end < len(text) { + out += "…" + } + return strings.Join(strings.Fields(out), " ") +} + +func prefixCount(terms map[string]int, prefix string) int { + if len(prefix) < 3 { + return 0 + } + n := 0 + for t, c := range terms { + if t != prefix && strings.HasPrefix(t, prefix) { + n += c + } + } + return n +} + +var tokenSplit = regexp.MustCompile(`[^\p{L}\p{N}_.\-/]+`) + +// tokenize lowercases and splits, keeping the punctuation that lives inside the +// names these artifacts are full of — `meta.json`, `--dry-run`, `specs/job-store` — +// and then also emitting the pieces, so a search for "json" still finds the first. +func tokenize(s string) []string { + var out []string + for _, raw := range tokenSplit.Split(strings.ToLower(s), -1) { + t := strings.Trim(raw, ".-/_") + if t == "" { + continue + } + out = append(out, t) + if strings.ContainsAny(t, "./-_") { + for _, part := range strings.FieldsFunc(t, func(r rune) bool { + return r == '.' || r == '/' || r == '-' || r == '_' + }) { + if len(part) > 1 { + out = append(out, part) + } + } + } + } + return out +} + +func joinRaw(a *Artifact, from, to int) string { + return strings.Join(strings.Fields(a.Text(from, to)), " ") +} + +func itoa(n int) string { + if n == 0 { + return "0" + } + var b []byte + for n > 0 { + b = append([]byte{byte('0' + n%10)}, b...) + n /= 10 + } + return string(b) +} diff --git a/internal/assets/assets.go b/internal/assets/assets.go index b0d88df..66a7dfb 100644 --- a/internal/assets/assets.go +++ b/internal/assets/assets.go @@ -71,7 +71,13 @@ import ( // each rule its own trigger line instead of running four of them together in a // sentence — project.md above all, since a build command nobody read is guessed — // and it stops telling a harness that preloads the rules to go and read them. -const Version = "10" +// 11: artifacts.md — plans and specs are addressable, so `scc map` answers a +// structural question and `scc patch` edits by address, and neither one costs the +// file. The entry file carries the reflex; the rule carries the how. +// 12: the entry file's layout block is one column across all three harnesses. The +// padding is computed from the profile rather than written into the template, +// because a run of spaces that lines up for `.codex/` is ragged for `.opencode/`. +const Version = "12" // The embedded tree. "all:" so nothing is silently dropped for having a name the // default embed pattern skips. @@ -144,6 +150,7 @@ func Workspace(h paths.Harness) []File { "specs.md", "knowledge-base.md", "code-search.md", + "artifacts.md", } { set = append(set, File{ Name: "rules/" + rule, @@ -338,6 +345,35 @@ type layout struct { // so a template can stop telling it to go and read them. See // paths.Harness.PreloadsRules. RulesPreloaded bool + + // The same three paths, trailing slash included, padded to the column the + // entry file's layout block puts its descriptions in. + // + // The padding is computed rather than written into the template because a + // harness's own directory names differ in width — `.codex/rules/` is three + // characters shorter than `.opencode/rules/` — so any literal run of spaces + // would line up in exactly one of the three and read as ragged in the other + // two. TestEntryLayoutBlockIsAligned is what keeps these and the template's + // hand-written left column agreeing. + RulesCol string + SkillsCol string + CommandsCol string +} + +// layoutColumn is where a description starts in the entry file's layout block. +// It has to clear both the widest path any harness produces (`.opencode/command/`, +// 18) and the widest literal in the template (`specs//`, 16). +const layoutColumn = 20 + +// column renders one path as the left half of that block: trailing slash, then +// spaces out to layoutColumn. A path too wide to pad still gets one space, so a +// future harness with a long name degrades to ragged rather than to a run-on line. +func column(p string) string { + s := p + "/" + if pad := layoutColumn - len([]rune(s)); pad > 0 { + return s + strings.Repeat(" ", pad) + } + return s + " " } func layoutOf(h paths.Harness) layout { @@ -352,8 +388,11 @@ func layoutOf(h paths.Harness) layout { Manifest: path.Join(h.Dir, paths.ManifestSeg), RulesPreloaded: h.PreloadsRules, } + l.RulesCol = column(l.Rules) + l.SkillsCol = column(l.Skills) if h.CommandsSeg != "" { l.Commands = path.Join(h.Dir, h.CommandsSeg) + l.CommandsCol = column(l.Commands) l.HasCommands = true } return l diff --git a/internal/assets/assets_test.go b/internal/assets/assets_test.go index a640ff2..4835025 100644 --- a/internal/assets/assets_test.go +++ b/internal/assets/assets_test.go @@ -800,3 +800,80 @@ func TestSplitMetaRejectsAMalformedHeader(t *testing.T) { } } } + +// The entry file's layout block is a column: paths on the left, what each holds +// on the right. Half that column is written by hand in the template and half is +// computed from the harness profile, so nothing but this test keeps the two +// halves agreeing — and the failure is invisible in review, because a template +// that lines up for one harness reads as ragged in the other two. +func TestEntryLayoutBlockIsAligned(t *testing.T) { + for _, h := range paths.Harnesses() { + raw, err := Render(h, entryFile(t, h)) + if err != nil { + t.Fatalf("%s: %v", h.ID, err) + } + lines := layoutBlock(t, h.ID, raw) + if len(lines) < 4 { + t.Fatalf("%s: layout block has %d lines, expected the whole tree", h.ID, len(lines)) + } + want := -1 + for _, line := range lines { + got := descriptionColumn(line) + if got < 0 { + t.Errorf("%s: %q has no column break", h.ID, line) + continue + } + if want < 0 { + want = got + continue + } + if got != want { + t.Errorf("%s: description starts at column %d, but the block's column is %d:\n %q", + h.ID, got, want, line) + } + } + if want != layoutColumn { + t.Errorf("%s: block column is %d, but layoutColumn is %d — the template's hand-written "+ + "left column and the computed one have drifted apart", h.ID, want, layoutColumn) + } + } +} + +// layoutBlock returns the non-blank lines inside the fenced block under ## Layout. +func layoutBlock(t *testing.T, id, raw string) []string { + t.Helper() + var out []string + inSection, inFence := false, false + for _, line := range strings.Split(raw, "\n") { + switch { + case strings.HasPrefix(line, "## Layout"): + inSection = true + case inSection && strings.HasPrefix(line, "```"): + if inFence { + return out + } + inFence = true + case inFence && strings.TrimSpace(line) != "": + out = append(out, line) + } + } + t.Fatalf("%s: no fenced layout block found", id) + return nil +} + +// descriptionColumn is where the description starts: the first rune after the +// run of two or more spaces that separates it from the path. +func descriptionColumn(line string) int { + runes := []rune(line) + for i := 0; i < len(runes)-1; i++ { + if runes[i] != ' ' || runes[i+1] != ' ' { + continue + } + for j := i; j < len(runes); j++ { + if runes[j] != ' ' { + return j + } + } + } + return -1 +} diff --git a/internal/assets/templates/entry.md b/internal/assets/templates/entry.md index 4140031..1c99ca9 100644 --- a/internal/assets/templates/entry.md +++ b/internal/assets/templates/entry.md @@ -1,15 +1,13 @@ # {{.Entry}} -Spec-driven development, scaffolded and checked by `scc`. Keep this file short — -the methodology lives in `{{.Rules}}/`. Never inline it here. +Spec-driven development, scaffolded and checked by `scc`. The methodology lives in `{{.Rules}}/` — never inline it here. ## Rules — `{{.Rules}}/.md` {{if .RulesPreloaded -}} -{{.Label}} loads `{{.Rules}}/` into your context at session start, so these are already -in front of you and there is nothing to open. What the triggers below tell you is *when* -each rule governs — the failure they prevent is not a rule you never read, it is a rule -you had all along and applied at the wrong moment, or not at all. +{{.Label}} loads `{{.Rules}}/` at session start: they are in front of you, nothing to open. +The triggers below say *when* each governs — the failure is not a rule you never read, +it is one you had all along and applied at the wrong moment, or not at all. {{- else -}} Nothing loads these for you. Open the file whose moment has arrived, and open it again in a new session: a rule you read yesterday is not a rule you have read. @@ -25,28 +23,38 @@ Triggered by where you are in the work: Triggered by what you are about to touch: -- `project.md` — **before you run any build, test, lint, or format command.** This - project's commands exist nowhere else: `scc` ships the file as a stub for the team - to fill in, and runs none of them itself. A command that did not come from there is - a guess, and a guessed test command that exits 0 looks exactly like a passing suite. -- `code-search.md` — **before you go looking for code you have not read yet.** This - workspace keeps a symbol graph, and a structural question answered from it costs one - call instead of a grep and six reads. +- `project.md` — **before any build, test, lint, or format command.** This project's + commands exist nowhere else, and a guessed test command that exits 0 looks exactly + like a passing suite. +- `code-search.md` — before going looking for code you have not read +- `artifacts.md` — before opening a plan or a spec - `specs.md` — writing requirements, design, or tasks for a spec - `tasks.md` — working through a spec's task list - `knowledge-base.md` — something was learned, or a decision was made +## Ask the index before you read the file + +**Code** — `scc graph query|explore `, or `codegraph_explore` where registered. +Read the source when you are about to change it, not to find it. + +**Plans and specs** — `scc map` · `map ` · `map tasks --next` · +`map find ""` · `map show
` · `map trace`. An address is a +name — `1.2` `R1.2` `#notes` `notes:7` `specs//` — never a line number. + +**Changing one** — `scc patch check 1.2`, plus `task` `add` `append` `fm`. Not +an editor: it resolves the address, re-validates, and rolls back an edit that adds a +finding — so you need not read a plan to change one line of it. + ## Layout ``` -specs// requirements.md · design.md · tasks.md -plans/.md structure, plus a checklist and/or spec references -docs/ knowledge base — wiki, adr, codewiki, glossary, stack - -{{.Rules}}/ — the methodology above -{{.Skills}}/ — authoring each part of docs/, and running a plan group by group +specs// requirements.md · design.md · tasks.md +plans/.md structure, plus a checklist and/or spec references +docs/ knowledge base — wiki, adr, codewiki, glossary, stack +{{.RulesCol}}the methodology above +{{.SkillsCol}}authoring each part of docs/, and running a plan group by group {{- if .HasCommands}} -{{.Commands}}/ — the same skills on demand: /scc-plan-run, /scc-wiki, /scc-adr, … +{{.CommandsCol}}the same skills on demand: /scc-plan-run, /scc-wiki, /scc-adr, … {{- end}} ``` @@ -56,6 +64,4 @@ docs/ knowledge base — wiki, adr, codewiki, glossary, stack `scc update` brings a newer scc's rules and agents in: it shows the plan, then asks. Exit `0` ok · `1` could not run · `2` ran and found something. A finding is an answer, not a crash. - -`scc` checks artifact *shape* only; it never reads source, so whether the code honors -the artifact is on you. +`scc` checks artifact *shape* only; it never reads source, so whether the code honors it is on you. diff --git a/internal/assets/templates/rules/artifacts.md b/internal/assets/templates/rules/artifacts.md new file mode 100644 index 0000000..990199a --- /dev/null +++ b/internal/assets/templates/rules/artifacts.md @@ -0,0 +1,55 @@ +# Plans and specs — address them, do not read them + +A plan is a structured document that happens to be Markdown. Reading one end to end +to answer a question about its structure is the most wasteful thing this workspace can +ask of you: a plan decomposing into thirty specs is tens of kilobytes of prose wrapped +around a dozen checkboxes, and once it is in context you carry it all session. `scc +map` answers those questions without loading the file; `scc patch` changes them +without loading it either. + +| The question | Ask | +|---|---| +| What is here, and how far along? | `scc map` | +| What is the shape of this one? | `scc map ` | +| What do I work on next? | `scc map tasks --next` | +| What is left in group 4? | `scc map tasks --open --group 4` | +| Where is the note about X? | `scc map find ""` | +| Show me exactly that piece | `scc map show
` | +| What else mentions this requirement? | `scc map trace specs//R1.2` | + +`` is a path, a plan name, or a feature name. **An address is a name, never +a line number**, so it survives an edit above it: + +``` +1.2 a task #notes a section, by anchor slug +R1.2 a requirement notes:7 the 7th paragraph of that section +specs/foo/ a leaf L120-160 an explicit range, the escape hatch +``` + +`find` returns addresses, which is what makes the pair work: search, then `show` only +the hit. A long `## Notes` with no headings inside it is still navigable — `scc map +blocks` indexes its paragraphs by their opening sentence. Read the file directly only +when the question is about *this exact text*: prose you are about to rewrite. + +## Writing + +**Tick boxes and amend tasks with `scc patch`, not with an editor.** + +``` +scc patch check 1.1 1.2 +scc patch task 1.2 --text "…" --method TDD --req R1.1,R1.2 +scc patch add --section tasks --number 1.3 --method Unit --text "…" +scc patch append '#notes' --text - reads stdin, for paragraphs +scc patch fm pr=per-plan +``` + +Each resolves its address with the parser that read the file, so a miss is an error +rather than a write to the wrong place. It then re-runs the validators and **rolls the +change back if it introduced a finding** — exit `2`, file untouched. `--dry-run` shows +the lines first; deleting more than a screenful stops and asks for `--force`. + +That is why you need not read a plan to change one line of it. Do not defeat it by +reading "to be safe": the printed before/after is the confirmation. + +A requirement id is scoped to its own spec — `R2.5` in one feature is not `R2.5` in +another — so cite it as `specs//R2.5` when the spec is not obvious. diff --git a/internal/assets/templates/rules/code-search.md b/internal/assets/templates/rules/code-search.md index 324913e..8c7fbd0 100644 --- a/internal/assets/templates/rules/code-search.md +++ b/internal/assets/templates/rules/code-search.md @@ -25,6 +25,8 @@ or from a harness with no MCP surface. **It indexes code, not this repository's knowledge.** `docs/` is Markdown and no part of it is in the graph: not the glossary, not the wiki, not an ADR, not a `design.md`. +Plans and specs are not in it either, and they have their own index — see +[artifacts.md](artifacts.md), which is the same rule for the other corpus. That matters more here than it would elsewhere, because this project deliberately keeps the *why* out of the code. A question the graph answers well — "where is this diff --git a/internal/assets/templates/rules/tasks.md b/internal/assets/templates/rules/tasks.md index 5f0f968..d28e81d 100644 --- a/internal/assets/templates/rules/tasks.md +++ b/internal/assets/templates/rules/tasks.md @@ -47,3 +47,7 @@ record: it survives the session, it gets reviewed, it gets committed, and it is A session ending with its todo list complete and the file untouched has lost everything except the code: neither the next session nor the reviewer knows which tasks were done. + +Check it with `scc patch check 1.2` rather than by editing the file. It +addresses the task by its number, so the file never has to be read to change one box, +and it re-validates afterwards — see [code-search.md](code-search.md). diff --git a/internal/assets/templates/skills/plan-run/SKILL.md b/internal/assets/templates/skills/plan-run/SKILL.md index a8ea4e5..a9db50e 100644 --- a/internal/assets/templates/skills/plan-run/SKILL.md +++ b/internal/assets/templates/skills/plan-run/SKILL.md @@ -1,6 +1,6 @@ --- name: plan-run -description: Drive a whole plan under plans/ to completion — read the plan, report the groups, take whatever the invocation already decided and ask only for the rest, then implement group by group and deliver either one PR per group or one at the end, settling CI before calling the plan delivered. Resumes from the repository rather than from memory. Use it when someone asks to implement an entire plan, to keep going until the plan is finished, or runs /scc-plan-run. Not for a single spec or a one-off change, which delivery.md already carries end to end on its own. +description: Drive a whole plan under plans/ to completion — map the plan, report the groups, take whatever the invocation already decided and ask only for the rest, then implement group by group and deliver either one PR per group or one at the end, settling CI before calling the plan delivered. Resumes from the repository rather than from memory. Use it when someone asks to implement an entire plan, to keep going until the plan is finished, or runs /scc-plan-run. Not for a single spec or a one-off change, which delivery.md already carries end to end on its own. --- You run a plan to the end. @@ -36,13 +36,23 @@ A **group** is the smallest part of the plan that can merge on its own. The order is the order they are written in, unless `## Notes` says otherwise. Notes wins — that heading exists precisely to say what must not be merged out of sequence. +**Do not read `## Notes` end to end to find that out.** It is the longest section of +any real plan and most of it decides nothing about order. `scc map blocks +notes` lists every paragraph by its opening sentence, with an address; `scc map show + notes:7` returns the one or two that actually constrain the sequence. On a +measured plan that is ~1.3k tokens instead of ~7.3k, for the same answer. + A plan with a flat, unnumbered checklist has exactly one group. Say so and run it once, rather than inventing a decomposition the author did not write. ## Before the first group — read, report, then ask what is still open -1. **Read the plan and work out the groups.** Ask nothing yet. The questions below - are only answerable by someone who can see what they are agreeing to. +1. **Map the plan and work out the groups.** `scc map ` gives you the sections, + the leaves, the task counts and the open numbers — the whole shape, without the + prose. Read the plan itself only where the map is not enough; a plan that + decomposes into thirty specs is tens of kilobytes you would otherwise carry for the + rest of the loop. Ask nothing yet: the questions below are only answerable by + someone who can see what they are agreeing to. 2. **Name the groups back, numbered, in order.** Order is the one thing a person can correct cheaply now and expensively after three merges. 3. **Take every answer the invocation already gave, and ask only for what is left.** @@ -86,6 +96,10 @@ that only shows up later: **Write every answer into the plan's frontmatter before starting**, then never ask again for this plan: +```bash +scc patch fm autonomy=auto ci=wait pr=per-plan worktree=per-group merge=auto +``` + ```yaml --- autonomy: auto @@ -96,6 +110,11 @@ merge: auto --- ``` +`patch fm` writes each key by name — replacing one that is already there, adding one +that is not — and re-validates afterwards, so a typo'd value is refused rather than +recorded as an answer nobody gave. It is also the first write of the loop, and doing +it this way means the plan never has to be in context to be configured. + That is what makes a resumed session pick up where this one stopped instead of interrogating the developer a second time. `scc validate` checks the values. @@ -123,8 +142,9 @@ you stop to deliver. validate`, both review subagents, commit, push, open the PR. 5. **Record the group's state in that same PR.** A task group's checkboxes are ticked in the plan file, in the branch that does the work, so `main` and the plan agree - the moment the merge lands. A leaf is never ticked — its state lives in the spec - and is read from there. + the moment the merge lands — `scc patch check 1.1 1.2 …`, which addresses + each task by number and re-validates the file. A leaf is never ticked: its state + lives in the spec and is read from there. 6. **CI and merge, exactly as answered.** `ci: wait` means watch the checks until they settle and fix what is red before merging. `merge: auto` means you merge once that answer is satisfied; `merge: manual` means you open the PR, say where it is, and @@ -144,8 +164,9 @@ Branch once from a green `main`, then for each group in order: later failure attributable: a break caught at group 3 is group 3's, while the same break found after group 9 costs a bisect. 3. **Commit the group on its own**, with the group in the subject, and tick its - checkboxes in the same commit. The commits are the granularity this shape gives up - in pull requests — do not squash the plan into one. + checkboxes in the same commit — `scc patch check 2.1 2.2 …`, as above. The + commits are the granularity this shape gives up in pull requests — do not squash + the plan into one. 4. **Report the group in one line**, then start the next. Do not push a PR yet. Then, once — and only once every group is in: @@ -186,19 +207,25 @@ plan's frontmatter — read them and carry on, do not ask again.** They were the developer's call once; asking a second time because your context died makes them pay for your problem. -Where you read your position from depends on the shape: - -- **`pr: per-group` — read `main`.** Pull it and re-read the plan there; the copy in - an old worktree is stale by construction. A task group whose boxes are ticked on - `main` is done, as is a leaf whose spec's `tasks.md` is fully ticked there. An open - PR means that group is mid-flight — under `merge: manual` that is the expected - resting state. Finish it before starting another; two open groups is the fan-out - this loop exists to avoid. -- **`pr: per-plan` — read the plan's branch.** Nothing reaches `main` until the end, - so `main` will say no group is done and it will be wrong. Find the branch, read its - log for the per-group commits, and re-read the plan **there**. If every group is - committed but no PR is open, the run died between the last group and the review - pass: run the subagents and push. If the PR is open, the run died waiting on CI. +Resuming is a question about state, not about prose, so read it as state: `scc map +` for the shape and `scc map tasks --open` for what is left. Re-reading a +whole plan to find one unticked box is the cost this loop would otherwise pay every +time a session dies. + +Which checkout you read that from depends on the shape: + +- **`pr: per-group` — read `main`.** Pull it and map the plan there; the copy in an + old worktree is stale by construction. A task group whose boxes are ticked on `main` + is done, as is a leaf whose spec's `tasks.md` is fully ticked there — `scc map trace + specs//` answers that in one call, without opening either file. An open PR + means that group is mid-flight; under `merge: manual` that is the expected resting + state. Finish it before starting another — two open groups is the fan-out this loop + exists to avoid. +- **`pr: per-plan` — read the plan's branch.** Nothing reaches `main` until the end, so + `main` will say no group is done and it will be wrong. Find the branch, read its log + for the per-group commits, and map the plan **there**. If every group is committed + but no PR is open, the run died between the last group and the review pass: run the + subagents and push. If the PR is open, the run died waiting on CI. A leftover worktree whose branch is already merged is debris. Remove it. @@ -216,6 +243,8 @@ run over. Ask what the invocation did not already answer. evidence you will get." That turns a degraded run into something the developer chose. Never let it look like every group was reviewed, and never call such a plan delivered. -- **The plan grows while the loop runs.** Re-read it at each group boundary. The - group list from step 2 is a report, not a contract, and a group appended after you - started is still part of the plan. +- **The plan grows while the loop runs.** Re-map it at each group boundary — `scc map + ` — because this is the read the loop performs most often and the one that + most tempts you to reach for the file instead. The group list from step 2 is a + report, not a contract, and a group appended after you started is still part of the + plan. diff --git a/internal/cli/cli.go b/internal/cli/cli.go index 4b53069..61ef5e0 100644 --- a/internal/cli/cli.go +++ b/internal/cli/cli.go @@ -70,6 +70,10 @@ func Run(args []string) int { return runSpec(args[1:]) case "plan": return runPlan(args[1:]) + case "map": + return runMap(args[1:]) + case "patch": + return runPatch(args[1:]) case "skill": return runSkill(args[1:]) case "validate": @@ -115,6 +119,8 @@ Commands: graph The workspace's symbol graph — build | sync | status | query | explore spec Create and inspect specs — new | list | show | delete | validate plan Create and inspect plans — new | list | delete | validate + map Read a plan or spec by address, not by the whole file — outline | tasks | show | blocks | find | trace + patch Change one at an address, without reading it first; verified and rolled back on findings skill Agent Skills conformance — validate validate Run every applicable validator; exit 2 on findings version Print the version diff --git a/internal/cli/map.go b/internal/cli/map.go new file mode 100644 index 0000000..600ffcc --- /dev/null +++ b/internal/cli/map.go @@ -0,0 +1,740 @@ +package cli + +import ( + "flag" + "fmt" + "os" + "strings" + + "github.com/protonspy/spec-claude-code/internal/artifact" + "github.com/protonspy/spec-claude-code/internal/paths" + "github.com/protonspy/spec-claude-code/internal/render" +) + +// runMap dispatches `scc map `, the read half of navigating an artifact +// without loading it. +// +// The bare form takes a path rather than a subcommand — `scc map plans/x.md` — because +// the outline is what a caller wants nine times out of ten and making them type the +// verb for the common case is a cost paid on every invocation. A first argument that +// matches a known verb is that verb; anything else is a path. +func runMap(args []string) int { + if len(args) == 0 { + return runMapIndex(nil) + } + switch args[0] { + case "index": + return runMapIndex(args[1:]) + case "outline": + return runMapOutline(args[1:]) + case "tasks": + return runMapTasks(args[1:]) + case "show": + return runMapShow(args[1:]) + case "blocks": + return runMapBlocks(args[1:]) + case "find": + return runMapFind(args[1:]) + case "trace": + return runMapTrace(args[1:]) + case "help", "-h", "--help": + mapUsage() + return ExitOK + default: + if strings.HasPrefix(args[0], "-") { + return runMapIndex(args) + } + return runMapOutline(args) + } +} + +// loadOne resolves a positional to exactly one artifact. Naming a spec resolves to +// its three files, which is right for an outline and ambiguous for everything else, +// so the commands that need one target say which file they want. +func loadOne(root, arg string) (*artifact.Artifact, bool) { + paths_, err := artifact.Resolve(root, arg) + if err != nil { + render.Err(err.Error()) + return nil, false + } + if len(paths_) > 1 { + render.Err(fmt.Sprintf("%q is a spec, which is %d files; name one of them", arg, len(paths_))) + for _, p := range paths_ { + render.Detail(" " + p) + } + return nil, false + } + a, err := artifact.Load(root, paths_[0]) + if err != nil { + render.Err(err.Error()) + return nil, false + } + return a, true +} + +func loadMany(root string, args []string) ([]*artifact.Artifact, bool) { + if len(args) == 0 { + all, err := artifact.Scan(root) + if err != nil { + render.Err(err.Error()) + return nil, false + } + return all, true + } + var out []*artifact.Artifact + for _, arg := range args { + files, err := artifact.Resolve(root, arg) + if err != nil { + render.Err(err.Error()) + return nil, false + } + for _, f := range files { + a, err := artifact.Load(root, f) + if err != nil { + render.Err(err.Error()) + return nil, false + } + out = append(out, a) + } + } + return out, true +} + +// indexEntry is one artifact in the workspace index: enough to decide whether to +// open it, and nothing more. +type indexEntry struct { + Path string `json:"path"` + Kind string `json:"kind"` + Name string `json:"name"` + Title string `json:"title"` + Tasks int `json:"tasks"` + Done int `json:"done"` + Leaves int `json:"leaves"` + Reqs int `json:"requirements"` + Lines int `json:"lines"` + Bytes int `json:"bytes"` +} + +func runMapIndex(args []string) int { + fs := flag.NewFlagSet("map index", flag.ContinueOnError) + fs.SetOutput(os.Stderr) + root := addRoot(fs) + jsonOut := addJSON(fs) + rest, err := parseFlags(fs, args) + if err != nil { + return ExitError + } + if !noPositionals(rest, "map index") { + return ExitError + } + target, ok := resolveRoot(*root) + if !ok || !requireWorkspace(target) { + return ExitError + } + arts, err := artifact.Scan(target) + if err != nil { + render.Err(err.Error()) + return ExitError + } + + entries := make([]indexEntry, 0, len(arts)) + totalBytes := 0 + for _, a := range arts { + done, total := a.Done() + entries = append(entries, indexEntry{ + Path: a.Path, Kind: string(a.Kind), Name: a.Name, Title: a.Title, + Tasks: total, Done: done, Leaves: len(a.Leaves), + Reqs: len(a.Requirements), Lines: a.LineCount(), Bytes: a.Bytes, + }) + totalBytes += a.Bytes + } + if *jsonOut { + return emitJSON(struct { + Artifacts []indexEntry `json:"artifacts"` + Count int `json:"count"` + Bytes int `json:"bytes"` + }{entries, len(entries), totalBytes}) + } + if len(entries) == 0 { + render.Info(fmt.Sprintf("no plans or specs yet — `%s plan new `", prog())) + return ExitOK + } + for _, e := range entries { + render.Info(fmt.Sprintf("%-44s %s", e.Path, indexFacts(e))) + } + render.Info(fmt.Sprintf("%d artifacts · %s", len(entries), sizeOf(totalBytes))) + return ExitOK +} + +func indexFacts(e indexEntry) string { + var parts []string + if e.Tasks > 0 { + parts = append(parts, fmt.Sprintf("%d/%d tasks", e.Done, e.Tasks)) + } + if e.Leaves > 0 { + parts = append(parts, fmt.Sprintf("%d leaves", e.Leaves)) + } + if e.Reqs > 0 { + parts = append(parts, fmt.Sprintf("%d reqs", e.Reqs)) + } + parts = append(parts, fmt.Sprintf("%dL", e.Lines)) + return strings.Join(parts, " · ") +} + +func runMapOutline(args []string) int { + fs := flag.NewFlagSet("map outline", flag.ContinueOnError) + fs.SetOutput(os.Stderr) + root := addRoot(fs) + depth := fs.Int("depth", 6, "deepest heading level to print") + jsonOut := addJSON(fs) + rest, err := parseFlags(fs, args) + if err != nil { + return ExitError + } + if len(rest) == 0 { + render.Err("map outline needs an artifact: a path, a plan name, or a feature name") + return ExitError + } + target, ok := resolveRoot(*root) + if !ok || !requireWorkspace(target) { + return ExitError + } + arts, ok := loadMany(target, rest) + if !ok { + return ExitError + } + if *jsonOut { + return emitJSON(struct { + Artifacts []*artifact.Artifact `json:"artifacts"` + }{arts}) + } + for i, a := range arts { + if i > 0 { + render.Info("") + } + printOutline(a, *depth) + } + return ExitOK +} + +// printOutline is the whole point of the command: the shape of the file, with the +// prose replaced by its size. The closing line states what was skipped, because a +// summary that hides how much it summarized is a summary you cannot calibrate. +func printOutline(a *artifact.Artifact, depth int) { + head := render.Bold(a.Title) + if fm := frontmatterLine(a); fm != "" { + head += " " + fm + } + render.Info(head) + render.Info(fmt.Sprintf("%s · %s · %d lines · %s", a.Path, a.Kind, a.LineCount(), sizeOf(a.Bytes))) + + blocks := map[string]int{} + for _, b := range a.Blocks() { + blocks[b.Section]++ + } + for _, s := range a.Sections { + if s.Level > depth || s.Level == 1 { + continue + } + indent := strings.Repeat(" ", s.Level-1) + var facts []string + if s.Tasks > 0 { + facts = append(facts, fmt.Sprintf("%d/%d tasks", s.Done, s.Tasks)) + } + if s.Leaves > 0 { + facts = append(facts, fmt.Sprintf("%d leaves", s.Leaves)) + } + if n := blocks[s.Slug]; n > 0 { + facts = append(facts, fmt.Sprintf("%d notes", n)) + } + facts = append(facts, fmt.Sprintf("L%d-%d", s.Line, s.End)) + render.Info(fmt.Sprintf("%s%-32s %s", indent, s.Title, strings.Join(facts, " · "))) + } + if done, total := a.Done(); total > 0 { + render.Info(fmt.Sprintf("tasks: %d/%d done · open: %s", done, total, openNumbers(a))) + } +} + +func openNumbers(a *artifact.Artifact) string { + var open []string + for _, t := range a.Tasks { + if !t.Checked { + open = append(open, t.Number) + } + } + if len(open) == 0 { + return "none" + } + if len(open) > 12 { + return strings.Join(open[:12], " ") + fmt.Sprintf(" … (%d more)", len(open)-12) + } + return strings.Join(open, " ") +} + +func frontmatterLine(a *artifact.Artifact) string { + var parts []string + for _, k := range []string{"autonomy", "ci", "pr", "worktree", "merge"} { + if v, ok := a.Frontmatter[k]; ok { + parts = append(parts, k+":"+v) + } + } + return strings.Join(parts, " · ") +} + +func runMapTasks(args []string) int { + fs := flag.NewFlagSet("map tasks", flag.ContinueOnError) + fs.SetOutput(os.Stderr) + root := addRoot(fs) + open := fs.Bool("open", false, "only tasks that are not done") + done := fs.Bool("done", false, "only tasks that are done") + group := fs.String("group", "", "only tasks in this numbering `group` (1, or 1.2)") + req := fs.String("req", "", "only tasks citing this `requirement` (R1.2)") + method := fs.String("method", "", "only tasks annotated `Unit` or TDD") + next := fs.Bool("next", false, "only the first open task — what a loop asks for, and it implies --open") + width := fs.Int("width", 96, "clip each description to this many `runes`") + jsonOut := addJSON(fs) + rest, err := parseFlags(fs, args) + if err != nil { + return ExitError + } + if *open && *done { + render.Err("--open and --done ask for opposite things") + return ExitError + } + // --next means the next task to work on, which is the first one nobody has done. + // Without this it would mean "the first task", and answer a question nobody asked. + if *next { + if *done { + render.Err("--next asks for the first open task; --done asks for finished ones") + return ExitError + } + *open = true + } + target, ok := resolveRoot(*root) + if !ok || !requireWorkspace(target) { + return ExitError + } + arts, ok := loadMany(target, rest) + if !ok { + return ExitError + } + + type row struct { + Path string `json:"path"` + artifact.Task + } + rows := []row{} + for _, a := range arts { + for _, t := range a.Tasks { + switch { + case *open && t.Checked, *done && !t.Checked: + continue + case *group != "" && t.Group() != *group && !strings.HasPrefix(t.Number, *group+"."): + continue + case *method != "" && !strings.EqualFold(t.Methodology, *method): + continue + } + if *req != "" && !cites(t, *req) { + continue + } + rows = append(rows, row{a.Path, t}) + if *next { + break + } + } + if *next && len(rows) > 0 { + break + } + } + + if *jsonOut { + return emitJSON(struct { + Tasks []row `json:"tasks"` + Count int `json:"count"` + }{rows, len(rows)}) + } + if len(rows) == 0 { + render.Info("no tasks match") + return ExitOK + } + for _, r := range rows { + box := "[ ]" + if r.Checked { + box = render.Green("[x]") + } + cite := "" + if len(r.Requirements) > 0 { + cite = " — " + strings.Join(r.Requirements, ", ") + } + render.Info(fmt.Sprintf("%s %-6s %-6s %s%s %s", box, r.Number, r.Methodology, + r.Summary(*width), cite, render.Cyan(fmt.Sprintf("%s:%d", r.Path, r.Line)))) + } + return ExitOK +} + +func cites(t artifact.Task, id string) bool { + for _, r := range t.Requirements { + if strings.EqualFold(r, id) { + return true + } + } + return false +} + +func runMapShow(args []string) int { + fs := flag.NewFlagSet("map show", flag.ContinueOnError) + fs.SetOutput(os.Stderr) + root := addRoot(fs) + numbers := fs.Bool("numbers", false, "prefix each line with its line number") + jsonOut := addJSON(fs) + rest, err := parseFlags(fs, args) + if err != nil { + return ExitError + } + if len(rest) < 2 { + render.Err("map show needs an artifact and at least one address: `map show plans/x.md 1.2`") + return ExitError + } + target, ok := resolveRoot(*root) + if !ok || !requireWorkspace(target) { + return ExitError + } + a, ok := loadOne(target, rest[0]) + if !ok { + return ExitError + } + + type piece struct { + artifact.Target + Text string `json:"text"` + } + pieces := make([]piece, 0, len(rest)-1) + for _, ref := range rest[1:] { + t, err := a.Find(ref) + if err != nil { + render.Err(err.Error()) + return ExitError + } + pieces = append(pieces, piece{t, a.Text(t.Line, t.End)}) + } + + if *jsonOut { + return emitJSON(struct { + Path string `json:"path"` + Pieces []piece `json:"pieces"` + }{a.Path, pieces}) + } + for i, p := range pieces { + if i > 0 { + fmt.Println() + } + render.Info(fmt.Sprintf("%s %s %s:%d-%d", p.Kind, render.Bold(p.Ref), a.Path, p.Line, p.End)) + if *numbers { + for n, line := range strings.Split(p.Text, "\n") { + fmt.Printf("%5d %s\n", p.Line+n, line) + } + continue + } + fmt.Println(p.Text) + } + return ExitOK +} + +// runMapBlocks prints the paragraph index — the lead sentence of every paragraph in +// a section, with its address. It is the answer to a section that is half the file +// and carries no headings to navigate by. +func runMapBlocks(args []string) int { + fs := flag.NewFlagSet("map blocks", flag.ContinueOnError) + fs.SetOutput(os.Stderr) + root := addRoot(fs) + section := fs.String("section", "", "only paragraphs under this `section` slug") + width := fs.Int("width", 96, "clip each lead to this many `runes`") + jsonOut := addJSON(fs) + rest, err := parseFlags(fs, args) + if err != nil { + return ExitError + } + if len(rest) == 0 { + render.Err("map blocks needs an artifact") + return ExitError + } + target, ok := resolveRoot(*root) + if !ok || !requireWorkspace(target) { + return ExitError + } + a, ok := loadOne(target, rest[0]) + if !ok { + return ExitError + } + if len(rest) > 1 && *section == "" { + *section = rest[1] + } + + blocks := []artifact.Block{} + for _, b := range a.Blocks() { + if *section != "" && b.Section != strings.TrimPrefix(*section, "#") { + continue + } + blocks = append(blocks, b) + } + if *jsonOut { + return emitJSON(struct { + Path string `json:"path"` + Blocks []artifact.Block `json:"blocks"` + Count int `json:"count"` + }{a.Path, blocks, len(blocks)}) + } + if len(blocks) == 0 { + render.Info("no paragraphs match") + return ExitOK + } + for _, b := range blocks { + ref := fmt.Sprintf("%s:%d", b.Section, b.Index) + render.Info(fmt.Sprintf("%-14s %s %s", render.Bold(ref), + clipRunes(b.Lead, *width), render.Cyan(fmt.Sprintf("L%d-%d", b.Line, b.End)))) + } + return ExitOK +} + +func runMapFind(args []string) int { + fs := flag.NewFlagSet("map find", flag.ContinueOnError) + fs.SetOutput(os.Stderr) + root := addRoot(fs) + in := fs.String("in", "", "restrict to one `artifact` (a path, plan name, or feature)") + kind := fs.String("kind", "", "restrict to one `kind`: task, requirement, leaf, block, section") + limit := fs.Int("limit", 10, "how many hits to `n`ame") + any := fs.Bool("any", false, "match any term rather than all of them") + rex := fs.Bool("regex", false, "treat the query as a regular expression") + jsonOut := addJSON(fs) + rest, err := parseFlags(fs, args) + if err != nil { + return ExitError + } + if len(rest) == 0 { + render.Err("map find needs something to look for") + return ExitError + } + target, ok := resolveRoot(*root) + if !ok || !requireWorkspace(target) { + return ExitError + } + var scope []string + if *in != "" { + scope = []string{*in} + } + arts, ok := loadMany(target, scope) + if !ok { + return ExitError + } + + hits, err := artifact.Search(arts, strings.Join(rest, " "), artifact.SearchOpts{ + Any: *any, Regex: *rex, Kind: *kind, Limit: *limit, + }) + if err != nil { + render.Err(err.Error()) + return ExitError + } + if *jsonOut { + return emitJSON(struct { + Query string `json:"query"` + Hits []artifact.Hit `json:"hits"` + Count int `json:"count"` + }{strings.Join(rest, " "), hits, len(hits)}) + } + if len(hits) == 0 { + render.Info("nothing matched") + return ExitOK + } + for _, h := range hits { + render.Info(fmt.Sprintf("%s %s %s", + render.Bold(h.Ref), render.Cyan(fmt.Sprintf("%s:%d-%d", h.Path, h.Line, h.End)), h.Kind)) + render.Detail(" " + clipRunes(h.Snippet, 150)) + } + render.Info(fmt.Sprintf("%d hits — `%s map show ` for the whole of one", len(hits), prog())) + return ExitOK +} + +// runMapTrace answers "what else knows about this?" across files: a requirement and +// the tasks that cite it, or a spec and the plan leaf that carries it. +func runMapTrace(args []string) int { + fs := flag.NewFlagSet("map trace", flag.ContinueOnError) + fs.SetOutput(os.Stderr) + root := addRoot(fs) + in := fs.String("in", "", "the `feature` whose numbering a requirement id belongs to") + jsonOut := addJSON(fs) + rest, err := parseFlags(fs, args) + if err != nil { + return ExitError + } + if len(rest) != 1 { + render.Err("map trace takes one reference: a requirement (R1.2) or a spec (specs//)") + return ExitError + } + target, ok := resolveRoot(*root) + if !ok || !requireWorkspace(target) { + return ExitError + } + arts, err := artifact.Scan(target) + if err != nil { + render.Err(err.Error()) + return ExitError + } + + ref := rest[0] + // A requirement id is scoped to its spec: R2.5 in one feature is a different + // requirement from R2.5 in another, and this workspace has both. So the id can be + // written with its scope — specs//R2.5 — and an unscoped one that is + // defined in more than one place is answered with the list rather than with every + // spec's trace concatenated, which is the same mistake as reading all of them. + scope := *in + if i := strings.LastIndex(ref, "/"); i >= 0 && artifact.RequirementIDRe.MatchString(ref[i+1:]) { + scope = strings.Trim(strings.TrimPrefix(ref[:i], paths.SpecsSeg+"/"), "/") + ref = ref[i+1:] + } + + type site struct { + Path string `json:"path"` + Role string `json:"role"` + Ref string `json:"ref"` + Line int `json:"line"` + Label string `json:"label"` + } + sites := []site{} + + if artifact.RequirementIDRe.MatchString(ref) && scope == "" { + var defined []string + for _, a := range arts { + if _, ok := a.Requirement(ref); ok { + defined = append(defined, a.Spec) + } + } + if len(defined) > 1 { + if *jsonOut { + return emitJSON(struct { + Ref string `json:"ref"` + Scoped bool `json:"scoped"` + Defined []string `json:"defined_in"` + }{ref, false, defined}) + } + render.Warn(fmt.Sprintf("%s is defined in %d specs — a requirement id is scoped to its own", ref, len(defined))) + for _, f := range defined { + render.Info(fmt.Sprintf(" %s/%s/%s", paths.SpecsSeg, f, ref)) + } + render.Detail(fmt.Sprintf(" name one: `%s map trace %s//%s`", prog(), paths.SpecsSeg, ref)) + return ExitOK + } + if len(defined) == 1 { + scope = defined[0] + } + } + if scope != "" { + var narrowed []*artifact.Artifact + for _, a := range arts { + if a.Spec == scope || a.Kind == artifact.KindPlan { + narrowed = append(narrowed, a) + } + } + arts = narrowed + } + + if strings.HasPrefix(ref, paths.SpecsSeg+"/") || !artifact.RequirementIDRe.MatchString(ref) { + feature := strings.Trim(strings.TrimPrefix(ref, paths.SpecsSeg+"/"), "/") + for _, a := range arts { + for _, l := range a.Leaves { + if l.Feature == feature { + sites = append(sites, site{a.Path, "decomposed-by", l.Ref, l.Line, clipRunes(l.Text, 88)}) + } + } + if a.Spec == feature { + done, total := a.Done() + var label string + switch { + case total > 0: + label = fmt.Sprintf("%d/%d tasks done", done, total) + case len(a.Requirements) > 0: + label = fmt.Sprintf("%d requirements", len(a.Requirements)) + default: + label = fmt.Sprintf("%d sections · %d lines", len(a.Sections), a.LineCount()) + } + sites = append(sites, site{a.Path, string(a.Kind), feature, 1, label}) + } + } + } else { + for _, a := range arts { + if r, ok := a.Requirement(ref); ok { + sites = append(sites, site{a.Path, "defined", r.ID, r.Line, clipRunes(r.Text, 88)}) + } + for _, t := range a.Tasks { + if cites(t, ref) { + sites = append(sites, site{a.Path, "implemented-by", t.Number, t.Line, t.Summary(88)}) + } + } + if a.Kind == artifact.KindDesign { + for i, line := range a.Doc().Body { + for _, id := range artifact.RequirementIDRe.FindAllString(line, -1) { + if strings.EqualFold(id, ref) { + sites = append(sites, site{a.Path, "designed", id, i + 1, clipRunes(line, 88)}) + } + } + } + } + } + } + + if *jsonOut { + return emitJSON(struct { + Ref string `json:"ref"` + Sites []site `json:"sites"` + Count int `json:"count"` + }{ref, sites, len(sites)}) + } + if len(sites) == 0 { + render.Info(fmt.Sprintf("nothing in this workspace mentions %s", ref)) + return ExitOK + } + for _, s := range sites { + render.Info(fmt.Sprintf("%-16s %-10s %s %s", render.Bold(s.Ref), s.Role, + render.Cyan(fmt.Sprintf("%s:%d", s.Path, s.Line)), s.Label)) + } + return ExitOK +} + +func clipRunes(s string, n int) string { + r := []rune(strings.TrimSpace(s)) + if len(r) <= n { + return string(r) + } + return strings.TrimRight(string(r[:n-1]), " ") + "…" +} + +func sizeOf(b int) string { + if b < 1024 { + return fmt.Sprintf("%dB", b) + } + return fmt.Sprintf("%.1fKB", float64(b)/1024) +} + +func mapUsage() { + fmt.Fprintf(os.Stderr, `Usage: + %s map every plan and spec, one line each + %s map the shape of one file: sections, counts, open tasks + %s map tasks […] [filters] --open --done --next --group N --req R1.2 --method TDD + %s map show
… exactly that piece, and nothing else + %s map blocks [
] the lead sentence of every paragraph, with its address + %s map find [--in ] ranked search over addressable units + %s map trace /> everything in the workspace that mentions it + +An is a path (plans/x.md), a plan name, or a feature name. + +Addresses, none of which is a line number — which is why one survives an edit above it: + + 1.2 a task, by its number + R1.2 a requirement, by its id + specs/foo/ a decomposition leaf + #notes a section, by anchor slug (or by its title as written) + notes:7 the 7th paragraph of that section + L120-160 an explicit line range, the escape hatch + +Use "%s patch" to change what "%s map" found, without reading the file first. +`, prog(), prog(), prog(), prog(), prog(), prog(), prog(), prog(), prog()) +} diff --git a/internal/cli/map_test.go b/internal/cli/map_test.go new file mode 100644 index 0000000..113b08e --- /dev/null +++ b/internal/cli/map_test.go @@ -0,0 +1,392 @@ +package cli + +import ( + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" +) + +// mapPlan is a plan with everything the commands address: a decomposition of leaves, +// a task list whose items run past their checkbox line, and a Notes section with no +// headings inside it — which is the shape that makes paragraph addressing necessary. +const mapPlan = `--- +autonomy: auto +ci: wait +--- + +# Sample plan + +## Why + +What this is for, and what done means for the whole of it. + +## Decomposition + +- ` + "`specs/thing/`" + ` — the leaf that carries most of the work, described over + two lines because that is what leaves do. + +## Tasks + +- [x] 1.1 (Unit) Build the parser, and prove with a test that it reads a fenced + block without treating the example inside it as a task +- [ ] 1.2 (TDD) Guard the credential before the provider client lands + +## Notes + +**Order matters.** The parser first, because everything downstream reads through it. + +**The free path wins.** The expensive command knows about the cheap one, never the +other way round. +` + +// mapWorkspace scaffolds a real workspace and drops the plan into it. init is used +// rather than a hand-made directory so the marker file the workspace walk needs is +// the one the product actually writes. +func mapWorkspace(t *testing.T) string { + t.Helper() + root := t.TempDir() + if _, _, code := run(t, "init", "--root", root); code != ExitOK { + t.Fatalf("init: exit %d", code) + } + dir := filepath.Join(root, "plans") + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, "sample.md"), []byte(mapPlan), 0o644); err != nil { + t.Fatal(err) + } + return root +} + +func planText(t *testing.T, root string) string { + t.Helper() + b, err := os.ReadFile(filepath.Join(root, "plans", "sample.md")) + if err != nil { + t.Fatal(err) + } + return string(b) +} + +// The outline reports structure, so its size must track the structure and not the +// prose. That is the property that makes it worth running: measured on a real +// 56KB plan the outline came to 878 bytes, and it would come to about the same on a +// plan twice as wordy. A ratio on a small fixture proves nothing; this does. +func TestMapOutlineDoesNotGrowWithTheProse(t *testing.T) { + root := mapWorkspace(t) + lean, stderr, code := run(t, "map", "sample", "--root", root) + if code != ExitOK { + t.Fatalf("exit %d (stderr: %s)", code, stderr) + } + for _, want := range []string{"Decomposition", "Tasks", "Notes", "1/2 tasks"} { + if !strings.Contains(lean, want) { + t.Errorf("outline missing %q:\n%s", want, lean) + } + } + if strings.Contains(lean, "never the") { + t.Error("the outline printed the prose it exists to replace") + } + + // Same structure, an order of magnitude more words under the last heading. + fat := mapPlan + strings.Repeat("\nMore prose that decides nothing and is here to be skipped.\n", 200) + if err := os.WriteFile(filepath.Join(root, "plans", "sample.md"), []byte(fat), 0o644); err != nil { + t.Fatal(err) + } + grown, _, code := run(t, "map", "sample", "--root", root) + if code != ExitOK { + t.Fatalf("exit %d", code) + } + if len(fat) < 10*len(mapPlan) { + t.Fatalf("fixture is not much bigger: %d vs %d", len(fat), len(mapPlan)) + } + // The one legitimate growth is the line count and the paragraph tally in the + // report, which is a handful of characters — never a multiple. + if len(grown) > len(lean)+120 { + t.Errorf("outline grew from %d to %d bytes while the file grew %dx — it is summarizing prose", + len(lean), len(grown), len(fat)/len(mapPlan)) + } +} + +// --next means the next task to work on. Answering with the first task in the file, +// done or not, answers a question nobody asked. +func TestMapTasksNextIsTheFirstOpenOne(t *testing.T) { + root := mapWorkspace(t) + stdout, _, code := run(t, "map", "tasks", "sample", "--next", "--root", root) + if code != ExitOK { + t.Fatalf("exit %d", code) + } + if !strings.Contains(stdout, "1.2") { + t.Errorf("--next = %q, want task 1.2", stdout) + } + if strings.Contains(stdout, "1.1") { + t.Error("--next returned a task that is already done") + } +} + +func TestMapShowReturnsOnlyThatPiece(t *testing.T) { + root := mapWorkspace(t) + stdout, stderr, code := run(t, "map", "show", "sample", "notes:1", "--root", root) + if code != ExitOK { + t.Fatalf("exit %d (stderr: %s)", code, stderr) + } + if !strings.Contains(stdout, "Order matters") { + t.Errorf("did not return the addressed paragraph:\n%s", stdout) + } + if strings.Contains(stdout, "The free path wins") { + t.Error("returned the next paragraph too") + } + if strings.Contains(stdout, "## Decomposition") { + t.Error("returned the rest of the file") + } +} + +// An address that misses must fail rather than return something adjacent, and must +// say what the file does have — the caller cannot see it. +func TestMapShowOnAMissNamesWhatExists(t *testing.T) { + root := mapWorkspace(t) + stdout, stderr, code := run(t, "map", "show", "sample", "9.9", "--root", root) + if code != ExitError { + t.Errorf("exit = %d, want %d", code, ExitError) + } + if stdout != "" { + t.Errorf("stdout = %q, want empty on a miss", stdout) + } + if !strings.Contains(stderr, "1.1") { + t.Errorf("stderr does not list the tasks that exist: %q", stderr) + } +} + +func TestMapFindReturnsAddresses(t *testing.T) { + root := mapWorkspace(t) + stdout, stderr, code := run(t, "map", "find", "credential provider", "--root", root, "--json") + if code != ExitOK { + t.Fatalf("exit %d (stderr: %s)", code, stderr) + } + var got struct { + Hits []struct { + Ref string `json:"ref"` + Kind string `json:"kind"` + Line int `json:"line"` + } `json:"hits"` + } + if err := json.Unmarshal([]byte(stdout), &got); err != nil { + t.Fatalf("not JSON (%v): %s", err, stdout) + } + if len(got.Hits) == 0 { + t.Fatal("no hits") + } + if got.Hits[0].Ref != "1.2" { + t.Errorf("top hit = %q, want the task that mentions both terms", got.Hits[0].Ref) + } + if got.Hits[0].Line < 1 { + t.Error("a hit with no line is not an address") + } +} + +func TestPatchCheckWritesAndReValidates(t *testing.T) { + root := mapWorkspace(t) + _, stderr, code := run(t, "patch", "check", "sample", "1.2", "--root", root) + if code != ExitOK { + t.Fatalf("exit %d (stderr: %s)", code, stderr) + } + if !strings.Contains(planText(t, root), "- [x] 1.2") { + t.Error("the box was not flipped on disk") + } +} + +// The guarantee that replaces reading the file first: a change that introduces a +// finding is undone, reported, and exits 2 — never left on disk. +func TestPatchRollsBackAChangeThatIntroducesAFinding(t *testing.T) { + root := mapWorkspace(t) + before := planText(t, root) + + stdout, stderr, code := run(t, "patch", "add", "sample", + "--section", "tasks", "--number", "1.3", "--method", "Unit", + "--text", "Port the queue out of specs/thing/ into the runner", "--root", root) + if code != ExitFindings { + t.Errorf("exit = %d, want %d (stdout: %s stderr: %s)", code, ExitFindings, stdout, stderr) + } + if got := planText(t, root); got != before { + t.Errorf("the file was left changed after a rollback:\n%s", got) + } + if !strings.Contains(stderr, "rolled back") { + t.Errorf("stderr does not say it rolled back: %q", stderr) + } + if !strings.Contains(stderr, "item-has-two-records") { + t.Errorf("stderr does not name the rule that fired: %q", stderr) + } +} + +// A pre-existing finding is not this edit's fault. Comparing whole sets rather than +// what the edit introduced would make every patch to an already-imperfect file fail. +func TestPatchIgnoresFindingsTheFileAlreadyHad(t *testing.T) { + root := mapWorkspace(t) + // specs/thing/ does not exist, so the plan already reports plan.unknown-spec. + if _, _, code := run(t, "plan", "validate", "--root", root); code != ExitFindings { + t.Fatal("this test needs a plan that already has findings") + } + if _, stderr, code := run(t, "patch", "check", "sample", "1.2", "--root", root); code != ExitOK { + t.Errorf("exit = %d, want %d — a pre-existing finding blocked an unrelated edit (%s)", + code, ExitOK, stderr) + } +} + +// An address is a name, not a span: `replace #notes` reads as a small edit and can +// resolve to most of the file. Since nobody read the file first, the scale of the +// deletion is the one thing the caller cannot have known. +func TestPatchRefusesALargeDeletionWithoutForce(t *testing.T) { + root := mapWorkspace(t) + path := writeBigPlan(t, root) + before := readFile(t, path) + + _, stderr, code := run(t, "patch", "replace", "big", "#notes", "--text", "gone", "--root", root) + if code != ExitError { + t.Errorf("exit = %d, want %d", code, ExitError) + } + if readFile(t, path) != before { + t.Error("the file was changed despite the refusal") + } + if !strings.Contains(stderr, "--force") { + t.Errorf("the refusal does not say how to proceed deliberately: %q", stderr) + } + + // --force is the separate decision, and it must actually work: a guard with no + // way past it is a guard that gets worked around with a text editor. + if _, stderr, code := run(t, "patch", "replace", "big", "#notes", "--text", "gone", + "--force", "--root", root); code != ExitOK { + t.Errorf("--force: exit = %d, want %d (%s)", code, ExitOK, stderr) + } + if after := readFile(t, path); !strings.Contains(after, "gone") || strings.Contains(after, "A line of prose") { + t.Error("--force did not apply the replacement") + } +} + +// writeBigPlan drops in a plan whose one section is far past the deletion threshold. +func writeBigPlan(t *testing.T, root string) string { + t.Helper() + body := strings.Repeat("A line of prose that is here to make the section long.\n", 40) + path := filepath.Join(root, "plans", "big.md") + if err := os.WriteFile(path, []byte("# Big\n\n## Notes\n\n"+body), 0o644); err != nil { + t.Fatal(err) + } + return path +} + +func readFile(t *testing.T, path string) string { + t.Helper() + b, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + return string(b) +} + +// And the refusal must not echo what it declined to delete: a confirmation that +// printed four hundred displaced lines would put the file in the caller's context by +// the back door, while reporting that it refused to touch it. +func TestPatchRefusalDoesNotEchoTheWholeRegion(t *testing.T) { + root := mapWorkspace(t) + writeBigPlan(t, root) + stdout, stderr, code := run(t, "patch", "replace", "big", "#notes", "--text", "gone", "--root", root) + if code != ExitError { + t.Fatalf("exit = %d, want %d", code, ExitError) + } + if n := strings.Count(stdout+stderr, "A line of prose"); n > 8 { + t.Errorf("the refusal echoed %d displaced lines; it must elide them", n) + } +} + +func TestPatchDryRunWritesNothing(t *testing.T) { + root := mapWorkspace(t) + before := planText(t, root) + stdout, _, code := run(t, "patch", "check", "sample", "1.2", "--dry-run", "--root", root) + if code != ExitOK { + t.Fatalf("exit %d", code) + } + if got := planText(t, root); got != before { + t.Error("--dry-run wrote to the file") + } + if !strings.Contains(stdout, "1.2") { + t.Errorf("--dry-run did not show the change: %q", stdout) + } +} + +// Checking an already-checked task is a loop re-running a step it finished. It is +// not an edit and not an error. +func TestPatchCheckIsIdempotent(t *testing.T) { + root := mapWorkspace(t) + if _, _, code := run(t, "patch", "check", "sample", "1.1", "--root", root); code != ExitOK { + t.Errorf("exit = %d, want %d", code, ExitOK) + } + if got := planText(t, root); got != mapPlan { + t.Error("a no-op check rewrote the file") + } +} + +// A requirement id is scoped to its own spec. Tracing an unscoped one across a +// workspace where several specs number theirs the same way is the same mistake as +// reading all of them. +func TestMapTraceRefusesToGuessARequirementScope(t *testing.T) { + root := mapWorkspace(t) + for _, feature := range []string{"alpha", "beta"} { + if _, _, code := run(t, "spec", "new", feature, "--root", root); code != ExitOK { + t.Fatalf("spec new %s: exit %d", feature, code) + } + path := filepath.Join(root, "specs", feature, "requirements.md") + body := "# " + feature + "\n\n## R1\n\n- **R1.1** The system shall do the " + feature + " thing\n" + if err := os.WriteFile(path, []byte(body), 0o644); err != nil { + t.Fatal(err) + } + } + stdout, stderr, code := run(t, "map", "trace", "R1.1", "--root", root) + if code != ExitOK { + t.Fatalf("exit %d", code) + } + if !strings.Contains(stderr, "scoped") { + t.Errorf("did not say the id is ambiguous: %q %q", stdout, stderr) + } + for _, want := range []string{"specs/alpha/R1.1", "specs/beta/R1.1"} { + if !strings.Contains(stdout, want) { + t.Errorf("did not offer %q:\n%s", want, stdout) + } + } +} + +func TestMapAndPatchRequireAWorkspace(t *testing.T) { + dir := t.TempDir() + for _, args := range [][]string{ + {"map", "--root", dir}, + {"map", "tasks", "--root", dir}, + {"patch", "check", "x", "1.1", "--root", dir}, + } { + if _, _, code := run(t, args...); code != ExitError { + t.Errorf("%v: exit = %d, want %d outside a workspace", args, code, ExitError) + } + } +} + +// Every command's --json must be a clean document on stdout with diagnostics on +// stderr, so a caller can pipe one into the next without filtering. +func TestMapJSONIsCleanOnStdout(t *testing.T) { + root := mapWorkspace(t) + for _, args := range [][]string{ + {"map", "--json", "--root", root}, + {"map", "sample", "--json", "--root", root}, + {"map", "tasks", "sample", "--json", "--root", root}, + {"map", "blocks", "sample", "--json", "--root", root}, + } { + stdout, stderr, code := run(t, args...) + if code != ExitOK { + t.Errorf("%v: exit %d (%s)", args, code, stderr) + continue + } + var v any + if err := json.Unmarshal([]byte(stdout), &v); err != nil { + t.Errorf("%v: stdout is not JSON (%v): %s", args, err, stdout) + } + if stderr != "" { + t.Errorf("%v: stderr = %q, want empty", args, stderr) + } + } +} diff --git a/internal/cli/patch.go b/internal/cli/patch.go new file mode 100644 index 0000000..29abcab --- /dev/null +++ b/internal/cli/patch.go @@ -0,0 +1,576 @@ +package cli + +import ( + "flag" + "fmt" + "io" + "os" + "strings" + + "github.com/protonspy/spec-claude-code/internal/artifact" + "github.com/protonspy/spec-claude-code/internal/finding" + "github.com/protonspy/spec-claude-code/internal/render" + "github.com/protonspy/spec-claude-code/internal/validate" + "github.com/protonspy/spec-claude-code/internal/workspace" +) + +// runPatch dispatches `scc patch ` — changing an artifact at an address, +// without having read it. +// +// The guard that reading-first was providing is replaced by three that are stronger +// for a structured file: the address is resolved by the parser rather than matched as +// a string, so a miss is an error and never a write to the wrong place; the file is +// re-validated afterwards and the write is rolled back if it introduced a finding; +// and the lines that changed are printed back, which is the confirmation the read was +// standing in for. +func runPatch(args []string) int { + if len(args) == 0 { + patchUsage() + return ExitError + } + switch args[0] { + case "check": + return runPatchCheck(args[1:], true) + case "uncheck": + return runPatchCheck(args[1:], false) + case "task": + return runPatchTask(args[1:]) + case "add": + return runPatchAdd(args[1:]) + case "rm": + return runPatchRemove(args[1:]) + case "append", "prepend", "replace": + return runPatchText(args[0], args[1:]) + case "fm": + return runPatchFrontmatter(args[1:]) + case "help", "-h", "--help": + patchUsage() + return ExitOK + default: + render.Err(fmt.Sprintf("unknown patch subcommand %q", args[0])) + patchUsage() + return ExitError + } +} + +// patchFlags is the set every patch subcommand shares, so the safety valves are +// spelled and behave identically across the surface. +type patchFlags struct { + root *string + dry *bool + force *bool + jsonOut *bool +} + +func addPatchFlags(fs *flag.FlagSet) patchFlags { + return patchFlags{ + root: addRoot(fs), + dry: fs.Bool("dry-run", false, "show the change and write nothing"), + force: fs.Bool("force", false, + "write even when the change introduces a validation finding"), + jsonOut: addJSON(fs), + } +} + +func runPatchCheck(args []string, done bool) int { + name := "patch uncheck" + if done { + name = "patch check" + } + fs := flag.NewFlagSet(name, flag.ContinueOnError) + fs.SetOutput(os.Stderr) + pf := addPatchFlags(fs) + rest, err := parseFlags(fs, args) + if err != nil { + return ExitError + } + if len(rest) < 2 { + render.Err(name + " needs an artifact and at least one task number") + return ExitError + } + return withEditor(pf, rest[0], func(e *artifact.Editor) { + for _, number := range rest[1:] { + e.Check(number, done) + } + }) +} + +func runPatchTask(args []string) int { + fs := flag.NewFlagSet("patch task", flag.ContinueOnError) + fs.SetOutput(os.Stderr) + pf := addPatchFlags(fs) + text := fs.String("text", "", "replace the description") + method := fs.String("method", "", "replace the methodology: `Unit` or TDD") + req := fs.String("req", "", "replace the citations: a comma-separated `list` of ids") + number := fs.String("number", "", "renumber the task") + state := fs.String("state", "", "set the box: `open` or done") + rest, err := parseFlags(fs, args) + if err != nil { + return ExitError + } + if len(rest) != 2 { + render.Err("patch task needs an artifact and one task number") + return ExitError + } + edit := artifact.TaskEdit{} + if isSet(fs, "text") { + edit.Text = text + } + if isSet(fs, "method") { + if !validMethod(*method) { + return ExitError + } + edit.Methodology = method + } + if isSet(fs, "req") { + ids := splitList(*req) + edit.Requirements = &ids + } + if isSet(fs, "number") { + edit.Number = number + } + if isSet(fs, "state") { + switch *state { + case "done": + t := true + edit.Checked = &t + case "open": + f := false + edit.Checked = &f + default: + render.Err(fmt.Sprintf("--state is `open` or `done`, got %q", *state)) + return ExitError + } + } + if edit.Text == nil && edit.Methodology == nil && edit.Requirements == nil && + edit.Number == nil && edit.Checked == nil { + render.Err("patch task changes nothing: pass --text, --method, --req, --number, or --state") + return ExitError + } + return withEditor(pf, rest[0], func(e *artifact.Editor) { e.SetTask(rest[1], edit) }) +} + +func runPatchAdd(args []string) int { + fs := flag.NewFlagSet("patch add", flag.ContinueOnError) + fs.SetOutput(os.Stderr) + pf := addPatchFlags(fs) + section := fs.String("section", "", "the `section` slug whose task list this joins") + number := fs.String("number", "", "the task's `number`") + method := fs.String("method", "Unit", "`Unit` or TDD") + text := fs.String("text", "", "the description") + req := fs.String("req", "", "the requirements it satisfies, comma-separated") + rest, err := parseFlags(fs, args) + if err != nil { + return ExitError + } + if len(rest) != 1 { + render.Err("patch add needs an artifact") + return ExitError + } + if *section == "" || *number == "" || strings.TrimSpace(*text) == "" { + render.Err("patch add needs --section, --number and --text") + return ExitError + } + if !validMethod(*method) { + return ExitError + } + t := artifact.NewTask{ + Section: *section, Number: *number, Methodology: *method, + Text: *text, Requirements: splitList(*req), + } + return withEditor(pf, rest[0], func(e *artifact.Editor) { e.AddTask(t) }) +} + +func runPatchRemove(args []string) int { + fs := flag.NewFlagSet("patch rm", flag.ContinueOnError) + fs.SetOutput(os.Stderr) + pf := addPatchFlags(fs) + rest, err := parseFlags(fs, args) + if err != nil { + return ExitError + } + if len(rest) != 2 { + render.Err("patch rm needs an artifact and one task number") + return ExitError + } + return withEditor(pf, rest[0], func(e *artifact.Editor) { e.RemoveTask(rest[1]) }) +} + +func runPatchText(op string, args []string) int { + fs := flag.NewFlagSet("patch "+op, flag.ContinueOnError) + fs.SetOutput(os.Stderr) + pf := addPatchFlags(fs) + text := fs.String("text", "", "the text to write; `-` reads stdin, which is how multi-line prose gets in") + file := fs.String("file", "", "read the text from this `path` instead") + rest, err := parseFlags(fs, args) + if err != nil { + return ExitError + } + if len(rest) != 2 { + render.Err(fmt.Sprintf("patch %s needs an artifact and one address", op)) + return ExitError + } + body, ok := readText(*text, *file) + if !ok { + return ExitError + } + if strings.TrimSpace(body) == "" { + render.Err("nothing to write: pass --text, --text - to read stdin, or --file") + return ExitError + } + return withEditor(pf, rest[0], func(e *artifact.Editor) { + switch op { + case "append": + e.Append(rest[1], body) + case "prepend": + e.Prepend(rest[1], body) + case "replace": + e.Replace(rest[1], body) + } + }) +} + +func runPatchFrontmatter(args []string) int { + fs := flag.NewFlagSet("patch fm", flag.ContinueOnError) + fs.SetOutput(os.Stderr) + pf := addPatchFlags(fs) + rest, err := parseFlags(fs, args) + if err != nil { + return ExitError + } + if len(rest) < 2 { + render.Err("patch fm needs an artifact and at least one key=value") + return ExitError + } + pairs := make([][2]string, 0, len(rest)-1) + for _, arg := range rest[1:] { + k, v, ok := strings.Cut(arg, "=") + if !ok || strings.TrimSpace(k) == "" { + render.Err(fmt.Sprintf("expected key=value, got %q", arg)) + return ExitError + } + pairs = append(pairs, [2]string{strings.TrimSpace(k), strings.TrimSpace(v)}) + } + return withEditor(pf, rest[0], func(e *artifact.Editor) { + for _, p := range pairs { + e.SetFrontmatter(p[0], p[1]) + } + }) +} + +// withEditor is the whole write path: load, apply, verify, and roll back. +// +// The verification is the part that earns the right to skip reading. A patch is +// checked against the same validators `scc validate` runs, before and after, and a +// finding the file did not already have means the edit is undone and reported rather +// than left on disk. An artifact scc has no validator for — a wiki page — is written +// and said to be unverified, because claiming a check that did not happen is worse +// than not checking. +func withEditor(pf patchFlags, target string, apply func(*artifact.Editor)) int { + root, ok := resolveRoot(*pf.root) + if !ok || !requireWorkspace(root) { + return ExitError + } + a, ok := loadOne(root, target) + if !ok { + return ExitError + } + + e := a.Edit() + apply(e) + content, err := e.Content() + if err != nil { + render.Err(err.Error()) + return ExitError + } + changes := e.Changes() + + if e.Empty() { + if *pf.jsonOut { + return emitJSON(patchReport{Path: a.Path, Changes: nil, Written: false, Verified: "unchanged"}) + } + render.Info("already in that state — nothing to write") + return ExitOK + } + if *pf.dry { + if *pf.jsonOut { + return emitJSON(patchReport{Path: a.Path, Changes: changes, Written: false, Verified: "not-run"}) + } + printChanges(a.Path, changes) + render.Info(fmt.Sprintf("--dry-run: nothing written (%s)", scaleOf(changes))) + return ExitOK + } + if lost := deleted(changes); lost > destructiveLines && !*pf.force { + // The failure mode a caller cannot see coming. An address is a name, not a + // span, so `replace #notes` reads as one small edit and resolves to four + // hundred lines — and the whole point of this command is that nobody looked at + // the file first. So a change that destroys more than a screenful stops and + // shows what it was about to remove, and --force is the separate decision. + if *pf.jsonOut { + emitJSON(patchReport{Path: a.Path, Changes: changes, Written: false, Verified: "refused"}) + return ExitError + } + printChanges(a.Path, changes) + render.Err(fmt.Sprintf("refusing to remove %d lines from %s without --force", lost, a.Path)) + render.Detail(" the address resolved to more than it looks like: check it with") + render.Detail(fmt.Sprintf(" %s map show %s %s", prog(), a.Name, changes[0].Ref)) + return ExitError + } + + original, err := os.ReadFile(a.Abs) + if err != nil { + render.Err(err.Error()) + return ExitError + } + before, checkable := validateArtifact(root, a) + if err := workspace.AtomicWrite(a.Abs, []byte(content), 0o644); err != nil { + render.Err(err.Error()) + return ExitError + } + + report := patchReport{Path: a.Path, Changes: changes, Written: true, Verified: "clean"} + if !checkable { + report.Verified = "no-validator" + } else { + after, _ := validateArtifact(root, a) + if introduced := newFindings(before, after); len(introduced) > 0 { + report.Introduced = introduced + if !*pf.force { + if writeErr := workspace.AtomicWrite(a.Abs, original, 0o644); writeErr != nil { + render.Err("the edit introduced findings and the rollback failed: " + writeErr.Error()) + return ExitError + } + report.Written = false + report.Verified = "rolled-back" + } else { + report.Verified = "forced" + } + } + } + + if *pf.jsonOut { + code := ExitOK + if len(report.Introduced) > 0 { + code = ExitFindings + } + if emitJSON(report) != ExitOK { + return ExitError + } + return code + } + return printPatchReport(report) +} + +// patchReport is what a patch says it did, in the one shape both renderers read. +type patchReport struct { + Path string `json:"path"` + Changes []artifact.Change `json:"changes"` + Written bool `json:"written"` + Verified string `json:"verified"` // clean | rolled-back | forced | refused | no-validator | not-run | unchanged + Introduced []finding.Finding `json:"introduced,omitempty"` +} + +func printPatchReport(r patchReport) int { + printChanges(r.Path, r.Changes) + switch r.Verified { + case "rolled-back": + render.Err(fmt.Sprintf("rolled back: the change introduced %d finding(s)", len(r.Introduced))) + for _, f := range r.Introduced { + render.Detail(fmt.Sprintf(" %s:%d %s %s", f.File, f.Line, f.Rule, f.Message)) + } + render.Detail(" fix the input, or pass --force to write it anyway") + return ExitFindings + case "forced": + render.Warn(fmt.Sprintf("written with %d new finding(s) because --force was given", len(r.Introduced))) + for _, f := range r.Introduced { + render.Detail(fmt.Sprintf(" %s:%d %s %s", f.File, f.Line, f.Rule, f.Message)) + } + return ExitFindings + case "no-validator": + render.OK(r.Path + " — written; scc has no validator for this artifact, so nothing was checked") + return ExitOK + default: + render.OK(fmt.Sprintf("%s — written and re-validated clean", r.Path)) + return ExitOK + } +} + +// destructiveLines is where a patch stops being an edit and starts being a +// deletion. A screenful: below it a caller can see the whole change in the report +// the command prints back, and above it they cannot. +const destructiveLines = 12 + +// deleted is how many lines the change set removes on net — what the caller loses if +// the address resolved to more than they meant. +func deleted(changes []artifact.Change) int { + lost := 0 + for _, c := range changes { + if n := len(c.Before) - len(c.After); n > 0 { + lost += n + } + } + return lost +} + +func scaleOf(changes []artifact.Change) string { + added, removed := 0, 0 + for _, c := range changes { + added += len(c.After) + removed += len(c.Before) + } + return fmt.Sprintf("+%d −%d lines", added, removed) +} + +// printChanges shows the displaced and the written lines. It is the confirmation +// that replaces having read the file: a caller sees exactly what its address +// resolved to. +func printChanges(path string, changes []artifact.Change) { + for _, c := range changes { + render.Info(fmt.Sprintf("%s %s %s:%d", c.Op, render.Bold(c.Ref), path, c.Line)) + printSide(c.Before, render.Red(" - ")) + printSide(c.After, render.Green(" + ")) + } +} + +// printSide prints one side of a change, elided in the middle. +// +// The elision is not cosmetic. This command exists so a caller never has to hold the +// file, and a confirmation that echoed four hundred displaced lines would put the +// file in their context by the back door — while reporting a refusal to touch it. +func printSide(lines []string, marker string) { + const shown = 3 + if len(lines) <= 2*shown+1 { + for _, line := range lines { + render.Detail(marker + line) + } + return + } + for _, line := range lines[:shown] { + render.Detail(marker + line) + } + render.Detail(fmt.Sprintf("%s… %d more lines", marker, len(lines)-2*shown)) + for _, line := range lines[len(lines)-shown:] { + render.Detail(marker + line) + } +} + +// validateArtifact runs the validator that owns this file. The second return is +// false when scc has none, which is not a failure — the knowledge base is Markdown +// too, and refusing to edit it would be the wrong lesson to draw from having no rule +// for it. +func validateArtifact(root string, a *artifact.Artifact) ([]finding.Finding, bool) { + var set *finding.Set + var err error + switch a.Kind { + case artifact.KindPlan: + set, err = validate.Plan(root, a.Name) + case artifact.KindRequirements, artifact.KindDesign, artifact.KindTasks: + set, err = validate.Spec(root, a.Spec) + default: + return nil, false + } + if err != nil || set == nil { + return nil, false + } + return set.Sorted(), true +} + +// newFindings is what the edit caused: findings whose rule and message are not in +// the set the file already had. +// +// Line numbers are deliberately not part of the identity. An insertion moves every +// finding below it, and comparing on line would report the whole tail of a +// pre-existing problem as newly caused by a task added above it. +func newFindings(before, after []finding.Finding) []finding.Finding { + had := map[string]int{} + for _, f := range before { + had[f.Rule+"\x00"+f.Message]++ + } + var out []finding.Finding + for _, f := range after { + key := f.Rule + "\x00" + f.Message + if had[key] > 0 { + had[key]-- + continue + } + out = append(out, f) + } + return out +} + +// readText resolves the three ways prose gets in: inline, from stdin, or from a +// file. Stdin exists because a note is paragraphs, and a shell argument is a bad +// place to put paragraphs. +func readText(text, file string) (string, bool) { + if file != "" { + b, err := os.ReadFile(file) + if err != nil { + render.Err(err.Error()) + return "", false + } + return string(b), true + } + if text == "-" { + b, err := io.ReadAll(os.Stdin) + if err != nil { + render.Err(err.Error()) + return "", false + } + return string(b), true + } + return text, true +} + +func validMethod(m string) bool { + if m == "Unit" || m == "TDD" { + return true + } + render.Err(fmt.Sprintf("a methodology is `Unit` or `TDD`, got %q", m)) + return false +} + +func splitList(s string) []string { + var out []string + for _, part := range strings.Split(s, ",") { + if p := strings.TrimSpace(part); p != "" { + out = append(out, p) + } + } + return out +} + +// isSet reports whether the caller actually passed a flag, which is what separates +// "clear the citations" from "leave them alone". +func isSet(fs *flag.FlagSet, name string) bool { + seen := false + fs.Visit(func(f *flag.Flag) { + if f.Name == name { + seen = true + } + }) + return seen +} + +func patchUsage() { + fmt.Fprintf(os.Stderr, `Usage: + %s patch check … mark tasks done + %s patch uncheck … mark tasks not done + %s patch task [--text|--method|--req|--number|--state] + %s patch add --section --number N --text "…" [--method|--req] + %s patch rm + %s patch append
--text "…"|--text -|--file + %s patch prepend
… + %s patch replace
… + %s patch fm key=value… write the frontmatter + +Changes an artifact at an address — the same addresses "%s map" prints — so the file +never has to be read first. Every address is resolved by the parser, so a miss is an +error and never a write to the wrong place. + +After writing, the file is re-validated. A change that introduces a finding is rolled +back and reported; --force writes it anyway. --dry-run shows the lines and writes +nothing. + +Exit codes: 0 written · 1 usage or runtime error · 2 rolled back, or forced with findings. +`, prog(), prog(), prog(), prog(), prog(), prog(), prog(), prog(), prog(), prog()) +} diff --git a/internal/validate/plan.go b/internal/validate/plan.go index d390551..5691ab5 100644 --- a/internal/validate/plan.go +++ b/internal/validate/plan.go @@ -3,18 +3,19 @@ package validate import ( "fmt" "os" - "regexp" "sort" "strings" + "github.com/protonspy/spec-claude-code/internal/artifact" "github.com/protonspy/spec-claude-code/internal/finding" "github.com/protonspy/spec-claude-code/internal/mdscan" "github.com/protonspy/spec-claude-code/internal/paths" ) // specReferenceRe matches a reference to a spec from inside a plan: `specs//`, -// with or without backticks around it. -var specReferenceRe = regexp.MustCompile(`\b` + paths.SpecsSeg + `/([a-z0-9][a-z0-9-]*)/?`) +// with or without backticks around it. Shared with the reader in internal/artifact, +// which resolves the same reference to a decomposition leaf. +var specReferenceRe = artifact.SpecRefRe // Plans validates every plan under plans/. func Plans(root string) (*finding.Set, error) { diff --git a/internal/validate/spec.go b/internal/validate/spec.go index 155c57f..3e8b4d8 100644 --- a/internal/validate/spec.go +++ b/internal/validate/spec.go @@ -3,16 +3,17 @@ package validate import ( "fmt" "os" - "regexp" "sort" "strings" + "github.com/protonspy/spec-claude-code/internal/artifact" "github.com/protonspy/spec-claude-code/internal/ears" "github.com/protonspy/spec-claude-code/internal/finding" "github.com/protonspy/spec-claude-code/internal/paths" ) -// requirementRe matches a numbered requirement, and only in that position: +// The requirement grammar, shared with the reader in internal/artifact so one +// definition governs both: // // - **R1.2** (MODIFIED) When the manifest is missing, the CLI shall exit 1 // @@ -20,7 +21,10 @@ import ( // alternative — parsing every sentence as EARS — is a validator that fires on the // document's own introduction, which is exactly how a tool teaches its user to // disbelieve it. -var requirementRe = regexp.MustCompile(`^\s*[-*+]\s+\*\*(R\d+(?:\.\d+)+)\*\*\s*(?:\(([^)]*)\)\s*)?(.*)$`) +var ( + requirementRe = artifact.RequirementRe + requirementIDRe = artifact.RequirementIDRe +) // The delta markers a change to an existing spec is written with. A change is // proposed as a delta so that adopting scc does not mean writing the spec for diff --git a/internal/validate/tasks.go b/internal/validate/tasks.go index 7e72604..5114fa6 100644 --- a/internal/validate/tasks.go +++ b/internal/validate/tasks.go @@ -1,98 +1,63 @@ package validate import ( - "regexp" - "strings" - + "github.com/protonspy/spec-claude-code/internal/artifact" "github.com/protonspy/spec-claude-code/internal/finding" "github.com/protonspy/spec-claude-code/internal/mdscan" ) -// The task grammar, in one place, because one grammar governs every task line — -// whether it sits in a spec's tasks.md or in a plan's checklist. The methodology is a -// property of the task, not of the vehicle that carried it, so the findings a user -// sees are identical in both and share one `task.` slug namespace. +// The task grammar governs every task line — whether it sits in a spec's tasks.md or +// in a plan's checklist — and it lives in internal/artifact, next to the reader that +// navigates by it. One grammar, two consumers: the reader that resolves `1.2` to a +// region, and this, which turns what the line failed to say into findings. // // - [ ] 1.1 (Unit) Parse the manifest file — R1.2, R1.4 -var ( - taskNumberRe = regexp.MustCompile(`^(\d+(?:\.\d+)*)\s+(.*)$`) - methodologyRe = regexp.MustCompile(`\((Unit|TDD)\)`) - looseMethodology = regexp.MustCompile(`(?i)\((unit|tdd|test[- ]?first|none)\)`) - requirementIDRe = regexp.MustCompile(`\bR\d+(?:\.\d+)+\b`) - citationSeparator = "—" // an em dash, per the grammar in .claude/rules/tasks.md -) - -// task is one parsed task line. -type task struct { - Line int - Checked bool - Number string - Methodology string // "Unit" or "TDD" - Text string - Requirements []string -} - -// parseTasks reads every checkbox in doc against the task grammar and reports what is -// missing. citations decides whether a task must cite requirements: in a spec's -// tasks.md it must, because that citation is what makes traceability checkable; a -// plan has no requirements to cite. // -// Only checkboxes are considered, and mdscan has already excluded the ones inside -// fenced blocks and HTML comments — which is what keeps a rule file that documents -// the grammar, or a template that shows an example, from producing findings. -func parseTasks(set *finding.Set, file string, doc *mdscan.Document, citations bool) []task { - var tasks []task +// The split is deliberate. The parser states facts about the line; deciding which +// fact is a defect is a validator's job, and keeping that decision here is what lets +// `scc map` read a malformed artifact instead of refusing it. A reader that enforced +// the grammar would be unusable on exactly the files a user most needs to inspect. +func parseTasks(set *finding.Set, file string, doc *mdscan.Document, citations bool) []artifact.Task { + tasks := artifact.ParseTasks(doc) seen := map[string]int{} - for _, box := range doc.Checkboxes { - t := task{Line: box.Line, Checked: box.Checked} - rest := box.Text - - if m := taskNumberRe.FindStringSubmatch(rest); m != nil { - t.Number, rest = m[1], m[2] + for _, t := range tasks { + if t.Number == "" { + set.Addf(file, t.Line, "task.missing-number", + "a task opens with its number: `- [ ] 1.1 (Unit) …`") + } else { if prior, dup := seen[t.Number]; dup { - set.Addf(file, box.Line, "task.duplicate-number", + set.Addf(file, t.Line, "task.duplicate-number", "task %s is already used on line %d; a number has to identify one task", t.Number, prior) } - seen[t.Number] = box.Line - } else { - set.Addf(file, box.Line, "task.missing-number", - "a task opens with its number: `- [ ] 1.1 (Unit) …`") + seen[t.Number] = t.Line } - switch found := methodologyRe.FindAllStringSubmatch(rest, -1); len(found) { + switch t.Methodologies { case 0: // The one finding this whole practice exists to produce. A task with no // methodology is a task where nobody decided. - if loose := looseMethodology.FindString(rest); loose != "" { - set.Addf(file, box.Line, "task.missing-methodology", - "%s is not a methodology: annotate the task `(Unit)` or `(TDD)`", loose) + if t.Loose != "" { + set.Addf(file, t.Line, "task.missing-methodology", + "%s is not a methodology: annotate the task `(Unit)` or `(TDD)`", t.Loose) } else { - set.Addf(file, box.Line, "task.missing-methodology", + set.Addf(file, t.Line, "task.missing-methodology", "every task carries `(Unit)` or `(TDD)`: without it, nobody decided how this gets built") } case 1: - t.Methodology = found[0][1] default: - set.Addf(file, box.Line, "task.multiple-methodologies", - "a task is built one way: found %d methodology annotations", len(found)) + set.Addf(file, t.Line, "task.multiple-methodologies", + "a task is built one way: found %d methodology annotations", t.Methodologies) } - text, tail, hasTail := strings.Cut(rest, citationSeparator) - t.Text = strings.TrimSpace(methodologyRe.ReplaceAllString(text, "")) - t.Requirements = requirementIDRe.FindAllString(tail, -1) - if t.Text == "" { - set.Addf(file, box.Line, "task.no-description", "the task says nothing about what to do") + set.Addf(file, t.Line, "task.no-description", "the task says nothing about what to do") } - if citations { - switch { - case !hasTail, len(t.Requirements) == 0: - set.Addf(file, box.Line, "task.missing-requirement", - "a task in a spec cites the requirements it satisfies: `… %s R1.1, R1.2`", citationSeparator) - } + if citations && (!t.HasCitation || len(t.Requirements) == 0) { + set.Addf(file, t.Line, "task.missing-requirement", + "a task in a spec cites the requirements it satisfies: `… %s R1.1, R1.2`", + artifact.CitationSeparator) } - tasks = append(tasks, t) } return tasks }