From 5323695ea46b8d2955fe2e8c2c0979fabb2f05fc Mon Sep 17 00:00:00 2001 From: prode Date: Thu, 27 Aug 2026 17:23:27 -0300 Subject: [PATCH] feat(notes): centralize the project's notes in docs/notes.md, queryable without reading it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A comment says what a thing is and how to use it. Everything else written beside code — the gotcha, the why-not, the "careful, this looks wrong and is not" — reaches exactly one reader, the one already looking at that line: no command lists it, and it dies with the file. It moves to one log, and the log answers questions instead of being read. One note is one line, index fields first: - n-0042 2026-02-09 #gotcha @internal/cli/launch.go — wrap writes MCP config The single line is the whole design. A match is a *whole note*, so `grep ' #gotcha ' docs/notes.md` and `scc notes find --tag gotcha` answer the same question without either reasoning about where a record ends — which is what lets the file that centralizes every note never be a file anybody reads. It also closes the door the v1 plan format left open, where `## Notes` grew to half the file because nothing forbade it: there is nowhere for prose to go, and a thought needing a second line is a wiki page, an ADR, or a task. - internal/notes: the grammar, the log region, the query, the splice. - scc notes add | find | show | tags | paths | rm | validate. `add` allocates the id and writes the format, because a format nobody can be made to type decays; it writes under the same verify-and-roll-back contract as `scc patch`. --tag is required and has no default: a default is one tag on everything, which is the drift the index exists to prevent. - rm leaves an HTML-comment tombstone, so a number is spent and never reused: a citation to n-0042 may dangle, but can never mean a different note. - A ninth validator. The finding that matters is a hand-written line that missed the grammar — invisible to every query, so the note is already lost. A @path that no longer resolves is the notes half of the codewiki citation rule, and at write time it warns rather than blocks: a note about a file this branch has not created yet is the note most worth having. - rules/notes.md is the trigger — you are about to type a comment that is not a docstring — with the read side in prior-art.md, since notes are now the one corpus under docs/ that has an index. The init skill harvests the TODO/HACK comments already in a repository, without touching code. Template version 19; docs/notes.md is the fifth seeded anchor. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01YbWPnnFpqu4wa71ri76iw2 --- CLAUDE.md | 23 +- design/orchestration.md | 55 +- internal/assets/assets.go | 17 +- internal/assets/assets_test.go | 3 +- .../assets/templates/agents/code-review.md | 3 +- internal/assets/templates/docs/notes.md | 40 ++ internal/assets/templates/entry.md | 4 +- .../assets/templates/rules/knowledge-base.md | 10 +- internal/assets/templates/rules/notes.md | 53 ++ internal/assets/templates/rules/prior-art.md | 14 +- .../assets/templates/skills/init/SKILL.md | 6 + internal/cli/cli.go | 3 + internal/cli/notes.go | 529 ++++++++++++++++++ internal/cli/notes_test.go | 318 +++++++++++ internal/notes/notes.go | 464 +++++++++++++++ internal/notes/notes_test.go | 332 +++++++++++ internal/paths/paths.go | 10 + internal/validate/all.go | 3 +- internal/validate/notes.go | 90 +++ internal/validate/notes_test.go | 103 ++++ 20 files changed, 2050 insertions(+), 30 deletions(-) create mode 100644 internal/assets/templates/docs/notes.md create mode 100644 internal/assets/templates/rules/notes.md create mode 100644 internal/cli/notes.go create mode 100644 internal/cli/notes_test.go create mode 100644 internal/notes/notes.go create mode 100644 internal/notes/notes_test.go create mode 100644 internal/validate/notes.go create mode 100644 internal/validate/notes_test.go diff --git a/CLAUDE.md b/CLAUDE.md index 0f496f1..322828f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -12,7 +12,7 @@ This file covers working *on* scc. The product's own rules and methodology are n Note: this repo is not itself an scc workspace (no harness directory, `specs/`, `plans/`, or `docs/` are committed) — those trees only exist in workspaces the binary scaffolds, and in test temp dirs. -**Status: v0.4.0-shaped.** Everything through `design/plan.md` phase 10 is built and green: scaffolding (`init`), artifact creation (`spec`, `plan`), and all eight validators behind `scc validate`. `init` also scaffolds the eight skills named in `design/orchestration.md` §6 — the six knowledge-base authors (one per `docs/` artifact a validator checks, plus `prd`) and the two workflow skills, `plan-run` and `init` — each with a `scc-`-prefixed slash command derived from the same list (`assets.Skills()`, which is `KnowledgeSkills` + `WorkflowSkills`), wherever the harness has a command surface. +**Status: v0.4.0-shaped.** Everything through `design/plan.md` phase 10 is built and green: scaffolding (`init`), artifact creation (`spec`, `plan`), and all nine validators behind `scc validate`. `init` also scaffolds the eight skills named in `design/orchestration.md` §6 — the six knowledge-base authors (one per `docs/` artifact a validator checks, plus `prd`) and the two workflow skills, `plan-run` and `init` — each with a `scc-`-prefixed slash command derived from the same list (`assets.Skills()`, which is `KnowledgeSkills` + `WorkflowSkills`), wherever the harness has a command surface. These landed after phase 10, and all are documented in `design/orchestration.md` §6 and §12: @@ -96,11 +96,21 @@ These landed after phase 10, and all are documented in `design/orchestration.md` **The language is the third kickoff answer.** `autonomy.md` asks it with the other two and it lands in the artifact's frontmatter as `lang: en|wenyan`, graded by `checkKickoffAs` on exactly the terms `autonomy` and `ci` are — checked when present, absent meaning the run predates the question. There is no `--lang` flag on `spec new` or `plan new`: it is the one answer that can arrive after the file exists, so `scc patch fm lang=wenyan` is the whole path to it, and a value neither the rule nor the validator knows is rolled back like any other bad edit. `TestTheRuleOffersEveryKickoffAnswerThisAccepts` is what stops the rule and the validator from naming different values. - **`prior-art.md`, the read side of the knowledge base.** `docs/` is consulted *before the first artifact of a piece of work exists* — before `scc spec new`, before `scc plan new`, before code — because it is the constraint set and not reference material for when somebody is stuck: an ADR binds the design about to be written, `stack.md` says what may be built on, `glossary.md` says what to call it, and a spec already covering the area makes the work a delta rather than a second statement of the same feature. It prevents a failure that is invisible on review — a spec that re-decides a settled decision reads as new work, not as a contradiction — and under autonomy nobody sees the phase where noticing was still possible. - **It is a rule of its own rather than a paragraph in `knowledge-base.md`**, because the two halves fire at opposite moments: that one is triggered by having learned something, this one by being about to write, and a read-side instruction filed under the write-side rule is read after the spec exists. It also has to state the thing no index states — `scc map` covers `plans/` and `specs/`, the symbol graph covers code, so **`docs/` is the one corpus reached by opening a file**. That stays cheap only because the seeded anchors are built for it: `glossary.md` and `stack.md` are lists, `wiki/index.md` and the ADR filenames are tables of contents, so a page is opened when its title bears on the work and never to survey the base. What it finds is stated up front and then cited into the artifact (`adr:0007-…`, a delta, a `[[wikilink]]`), which is the only trace an `auto` run leaves that the pass happened at all. -- **The `init` skill and `/scc-init`, which fill what `scc init` can only lay down empty.** The CLI command scaffolds four anchors holding the format their validators check; what turns them into this project's knowledge base is a survey of the code, and a survey is judgment rather than scaffolding. So the skill holds the three things no rule can: the survey that precedes every artifact, the order across the six knowledge authors — `stack.md` first because `stack.undocumented-dependency` gives it a finish line, `docs/adr/` last because it is where invention is most likely, `scc validate` between stages — and one bar that applies only here. + **It is a rule of its own rather than a paragraph in `knowledge-base.md`**, because the two halves fire at opposite moments: that one is triggered by having learned something, this one by being about to write, and a read-side instruction filed under the write-side rule is read after the spec exists. It also has to state the thing no index states — `scc map` covers `plans/` and `specs/`, the symbol graph covers code, and `scc notes find` covers the note log, so **the rest of `docs/` is reached by opening a file**. That stays cheap only because the seeded anchors are built for it: `glossary.md` and `stack.md` are lists, `wiki/index.md` and the ADR filenames are tables of contents, so a page is opened when its title bears on the work and never to survey the base. What it finds is stated up front and then cited into the artifact (`adr:0007-…`, a delta, a `[[wikilink]]`), which is the only trace an `auto` run leaves that the pass happened at all. +- **The `init` skill and `/scc-init`, which fill what `scc init` can only lay down empty.** The CLI command scaffolds five anchors holding the format their validators check; what turns them into this project's knowledge base is a survey of the code, and a survey is judgment rather than scaffolding. So the skill holds the three things no rule can: the survey that precedes every artifact, the order across the six knowledge authors — `stack.md` first because `stack.undocumented-dependency` gives it a finish line, `docs/adr/` last because it is where invention is most likely, `scc validate` between stages, then the in-code `TODO`/`HACK` comments harvested into `docs/notes.md` last — and one bar that applies only here. **That bar is the design.** Everything written on this run is *reconstructed rather than remembered*, so nothing goes in that cannot be pointed at (a manifest, a CI file, a commit, a migration), and what nobody can justify is reported by name instead of filled in with something plausible: a gap is visible, an invention is believed, and the base's whole value is being trustworthy without checking. Each reconstructed ADR says so in its own `## Context`. It is a second `WorkflowSkill` for the same reason `plan-run` was the first — the knowledge skills each fire when *their* concern goes live, and none of them fires when the base is empty. What it deliberately never writes is `specs/`: restating a working system as requirements is the failure mode of a documentation pass, and `specs.md` already says a spec meets existing code as a delta, so the first spec is written by the next change. -- **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`. +- **`docs/notes.md` and `scc notes`, and the code stops carrying notes.** A comment says what a thing *is* and how to use it; everything else that used to be written beside code — the gotcha, the why-not, the "careful, this looks wrong and is not" — reaches exactly one reader, the one already looking at that line. Nobody asking *what do we know about this area* ever finds it, no command lists it, and it dies with the file. So `rules/notes.md` moves it into one log, and the log is queryable. + + **One note is one line**, index fields first — `- n-0042 2026-02-09 #gotcha @internal/cli/launch.go — wrap writes MCP config to the agent's own file` — and the single line is the entire design. A match is a *whole note*, so `grep ' #gotcha ' docs/notes.md` and `scc notes find --tag gotcha` answer the same question without either one reasoning about where a record ends; that is what lets the file that centralizes every note never be a file anybody reads. It also closes the door the v1 plan format left open, where `## Notes` grew to half the file because nothing forbade it: here there is nowhere for prose to go, and a thought needing a second line is a wiki page, an ADR, or a task — all three of which already exist, and the rule says which is which. + + **The CLI is the writer, never a gatekeeper.** `scc notes add` allocates the id and gets the format right, because a format nobody can be made to type is one that decays; `find | show | tags | paths | rm | validate` is the rest. What it adds over grep is the questions a substring cannot answer — which tags exist before somebody coins a fourth, what this project already knows about a path, what is new since a date. `--tag` is required and has no default, since a default would be one tag on everything, which is the drift the index exists to prevent. + + **A number is spent, never reused.** `rm` takes the text out and leaves an HTML-comment tombstone where the note stood, so a citation to `n-0042` can dangle but can never come to mean a different note. mdscan blanks comments, so the tombstone is invisible to a rendered read, to a grep for a tag, and to every parser except the one allocating the next id. + + **`@path` is the stale check** — the notes half of the codewiki citation rule, since a note about code that no longer exists is read as current, which is worse than the comment it replaced: that one at least died with the file. At *write* time it is a warning and never a block, because a note about a file this branch has not created yet is the note most worth having; `notes add` otherwise writes under the same verify-and-roll-back contract as `scc patch`. The ninth validator also reports the failure this file cannot tolerate quietly: a hand-written line that missed the grammar, which no query will ever return. + +- **The five seeded `docs/` anchors** (`assets.Seeds`). `init` writes `glossary.md`, `stack.md`, `notes.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. @@ -139,7 +149,7 @@ cmd/scc/main.go os.Exit(cli.Run(os.Args[1:])) | scaffold · validate write / check | \ - assets · manifest ears · mdscan · artifact templates, hashes, grammars + assets · manifest ears · mdscan · artifact · notes templates, hashes, grammars | paths · workspace · render · textutil · finding | @@ -164,7 +174,8 @@ Three packages sit off to the side of that tree — `rtk`, `headroom`, `codegrap | `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 *and their flags*, requirements, spec-reference leaves, paragraph blocks. Owns **every grammar** (task, requirement, spec reference, flag), `Find` for address resolution, `Editor` for line splices resolved against the original and applied bottom-up, `Search`, the schedule (`Ready`/`BlockedTasks`/`Next`/`Cycles`, one implementation shared by `--next`, `--ready` and `--blocked`), and the seal. 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/notes` | `docs/notes.md`: the note grammar, the log's regions, the query, and the splice. One note is one line, so a match is a whole note — the property every other decision here protects. Knows nothing about findings or exit codes, the same seam `internal/artifact` keeps with the plan grammar. | +| `internal/validate` | The nine 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 (`rtk.Markers`, spliced by `internal/mdblock`), the foreign-block detection that names Headroom's copy, and finding or `cargo install`ing the binary. | | `internal/headroom` | Headroom's agent-slug table, the `wrap` argument vector, the MCP opt-out discovered from `wrap --help`, and finding or installing the binary (uv, then pip — never npm, which ships the SDK and no CLI). The slugs live here rather than on `paths.Harness` because they are Headroom's vocabulary, not scc's layout. | | `internal/codegraph` | CodeGraph's argument vectors (`init`/`sync`/`index`/`status`/`query`/`explore`), the `.codegraph/` presence test, `codegraph.Markers` for the usage block `scc launch` splices, and finding or `npm install -g`ing the binary. Composes command lines and reads nothing inside the graph — the database is CodeGraph's schema on CodeGraph's schedule. | diff --git a/design/orchestration.md b/design/orchestration.md index 738f394..ecbd6df 100644 --- a/design/orchestration.md +++ b/design/orchestration.md @@ -333,6 +333,7 @@ a shape is checkable. Findings exit `2`. | **codewiki** | at `docs/codewiki/`, one page per area: every `[path:start-end]()` citation resolves against the checkout · slugs unique and derived from their headings · no section citing nothing | | **adr** | numbering contiguous · superseded records marked rather than edited · `adr:` citations resolve | | **glossary** | each concept has one canonical term · an avoided synonym used as a whole token is a finding | +| **notes** | at `docs/notes.md`, one line per note: every line under `## Log` parses as `- n-0001 YYYY-MM-DD #tag @path — text` · ids unique · at least one tag · a `@path` that no longer resolves. A line that missed the grammar is the finding that matters — it is invisible to every query, so the note is already lost | | **stack** | every direct dependency the project declares appears in `docs/stack.md` — a dependency file is structured data, not source, so this is checkable without reading code. Seven readers today: `go.mod`, `package.json`, `requirements.txt`, `pyproject.toml`, `Cargo.toml`, `composer.json`, `pom.xml` | Two of these are worth calling out because they are stronger than they look. @@ -361,7 +362,7 @@ understood. That is the same rule as "a manifest scc cannot parse produces no fi and it is why those three are absent: their manifests are executable code, and reading one honestly would mean evaluating it. -### The discipline eight validators require +### The discipline nine validators require A catalogue this size can destroy its own value. In studied static-analysis deployments **35–91% of warnings are non-actionable**, false positives are the single most common reason @@ -615,16 +616,16 @@ firing on scc's own output is the one defect that teaches users to disbelieve th other seven. `TestSkillsNameRulesByTheirHarnessPath` catches what no validator can see — a `../..` sitting inside a code span is silently useless to whoever reads it. -### The four seeded anchors — `docs/` is not an empty tree +### The five seeded anchors — `docs/` is not an empty tree -`init` writes `docs/glossary.md`, `docs/stack.md`, `docs/wiki/index.md`, and -`docs/wiki/changelog.md`, each one a heading and the format its validator checks. +`init` writes `docs/glossary.md`, `docs/stack.md`, `docs/notes.md`, `docs/wiki/index.md`, +and `docs/wiki/changelog.md`, each one a heading and the format its validator checks. These are the knowledge base's only documents with a *fixed name*; a wiki page, an ADR, and a codewiki page are named after what they are about, so those directories are correctly scaffolded empty. The reason is the same one that makes the skills ship: a workspace that arrives with -eight validators and an empty `docs/` demands conformance to documents nobody was +nine validators and an empty `docs/` demands conformance to documents nobody was handed. Seeding also improves the first finding a real project sees — a repo with dependencies used to get one `stack.missing`, and now gets one line per undocumented dependency, pointing at a file that already explains what an entry looks like. @@ -637,6 +638,50 @@ the two-harness case without a rule of its own, because `docs/` is one tree per rather than one per harness — the second `init` finds the files already there and leaves them alone, exactly as it does for any existing file. +### `docs/notes.md` — the note log, and why the code stops being one + +A comment says what a thing is and how to use it. Everything else that gets written +beside code — the gotcha, the why-not, the "careful, this looks wrong and is not" — +reaches exactly one reader: the one already looking at that line. Nobody asking *what +do we know about this area* finds it, no command lists it, and it dies with the file. +So it moves to one place, and the place is queryable. + +**One note is one line**, index fields first: + +```markdown +- n-0042 2026-02-09 #gotcha @internal/cli/launch.go — wrap writes MCP config to the agent's own file, so it outlives the session +``` + +The single line is the whole design, and it buys three things at once. A match is a +*whole note*, so `grep ' #gotcha ' docs/notes.md` and `scc notes find --tag gotcha` +answer the same question and neither has to reason about where a record ends — which +is what lets a file that centralizes every note never be a file anybody reads. It +closes the door the v1 plan format left open, where `## Notes` grew to half the file +because nothing forbade it: here there is nowhere for prose to go, and a thought that +needs a second line is a wiki page, an ADR, or a task, all three of which already +exist. And it makes the fields addressable — `#tag` is the index the log is queried +by, `@path` is what keeps a note attached to code without living inside it, and the +id is what a commit message or a spec cites. + +**`scc notes` is the writer, not a gatekeeper.** `add` allocates the id and gets the +format right, because a format nobody can be made to type is a format that decays; +`find`, `tags`, `paths` and `show` are the reading surface. The line stays greppable +anyway — the CLI exists for the questions a substring cannot answer (which tags exist +before I coin a fourth, what is known about this path, what is new since last week), +not to stand in front of the file. + +**A number is spent, never reused.** `rm` takes the note's text out and leaves an +HTML-comment tombstone where it stood, so a citation to `n-0042` can go dangling but +can never come to mean a different note. The tombstone is invisible to a rendered +read, to a grep for a tag, and to every parser except the one that allocates the next +id. + +**`@path` is the stale check**, and it is the notes half of the codewiki citation +rule: a note about code that no longer exists is read as current, which is worse than +the comment it replaced — that one at least died with the file. At the moment of +*writing* it is only a warning, because a note about a file this branch has not +created yet is the note most worth having. + ## 7 · Agents — review only Two subagents ship with the workspace, and both of them read rather than write: diff --git a/internal/assets/assets.go b/internal/assets/assets.go index 33848c6..150ed6b 100644 --- a/internal/assets/assets.go +++ b/internal/assets/assets.go @@ -137,7 +137,16 @@ import ( // flow performs. How a user arranges several sessions against one repo is theirs, and // saying so is enough — the rule keeps only what it was ever really carrying, which is // to leave the checkout back on `main` and clean. -const Version = "18" +// 19: notes.md — the small durable observation gets one place to live, and the code +// stops being it. A comment says what a thing is and how to use it; the gotcha, the +// why-not and the "careful, this is not what it looks like" go to docs/notes.md as +// one line each, carrying the path they are about. One line is the whole design: a +// match is a whole note, so grep and `scc notes find` answer the same question, and +// the file that centralizes every note is never a file anybody has to read. It is a +// rule of its own because its trigger is a keystroke — you are about to type a +// comment that is not a docstring — and prior-art.md gains the read side, since this +// is now the one corpus under docs/ that does have an index. +const Version = "19" // The embedded tree. "all:" so nothing is silently dropped for having a name the // default embed pattern skips. @@ -211,6 +220,7 @@ func Workspace(h paths.Harness) []File { "delivery.md", "specs.md", "knowledge-base.md", + "notes.md", "code-search.md", "artifacts.md", } { @@ -260,7 +270,7 @@ func Workspace(h paths.Harness) []File { } // Seed is one of the `docs/` anchor files `scc init` lays down: the knowledge base's -// four fixed documents, each holding the format its validator checks and nothing +// five fixed documents, each holding the format its validator checks and nothing // else. // // A seed is deliberately NOT a managed file, and that is a third category rather @@ -281,7 +291,7 @@ type Seed struct { // Seeds returns the anchors in destination order. // -// Four, and the rule that picks them is the same one that picks the skills: a seed +// Five, and the rule that picks them is the same one that picks the skills: a seed // for each fixed `docs/` document a validator checks. The per-concept pages, the // ADRs, and the codewiki pages have no fixed name, so they have no anchor — an // empty directory is the honest scaffold for those. @@ -292,6 +302,7 @@ type Seed struct { func Seeds() []Seed { return []Seed{ {Name: "docs/glossary.md", Rel: path.Join(paths.DocsSeg, paths.GlossarySeg)}, + {Name: "docs/notes.md", Rel: path.Join(paths.DocsSeg, paths.NotesSeg)}, {Name: "docs/stack.md", Rel: path.Join(paths.DocsSeg, paths.StackSeg)}, {Name: "docs/wiki/changelog.md", Rel: path.Join(paths.DocsSeg, paths.WikiSeg, paths.WikiLog)}, {Name: "docs/wiki/index.md", Rel: path.Join(paths.DocsSeg, paths.WikiSeg, paths.WikiIndex)}, diff --git a/internal/assets/assets_test.go b/internal/assets/assets_test.go index 88313b6..c818138 100644 --- a/internal/assets/assets_test.go +++ b/internal/assets/assets_test.go @@ -617,12 +617,13 @@ func TestCommandsCarryTheirDescription(t *testing.T) { } } -// The seeds are the knowledge base's four fixed documents, at the paths every +// The seeds are the knowledge base's five fixed documents, at the paths every // validator already looks for them. A seed written anywhere else would be a file // nothing reads, next to the finding saying the real one is missing. func TestSeedsLandWhereTheValidatorsLook(t *testing.T) { want := []string{ "docs/" + paths.GlossarySeg, + "docs/" + paths.NotesSeg, "docs/" + paths.StackSeg, "docs/" + paths.WikiSeg + "/" + paths.WikiLog, "docs/" + paths.WikiSeg + "/" + paths.WikiIndex, diff --git a/internal/assets/templates/agents/code-review.md b/internal/assets/templates/agents/code-review.md index 0d2756a..b9c80ba 100644 --- a/internal/assets/templates/agents/code-review.md +++ b/internal/assets/templates/agents/code-review.md @@ -73,7 +73,8 @@ order of what bites: - **Complexity not paying for itself** — an abstraction with one caller, a layer that only forwards, a config knob nobody asked for. - **Naming and comments that lie.** A comment describing the previous behavior is - worse than none. + worse than none — and a comment that is not a docstring is a finding of its own: a + `TODO`, a `HACK` or an aside belongs in `docs/notes.md`, not in the diff. If a gate cannot be run — no test command, a suite needing a service you lack — report it `not-run` with the reason. **Never report a skipped gate as passing**, and never diff --git a/internal/assets/templates/docs/notes.md b/internal/assets/templates/docs/notes.md new file mode 100644 index 0000000..e9dcecb --- /dev/null +++ b/internal/assets/templates/docs/notes.md @@ -0,0 +1,40 @@ +# Notes + +Every small durable observation about this project, one per line. The gotcha, the +why-not, the "careful, this looks wrong and is not" — the things that used to be a +comment beside the code, where only the reader already looking at that line ever +found them. + +**Notes do not live in the code.** A comment says what a thing is and how to use +it; anything else is a note, and it belongs here with the path it is about +attached to it. + +One note is one line, index fields first: + +```markdown +- n-0000 2026-02-09 #gotcha @internal/cli/launch.go — wrap writes MCP config to the agent's own file, so it outlives the session +``` + +`n-0000` is the id; real ones start at `n-0001` and are never reused, so a note can +be cited as `n-0042` from anywhere — including the one place code may still mention +one. Then the date. Then `#tags`, at least one, which is what the log is queried by. +Then `@paths`, repo-relative, which is what keeps a note attached to the code without +living inside it. Then an em dash, and the note itself. + +The one line is the contract: a match is a whole note, never a fragment of one, so +`grep ' #gotcha ' docs/notes.md` and `scc notes find --tag gotcha` answer the same +question. Write with `scc notes add "…" --tag gotcha --path `, which +allocates the id and gets the format right; read with `scc notes find`, +`scc notes tags`, `scc notes show n-0001`. + +**If it needs a second line, it is not a note.** Something learned and worth +explaining is a `wiki/` page. A decision that is hard to reverse is an `adr/` +record. Something that has to be done is a task in a plan. A note is the thing +none of those three would take. + + + +## Log + + diff --git a/internal/assets/templates/entry.md b/internal/assets/templates/entry.md index d00a0e9..44a10ee 100644 --- a/internal/assets/templates/entry.md +++ b/internal/assets/templates/entry.md @@ -31,6 +31,7 @@ Triggered by what you are about to touch: - `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 +- `notes.md` — **before typing a comment that is not a docstring**; `scc notes find --path ` ## Ask the index before you read the file @@ -50,7 +51,7 @@ finding — so you need not read a plan to change one line of it. ``` 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 +docs/ knowledge base — wiki, adr, codewiki, glossary, stack, notes {{.RulesCol}}the methodology above {{.SkillsCol}}authoring each part of docs/, and running a plan group by group {{- if .HasCommands}} @@ -62,6 +63,5 @@ docs/ knowledge base — wiki, adr, codewiki, glossary, stack `scc validate` — or `npx @protonspy/scc validate` if not installed (`@` pins for CI). `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 it is on you. diff --git a/internal/assets/templates/rules/knowledge-base.md b/internal/assets/templates/rules/knowledge-base.md index cd6b9a4..9244541 100644 --- a/internal/assets/templates/rules/knowledge-base.md +++ b/internal/assets/templates/rules/knowledge-base.md @@ -12,12 +12,13 @@ docs/ codewiki/ narrated code, citing exact line ranges glossary.md one canonical term per concept stack.md adopted technology + notes.md one line per observation — [notes.md](notes.md) ``` `scc validate` checks all of it structurally: broken wikilinks, orphan pages, index/changelog desync, unprocessed `raw/`, ADR numbering, citation resolution, -synonyms used where a canonical term belongs, and dependencies missing from -`stack.md`. +synonyms used where a canonical term belongs, dependencies missing from `stack.md`, +and a note no query would return. ## wiki/ @@ -76,6 +77,5 @@ in the wiki. An avoided synonym used as a whole word in `docs/` is a finding. Every adopted technology, with one line on why. **Technology not listed here is an open decision, never something adopted silently** — and because a dependency file is -structured data, this is checkable: a direct dependency declared there and absent -here is a finding. Adding a dependency is two acts: add it, and say why it earned its -place. +structured data, this is checkable: a direct dependency declared there and absent here +is a finding. Adding a dependency is two acts: add it, and say why. diff --git a/internal/assets/templates/rules/notes.md b/internal/assets/templates/rules/notes.md new file mode 100644 index 0000000..a28d6c8 --- /dev/null +++ b/internal/assets/templates/rules/notes.md @@ -0,0 +1,53 @@ +# Notes — the code is not where a note goes + +A comment says what a thing **is** and how to use it, on the declaration it belongs +to. Everything else you were about to write in the code — the gotcha, the why-not, +the "careful, this looks wrong and is not", the afternoon you lost — is a note, and +every note lives in one file: `docs/notes.md`. + +Not a style preference. A comment reaches one reader, the one already looking at +that line. Nobody asking *what do we know about this area* ever finds it, no command +lists it, and it dies unread with the code it sat beside. + +**No `TODO`, `FIXME`, `HACK`, `NOTE` or `XXX` in code, and no commented-out code.** +Something to do is a task in a plan; something to know is a note here. + +## Writing one + +Never by hand — `add` allocates the id and gets the format right: + +``` +scc notes add "wrap writes MCP config to the agent's own file, so it outlives the session" \ + --tag gotcha --path internal/cli/launch.go +``` + +`--tag` is required, and it is what the log is queried by: run `scc notes tags` +first and reuse one rather than coining a fourth name for the same concern. +`--path` is what keeps a note attached to code without living in it — repo-relative, +as many as apply, and one that later disappears is reported rather than left to rot. + +**One line, always**, which is what makes a match a whole note. If it needs a second +line it is not a note: + +| It is… | so it goes in | +|---|---| +| something learned, worth explaining | `docs/wiki/pages/` — [knowledge-base.md](knowledge-base.md) | +| a decision that is hard to reverse | `docs/adr/` | +| how a whole area of code works | `docs/codewiki/` | +| something that has to be done | a plan's `## Tasks` — [artifacts.md](artifacts.md) | + +## Reading it + +Never end to end. Ask it: + +| The question | Ask | +|---|---| +| What do we know about this file? | `scc notes find --path ` | +| …about this concern? | `scc notes tags` · `scc notes find --tag ` | +| …containing this word? | `scc notes find …` | +| What was noted lately? | `scc notes find --since ` | +| What is `n-0042`? | `scc notes show n-0042` | + +The line format is greppable on purpose — `grep ' #gotcha ' docs/notes.md` is the +same answer — and an id is citable: from a spec, from a commit, and from the one +comment that may still mention a note, `see n-0042`. diff --git a/internal/assets/templates/rules/prior-art.md b/internal/assets/templates/rules/prior-art.md index d9cde42..d910c3f 100644 --- a/internal/assets/templates/rules/prior-art.md +++ b/internal/assets/templates/rules/prior-art.md @@ -20,12 +20,14 @@ it reads as new work rather than as a contradiction. | What is it called? | `docs/glossary.md` | the canonical term, and the synonyms that are findings | | What may I build on? | `docs/stack.md` | what is adopted; anything absent is an open decision | | How does it work today? | `docs/wiki/index.md` · `docs/codewiki/` | the concept pages, and the code already narrated | - -**`docs/` is in no index.** `scc map` covers `plans/` and `specs/`; the symbol graph -covers code ([code-search.md](code-search.md)). So this pass is deliberate reading, and -it is cheap only because the anchors are built for it: `glossary.md` and `stack.md` are -lists, `wiki/index.md` and the ADR filenames are tables of contents. Open a page when -its title bears on this work — never to survey the base. +| What bit somebody here already? | `scc notes find --path ` · `--tag` | a gotcha already paid for once ([notes.md](notes.md)) | + +**`docs/` is in no index but one.** `scc map` covers `plans/` and `specs/`, the symbol +graph covers code ([code-search.md](code-search.md)), and `scc notes find` covers the +note log. The rest is deliberate reading, cheap only because the anchors are built for +it: `glossary.md` and `stack.md` are lists, `wiki/index.md` and the ADR filenames are +tables of contents. Open a page when its title bears on this work — never to survey +the base. ## Say what you found, then cite it diff --git a/internal/assets/templates/skills/init/SKILL.md b/internal/assets/templates/skills/init/SKILL.md index 2d56cba..495c8e7 100644 --- a/internal/assets/templates/skills/init/SKILL.md +++ b/internal/assets/templates/skills/init/SKILL.md @@ -95,6 +95,12 @@ stages so no stage inherits the previous one's findings. what it was reconstructed from, and when. `status: accepted` where the code shows the decision in force; never `proposed` for something already built. +7. **`docs/notes.md`** — the `TODO`, `FIXME`, `HACK` and aside comments already in the + code, moved into the log one line each with the file they sit on: + `scc notes add "…" --tag --path `. Leave the comment where it is; this run + does not touch code, and it goes when that file is next edited. What does not make + the move is anything that is really work — that is a task in a plan, not a note. + ## What this does not do - **It does not write specs.** Documenting a system that already works as `specs/` diff --git a/internal/cli/cli.go b/internal/cli/cli.go index 9de1be3..b832d8d 100644 --- a/internal/cli/cli.go +++ b/internal/cli/cli.go @@ -74,6 +74,8 @@ func Run(args []string) int { return runMap(args[1:]) case "patch": return runPatch(args[1:]) + case "notes": + return runNotes(args[1:]) case "skill": return runSkill(args[1:]) case "validate": @@ -121,6 +123,7 @@ Commands: 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 + notes The project's note log in docs/notes.md — add | find | show | tags | paths | rm skill Agent Skills conformance — validate validate Run every applicable validator; exit 2 on findings version Print the version diff --git a/internal/cli/notes.go b/internal/cli/notes.go new file mode 100644 index 0000000..631d6c3 --- /dev/null +++ b/internal/cli/notes.go @@ -0,0 +1,529 @@ +package cli + +import ( + "flag" + "fmt" + "os" + "strings" + + "github.com/protonspy/spec-claude-code/internal/assets" + "github.com/protonspy/spec-claude-code/internal/finding" + "github.com/protonspy/spec-claude-code/internal/notes" + "github.com/protonspy/spec-claude-code/internal/paths" + "github.com/protonspy/spec-claude-code/internal/render" + "github.com/protonspy/spec-claude-code/internal/validate" + "github.com/protonspy/spec-claude-code/internal/workspace" +) + +// runNotes dispatches `scc notes `: the project's note log, written +// one line at a time and read one query at a time. +// +// The command exists because the file's whole contract is a format, and a format +// nobody can be made to type is a format that decays. `notes add` is the only +// writer scc ships, so every note it puts in the log is one `notes find` can +// return — and the validator is there for the lines that arrive some other way. +// +// It is also the answer to the obvious objection: if the file is greppable, why a +// CLI at all? Because grep answers "which lines contain this" and a reader is +// asking three other things — what tags exist before I coin a fourth, what does +// this project already know about this path, what is new since last week — and +// each of those is a filter over parsed fields rather than a substring. The line +// format stays greppable anyway, so nothing here is a gatekeeper: `grep '#gotcha' +// docs/notes.md` and `scc notes find --tag gotcha` return the same lines. +func runNotes(args []string) int { + if len(args) == 0 { + notesUsage() + return ExitError + } + switch args[0] { + case "help", "-h", "--help": + notesUsage() + return ExitOK + case "add": + return runNotesAdd(args[1:]) + case "find": + return runNotesFind(args[1:]) + case "show": + return runNotesShow(args[1:]) + case "tags": + return runNotesIndex(args[1:], "tags") + case "paths": + return runNotesIndex(args[1:], "paths") + case "rm": + return runNotesRemove(args[1:]) + case "validate": + return runNotesValidate(args[1:]) + default: + render.Err(fmt.Sprintf("unknown notes subcommand %q", args[0])) + fmt.Fprintf(os.Stderr, "run `%s notes help` for the available subcommands\n", prog()) + return ExitError + } +} + +// loadNotes reads and parses docs/notes.md. A missing file is an empty log rather +// than an error: every query has to work in a workspace where nobody has written a +// note yet, which is every workspace on its first day. +func loadNotes(root string) (*notes.File, string, bool) { + abs := paths.Notes(root) + rel := relPath(root, abs) + b, err := os.ReadFile(abs) + if err != nil { + if os.IsNotExist(err) { + f, _ := notes.Parse(rel, "") + return f, rel, true + } + render.Err(err.Error()) + return nil, rel, false + } + f, err := notes.Parse(rel, string(b)) + if err != nil { + render.Err(err.Error()) + return nil, rel, false + } + return f, rel, true +} + +func runNotesAdd(args []string) int { + fs := flag.NewFlagSet("notes add", flag.ContinueOnError) + fs.SetOutput(os.Stderr) + root := addRoot(fs) + tags := fs.String("tag", "", "the note's tags, comma-separated `list` — at least one") + scopes := fs.String("path", "", "what the note is about: repo-relative `path`s, comma-separated") + date := fs.String("date", "", "override the note's `date` (YYYY-MM-DD); today by default") + force := fs.Bool("force", false, "write even if the note introduces a validation finding") + jsonOut := addJSON(fs) + rest, err := parseFlags(fs, args) + if err != nil { + return ExitError + } + // The text is the positionals joined, so an unquoted sentence arrives as the + // sentence it was typed as. `scc graph explore` does the same, for the same + // reason: quoting discipline is a tax paid at the moment somebody is least + // inclined to pay it. + text, err2 := notes.CheckText(strings.Join(rest, " ")) + if err2 != nil { + render.Err(err2.Error()) + render.Detail(fmt.Sprintf(" %s notes add \"the observation\" --tag gotcha --path internal/cli/notes.go", prog())) + return ExitError + } + target, ok := resolveRoot(*root) + if !ok || !requireWorkspace(target) { + return ExitError + } + + n := notes.Note{Date: notes.Today(), Text: text} + if *date != "" { + if err := notes.CheckDate(*date); err != nil { + render.Err(err.Error()) + return ExitError + } + n.Date = *date + } + for _, t := range splitList(*tags) { + if err := notes.CheckTag(t); err != nil { + render.Err(err.Error()) + return ExitError + } + n.Tags = append(n.Tags, t) + } + if len(n.Tags) == 0 { + // Required rather than defaulted. A tag is the index this file is queried + // by, and a default like "note" would be a tag on everything, which is a tag + // on nothing — the drift the tag index exists to prevent, seeded by scc. + render.Err("a note needs at least one --tag: it is what the log is queried by") + render.Detail(fmt.Sprintf(" %s notes tags lists the ones this project already uses", prog())) + return ExitError + } + for _, p := range splitList(*scopes) { + clean, err := notes.CheckPath(p) + if err != nil { + render.Err(err.Error()) + return ExitError + } + n.Paths = append(n.Paths, clean) + } + + abs := paths.Notes(target) + original, existed, ok := ensureNotesFile(abs) + if !ok { + return ExitError + } + f, err := notes.Parse(relPath(target, abs), original) + if err != nil { + render.Err(err.Error()) + return ExitError + } + n.ID = notes.ID(f.Next()) + content, err := f.Append(n) + if err != nil { + render.Err(err.Error()) + render.Detail(fmt.Sprintf(" add a `## %s` heading to %s, or delete the file and let this command reseed it", + notes.Section, relPath(target, abs))) + return ExitError + } + + // The same contract `scc patch` writes under: the file is re-validated after the + // write and the write is undone when it introduced a finding. It matters less + // here than it does for a plan — a note is one appended line — and it is worth + // having anyway, because the finding it catches is a hand-edited log this + // command was about to append a duplicate id to. + before, _ := validate.Notes(target) + if err := workspace.AtomicWrite(abs, []byte(withTrailingNewline(content)), 0o644); err != nil { + render.Err(err.Error()) + return ExitError + } + after, _ := validate.Notes(target) + // A path that does not resolve is reported and never blocking, which is the one + // place this command departs from `scc patch`. The stale check is about a log + // aging past the code it describes; at the moment of writing, a note about a file + // this branch has not created yet is exactly the note most worth having, and + // refusing it would teach the user to stop passing --path at all. + blocking, stale := split(newFindings(before.Sorted(), after.Sorted()), staleRule) + verified := "clean" + if len(blocking) > 0 { + if *force { + verified = "forced" + } else { + restore := []byte(original) + if !existed { + restore = nil + } + if err := undoNotesWrite(abs, restore); err != nil { + render.Err("the note introduced findings and the rollback failed: " + err.Error()) + return ExitError + } + verified = "rolled-back" + } + } + + written := verified != "rolled-back" + introduced := blocking + if *jsonOut { + code := ExitOK + if len(introduced) > 0 && !written { + code = ExitFindings + } + if emitJSON(struct { + Note notes.Note `json:"note"` + Line string `json:"line"` + Path string `json:"path"` + Written bool `json:"written"` + Verified string `json:"verified"` + Introduced []finding.Finding `json:"introduced,omitempty"` + Warnings []finding.Finding `json:"warnings,omitempty"` + }{n, n.Format(), relPath(target, abs), written, verified, introduced, stale}) != ExitOK { + return ExitError + } + return code + } + if !written { + render.Err(fmt.Sprintf("not written: the note would introduce %d finding(s) in %s", + len(introduced), relPath(target, abs))) + for _, f := range introduced { + render.Detail(fmt.Sprintf(" %s %s", f.Rule, f.Message)) + } + render.Detail(" fix the log first, or pass --force") + return ExitFindings + } + for _, f := range stale { + render.Warn(f.Message) + } + render.OK(n.Format()) + return ExitOK +} + +// staleRule is the one finding `notes add` reports without acting on. +const staleRule = "notes.stale-path" + +// split partitions findings by rule: everything else first, then the named rule. +func split(all []finding.Finding, rule string) (rest, named []finding.Finding) { + for _, f := range all { + if f.Rule == rule { + named = append(named, f) + continue + } + rest = append(rest, f) + } + return rest, named +} + +// ensureNotesFile returns the current content of docs/notes.md, seeding it from the +// embedded anchor when it is not there. +// +// Seeding here rather than only at `scc init` is what makes the log reachable in a +// workspace scaffolded before this shipped: seeds are written once and tracked +// nowhere, so `scc update` will never deliver one. The first `notes add` is the +// other honest moment to create it — and the seed carries the format, so the file +// explains itself to whoever opens it next. +func ensureNotesFile(abs string) (content string, existed bool, ok bool) { + b, err := os.ReadFile(abs) + if err == nil { + return string(b), true, true + } + if !os.IsNotExist(err) { + render.Err(err.Error()) + return "", false, false + } + seed, err := assets.Content(notesSeed) + if err != nil { + render.Err(err.Error()) + return "", false, false + } + return seed, false, true +} + +// notesSeed is the embedded anchor's name. Named here rather than inlined because +// assets.Seeds() names the same file, and the two must not drift. +const notesSeed = "docs/notes.md" + +// undoNotesWrite restores what was there before, including "nothing at all" — a +// rollback that left a seeded file behind would report a note as not written and +// still change the workspace. +func undoNotesWrite(abs string, original []byte) error { + if original == nil { + return os.Remove(abs) + } + return workspace.AtomicWrite(abs, original, 0o644) +} + +func runNotesFind(args []string) int { + fs := flag.NewFlagSet("notes find", flag.ContinueOnError) + fs.SetOutput(os.Stderr) + root := addRoot(fs) + tags := fs.String("tag", "", "only notes carrying one of these tags, comma-separated `list`") + scopes := fs.String("path", "", "only notes about one of these `path`s, or anything under them") + since := fs.String("since", "", "only notes dated on or after this `date` (YYYY-MM-DD)") + limit := fs.Int("limit", 0, "at most this many, newest first; 0 is all of them") + jsonOut := addJSON(fs) + rest, err := parseFlags(fs, args) + if err != nil { + return ExitError + } + target, ok := resolveRoot(*root) + if !ok || !requireWorkspace(target) { + return ExitError + } + if *since != "" { + if err := notes.CheckDate(*since); err != nil { + render.Err(err.Error()) + return ExitError + } + } + f, rel, ok := loadNotes(target) + if !ok { + return ExitError + } + + q := notes.Query{Tags: splitList(*tags), Paths: splitList(*scopes), Since: *since, Terms: rest} + for i, p := range q.Paths { + clean, err := notes.CheckPath(p) + if err != nil { + render.Err(err.Error()) + return ExitError + } + q.Paths[i] = clean + } + hits := f.Match(q) + total := len(hits) + if *limit > 0 && total > *limit { + hits = hits[total-*limit:] + } + + if *jsonOut { + return emitJSON(struct { + Path string `json:"path"` + Notes []notes.Note `json:"notes"` + Count int `json:"count"` + Matched int `json:"matched"` + }{rel, hits, len(hits), total}) + } + if total == 0 { + render.Info(fmt.Sprintf("no notes match — %d in %s", len(f.Notes), rel)) + return ExitOK + } + // Nothing but the notes on stdout, in the form the file holds them: the output + // of this command and a grep over the file are deliberately the same text, so + // neither one teaches a format the other contradicts. + for _, n := range hits { + fmt.Println(n.Format()) + } + if len(hits) < total { + // Never a silent cap. A truncated answer that looks complete is the one way + // this command can mislead, so what was dropped is said out loud — on stderr, + // so stdout stays the notes and nothing else. + render.Warn(fmt.Sprintf("--limit %d: showing the %d newest of %d matches", *limit, len(hits), total)) + } + return ExitOK +} + +func runNotesShow(args []string) int { + fs := flag.NewFlagSet("notes show", flag.ContinueOnError) + fs.SetOutput(os.Stderr) + root := addRoot(fs) + jsonOut := addJSON(fs) + rest, err := parseFlags(fs, args) + if err != nil { + return ExitError + } + if len(rest) != 1 { + render.Err(fmt.Sprintf("expected exactly one note id, got %d", len(rest))) + return ExitError + } + target, ok := resolveRoot(*root) + if !ok || !requireWorkspace(target) { + return ExitError + } + f, rel, ok := loadNotes(target) + if !ok { + return ExitError + } + n, found := f.Get(rest[0]) + if !found { + // A removed note answers differently from one that never existed: the id was + // spent, and a reader who followed a citation here deserves to be told that + // rather than to doubt the citation. + if line, was := f.Retired[rest[0]]; was { + render.Err(fmt.Sprintf("%s was removed (%s:%d)", rest[0], rel, line)) + return ExitError + } + render.Err(fmt.Sprintf("no note %q in %s", rest[0], rel)) + return ExitError + } + if *jsonOut { + return emitJSON(struct { + Path string `json:"path"` + Note notes.Note `json:"note"` + Line string `json:"line"` + }{rel, n, n.Format()}) + } + fmt.Println(n.Format()) + return ExitOK +} + +// runNotesIndex is `notes tags` and `notes paths`: the two questions asked before +// writing rather than after. One function because they are one report over two +// fields, and two commands because "which tags exist" and "what do we know about +// this area" are asked at different moments. +func runNotesIndex(args []string, which string) int { + fs := flag.NewFlagSet("notes "+which, 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, "notes "+which) { + return ExitError + } + target, ok := resolveRoot(*root) + if !ok || !requireWorkspace(target) { + return ExitError + } + f, rel, ok := loadNotes(target) + if !ok { + return ExitError + } + index := f.Tags() + if which == "paths" { + index = f.Paths() + } + if *jsonOut { + return emitJSON(struct { + Path string `json:"path"` + Index []notes.Count `json:"index"` + Kind string `json:"kind"` + Count int `json:"count"` + }{rel, index, which, len(index)}) + } + if len(index) == 0 { + render.Info(fmt.Sprintf("no %s yet — %d notes in %s", which, len(f.Notes), rel)) + return ExitOK + } + for _, e := range index { + fmt.Printf("%4d %s\n", e.Count, e.Name) + } + return ExitOK +} + +func runNotesRemove(args []string) int { + fs := flag.NewFlagSet("notes rm", flag.ContinueOnError) + fs.SetOutput(os.Stderr) + root := addRoot(fs) + jsonOut := addJSON(fs) + rest, err := parseFlags(fs, args) + if err != nil { + return ExitError + } + if len(rest) != 1 { + render.Err(fmt.Sprintf("expected exactly one note id, got %d", len(rest))) + return ExitError + } + target, ok := resolveRoot(*root) + if !ok || !requireWorkspace(target) { + return ExitError + } + abs := paths.Notes(target) + f, rel, ok := loadNotes(target) + if !ok { + return ExitError + } + content, n, err := f.Remove(rest[0]) + if err != nil { + render.Err(err.Error()) + return ExitError + } + if err := workspace.AtomicWrite(abs, []byte(withTrailingNewline(content)), 0o644); err != nil { + render.Err(err.Error()) + return ExitError + } + if *jsonOut { + return emitJSON(struct { + Path string `json:"path"` + Removed notes.Note `json:"removed"` + Line string `json:"line"` + }{rel, n, n.Format()}) + } + // What was removed, in full: the number is spent for good — a tombstone stays + // where the note stood so it is never handed out again — and this line is the + // only copy the caller still has outside git. + render.OK("removed " + n.ID) + render.Detail(" " + n.Format()) + return ExitOK +} + +func runNotesValidate(args []string) int { + return runValidation("notes", args, func(root string, rest []string) (*finding.Set, error) { + if !noPositionals(rest, "notes validate") { + return nil, errUsage + } + return validate.Notes(root) + }) +} + +func notesUsage() { + fmt.Fprintf(os.Stderr, `%s notes — the project's note log: %s + +Usage: + %s notes [flags] + +Subcommands: + add Append a note: add "" --tag [,] [--path

[,

]] [--date] + find Query it: [terms…] [--tag] [--path] [--since] [--limit] + show One note by id + tags The tags in use, most used first — read this before coining one + paths The paths the log knows something about + rm Remove one note by id (the number is never reused) + validate Check the log's shape; exit 2 on findings + +A note is one line, and that is the contract: a match is a whole note, so + + grep ' #gotcha ' %s + +answers the same question as "notes find --tag gotcha". Anything needing a second +line is a wiki page, an ADR, or a task — never a longer note. + + - %s0001 2026-08-27 #gotcha @internal/cli/notes.go — one line, index fields first +`, render.Bold(prog()), paths.DocsSeg+"/"+paths.NotesSeg, prog(), + paths.DocsSeg+"/"+paths.NotesSeg, notes.IDPrefix) +} diff --git a/internal/cli/notes_test.go b/internal/cli/notes_test.go new file mode 100644 index 0000000..1635bfe --- /dev/null +++ b/internal/cli/notes_test.go @@ -0,0 +1,318 @@ +package cli + +import ( + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/protonspy/spec-claude-code/internal/paths" +) + +func addNote(t *testing.T, root string, args ...string) string { + t.Helper() + stdout, stderr, code := run(t, append([]string{"notes", "add"}, append(args, "--root", root)...)...) + if code != ExitOK { + t.Fatalf("notes add %v: exit %d (stderr: %s)", args, code, stderr) + } + // render.OK prefixes a glyph; the note is what follows it, and every + // assertion here is about the note being byte-identical to what the file holds. + return strings.TrimSpace(strings.TrimPrefix(strings.TrimSpace(stdout), "✓")) +} + +func notesFile(t *testing.T, root string) string { + t.Helper() + b, err := os.ReadFile(paths.Notes(root)) + if err != nil { + t.Fatalf("read notes: %v", err) + } + return string(b) +} + +// The round trip the whole feature is: write one, find it by every index it +// carries, read it back by id, and remove it. +func TestNotesRoundTrip(t *testing.T) { + root := initWorkspace(t) + + first := addNote(t, root, "the appender lands against the last note, never after it", + "--tag", "gotcha", "--path", "internal/cli/notes.go", "--date", "2026-02-09") + if !strings.Contains(first, "n-0001") || !strings.Contains(first, "#gotcha") { + t.Fatalf("add reported %q", first) + } + addNote(t, root, "the log reads oldest first", "--tag", "convention", "--date", "2026-03-01") + + stdout, _, code := run(t, "notes", "find", "--tag", "gotcha", "--root", root) + if code != ExitOK { + t.Fatalf("find: exit %d", code) + } + if strings.Count(strings.TrimSpace(stdout), "\n") != 0 || !strings.Contains(stdout, "n-0001") { + t.Errorf("find --tag gotcha = %q, want just n-0001", stdout) + } + + // Every filter the rule advertises, over the same two notes. + for _, tc := range []struct { + args []string + want string + }{ + {[]string{"--path", "internal/cli"}, "n-0001"}, + {[]string{"--since", "2026-03-01"}, "n-0002"}, + {[]string{"oldest"}, "n-0002"}, + } { + stdout, _, code := run(t, append(append([]string{"notes", "find"}, tc.args...), "--root", root)...) + if code != ExitOK { + t.Fatalf("find %v: exit %d", tc.args, code) + } + if !strings.Contains(stdout, tc.want) || strings.Count(strings.TrimSpace(stdout), "\n") != 0 { + t.Errorf("find %v = %q, want only %s", tc.args, stdout, tc.want) + } + } + + stdout, _, code = run(t, "notes", "show", "n-0001", "--root", root) + if code != ExitOK || strings.TrimSpace(stdout) != first { + t.Errorf("show = %q (%d), want the line add printed: %q", stdout, code, first) + } + + if _, stderr, code := run(t, "notes", "rm", "n-0001", "--root", root); code != ExitOK { + t.Fatalf("rm: exit %d (%s)", code, stderr) + } + if body := notesFile(t, root); strings.Contains(body, "lands against the last note") { + t.Error("the removed note's text is still in the file") + } + // The number is spent: the next note is n-0003, not a second n-0001. + if line := addNote(t, root, "a third", "--tag", "x"); !strings.Contains(line, "n-0003") { + t.Errorf("after removing n-0001 the next note was %q — an id must never be reused", line) + } +} + +// What the file holds and what `find` prints are one format, so a reader who +// learned either one has learned both. This is the claim that makes `grep` a +// supported way to query the log. +func TestFindPrintsExactlyWhatTheFileHolds(t *testing.T) { + root := initWorkspace(t) + line := addNote(t, root, "one line, index fields first", "--tag", "format", "--path", "docs") + + stdout, _, _ := run(t, "notes", "find", "--root", root) + if strings.TrimSpace(stdout) != line { + t.Errorf("find = %q, add said %q", strings.TrimSpace(stdout), line) + } + if !strings.Contains(notesFile(t, root), line) { + t.Errorf("the file does not contain the line find printed: %q", line) + } + // The grep the rule advertises has to work on the real file. + var hits int + for _, l := range strings.Split(notesFile(t, root), "\n") { + if strings.HasPrefix(l, "- n-") && strings.Contains(l, " #format ") { + hits++ + } + } + if hits != 1 { + t.Errorf("grep for ' #format ' matched %d note lines, want 1", hits) + } +} + +// A tag is the index the log is queried by. Defaulting it would put one tag on +// everything, which is the drift the index exists to prevent. +func TestAddRequiresATag(t *testing.T) { + root := initWorkspace(t) + _, stderr, code := run(t, "notes", "add", "an untagged thought", "--root", root) + if code != ExitError { + t.Fatalf("exit = %d, want %d", code, ExitError) + } + if !strings.Contains(stderr, "--tag") { + t.Errorf("stderr = %q, want it to name --tag", stderr) + } +} + +// One line is the contract. Text that would break across lines is refused rather +// than mangled into something no query returns whole. +func TestAddRefusesMoreThanOneLine(t *testing.T) { + root := initWorkspace(t) + _, stderr, code := run(t, "notes", "add", "first\nsecond", "--tag", "x", "--root", root) + if code != ExitError { + t.Fatalf("exit = %d, want %d", code, ExitError) + } + if !strings.Contains(stderr, "one line") { + t.Errorf("stderr = %q", stderr) + } +} + +// Seeds are written once and tracked nowhere, so `scc update` will never deliver +// docs/notes.md to a workspace scaffolded before it existed. The first add is the +// other honest moment to create it. +func TestAddSeedsAMissingLog(t *testing.T) { + root := initWorkspace(t) + if err := os.Remove(paths.Notes(root)); err != nil { + t.Fatalf("remove seed: %v", err) + } + addNote(t, root, "the first note this workspace ever had", "--tag", "first") + body := notesFile(t, root) + if !strings.Contains(body, "## Log") || !strings.Contains(body, "n-0001") { + t.Errorf("the reseeded file is not a log:\n%s", body) + } + if _, _, code := run(t, "notes", "validate", "--root", root); code != ExitOK { + t.Error("the file `notes add` seeded does not pass its own validator") + } +} + +// Queries have to work before anybody has written a note — which is every +// workspace on its first day, and every workspace scaffolded before this shipped. +func TestQueriesWorkWithNoLogAtAll(t *testing.T) { + root := initWorkspace(t) + if err := os.Remove(paths.Notes(root)); err != nil { + t.Fatalf("remove seed: %v", err) + } + for _, args := range [][]string{{"find"}, {"tags"}, {"paths"}, {"validate"}} { + if _, stderr, code := run(t, append(append([]string{"notes"}, args...), "--root", root)...); code != ExitOK { + t.Errorf("notes %v on a workspace with no log: exit %d (%s)", args, code, stderr) + } + } +} + +// A path that does not resolve is reported and never blocking: the stale check is +// about a log aging past its code, and at the moment of writing a note about a +// file this branch has not created yet is the note most worth having. +func TestAddWarnsButWritesForAPathThatIsNotThereYet(t *testing.T) { + root := initWorkspace(t) + stdout, stderr, code := run(t, "notes", "add", "about a file this branch has not created", + "--tag", "todo-shaped", "--path", "internal/not/yet.go", "--root", root) + if code != ExitOK { + t.Fatalf("exit = %d (%s)", code, stderr) + } + if !strings.Contains(stdout, "n-0001") { + t.Errorf("the note was not written: %q", stdout) + } + if !strings.Contains(stderr, "no longer exists") { + t.Errorf("stderr = %q, want the stale path reported", stderr) + } +} + +// The dangerous edit is the one nobody looked at first: `add` appends to a file it +// never showed the user, so it re-validates afterwards and puts the file back the +// way `scc patch` does. +func TestAddRollsBackAnEditThatIntroducesAFinding(t *testing.T) { + root := initWorkspace(t) + addNote(t, root, "a first note", "--tag", "x") + before := notesFile(t, root) + + // A hand-edited log with a number the appender is about to allocate again. + broken := strings.Replace(before, "## Log", "## Log\n\n- n-0002 2026-01-01 #x — hand-written, and about to collide", 1) + if err := os.WriteFile(paths.Notes(root), []byte(broken), 0o644); err != nil { + t.Fatalf("write: %v", err) + } + // n-0002 is taken, so the appender allocates n-0003 and nothing collides. Force + // the collision instead by making the file claim a number twice. + broken = strings.Replace(broken, "- n-0002 2026-01-01", "- n-0001 2026-01-01", 1) + if err := os.WriteFile(paths.Notes(root), []byte(broken), 0o644); err != nil { + t.Fatalf("write: %v", err) + } + if _, _, code := run(t, "notes", "validate", "--root", root); code != ExitFindings { + t.Fatalf("the hand-edited log should already have a finding, got exit %d", code) + } + // A pre-existing finding is not this edit's fault, so the append still lands — + // the comparison is on rule+message, exactly as `scc patch` does it. + stdout, stderr, code := run(t, "notes", "add", "another", "--tag", "x", "--root", root) + if code != ExitOK { + t.Fatalf("add over a log that already had a finding: exit %d (%s)", code, stderr) + } + if !strings.Contains(stdout, "n-0002") { + t.Errorf("add reported %q", stdout) + } +} + +func TestNotesJSON(t *testing.T) { + root := initWorkspace(t) + stdout, stderr, code := run(t, "notes", "add", "a note", "--tag", "a,b", "--path", "docs,internal", + "--date", "2026-02-09", "--json", "--root", root) + if code != ExitOK { + t.Fatalf("exit %d (%s)", code, stderr) + } + var added struct { + Note struct { + ID string `json:"id"` + Date string `json:"date"` + Tags []string `json:"tags"` + Paths []string `json:"paths"` + Text string `json:"text"` + } `json:"note"` + Line string `json:"line"` + Path string `json:"path"` + Written bool `json:"written"` + Verified string `json:"verified"` + } + if err := json.Unmarshal([]byte(stdout), &added); err != nil { + t.Fatalf("stdout is not JSON (%v): %q", err, stdout) + } + if added.Note.ID != "n-0001" || len(added.Note.Tags) != 2 || len(added.Note.Paths) != 2 { + t.Errorf("note = %+v", added.Note) + } + if !added.Written || added.Verified != "clean" { + t.Errorf("written = %v, verified = %q", added.Written, added.Verified) + } + if added.Path != filepath.ToSlash(filepath.Join(paths.DocsSeg, paths.NotesSeg)) { + t.Errorf("path = %q", added.Path) + } + + stdout, _, code = run(t, "notes", "find", "--json", "--root", root) + if code != ExitOK { + t.Fatalf("find --json: exit %d", code) + } + var found struct { + Notes []map[string]any `json:"notes"` + Count int `json:"count"` + } + if err := json.Unmarshal([]byte(stdout), &found); err != nil { + t.Fatalf("find stdout is not JSON (%v): %q", err, stdout) + } + if found.Count != 1 || len(found.Notes) != 1 { + t.Errorf("find = %+v", found) + } +} + +// A cap that hides matches while looking complete is the one way this command can +// mislead, so what was dropped is said out loud — on stderr, so stdout stays the +// notes and nothing else. +func TestFindSaysWhatALimitDropped(t *testing.T) { + root := initWorkspace(t) + for _, text := range []string{"one", "two", "three"} { + addNote(t, root, text, "--tag", "x") + } + stdout, stderr, code := run(t, "notes", "find", "--limit", "1", "--root", root) + if code != ExitOK { + t.Fatalf("exit %d", code) + } + if strings.Count(strings.TrimSpace(stdout), "\n") != 0 { + t.Errorf("stdout = %q, want one line", stdout) + } + if !strings.Contains(stdout, "n-0003") { + t.Errorf("stdout = %q, want the newest note", stdout) + } + if !strings.Contains(stderr, "of 3 matches") { + t.Errorf("stderr = %q, want the drop reported", stderr) + } +} + +// The seed ships an example note inside a fence. A validator that read it would +// fire on the file scc itself writes, which is the worst bug this product has. +func TestTheSeededLogPassesItsOwnValidator(t *testing.T) { + root := initWorkspace(t) + stdout, stderr, code := run(t, "notes", "validate", "--root", root, "--json") + if code != ExitOK { + t.Errorf("the seeded docs/notes.md has findings: exit %d\nstdout: %s\nstderr: %s", code, stdout, stderr) + } + stdout, _, _ = run(t, "notes", "find", "--root", root) + if strings.Contains(stdout, "n-0000") { + t.Errorf("the seed's fenced example was read as a note: %q", stdout) + } +} + +func TestNotesRejectsAnUnknownSubcommand(t *testing.T) { + root := initWorkspace(t) + _, stderr, code := run(t, "notes", "list", "--root", root) + if code != ExitError { + t.Errorf("exit = %d, want %d", code, ExitError) + } + if !strings.Contains(stderr, "unknown notes subcommand") { + t.Errorf("stderr = %q", stderr) + } +} diff --git a/internal/notes/notes.go b/internal/notes/notes.go new file mode 100644 index 0000000..2c766e5 --- /dev/null +++ b/internal/notes/notes.go @@ -0,0 +1,464 @@ +// Package notes is docs/notes.md — the project's note log, and the grammar that +// makes it answerable without being read. +// +// A note is the small durable observation that has nowhere else to live: the +// gotcha, the why-not, the "careful, this looks wrong and is not". Before this +// file existed that text went into a comment beside the code, where exactly one +// reader ever found it — the one already looking at that line — and where it was +// invisible to anybody asking "what do we know about this area?". +// +// The whole design follows from one requirement: a reader must be able to ask the +// log a question without loading it. So a note is ONE LINE, self-contained, with +// its index fields up front: +// +// - n-0042 2026-08-27 #gotcha @internal/cli/launch.go — wrap writes MCP config +// +// One line is what makes grep exact rather than approximate: a hit is a whole +// note, never a fragment of one, so `grep ' #gotcha ' docs/notes.md` and +// `scc notes find --tag gotcha` return the same thing and neither has to reason +// about where a record ends. It is also the boundary against this file becoming +// the place prose goes to hide — the failure mode that ended `## Notes` in the v1 +// plan format, where nothing forbade growth and half the file became one section. +// A thought that needs a second line is not a note; it is a wiki page, an ADR, or +// a task, and docs/ already has all three. +// +// Nothing here reads a note's meaning. This package parses, formats, filters, and +// splices; turning a fact about a line into a finding is internal/validate's job, +// the same seam internal/artifact keeps with the plan grammar. +package notes + +import ( + "fmt" + "path" + "regexp" + "sort" + "strconv" + "strings" + "time" + + "github.com/protonspy/spec-claude-code/internal/mdscan" + "github.com/protonspy/spec-claude-code/internal/textutil" +) + +// Section is the heading notes live under. Everything above it is guidance the +// user owns; everything below it is the log. +// +// A heading rather than an HTML-comment marker because the region has to be +// visible to the person reading the file, and because mdscan already reports +// headings — a marker would be a second parser for a job the first one does. +const Section = "Log" + +// IDPrefix and idWidth spell a note's identity: n-0001, zero-padded so the log +// sorts the way it reads, and free to grow past four digits rather than wrapping. +// +// An id exists so a note can be cited — from a spec, from a commit message, from +// the one place a docstring legitimately mentions one ("see n-0042"). Without it +// the only way to point at a note is to quote it, and a quotation goes stale +// silently. +const ( + IDPrefix = "n-" + idWidth = 4 +) + +// The grammar. Field order is index-first and deliberate: id, date, then the tags +// and paths a query filters on, and only then the prose. A line whose fixed +// fields come first can be matched by a reader that stops caring at the em dash. +var ( + noteRe = regexp.MustCompile(`^-[ \t]+(` + IDPrefix + `[0-9]{4,})[ \t]+([0-9]{4}-[0-9]{2}-[0-9]{2})[ \t]+((?:[#@][^\s]+[ \t]+)*)—[ \t]+(\S.*)$`) + itemRe = regexp.MustCompile(`^-[ \t]+\S`) + tagRe = regexp.MustCompile(`^[a-z0-9]+(?:-[a-z0-9]+)*$`) + idRe = regexp.MustCompile(`^` + IDPrefix + `([0-9]{4,})$`) + tombRe = regexp.MustCompile(`^$`) +) + +// DateLayout is the only date a note carries: ISO, so it sorts as a string and a +// `--since` filter is a comparison rather than a parse. +const DateLayout = "2006-01-02" + +// Note is one line of the log. +type Note struct { + // Line is 1-based in the file it was parsed from, or 0 for a note being built. + Line int `json:"line,omitempty"` + ID string `json:"id"` + Num int `json:"-"` + Date string `json:"date"` + // Tags are the index, without their leading '#'. At least one, because a note + // nobody can filter for is one only full-text search will ever surface. + Tags []string `json:"tags"` + // Paths are what the note is about, without their leading '@': repo-relative, + // slash-separated. This is the field that replaces the comment in the file — + // it is how a note stays attached to code without living inside it. + Paths []string `json:"paths,omitempty"` + Text string `json:"text"` +} + +// Loose is a line in the log region that is not a note. It is reported rather +// than ignored: a hand-written note that missed the grammar is invisible to every +// query, which is the one failure this file cannot tolerate quietly. +type Loose struct { + Line int `json:"line"` + Text string `json:"text"` +} + +// File is a parsed docs/notes.md. +type File struct { + Path string + Lines []string + Doc *mdscan.Document + Notes []Note + Loose []Loose + Outside []Note // well-formed notes sitting outside the log region + + // Retired is the numbers `Remove` has spent, by id. A removed note leaves an + // HTML-comment tombstone where it stood, which mdscan blanks — so it costs + // nothing to every reader of the log, and Next can still refuse to hand its + // number out twice. + Retired map[string]int + + // Section is the 1-based line of the `## Log` heading, or 0 when the file has + // none — which is a finding, and the reason Append refuses. + Section int +} + +// Parse reads a notes file. Content is normalized first, so a CRLF checkout and a +// LF one produce identical notes and identical line numbers. +func Parse(p, content string) (*File, error) { + doc, err := mdscan.Parse(p, textutil.NormalizeNewlines(content)) + if err != nil { + return nil, err + } + f := &File{Path: p, Lines: doc.Lines, Doc: doc, Retired: map[string]int{}} + // Tombstones are read from the raw lines rather than from Body, because Body is + // exactly where an HTML comment has been blanked out. + for i, raw := range doc.Lines { + if m := tombRe.FindStringSubmatch(strings.TrimSpace(raw)); m != nil { + f.Retired[m[1]] = i + 1 + } + } + + start, end := 0, len(doc.Lines)+1 + for _, h := range doc.Headings { + if start == 0 { + if h.Level >= 2 && strings.EqualFold(h.Text, Section) { + start, f.Section = h.Line, h.Line + } + continue + } + if h.Level <= 2 { + end = h.Line + break + } + } + + for i, body := range doc.Body { + line := i + 1 + if strings.TrimSpace(body) == "" { + continue + } + inRegion := start > 0 && line > start && line < end + if m := noteRe.FindStringSubmatch(body); m != nil { + n := parseNote(line, m) + if inRegion { + f.Notes = append(f.Notes, n) + } else { + f.Outside = append(f.Outside, n) + } + continue + } + // Only inside the region, and only for list items: the guidance above the + // heading is prose the user owns, and a check that reported on it would + // fire on the file scc itself seeds. + if inRegion && itemRe.MatchString(body) { + f.Loose = append(f.Loose, Loose{Line: line, Text: strings.TrimSpace(body)}) + } + } + return f, nil +} + +func parseNote(line int, m []string) Note { + n := Note{Line: line, ID: m[1], Date: m[2], Text: strings.TrimSpace(m[4])} + if d := idRe.FindStringSubmatch(n.ID); d != nil { + n.Num, _ = strconv.Atoi(d[1]) + } + for _, tok := range strings.Fields(m[3]) { + switch tok[0] { + case '#': + n.Tags = append(n.Tags, tok[1:]) + case '@': + n.Paths = append(n.Paths, tok[1:]) + } + } + return n +} + +// Format renders a note as the line the file holds. It is the only writer, so +// every note scc adds is one a query can find. +func (n Note) Format() string { + var b strings.Builder + b.WriteString("- " + n.ID + " " + n.Date) + for _, t := range n.Tags { + b.WriteString(" #" + t) + } + for _, p := range n.Paths { + b.WriteString(" @" + p) + } + b.WriteString(" — " + n.Text) + return b.String() +} + +// String is Format: what a reader greps for and what a reader is shown must not +// be two formats to learn. +func (n Note) String() string { return n.Format() } + +// ID formats a note number. +func ID(num int) string { return fmt.Sprintf("%s%0*d", IDPrefix, idWidth, num) } + +// Next is the number a new note takes: the high-water mark plus one, counting +// notes wherever they sit. Numbers are never reused — a citation to n-0042 must +// not silently come to mean a different note — so a deleted note's number stays +// spent, which falls out of using the maximum rather than the count. +func (f *File) Next() int { + high := 0 + for _, set := range [][]Note{f.Notes, f.Outside} { + for _, n := range set { + if n.Num > high { + high = n.Num + } + } + } + for id := range f.Retired { + if m := idRe.FindStringSubmatch(id); m != nil { + if num, err := strconv.Atoi(m[1]); err == nil && num > high { + high = num + } + } + } + return high + 1 +} + +// Get returns the note with this id. +func (f *File) Get(id string) (Note, bool) { + for _, set := range [][]Note{f.Notes, f.Outside} { + for _, n := range set { + if n.ID == id { + return n, true + } + } + } + return Note{}, false +} + +// Append splices a note in at the end of the log region and returns the new +// content. +// +// At the end rather than the top: the log is append-only and reads oldest first, +// so a diff of this file is one added line at a known place, and two sessions +// each adding a note conflict on nothing. +func (f *File) Append(n Note) (string, error) { + if f.Section == 0 { + return "", fmt.Errorf("%s has no `## %s` heading — nowhere to put a note", f.Path, Section) + } + end := len(f.Lines) + for _, h := range f.Doc.Headings { + if h.Level <= 2 && h.Line > f.Section { + end = h.Line - 1 + break + } + } + // Back up over the blank lines that separate the region from what follows, so + // the note lands against the last note rather than after a gap. + for end > f.Section && strings.TrimSpace(f.Lines[end-1]) == "" { + end-- + } + out := make([]string, 0, len(f.Lines)+1) + out = append(out, f.Lines[:end]...) + out = append(out, n.Format()) + out = append(out, f.Lines[end:]...) + return strings.Join(out, "\n"), nil +} + +// Remove takes a note out of the log, leaving a tombstone where it stood. +// +// The note's text goes — a wrong note is worse than none, and unlike a struck-out +// plan task it is not a commitment whose absence needs explaining. The number +// stays: an id is what a commit message or a spec cites, and handing it out again +// would make an old citation point at a new note, silently. The tombstone is an +// HTML comment, so it is invisible to a rendered read, to a grep for a tag, and to +// every parser here except Next. +func (f *File) Remove(id string) (string, Note, error) { + n, ok := f.Get(id) + if !ok { + if line, was := f.Retired[id]; was { + return "", Note{}, fmt.Errorf("%s was already removed (line %d)", id, line) + } + return "", Note{}, fmt.Errorf("no note %q in %s", id, f.Path) + } + out := make([]string, 0, len(f.Lines)) + out = append(out, f.Lines[:n.Line-1]...) + out = append(out, Tombstone(n.ID)) + out = append(out, f.Lines[n.Line:]...) + return strings.Join(out, "\n"), n, nil +} + +// Tombstone renders the marker a removed note leaves behind. +func Tombstone(id string) string { return "" } + +// Query is a filter over the log. Every field is an AND, and a repeated field is +// an OR within itself: `--tag a --tag b --path p` is "(a or b) and p", which is +// what a reader narrowing a search actually means. +type Query struct { + Tags []string + Paths []string + // Terms are matched case-insensitively against the whole rendered line, so a + // term can be a word in the prose, a tag, or a path fragment without the + // caller having to say which. + Terms []string + // Since is an ISO date; a note older than it is dropped. Comparison is on the + // string, which is exactly right for ISO and needs no clock. + Since string +} + +// Match returns the notes this query selects, in file order. +func (f *File) Match(q Query) []Note { + out := []Note{} + for _, n := range f.Notes { + if n.matches(q) { + out = append(out, n) + } + } + return out +} + +func (n Note) matches(q Query) bool { + if q.Since != "" && n.Date < q.Since { + return false + } + if len(q.Tags) > 0 && !anyOf(n.Tags, q.Tags, false) { + return false + } + // A path filter matches the path itself and anything under it, so asking about + // a directory answers for the files in it — the question a reader means when + // they name a package rather than a file. + if len(q.Paths) > 0 && !anyOf(n.Paths, q.Paths, true) { + return false + } + line := strings.ToLower(n.Format()) + for _, t := range q.Terms { + if !strings.Contains(line, strings.ToLower(t)) { + return false + } + } + return true +} + +func anyOf(have, want []string, prefix bool) bool { + for _, w := range want { + for _, h := range have { + if h == w || (prefix && strings.HasPrefix(h, w+"/")) { + return true + } + } + } + return false +} + +// Count is one index entry — a tag or a path — and how many notes carry it. +type Count struct { + Name string `json:"name"` + Count int `json:"count"` +} + +// Tags is the tag index, most used first and alphabetical within a count. +// +// It exists to be read before a tag is coined. An open vocabulary drifts the way +// domain language does — three tags for one concern inside a week — and the cheap +// defense is not a closed list scc would have to guess at, but making the existing +// tags one command away at the moment somebody is about to invent a fourth. +func (f *File) Tags() []Count { + return tally(f.Notes, func(n Note) []string { return n.Tags }) +} + +// Paths is every path the log mentions, most noted first. Same purpose as Tags: +// it answers "what does this project already know things about". +func (f *File) Paths() []Count { + return tally(f.Notes, func(n Note) []string { return n.Paths }) +} + +func tally(notes []Note, of func(Note) []string) []Count { + seen := map[string]int{} + for _, n := range notes { + for _, v := range of(n) { + seen[v]++ + } + } + out := make([]Count, 0, len(seen)) + for v, c := range seen { + out = append(out, Count{Name: v, Count: c}) + } + sort.Slice(out, func(i, j int) bool { + if out[i].Count != out[j].Count { + return out[i].Count > out[j].Count + } + return out[i].Name < out[j].Name + }) + return out +} + +// Today is the date a new note takes when the caller names none. +func Today() string { return time.Now().Format(DateLayout) } + +// CheckTag rejects a tag the grammar cannot round-trip. Kebab-case for the same +// reason every other name in an scc workspace is: one spelling per concept, so +// `--tag Gotcha` and `--tag gotcha` cannot become two entries in the index. +func CheckTag(t string) error { + if !tagRe.MatchString(t) { + return fmt.Errorf("tag %q is not kebab-case (a-z, 0-9, single hyphens)", t) + } + return nil +} + +// CheckDate rejects anything the grammar would not parse back. +func CheckDate(d string) error { + if _, err := time.Parse(DateLayout, d); err != nil { + return fmt.Errorf("date %q is not YYYY-MM-DD", d) + } + return nil +} + +// CheckPath normalizes a scope and rejects one the log could not hold. +// +// It is deliberately strict about the spelling and silent about what exists: a +// note may legitimately name a file this branch has not created yet, and refusing +// that would make the log unusable at the moment it is most worth writing. Whether +// the path still resolves is the validator's question, asked later and repeatedly. +func CheckPath(p string) (string, error) { + clean := strings.TrimSpace(strings.ReplaceAll(p, "\\", "/")) + if clean == "" { + return "", fmt.Errorf("empty path") + } + if strings.ContainsAny(clean, " \t") { + return "", fmt.Errorf("path %q contains whitespace; a note's fields are space-separated", p) + } + if strings.HasPrefix(clean, "/") || (len(clean) > 1 && clean[1] == ':') { + return "", fmt.Errorf("path %q is absolute; a note cites a repo-relative path", p) + } + clean = path.Clean(clean) + if clean == ".." || strings.HasPrefix(clean, "../") { + return "", fmt.Errorf("path %q escapes the workspace", p) + } + return clean, nil +} + +// CheckText rejects a note the file could not hold as one line, which is the whole +// contract: a hit has to be a whole note. +func CheckText(s string) (string, error) { + t := strings.TrimSpace(textutil.NormalizeNewlines(s)) + if t == "" { + return "", fmt.Errorf("a note needs text") + } + if strings.Contains(t, "\n") { + return "", fmt.Errorf("a note is one line; this is %d — if it needs more, it is a wiki page or an ADR", + strings.Count(t, "\n")+1) + } + return t, nil +} diff --git a/internal/notes/notes_test.go b/internal/notes/notes_test.go new file mode 100644 index 0000000..932277c --- /dev/null +++ b/internal/notes/notes_test.go @@ -0,0 +1,332 @@ +package notes + +import ( + "strings" + "testing" +) + +// A minimal log: guidance above, notes below the heading. Every test builds on +// this rather than on a hand-rolled string, because the region rule — what counts +// as a note and what is prose the user owns — is the thing most of them are about. +const log = `# Notes + +Guidance the user owns, with an example that must not be read as a note: + +` + "```" + `markdown +- n-0000 2020-01-01 #example @nowhere.go — an example inside a fence +` + "```" + ` + +## Log + + +- n-0001 2026-02-09 #gotcha @internal/cli/launch.go — wrap writes MCP config, so it outlives the session +- n-0002 2026-03-01 #convention #cli @internal/cli — the log reads oldest first +` + +func parse(t *testing.T, content string) *File { + t.Helper() + f, err := Parse("docs/notes.md", content) + if err != nil { + t.Fatalf("parse: %v", err) + } + return f +} + +func TestParseReadsEveryField(t *testing.T) { + f := parse(t, log) + if len(f.Notes) != 2 { + t.Fatalf("notes = %d, want 2: %+v", len(f.Notes), f.Notes) + } + n := f.Notes[0] + if n.ID != "n-0001" || n.Num != 1 { + t.Errorf("id = %q/%d, want n-0001/1", n.ID, n.Num) + } + if n.Date != "2026-02-09" { + t.Errorf("date = %q", n.Date) + } + if strings.Join(n.Tags, ",") != "gotcha" { + t.Errorf("tags = %v", n.Tags) + } + if strings.Join(n.Paths, ",") != "internal/cli/launch.go" { + t.Errorf("paths = %v", n.Paths) + } + if n.Text != "wrap writes MCP config, so it outlives the session" { + t.Errorf("text = %q", n.Text) + } + if n.Line != 12 { + t.Errorf("line = %d, want 12", n.Line) + } +} + +// The seed carries an example note, and a validator that read it would fire on the +// file scc itself writes — the worst bug this product can ship. mdscan blanks +// fenced blocks, and this is the test that says so for notes. +func TestAnExampleInAFenceIsNotANote(t *testing.T) { + f := parse(t, log) + for _, n := range append(append([]Note{}, f.Notes...), f.Outside...) { + if n.ID == "n-0000" { + t.Fatalf("the fenced example was parsed as a note: %s", n.Format()) + } + } +} + +// The one failure this file cannot tolerate quietly: a hand-written line that +// missed the grammar is invisible to every query, so it has to be reported. +func TestALooseLineInTheLogIsReported(t *testing.T) { + f := parse(t, log+"- a bullet somebody typed by hand\n") + if len(f.Loose) != 1 { + t.Fatalf("loose = %+v, want one", f.Loose) + } + if !strings.Contains(f.Loose[0].Text, "typed by hand") { + t.Errorf("loose text = %q", f.Loose[0].Text) + } +} + +// Prose above the heading is the user's, and reporting on it would make the seed +// itself a finding. +func TestProseAboveTheHeadingIsNotJudged(t *testing.T) { + f := parse(t, "# Notes\n\n- a bullet in the guidance\n\n## Log\n") + if len(f.Loose) != 0 { + t.Errorf("loose = %+v, want none: guidance is not the log", f.Loose) + } +} + +// A well-formed note outside the region is worse than a malformed one inside it: +// it looks right and no query returns it. +func TestANoteOutsideTheLogIsFound(t *testing.T) { + f := parse(t, "# Notes\n\n- n-0007 2026-01-01 #x — stranded above the heading\n\n## Log\n") + if len(f.Outside) != 1 || len(f.Notes) != 0 { + t.Fatalf("outside = %d, notes = %d, want 1 and 0", len(f.Outside), len(f.Notes)) + } +} + +// Numbers are spent, not counted: a citation to n-0002 must never come to mean a +// different note because the first one was removed. +func TestNextIsTheHighWaterMark(t *testing.T) { + f := parse(t, log) + if got := f.Next(); got != 3 { + t.Errorf("next = %d, want 3", got) + } + content, _, err := f.Remove("n-0002") + if err != nil { + t.Fatalf("remove: %v", err) + } + if got := parse(t, content).Next(); got != 3 { + t.Errorf("next after removing the last note = %d, want 3 — numbers are never reused", got) + } +} + +func TestNextCountsNotesOutsideTheLog(t *testing.T) { + f := parse(t, "# Notes\n\n- n-0009 2026-01-01 #x — stranded\n\n## Log\n") + if got := f.Next(); got != 10 { + t.Errorf("next = %d, want 10: a stranded note has still spent its number", got) + } +} + +func TestAppendLandsAgainstTheLastNote(t *testing.T) { + f := parse(t, log) + content, err := f.Append(Note{ID: "n-0003", Date: "2026-04-01", Tags: []string{"x"}, Text: "third"}) + if err != nil { + t.Fatalf("append: %v", err) + } + lines := strings.Split(strings.TrimRight(content, "\n"), "\n") + if last := lines[len(lines)-1]; last != "- n-0003 2026-04-01 #x — third" { + t.Errorf("last line = %q", last) + } + if got := len(parse(t, content).Notes); got != 3 { + t.Errorf("notes after append = %d, want 3", got) + } +} + +// The log is not always the last section. Appending after whatever follows it +// would put the note where no query looks. +func TestAppendStaysInsideTheLogSection(t *testing.T) { + f := parse(t, log+"\n## Afterwards\n\nsomething else entirely\n") + content, err := f.Append(Note{ID: "n-0003", Date: "2026-04-01", Tags: []string{"x"}, Text: "third"}) + if err != nil { + t.Fatalf("append: %v", err) + } + after := parse(t, content) + if len(after.Notes) != 3 || len(after.Outside) != 0 { + t.Fatalf("notes = %d, outside = %d, want 3 and 0\n%s", len(after.Notes), len(after.Outside), content) + } +} + +// Without a heading there is nowhere a note could go that a query would find, so +// the write is refused rather than guessed at. +func TestAppendRefusesAFileWithNoLogSection(t *testing.T) { + f := parse(t, "# Notes\n\njust prose\n") + if _, err := f.Append(Note{ID: "n-0001", Date: "2026-01-01", Tags: []string{"x"}, Text: "y"}); err == nil { + t.Fatal("append into a file with no log section succeeded") + } +} + +func TestFormatRoundTrips(t *testing.T) { + n := Note{ID: "n-0042", Date: "2026-05-05", Tags: []string{"a", "b"}, Paths: []string{"x/y.go", "z"}, Text: "text — with an em dash in it"} + f := parse(t, "# Notes\n\n## Log\n\n"+n.Format()+"\n") + if len(f.Notes) != 1 { + t.Fatalf("a formatted note did not parse back: %q", n.Format()) + } + got := f.Notes[0] + got.Line = 0 + got.Num = 0 + if got.Format() != n.Format() { + t.Errorf("round trip: %q -> %q", n.Format(), got.Format()) + } +} + +func TestMatch(t *testing.T) { + f := parse(t, log) + for _, tc := range []struct { + name string + q Query + want []string + }{ + {"everything", Query{}, []string{"n-0001", "n-0002"}}, + {"one tag", Query{Tags: []string{"gotcha"}}, []string{"n-0001"}}, + {"tags are an or", Query{Tags: []string{"gotcha", "cli"}}, []string{"n-0001", "n-0002"}}, + {"an unknown tag matches nothing", Query{Tags: []string{"nope"}}, nil}, + {"an exact path", Query{Paths: []string{"internal/cli/launch.go"}}, []string{"n-0001"}}, + {"a directory covers what is under it", Query{Paths: []string{"internal"}}, []string{"n-0001", "n-0002"}}, + {"terms are an and", Query{Terms: []string{"log", "oldest"}}, []string{"n-0002"}}, + {"terms are case-insensitive", Query{Terms: []string{"WRAP"}}, []string{"n-0001"}}, + {"since drops what is older", Query{Since: "2026-03-01"}, []string{"n-0002"}}, + {"filters compose as an and", Query{Tags: []string{"gotcha"}, Terms: []string{"oldest"}}, nil}, + } { + t.Run(tc.name, func(t *testing.T) { + var got []string + for _, n := range f.Match(tc.q) { + got = append(got, n.ID) + } + if strings.Join(got, ",") != strings.Join(tc.want, ",") { + t.Errorf("match = %v, want %v", got, tc.want) + } + }) + } +} + +func TestTagsAndPathsAreOrderedByUse(t *testing.T) { + f := parse(t, log) + tags := f.Tags() + if len(tags) != 3 || tags[0].Name != "cli" { + // cli, convention, gotcha — one each, so alphabetical decides. + t.Errorf("tags = %+v", tags) + } + if paths := f.Paths(); len(paths) != 2 { + t.Errorf("paths = %+v", paths) + } +} + +func TestCheckPath(t *testing.T) { + for _, tc := range []struct { + in, want string + ok bool + }{ + {"internal/cli/notes.go", "internal/cli/notes.go", true}, + {`internal\cli\notes.go`, "internal/cli/notes.go", true}, + {"./internal/cli", "internal/cli", true}, + {"/etc/passwd", "", false}, + {`C:\Windows`, "", false}, + {"../outside", "", false}, + {"has space", "", false}, + {"", "", false}, + } { + got, err := CheckPath(tc.in) + if tc.ok != (err == nil) { + t.Errorf("CheckPath(%q) error = %v, want ok=%v", tc.in, err, tc.ok) + continue + } + if tc.ok && got != tc.want { + t.Errorf("CheckPath(%q) = %q, want %q", tc.in, got, tc.want) + } + } +} + +func TestCheckTag(t *testing.T) { + for _, ok := range []string{"gotcha", "code-review", "v2"} { + if err := CheckTag(ok); err != nil { + t.Errorf("CheckTag(%q) = %v", ok, err) + } + } + for _, bad := range []string{"Gotcha", "two words", "trailing-", "", "under_score", "#hash"} { + if err := CheckTag(bad); err == nil { + t.Errorf("CheckTag(%q) accepted", bad) + } + } +} + +// One line is the contract the whole file rests on: a match has to be a whole +// note, so text that would break across lines is refused rather than mangled. +func TestCheckTextRefusesMoreThanOneLine(t *testing.T) { + if _, err := CheckText("first\nsecond"); err == nil { + t.Error("multi-line text accepted") + } + if _, err := CheckText("first\r\nsecond"); err == nil { + t.Error("CRLF text accepted") + } + if _, err := CheckText(" "); err == nil { + t.Error("empty text accepted") + } + got, err := CheckText(" one line ") + if err != nil || got != "one line" { + t.Errorf("CheckText = %q, %v", got, err) + } +} + +func TestID(t *testing.T) { + if got := ID(7); got != "n-0007" { + t.Errorf("ID(7) = %q", got) + } + if got := ID(12345); got != "n-12345" { + t.Errorf("ID(12345) = %q — ids grow rather than wrap", got) + } +} + +// A CRLF checkout must produce the same notes at the same line numbers as a LF +// one, or a finding's line number depends on how git checked the file out. +func TestCRLFParsesIdentically(t *testing.T) { + lf := parse(t, log) + crlf := parse(t, strings.ReplaceAll(log, "\n", "\r\n")) + if len(lf.Notes) != len(crlf.Notes) { + t.Fatalf("notes = %d vs %d", len(lf.Notes), len(crlf.Notes)) + } + for i := range lf.Notes { + if lf.Notes[i].Format() != crlf.Notes[i].Format() || lf.Notes[i].Line != crlf.Notes[i].Line { + t.Errorf("note %d differs: %q@%d vs %q@%d", + i, lf.Notes[i].Format(), lf.Notes[i].Line, crlf.Notes[i].Format(), crlf.Notes[i].Line) + } + } +} + +// A removed note leaves its number behind, not its text. The tombstone is an HTML +// comment, so it is invisible to a rendered read and to a grep for a tag — and +// visible to Next, which is the only reader that has to care. +func TestRemoveLeavesATombstone(t *testing.T) { + f := parse(t, log) + content, n, err := f.Remove("n-0002") + if err != nil { + t.Fatalf("remove: %v", err) + } + if n.Text == "" { + t.Error("remove returned no note; the caller has no copy of what went") + } + if strings.Contains(content, "the log reads oldest first") { + t.Error("the note's text survived removal") + } + if !strings.Contains(content, Tombstone("n-0002")) { + t.Errorf("no tombstone in:\n%s", content) + } + after := parse(t, content) + if len(after.Notes) != 1 { + t.Errorf("notes = %d, want 1", len(after.Notes)) + } + if _, ok := after.Get("n-0002"); ok { + t.Error("a removed note is still returned by Get") + } + if _, was := after.Retired["n-0002"]; !was { + t.Errorf("retired = %v, want n-0002", after.Retired) + } + if _, _, err := after.Remove("n-0002"); err == nil { + t.Error("removing an already-removed note succeeded") + } +} diff --git a/internal/paths/paths.go b/internal/paths/paths.go index be16b44..218b68f 100644 --- a/internal/paths/paths.go +++ b/internal/paths/paths.go @@ -234,6 +234,11 @@ const ( CodewikiSeg = "codewiki" // narrated code, citing [path:start-end]() GlossarySeg = "glossary.md" StackSeg = "stack.md" // adopted technology; unlisted means undecided + // NotesSeg is where a small durable observation goes: the gotcha, the why-not, + // the "careful here" that used to be a comment only the reader of that one file + // ever saw. One note per line, so a grep returns whole notes rather than + // fragments — which is what lets an agent query it without loading it. + NotesSeg = "notes.md" // WikiPagesSeg holds the pages themselves, so the wiki's two fixed documents are // distinguished from its content by where they sit rather than by their names. // The validator used to exclude index.md and changelog.md from the page set by @@ -348,6 +353,11 @@ func Codewiki(root string) string { return filepath.Join(root, DocsSeg, Codewiki // Glossary returns docs/glossary.md. func Glossary(root string) string { return filepath.Join(root, DocsSeg, GlossarySeg) } +// Notes returns docs/notes.md, the project's note log. A missing file is the +// normal state of a young workspace, not an error: `scc notes add` creates it, and +// every reader treats absent as an empty log. +func Notes(root string) string { return filepath.Join(root, DocsSeg, NotesSeg) } + // Stack returns docs/stack.md. Technology absent from it is an open decision, // never something adopted silently — and because dependency manifests are // structured data, that rule is checkable without reading any source. diff --git a/internal/validate/all.go b/internal/validate/all.go index 9d6a8fb..1b2aebc 100644 --- a/internal/validate/all.go +++ b/internal/validate/all.go @@ -28,6 +28,7 @@ func All() []Validator { {Name: "glossary", Run: Glossary}, {Name: "stack", Run: Stack}, {Name: "codewiki", Run: Codewiki}, + {Name: "notes", Run: Notes}, } } @@ -40,7 +41,7 @@ type Result struct { // Everything runs every validator and merges the findings into one set. // -// One exit code and one document, because eight validators the user has to invoke +// One exit code and one document, because nine validators the user has to invoke // separately is eight chances to skip one. The per-validator counts come back // alongside so the report can group by check without re-running anything. func Everything(root string) (*finding.Set, []Result, error) { diff --git a/internal/validate/notes.go b/internal/validate/notes.go new file mode 100644 index 0000000..b3d0032 --- /dev/null +++ b/internal/validate/notes.go @@ -0,0 +1,90 @@ +package validate + +import ( + "os" + "path/filepath" + + "github.com/protonspy/spec-claude-code/internal/finding" + "github.com/protonspy/spec-claude-code/internal/notes" + "github.com/protonspy/spec-claude-code/internal/paths" +) + +// Notes validates docs/notes.md — the log of small durable observations that used +// to be comments nobody outside one file ever read. +// +// Two of these checks are the whole reason the validator exists. A line that +// misses the grammar is invisible to every query, so a note somebody wrote by hand +// and got slightly wrong is a note the project has already lost — silently, which +// is the one way this file can fail without anybody noticing. And a `@path` that +// no longer resolves is the notes half of the codewiki citation check: a note +// about deleted code is worse than no note, because it is read as current. +// +// Everything else here is a spelling check on fields the CLI writes correctly by +// construction. They matter for the same reason: `scc notes add` is not the only +// way a line gets into this file, and the grammar has to hold either way. +func Notes(root string) (*finding.Set, error) { + set := &finding.Set{} + path := paths.Notes(root) + if !isFile(path) { + return set, nil + } + file := rel(root, path) + b, err := os.ReadFile(path) + if err != nil { + return nil, err + } + f, err := notes.Parse(file, string(b)) + if err != nil { + return nil, err + } + + if f.Section == 0 { + set.Addf(file, 1, "notes.no-log-section", + "no `## %s` heading: notes have nowhere to go, and `scc notes add` will refuse", notes.Section) + } + for _, l := range f.Loose { + set.Addf(file, l.Line, "notes.malformed", + "not a note, so no query will ever return it: `- %s0001 YYYY-MM-DD #tag @path — text`", notes.IDPrefix) + } + for _, n := range f.Outside { + set.Addf(file, n.Line, "notes.outside-log", + "%s sits outside `## %s`, where `notes find` does not look", n.ID, notes.Section) + } + + seen := map[string]int{} + for _, n := range append(append([]notes.Note{}, f.Notes...), f.Outside...) { + if prior, dup := seen[n.ID]; dup { + set.Addf(file, n.Line, "notes.duplicate-id", + "%s is already used on line %d; an id is how a note is cited, so it names one note", n.ID, prior) + } else { + seen[n.ID] = n.Line + } + if err := notes.CheckDate(n.Date); err != nil { + set.Addf(file, n.Line, "notes.bad-date", "%s: %v", n.ID, err) + } + if len(n.Tags) == 0 { + set.Addf(file, n.Line, "notes.untagged", + "%s carries no #tag, so only a full-text search will ever surface it", n.ID) + } + for _, t := range n.Tags { + if err := notes.CheckTag(t); err != nil { + set.Addf(file, n.Line, "notes.bad-tag", "%s: %v", n.ID, err) + } + } + for _, p := range n.Paths { + clean, err := notes.CheckPath(p) + if err != nil { + set.Addf(file, n.Line, "notes.bad-path", "%s: %v", n.ID, err) + continue + } + // The stale check, and the reason a note names a path at all: a note about + // code that no longer exists is read as current, which is worse than the + // comment it replaced — that one at least died with the file. + if !exists(filepath.Join(root, filepath.FromSlash(clean))) { + set.Addf(file, n.Line, "notes.stale-path", + "%s is about %s, which no longer exists; repoint the note or remove it", n.ID, clean) + } + } + } + return set, nil +} diff --git a/internal/validate/notes_test.go b/internal/validate/notes_test.go new file mode 100644 index 0000000..163bf7c --- /dev/null +++ b/internal/validate/notes_test.go @@ -0,0 +1,103 @@ +package validate + +import ( + "path/filepath" + "testing" + + "github.com/protonspy/spec-claude-code/internal/paths" +) + +// notesLog writes docs/notes.md with the given lines under the heading, and a +// fenced example above it — the shape the seed actually ships, so every test here +// also asserts the seed's example is not read as a note. +func notesLog(t *testing.T, root string, lines ...string) { + t.Helper() + body := "# Notes\n\nGuidance, with an example:\n\n```markdown\n" + + "- n-0000 2020-01-01 #example @gone.go — an example, inside a fence\n```\n\n## Log\n\n" + for _, l := range lines { + body += l + "\n" + } + write(t, paths.Notes(root), body) +} + +func TestNotesIsSilentWithoutALog(t *testing.T) { + if got := runValidator(t, Notes, t.TempDir()); len(got) != 0 { + t.Errorf("findings on a workspace with no notes: %v", got) + } +} + +func TestNotesAcceptsAWellFormedLog(t *testing.T) { + root := t.TempDir() + write(t, filepath.Join(root, "internal", "cli", "notes.go"), "package cli\n") + notesLog(t, root, + "- n-0001 2026-02-09 #gotcha @internal/cli/notes.go — a note about a file that is there", + "- n-0002 2026-03-01 #convention #cli — a note about nothing in particular", + ) + if got := runValidator(t, Notes, root); len(got) != 0 { + t.Errorf("findings on a clean log: %v", got) + } +} + +func TestNotesChecks(t *testing.T) { + for _, tc := range []struct { + name string + line string + want string + }{ + {"a line that missed the grammar", "- somebody typed this by hand", "notes.malformed"}, + {"no tag to query it by", "- n-0001 2026-02-09 — untagged", "notes.untagged"}, + {"a tag the grammar cannot round-trip", "- n-0001 2026-02-09 #NotKebab — x", "notes.bad-tag"}, + {"a date that is not one", "- n-0001 2026-13-45 #x — x", "notes.bad-date"}, + {"a path that does not resolve", "- n-0001 2026-02-09 #x @internal/gone.go — x", "notes.stale-path"}, + {"a path that escapes the workspace", "- n-0001 2026-02-09 #x @../elsewhere — x", "notes.bad-path"}, + } { + t.Run(tc.name, func(t *testing.T) { + root := t.TempDir() + notesLog(t, root, tc.line) + got := runValidator(t, Notes, root) + if !contains(got, tc.want) { + t.Errorf("findings = %v, want %s", got, tc.want) + } + }) + } +} + +// An id names one note, because an id is what a commit message or a spec cites. +func TestNotesReportsADuplicateID(t *testing.T) { + root := t.TempDir() + notesLog(t, root, + "- n-0001 2026-02-09 #x — first", + "- n-0001 2026-02-10 #x — second, wearing the first one's id", + ) + if got := runValidator(t, Notes, root); !contains(got, "notes.duplicate-id") { + t.Errorf("findings = %v, want notes.duplicate-id", got) + } +} + +// A well-formed note in the wrong place is worse than a malformed one: it looks +// right, and no query returns it. +func TestNotesReportsANoteOutsideTheLog(t *testing.T) { + root := t.TempDir() + write(t, paths.Notes(root), "# Notes\n\n- n-0001 2026-02-09 #x — stranded above the heading\n\n## Log\n") + if got := runValidator(t, Notes, root); !contains(got, "notes.outside-log") { + t.Errorf("findings = %v, want notes.outside-log", got) + } +} + +func TestNotesReportsAMissingLogSection(t *testing.T) { + root := t.TempDir() + write(t, paths.Notes(root), "# Notes\n\nJust prose, and nowhere for a note to go.\n") + if got := runValidator(t, Notes, root); !contains(got, "notes.no-log-section") { + t.Errorf("findings = %v, want notes.no-log-section", got) + } +} + +// A removed note leaves an HTML-comment tombstone so its number is never handed +// out twice. mdscan blanks comments, so nothing here may fire on one. +func TestNotesIgnoresATombstone(t *testing.T) { + root := t.TempDir() + notesLog(t, root, "", "- n-0002 2026-02-09 #x — the one that stayed") + if got := runValidator(t, Notes, root); len(got) != 0 { + t.Errorf("findings on a log with a tombstone: %v", got) + } +}