diff --git a/CLAUDE.md b/CLAUDE.md index 6495b2e..3f36b0a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -49,6 +49,11 @@ These landed after phase 10, and all are documented in `design/orchestration.md` **`internal/artifact` owns the grammars, and `internal/validate` consumes them.** The task grammar used to live in the validator; a reader that disagreed with the validator about what a task is would be worse than no reader. The parser now states facts about a line (`Methodologies`, `Loose`, `HasCitation`) and turning a fact into a finding stays in `validate` — which is also what lets `map` read a malformed artifact instead of refusing exactly the file a user most needs to inspect. **No search engine.** The obvious reach for `find` is an inverted index; at 352KB and 94 artifacts a linear pass ranks the whole workspace in 55ms, and Tantivy or its kin would cost a CGO surface or a second binary against a stdlib-only `go.mod` and a six-platform cross-compile. What precision needed here was not a better index but a better *unit*: BM25 over addressable regions rather than lines, so a hit comes back as something `show` accepts. The seam is `artifact.Search` — it takes artifacts and returns hits, and nothing outside that file knows how it found them. +- **`caveman.md`, the register the agent answers in.** The output budget belongs to the code: prose about the work is written once and then carried in every later request of the session, so narration is the part of a long run that can be cut without losing a fact. It ships as a *rule* rather than a skill because it is on by default, and a default the model has to decide to load is not one — the cost is what every rule costs, since the harnesses that read `rules/` preload it. One level (ultra) rather than a dial, because three descriptions of the register are three things to keep true instead of one, and nobody turns the dial. + + What it must never compress is the line that keeps it honest: artifacts under `specs/`, `plans/`, `docs/`, anything a validator parses or a shell runs, quoted output, commit and PR bodies, and questions asked of the user. A denser EARS line is a finding, not a saving. + + **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. - **The four seeded `docs/` anchors** (`assets.Seeds`). `init` writes `glossary.md`, `stack.md`, `wiki/index.md`, and `wiki/changelog.md` — the knowledge base's only fixed-name documents, each holding the format its validator checks. A seed is written once and tracked nowhere: not in the manifest, not by `scc update`. `scc` is a redesign of `csdd` (`github.com/protonspy/csdd`), narrowed to spec-driven development and deliberately leaner. When reaching for something from there, port the *decision*, not the file. Already decided against: a TUI, an embedded web dashboard, an MCP server, a devcontainer. diff --git a/internal/artifact/address.go b/internal/artifact/address.go index 4db1a5d..efeb592 100644 --- a/internal/artifact/address.go +++ b/internal/artifact/address.go @@ -49,8 +49,8 @@ var ( // 1.2 a task, by its number // R1.2 a requirement, by its id // specs/foo/ a decomposition leaf, by the spec it names -// #notes | Notes a section, by anchor slug or by the title as written -// notes:7 the 7th paragraph of that section +// #risks | Risks a section, by anchor slug or by the title as written +// risks:2 the 2nd paragraph of that section // L120-160 an explicit line range, the escape hatch // // None of them is a line number except the last, which is why an address survives an diff --git a/internal/assets/assets.go b/internal/assets/assets.go index 66a7dfb..3856fba 100644 --- a/internal/assets/assets.go +++ b/internal/assets/assets.go @@ -77,7 +77,24 @@ import ( // 12: the entry file's layout block is one column across all three harnesses. The // padding is computed from the profile rather than written into the template, // because a run of spaces that lines up for `.codex/` is ragged for `.opencode/`. -const Version = "12" +// 13: caveman.md — the output budget belongs to the code, and narration is the part of +// a long run that can be cut without losing a fact. A rule rather than a skill because +// it is on by default, and a default the model has to choose to load is not one. One +// level (ultra) rather than three, because a dial nobody turns is three descriptions of +// the register to keep true instead of one. The language it answers in is the third +// kickoff question in autonomy.md, recorded as `lang:` beside the two that were already +// there — the register is a decision about the whole run, so it belongs where the run's +// other two decisions are and not in a preference asked again every session. In the +// same pass, three places a real workspace showed scc stating a mechanism exactly and +// leaving the judgment unsaid — where an agent fills the gap with the cheapest +// reading. The plan-run command said "read the plan" where the skill it invokes says +// map it; the review agents asked for surrounding code without ever saying the diff +// already held it; and the wiki skill explained how a slug resolves at the moment the +// page is being named, without saying the name has to name the concept. +// Also: the wiki's pages move to docs/wiki/pages/, so index.md and changelog.md are +// told apart from content by where they sit rather than by their names — which is what +// stopped any other .md dropped into wiki/ from becoming a page, and then an orphan. +const Version = "13" // The embedded tree. "all:" so nothing is silently dropped for having a name the // default embed pattern skips. @@ -141,6 +158,7 @@ func Workspace(h paths.Harness) []File { // The methodology. Every one of these is scc's own content: an upgrade should // deliver improvements to them, so none is Owned. for _, rule := range []string{ + "caveman.md", "routing.md", "autonomy.md", "methodology.md", @@ -282,6 +300,11 @@ var WorkflowSkills = []string{"plan-run"} // slash command are derived from this one list, so the two cannot drift apart, and a // skill added to either half above reaches workspaces that already exist through // `scc update` on the same terms as any other managed file. +// +// The register the agent answers in was briefly a skill here and is now caveman.md, +// because it is on by default: a skill nobody invokes does nothing, and one the model +// must decide to load is not a default. What it costs is what every rule costs — it is +// preloaded where the harness reads rules/ — and that is the price of it being on. func Skills() []string { return append(append([]string{}, KnowledgeSkills...), WorkflowSkills...) } @@ -306,6 +329,7 @@ func Dirs(h paths.Harness) []string { paths.PlansSeg, paths.DocsSeg, path.Join(paths.DocsSeg, paths.WikiSeg), + path.Join(paths.DocsSeg, paths.WikiSeg, paths.WikiPagesSeg), path.Join(paths.DocsSeg, paths.RawSeg), path.Join(paths.DocsSeg, paths.ADRSeg), path.Join(paths.DocsSeg, paths.CodewikiSeg), diff --git a/internal/assets/templates/agents/code-review.md b/internal/assets/templates/agents/code-review.md index 2581898..7e4cadc 100644 --- a/internal/assets/templates/agents/code-review.md +++ b/internal/assets/templates/agents/code-review.md @@ -20,10 +20,16 @@ git diff main...HEAD # or the base branch the work targets git diff --stat main...HEAD ``` -Then read what the work was supposed to be: `specs//requirements.md`, -`design.md`, `tasks.md`, or the checklist in `plans/.md`. **The artifact is the -standard the code is held to** — not the implementation's apparent intent. Build, test -and lint commands are in `{{.Rules}}/project.md`. +**The diff is your source; the repository is not.** It already carries every changed +line, so re-reading a file to look at them buys nothing. Open a file only when the diff +is genuinely not enough to judge a change, only if the diff touches it, and only once — +a review that fetches the same source three times spent its context on what it was +handed. When the work is in a worktree, read that path, never the main checkout's copy. + +Then read what the work was supposed to be, on the same terms: `scc map ` for +its shape and `scc map show
` for the part you need. **The artifact +is the standard the code is held to** — not the implementation's apparent intent. Build, +test and lint commands are in `{{.Rules}}/project.md`. ## The five gates — run every one, in this order diff --git a/internal/assets/templates/agents/security-review.md b/internal/assets/templates/agents/security-review.md index a7849e8..bed5d5b 100644 --- a/internal/assets/templates/agents/security-review.md +++ b/internal/assets/templates/agents/security-review.md @@ -27,6 +27,10 @@ git diff --stat main...HEAD Judge what the change makes *possible*, not what the codebase already was. Pre-existing issues in untouched code are worth one line at the end, not the body of the review. +That scope is also your read budget. The diff carries the changed lines already: open a +file only to follow reachability the diff cannot show you, only if the diff touches it, +and once. In a worktree, read that path rather than the main checkout's copy. + ## The method — four passes, in this order **1 · Map what the change adds to the attack surface.** Before judging anything, list diff --git a/internal/assets/templates/artifacts/plan.md b/internal/assets/templates/artifacts/plan.md index 6736c7d..bc2be58 100644 --- a/internal/assets/templates/artifacts/plan.md +++ b/internal/assets/templates/artifacts/plan.md @@ -13,6 +13,9 @@ ci: {{.CI}} is read from there). Never both — two records of one fact disagree, and the copy is the one that goes stale. + The same rule decides order: the order is the order things are written in, and + nothing restates it in prose. A group that must come first is moved up the list. + Delete this comment. --> ## Why @@ -36,7 +39,10 @@ ci: {{.CI}} - [ ] 1.1 (Unit) - [ ] 1.2 (TDD) -## Notes + + A constraint that is none of those — "cannot merge before the migration window" — + goes on the item's own line, where whoever reads that item will see it. --> diff --git a/internal/assets/templates/commands/scc-plan-run.md b/internal/assets/templates/commands/scc-plan-run.md index 3cc2f1d..83d9eec 100644 --- a/internal/assets/templates/commands/scc-plan-run.md +++ b/internal/assets/templates/commands/scc-plan-run.md @@ -7,9 +7,14 @@ Use the `plan-run` skill. Plan, and how to run it: $ARGUMENTS -Read the plan and name the groups back, numbered and in order, before writing any -code. The order is the one thing the user can correct cheaply now and expensively -after three merges. +Map the plan — `scc map ` — and name the groups back, numbered and in order, +before writing any code. The order is the one thing the user can correct cheaply now +and expensively after three merges. + +**Map it; do not open it.** A real plan runs to tens of kilobytes, and opening one as +the first act of a run puts all of it in context for every turn of a loop that lasts +hours. `map` answers the question you actually have here — the sections, the leaves, +the task counts, what is still open — and `map show` fetches the one part it did not. Then take every answer the line above already gave and ask only for what is left. "Implement the whole plan, one PR at the end, delivered when CI is green" has settled diff --git a/internal/assets/templates/docs/wiki/index.md b/internal/assets/templates/docs/wiki/index.md index 893a383..12578e8 100644 --- a/internal/assets/templates/docs/wiki/index.md +++ b/internal/assets/templates/docs/wiki/index.md @@ -1,12 +1,15 @@ # Wiki -The entry point. Every page under `wiki/` has to be reachable from here — directly, -or through a page that is — because a page nothing links to is a page nobody will -find again. +The entry point. Every page under `wiki/pages/` has to be reachable from here — +directly, or through a page that is — because a page nothing links to is a page +nobody will find again. Pages link to each other as `[[page-slug]]`, where the slug is the filename without -its extension. A link that resolves to no page is reported, and so is a page this -index cannot reach. +its extension and without its directory. A link that resolves to no page is reported, +and so is a page this index cannot reach. + +This file and `changelog.md` live here rather than in `pages/`: they are the wiki's +fixed documents, not pages, and neither is ever an orphan. ## Pages diff --git a/internal/assets/templates/entry.md b/internal/assets/templates/entry.md index 1c99ca9..7d3d48c 100644 --- a/internal/assets/templates/entry.md +++ b/internal/assets/templates/entry.md @@ -13,7 +13,7 @@ Nothing loads these for you. Open the file whose moment has arrived, and open it in a new session: a rule you read yesterday is not a rule you have read. {{- end}} -Triggered by where you are in the work: +`caveman.md` is always on: the register you answer in. The rest, by where you are: - `autonomy.md` — at kickoff, before writing anything - `routing.md` — work arrives and needs a vehicle: a spec, or a plan @@ -39,7 +39,7 @@ Read the source when you are about to change it, not to find it. **Plans and specs** — `scc map` · `map ` · `map tasks --next` · `map find ""` · `map show
` · `map trace`. An address is a -name — `1.2` `R1.2` `#notes` `notes:7` `specs//` — never a line number. +name — `1.2` `R1.2` `#risks` `risks:2` `specs//` — never a line number. **Changing one** — `scc patch check 1.2`, plus `task` `add` `append` `fm`. Not an editor: it resolves the address, re-validates, and rolls back an edit that adds a diff --git a/internal/assets/templates/rules/artifacts.md b/internal/assets/templates/rules/artifacts.md index 990199a..b76368f 100644 --- a/internal/assets/templates/rules/artifacts.md +++ b/internal/assets/templates/rules/artifacts.md @@ -21,13 +21,13 @@ without loading it either. a line number**, so it survives an edit above it: ``` -1.2 a task #notes a section, by anchor slug -R1.2 a requirement notes:7 the 7th paragraph of that section +1.2 a task #risks a section, by anchor slug +R1.2 a requirement risks:2 the 2nd paragraph of that section specs/foo/ a leaf L120-160 an explicit range, the escape hatch ``` `find` returns addresses, which is what makes the pair work: search, then `show` only -the hit. A long `## Notes` with no headings inside it is still navigable — `scc map +the hit. A long section with no headings inside it is still navigable — `scc map blocks` indexes its paragraphs by their opening sentence. Read the file directly only when the question is about *this exact text*: prose you are about to rewrite. @@ -39,7 +39,7 @@ when the question is about *this exact text*: prose you are about to rewrite. scc patch check 1.1 1.2 scc patch task 1.2 --text "…" --method TDD --req R1.1,R1.2 scc patch add --section tasks --number 1.3 --method Unit --text "…" -scc patch append '#notes' --text - reads stdin, for paragraphs +scc patch append '#risks' --text - reads stdin, for paragraphs scc patch fm pr=per-plan ``` diff --git a/internal/assets/templates/rules/autonomy.md b/internal/assets/templates/rules/autonomy.md index 953d646..948b082 100644 --- a/internal/assets/templates/rules/autonomy.md +++ b/internal/assets/templates/rules/autonomy.md @@ -3,23 +3,29 @@ The spec phases are **autonomous by default**: write requirements, design, and tasks, then start implementing. Do not stop for approval at each phase. -But autonomy is the user's call, so **ask, once, before writing anything** — two +But autonomy is the user's call, so **ask, once, before writing anything** — three questions, together, in the same breath: 1. **Run automatically, or gate each phase for review?** 2. **When the PR is open, wait for CI, or finish there?** +3. **Answer in English, or in 文言文?** Classical Chinese at maximum terseness — + particles (之/乃/為/其), verb before object, subject dropped. Say the cost: its + "80-90% reduction" counts **characters, not tokens**, and CJK spends more tokens per + character, so the real saving is smaller and unmeasured. Governs speech, not artifacts. -Record both answers in the artifact's frontmatter, then never ask again for this -piece of work: +Record the answers in the artifact's frontmatter (`requirements.md` for a spec), +then never ask again for this piece of work: ```yaml --- autonomy: auto # or: gated ci: wait # or: no-wait +lang: en # or: wenyan — omit to mirror the user --- ``` -`scc spec new --autonomy=auto --ci=wait` writes them for you. +`scc spec new --autonomy=auto --ci=wait` writes the first two; +`scc patch fm lang=wenyan` writes the third without opening the file. Recording them is what makes the run reproducible from the file and what stops a second session from re-asking. Ask in conversation rather than reading a flag, diff --git a/internal/assets/templates/rules/caveman.md b/internal/assets/templates/rules/caveman.md new file mode 100644 index 0000000..328f1aa --- /dev/null +++ b/internal/assets/templates/rules/caveman.md @@ -0,0 +1,55 @@ +# Caveman — the register you answer in + +You talk short. You do not think short. **Ultra, on by default**, in every response from +the first, and it does not lapse because the session got long. One level, no dial: the +only decision available is turning it off ("stop caveman" / "modo normal"). + +**Ultra.** Strip conjunctions where cause and effect stay unambiguous. One word where +one word is enough. State each fact once — a fact you already gave does not come back +as a summary. + +> Inline obj prop, new ref, re-render. `useMemo`. + +**The output budget belongs to the code.** What you write is not only an answer, it is +context every later request of the session carries — so prose about the work is paid on +every turn after the one that produced it. The diff is the part that had to exist. + +Drop articles, filler (just, really, basically, simply), pleasantries (sure, certainly, +happy to), hedging. Fragments are the norm. No narration of tool calls, no decorative +tables, no emoji, no preamble announcing the answer before the answer. + +**Never invent abbreviations** — not `cfg`, `impl`, `req`, `auth`. The tokenizer splits +an invented short form into the same pieces as the full word: the saving measures zero +and the reader still decodes it. Standard acronyms are fine — DB, API, HTTP, CI, PR. +**No causal arrows**: `→` is its own token, replacing a word that was also one. Both are +compression that measures as nothing and costs clarity, which is the one trade never +worth taking. + +**Language is the kickoff answer** — `lang:` in the artifact's frontmatter, `en` or +`wenyan`. Absent, mirror the user: Portuguese in, Portuguese out, compressed. + +**Never name the mode.** No announcement, no third-person tag, no full answer followed +by a short recap. The next answer being short is the whole confirmation. + +## What never compresses + +The line is who reads the bytes, not taste. Compressing something a validator parses, a +shell runs, or a person greps for is not compression — it is damage. + +- **Artifacts** under `specs/`, `plans/`, `docs/`. EARS lines, task lines and headings + are graded by `scc validate`; a denser requirement is a finding, not a saving. +- **Code, commands, paths, identifiers, error strings** — byte for byte. +- **Quoted output**: an error, a finding, an exit code. Quote the shortest decisive line + rather than the whole log, and quote that line exactly. +- **Commit messages and PR bodies.** [delivery.md](delivery.md) needs the body to say + what changed, which spec, and how it was verified — read by a person months later + with none of your context. +- **Questions you ask.** A compressed question gets a wrong answer you pay for all run. + +## Where it lifts + +For that passage only, with no announcement either way, wherever a misread is expensive: +a security warning · confirming something irreversible · a multi-step sequence whose +order blurs without conjunctions · anywhere the compression itself introduced the +ambiguity · any question the user had to repeat, which is evidence the short answer +failed. Answer that one in full, then carry on. diff --git a/internal/assets/templates/rules/knowledge-base.md b/internal/assets/templates/rules/knowledge-base.md index 3b1c9c9..cd6b9a4 100644 --- a/internal/assets/templates/rules/knowledge-base.md +++ b/internal/assets/templates/rules/knowledge-base.md @@ -6,7 +6,7 @@ what lets a spec stay anchored to one feature. ``` docs/ - wiki/ index.md, changelog.md, and one page per concept + wiki/ index.md and changelog.md, with pages/ — one page per concept raw/ sources dropped in to be processed — a file here is unfinished work adr/ numbered decision records codewiki/ narrated code, citing exact line ranges @@ -21,9 +21,9 @@ synonyms used where a canonical term belongs, and dependencies missing from ## wiki/ -One page per concept, linked with `[[wikilinks]]`, every page reachable from -`index.md` — an orphan is a page nobody will ever find again. Record what changed in -`changelog.md`. +One page per concept in `wiki/pages/`, linked with `[[wikilinks]]` — the slug is the +filename, never the path — and reachable from `index.md`, which stays a level up with +`changelog.md`. An orphan is a page nobody will find again. Log what you change. **`raw/` is a drop box, not storage.** Outside material goes there to be read, distilled into a wiki page, and removed. A file still sitting there is a finding: it diff --git a/internal/assets/templates/rules/methodology.md b/internal/assets/templates/rules/methodology.md index 6ee75c2..328006a 100644 --- a/internal/assets/templates/rules/methodology.md +++ b/internal/assets/templates/rules/methodology.md @@ -37,8 +37,9 @@ of writing code first, and `scc` can check neither — you are accountable for t Write the failing test first, **watch it fail**, then make it pass, then refactor. Skipping RED is not TDD: a test that has never failed has not been shown to test -anything. Say, in the task's notes, that you observed the red. RED/GREEN belongs to -TDD and only to TDD — Unit is not a lazier TDD, and TDD is not a stricter Unit. +anything. The commit that adds the failing test *is* the record that you watched it +fail — do not also write it down. RED/GREEN belongs to TDD and only to TDD: Unit is +not a lazier TDD, and TDD is not a stricter Unit. **Mandatory when the cost of being wrong is high:** money, in any calculation, rounding, split, or conversion involving currency · complex algorithms, whose diff --git a/internal/assets/templates/skills/plan-run/SKILL.md b/internal/assets/templates/skills/plan-run/SKILL.md index a9db50e..5b53ed0 100644 --- a/internal/assets/templates/skills/plan-run/SKILL.md +++ b/internal/assets/templates/skills/plan-run/SKILL.md @@ -33,14 +33,9 @@ A **group** is the smallest part of the plan that can merge on its own. | `## Decomposition` | one leaf, `specs//` | an ordinary spec — the whole cycle, by the ordinary rules | | `## Tasks` | one family of task numbers sharing a major number (`1.1`, `1.2` → group 1) | those tasks, in order | -The order is the order they are written in, unless `## Notes` says otherwise. Notes -wins — that heading exists precisely to say what must not be merged out of sequence. - -**Do not read `## Notes` end to end to find that out.** It is the longest section of -any real plan and most of it decides nothing about order. `scc map blocks -notes` lists every paragraph by its opening sentence, with an address; `scc map show - notes:7` returns the one or two that actually constrain the sequence. On a -measured plan that is ~1.3k tokens instead of ~7.3k, for the same answer. +**The order is the order they are written in.** There is no prose anywhere overriding +it, and you do not go looking for any: a plan that wanted a different order would have +been reordered. `scc map ` gives you that sequence without the file. A plan with a flat, unnumbered checklist has exactly one group. Say so and run it once, rather than inventing a decomposition the author did not write. diff --git a/internal/assets/templates/skills/prd/SKILL.md b/internal/assets/templates/skills/prd/SKILL.md index 9546dae..99d3143 100644 --- a/internal/assets/templates/skills/prd/SKILL.md +++ b/internal/assets/templates/skills/prd/SKILL.md @@ -63,8 +63,8 @@ Then fill it in. The rules the file must hold to: - **Each spec-sized leaf is one coherent feature** — something a person could describe in a sentence and verify on its own. If a leaf needs three sentences and an "and", it is two leaves. -- **Say what order they go in, and what depends on what**, under `## Notes`. This is - the part only you know right now, and the part the next session most needs. +- **Put them in the order they have to happen.** The order is the list — a plan does + not carry prose restating it, so a leaf that must come first is written first. - **A plan is work, not knowledge.** It lives in `plans/`, never in `docs/`. Referenced specs must exist, or `scc validate` reports `plan.unknown-spec`. Create @@ -81,5 +81,5 @@ placeholder; a reference to nothing is a broken plan. - **Check the vocabulary.** Every term the plan coins is a term three specs will inherit. If any of them is contested or new, use the `glossary` skill now, while it costs one edit. -- **A choice made here that is hard to reverse is an ADR**, not a line in the plan's - notes. Use the `adr` skill. +- **A choice made here that is hard to reverse is an ADR**, not prose in the plan. + Use the `adr` skill, and cite the record from the item it governs. diff --git a/internal/assets/templates/skills/wiki/SKILL.md b/internal/assets/templates/skills/wiki/SKILL.md index b73c875..87e0fee 100644 --- a/internal/assets/templates/skills/wiki/SKILL.md +++ b/internal/assets/templates/skills/wiki/SKILL.md @@ -23,8 +23,23 @@ This skill is the procedure. 3. **Check `docs/glossary.md` first.** If the source names something the project already has a canonical term for, use the canonical term. If it coins a term worth keeping, that is the `glossary` skill's job — do it, then come back. -4. **Write the page** at `docs/wiki/.md`. The filename is the slug, so - `order-total.md` is what `[[order-total]]` resolves to. +4. **Write the page** at `docs/wiki/pages/.md`. The filename is the slug, so + `order-total.md` is what `[[order-total]]` resolves to — the slug never carries the + directory. `index.md` and `changelog.md` stay one level up, in `docs/wiki/`: they + are the wiki's fixed documents rather than pages, and keeping them out of `pages/` + is what lets the validator tell content from structure without matching filenames. + + **Name it for the concept, as something that stands on its own.** A `[[wikilink]]` + shows the reader the slug and nothing else, so the slug carries the whole signal: + it has to answer "what is this page about?" with no other context on screen. + `order-total.md`, `retry-policy.md`, `sprite-atlas.md` do. A slug lifted out of a + sentence does not — `into-an-engine.md` is the tail of somebody's heading, and a + year later nobody can tell what page that was without opening it. Test it by + reading the filename alone: if it is not a thing this project has, rename it. + + No validator catches this. Every wiki check is about the graph — whether the link + resolves, whether the page is reachable — and a badly named page passes all of + them, which is exactly why it is on you here. 5. **Link it in.** Add a `[[wikilink]]` from `index.md`, or from a page already reachable from it. A page nobody links is a page nobody finds again. 6. **Record it in `changelog.md`** — what changed, naming the pages with wikilinks. @@ -54,6 +69,8 @@ someone reconstructs the same thing is the signal, not the first. | `wiki.orphan-page` | The page exists and nothing reaches it. | Link it from `index.md` or from a reachable page. Or delete it — an unreachable page nobody has missed is a candidate. | | `wiki.missing-changelog` | The wiki has pages and no log of how it got that way. | Write `changelog.md`. | | `wiki.changelog-desync` | The log names a page that no longer exists. | The page was renamed or removed; the log is a record, so correct the entry rather than rewriting history around it. | +| `wiki.legacy-page` | A page sits directly in `docs/wiki/`, from before `pages/`. | `git mv` it into `docs/wiki/pages/`. The slug does not change, so no wikilink moves with it. | +| `wiki.duplicate-page` | Two files claim one slug, so `[[it]]` is ambiguous. | Usually half a migration: keep the copy in `pages/`, delete the other once you have merged anything only it has. | | `wiki.unprocessed-source` | A file is still in `docs/raw/`. | Ingest it, above. | **`index.md` is a map, not a dump.** When it grows into a flat list of every page, diff --git a/internal/cli/graph.go b/internal/cli/graph.go index 91cddcd..a5b51f7 100644 --- a/internal/cli/graph.go +++ b/internal/cli/graph.go @@ -253,7 +253,7 @@ func ensureCodeGraph(opts graphInstall) (string, bool) { if ask && !opts.yes { render.Warn(fmt.Sprintf("%s is not on PATH — it is what builds and answers the graph", codegraph.Bin)) render.Detail(" " + codegraph.Repo) - ask = confirm(promptIn, fmt.Sprintf("Install it now with `%s`?", installer.Cmd)) + ask = confirmInstall(promptIn, fmt.Sprintf("Install it now with `%s`?", installer.Cmd)) } if !ask { render.Err(codegraph.Bin + " is not on PATH") diff --git a/internal/cli/launch.go b/internal/cli/launch.go index 9f0d691..a0deaea 100644 --- a/internal/cli/launch.go +++ b/internal/cli/launch.go @@ -12,6 +12,7 @@ import ( "github.com/protonspy/spec-claude-code/internal/headroom" "github.com/protonspy/spec-claude-code/internal/paths" "github.com/protonspy/spec-claude-code/internal/render" + "github.com/protonspy/spec-claude-code/internal/rtk" "github.com/protonspy/spec-claude-code/internal/workspace" ) @@ -60,7 +61,8 @@ func runLaunch(args []string) int { contextTool := fs.Bool("headroom-context-tool", false, "let Headroom set up its own CLI context tool (RTK or lean-ctx) and append its guidance to the entry file") noGraph := fs.Bool("no-graph", false, "start the agent without building or refreshing the symbol graph") - noInstall := fs.Bool("no-install", false, "never install anything; use Headroom and CodeGraph only if they are already on PATH") + noRTK := fs.Bool("no-rtk", false, "start the agent without setting up RTK's binary or its usage block") + noInstall := fs.Bool("no-install", false, "never install anything; use Headroom, CodeGraph and RTK only if they are already on PATH") yes := fs.Bool("yes", false, "answer the install prompts with yes, for an unattended run") dryRun := fs.Bool("dry-run", false, "print the command this would run, and run nothing") jsonOut := addJSON(fs) @@ -117,6 +119,16 @@ func runLaunch(args []string) int { plan: plan, quiet: *jsonOut, }) + // RTK last of the three, because it is the only one that writes to a file the + // user owns: a run the user aborts at the Headroom or CodeGraph prompt should not + // already have edited their entry file. + cmd.RTK = resolveRTK(target, rtkLaunchOptions{ + disabled: *noRTK, + noInstall: *noInstall, + yes: *yes, + plan: plan, + quiet: *jsonOut, + }) if cmd.Args == nil { // A JSON consumer gets [] rather than null: the field is a command line, // and an empty one is still a list. @@ -148,12 +160,13 @@ func runLaunch(args []string) int { // launchCommand is both the frozen JSON shape and what the human line is printed // from, so the two cannot describe different commands. type launchCommand struct { - Harness string `json:"harness"` - Dir string `json:"dir"` - Bin string `json:"bin"` - Args []string `json:"args"` - Headroom *headroomReport `json:"headroom,omitempty"` - Graph *graphReport `json:"graph,omitempty"` + Harness string `json:"harness"` + Dir string `json:"dir"` + Bin string `json:"bin"` + Args []string `json:"args"` + Headroom *headroomReport `json:"headroom,omitempty"` + Graph *graphReport `json:"graph,omitempty"` + RTK *rtkLaunchReport `json:"rtk,omitempty"` } // String is the command as a person would type it. Not shell-quoted, because it @@ -246,7 +259,7 @@ func resolveHeadroom(h paths.Harness, opts headroomOptions) *headroomReport { default: render.Warn(fmt.Sprintf("%s is not on PATH — %s compresses the agent's context before it reaches the model", headroom.Bin, headroom.Bin)) render.Detail(" " + headroom.Repo) - if !confirm(promptIn, fmt.Sprintf("Install it now with `%s`?", installer.Cmd)) { + if !confirmInstall(promptIn, fmt.Sprintf("Install it now with `%s`?", installer.Cmd)) { report.Reason = "install declined" } } @@ -320,6 +333,91 @@ func wrapWith(report *headroomReport, opts headroomOptions) { render.Detail(" `" + headroom.Bin + " wrap " + report.Agent + " --help`, and `" + headroom.Bin + " unwrap " + report.Agent + "` to undo what it registered") } +// rtkLaunchReport says what happened to RTK on the way to starting the agent. It is +// deliberately not cli.rtkReport: that one is `scc rtk`'s frozen shape, describing a +// run of that command, and a launch answers a narrower question — is the binary there, +// and does the entry file carry the block. +type rtkLaunchReport struct { + // Install is what happened to the binary, in the same vocabulary the other two + // integrations use: present | installed | skipped | failed. + Install string `json:"install"` + Path string `json:"path,omitempty"` + Version string `json:"version,omitempty"` + // Block is what happened to the usage block in the entry file: added | present | + // replaced | missing | skipped. + Block string `json:"block"` + // Reason names why RTK was not set up, for the run where that is a surprise. + Reason string `json:"reason,omitempty"` +} + +type rtkLaunchOptions struct { + disabled bool + noInstall bool + yes bool + plan bool + quiet bool +} + +// resolveRTK makes sure the agent about to start can actually use the prefix its entry +// file tells it to use. +// +// This is the one integration whose setup writes to a file the user owns, and that is +// why it is the one whose prompt has to name both halves. `scc rtk` exists as a +// separate opt-in command precisely because splicing into somebody's CLAUDE.md is not +// a thing to do on the side; offering it here is the same decision put where it is +// actually actionable — at the moment the session that would benefit is starting. +// Saying no is free, the block is idempotent once written, and --no-rtk is the +// standing answer. +// +// It degrades the way the other two do: RTK is an enhancement, so a missing cargo, a +// declined install, or a failed build all end in the agent starting anyway. +func resolveRTK(root string, opts rtkLaunchOptions) *rtkLaunchReport { + if opts.disabled { + return nil + } + report := &rtkLaunchReport{Install: installSkipped, Block: "skipped"} + + if _, ok := rtk.Path(); !ok { + switch { + case opts.noInstall || opts.plan: + report.Reason = rtk.Bin + " is not on PATH" + case !rtk.Available(): + report.Reason = fmt.Sprintf("cargo is not on PATH, so %s cannot be built", rtk.Bin) + case opts.yes: + // Asked for by flag; no question to put. + case opts.quiet || !interactive(): + report.Reason = fmt.Sprintf("%s is not on PATH, and nobody is here to answer the install prompt", rtk.Bin) + default: + render.Warn(fmt.Sprintf("%s is not on PATH — it filters command output before it reaches the model", rtk.Bin)) + render.Detail(" " + rtk.Repo) + // The question names the file, because consenting to an install is not + // consenting to an edit and the user cannot see the second one coming. + if !confirmInstall(promptIn, fmt.Sprintf("Build it with `%s` and add its usage block to the entry file?", rtk.InstallCmd())) { + report.Reason = "install declined" + } + } + if report.Reason != "" { + if !opts.quiet && !opts.plan { + render.Warn("starting without " + rtk.Bin + ": " + report.Reason) + } + return report + } + } + + // applyRTK owns both halves and is what `scc rtk` runs, so a launch cannot drift + // from the command. noInstall is false here only because the prompt above already + // settled it — this call is what actually builds the binary and writes the block. + sub, _ := applyRTK(root, rtkOptions{quiet: opts.quiet}) + report.Install, report.Path, report.Version = sub.Install, sub.Path, sub.Version + if sub.Error != "" { + report.Reason = sub.Error + } + if len(sub.Files) > 0 { + report.Block = sub.Files[0].Action + } + return report +} + // graphReport says what happened to the symbol graph on the way to starting the // agent. type graphReport struct { @@ -382,7 +480,7 @@ func resolveGraph(root string, opts graphOptions) *graphReport { default: render.Warn(fmt.Sprintf("%s is not on PATH — it gives the agent a symbol graph instead of file-by-file reading", codegraph.Bin)) render.Detail(" " + codegraph.Repo) - if !confirm(promptIn, fmt.Sprintf("Install it now with `%s`?", installer.Cmd)) { + if !confirmInstall(promptIn, fmt.Sprintf("Install it now with `%s`?", installer.Cmd)) { report.Reason = "install declined" } } diff --git a/internal/cli/launch_test.go b/internal/cli/launch_test.go index 869f6c1..483f482 100644 --- a/internal/cli/launch_test.go +++ b/internal/cli/launch_test.go @@ -646,3 +646,82 @@ func sameDir(t *testing.T, a, b string) bool { } return os.SameFile(fa, fb) } + +// RTK joins the preflight the other two integrations already went through, so a +// launch reports all three. --no-rtk is the standing "not in this workspace", and it +// has to leave the field out entirely rather than report a skip: the two are +// different answers, and only one of them means somebody decided. +func TestLaunchReportsRTKAndNoRTKOmitsIt(t *testing.T) { + root := initWorkspace(t) + isolatedPath(t, "claude") + + var with launchCommand + stdout, _, code := run(t, "launch", "--root", root, "--json") + if code != ExitOK { + t.Fatalf("exit = %d", code) + } + if err := json.Unmarshal([]byte(stdout), &with); err != nil { + t.Fatalf("unmarshal: %v\n%s", err, stdout) + } + if with.RTK == nil { + t.Fatal("a launch reported no rtk field at all") + } + if with.RTK.Install != installSkipped { + t.Errorf("install = %q, want %q with cargo absent", with.RTK.Install, installSkipped) + } + if with.RTK.Reason == "" { + t.Error("rtk was skipped without saying why") + } + + var without launchCommand + stdout, _, code = run(t, "launch", "--root", root, "--json", "--no-rtk") + if code != ExitOK { + t.Fatalf("exit = %d", code) + } + if err := json.Unmarshal([]byte(stdout), &without); err != nil { + t.Fatalf("unmarshal: %v\n%s", err, stdout) + } + if without.RTK != nil { + t.Errorf("--no-rtk still reported %+v", without.RTK) + } +} + +// --no-install covers RTK too. The flag says "never build anything", and a cargo +// build that takes minutes is the most expensive thing it governs. +func TestLaunchNoInstallCoversRTK(t *testing.T) { + root := initWorkspace(t) + isolatedPath(t, "cargo", "claude") + withPrompt(t, "\n") + withLaunchExec(t, 0) + + stdout, stderr, code := run(t, "launch", "--root", root, "--no-install") + if code != ExitOK { + t.Fatalf("exit = %d", code) + } + if strings.Contains(stdout+stderr, "add its usage block") { + t.Errorf("--no-install still offered the RTK setup: %q", stdout+stderr) + } +} + +// An install prompt defaults to yes and a bare Enter takes it, which is the opposite +// of confirm(). The balance is opposite too: a wrong yes here is a tool on the +// machine, where a wrong yes in confirm() is somebody's file. +func TestConfirmInstallDefaultsToYes(t *testing.T) { + for _, tc := range []struct { + in string + want bool + }{ + {"\n", true}, + {"y\n", true}, + {" \n", true}, + {"n\n", false}, + {"N\n", false}, + {"no\n", false}, + {"não\n", false}, + {"", false}, // end of input is not an answer + } { + if got := confirmInstall(strings.NewReader(tc.in), "Install?"); got != tc.want { + t.Errorf("confirmInstall(%q) = %v, want %v", tc.in, got, tc.want) + } + } +} diff --git a/internal/cli/map.go b/internal/cli/map.go index 600ffcc..ba76eb4 100644 --- a/internal/cli/map.go +++ b/internal/cli/map.go @@ -731,8 +731,8 @@ Addresses, none of which is a line number — which is why one survives an edit 1.2 a task, by its number R1.2 a requirement, by its id specs/foo/ a decomposition leaf - #notes a section, by anchor slug (or by its title as written) - notes:7 the 7th paragraph of that section + #risks a section, by anchor slug (or by its title as written) + risks:2 the 2nd paragraph of that section L120-160 an explicit line range, the escape hatch Use "%s patch" to change what "%s map" found, without reading the file first. diff --git a/internal/cli/map_test.go b/internal/cli/map_test.go index 113b08e..cbe9e36 100644 --- a/internal/cli/map_test.go +++ b/internal/cli/map_test.go @@ -324,6 +324,29 @@ func TestPatchCheckIsIdempotent(t *testing.T) { } } +// The kickoff language is the one answer `spec new` and `plan new` take no flag for, +// so `patch fm` is the whole path to it — and the path is only useful if it is the one +// autonomy.md tells the agent to type. A wrong value takes the same road as any other +// bad edit: the validator that owns the file fires, the write is undone, exit 2. +func TestPatchFrontmatterWritesTheKickoffLanguage(t *testing.T) { + root := mapWorkspace(t) + if _, stderr, code := run(t, "patch", "fm", "sample", "lang=wenyan", "--root", root); code != ExitOK { + t.Fatalf("exit = %d, want %d (%s)", code, ExitOK, stderr) + } + if !strings.Contains(planText(t, root), "lang: wenyan") { + t.Errorf("the answer was not recorded:\n%s", planText(t, root)) + } + + before := planText(t, root) + _, stderr, code := run(t, "patch", "fm", "sample", "lang=pt-BR", "--root", root) + if code != ExitFindings { + t.Errorf("exit = %d, want %d for an undocumented language (%s)", code, ExitFindings, stderr) + } + if got := planText(t, root); got != before { + t.Errorf("the rejected language was left on disk:\n%s", got) + } +} + // A requirement id is scoped to its own spec. Tracing an unscoped one across a // workspace where several specs number theirs the same way is the same mistake as // reading all of them. diff --git a/internal/cli/update.go b/internal/cli/update.go index 002b618..795a331 100644 --- a/internal/cli/update.go +++ b/internal/cli/update.go @@ -261,6 +261,32 @@ func reportPlans(harnesses []paths.Harness, plans []*scaffold.UpdatePlan, force // confirm asks a yes/no question, defaulting to no. Anything other than an // explicit yes leaves the workspace untouched: the cost of a wrong "no" is // running the command again, and the cost of a wrong "yes" is somebody's work. +// confirmInstall asks a yes/no question defaulting to *yes*, and is only ever used to +// offer installing one of the third-party tools a launch wants. +// +// The opposite default from confirm(), on purpose, because the balance is the opposite. +// confirm() guards a write to the user's own files, where a wrong yes costs somebody's +// work. Here a wrong yes costs a tool on the machine that can be uninstalled, and a +// wrong no costs a whole session running degraded — so the cheap mistake is the one to +// default to. A question that also writes to a file the user owns has to say so in its +// own text; there is no third default for that. +// +// End of input is not an answer and never installs: --yes is how an unattended run +// says yes, and the callers reach this only when a human is present. +func confirmInstall(in io.Reader, question string) bool { + render.Ask(question + " [n/Y]: ") + scanner := bufio.NewScanner(in) + if !scanner.Scan() { + fmt.Println() + return false + } + switch strings.ToLower(strings.TrimSpace(scanner.Text())) { + case "n", "no", "nao", "não": + return false + } + return true +} + func confirm(in io.Reader, question string) bool { render.Ask(question + " [y/N]: ") scanner := bufio.NewScanner(in) diff --git a/internal/paths/paths.go b/internal/paths/paths.go index 6477813..be16b44 100644 --- a/internal/paths/paths.go +++ b/internal/paths/paths.go @@ -228,14 +228,20 @@ const ( // decisions, the material read from outside — while a spec answers what one // feature does now. Neither replaces the other. const ( - WikiSeg = "wiki" // the graph of durable knowledge - ADRSeg = "adr" // architecture decision records, numbered - RawSeg = "raw" // sources dropped in to be processed into the wiki - CodewikiSeg = "codewiki" // narrated code, citing [path:start-end]() - GlossarySeg = "glossary.md" // one canonical term per concept - StackSeg = "stack.md" // adopted technology; unlisted means undecided - WikiIndex = "index.md" // the wiki's entry point; an unreachable page is an orphan - WikiLog = "changelog.md" // what changed in the wiki, and when + WikiSeg = "wiki" // the graph of durable knowledge + ADRSeg = "adr" // architecture decision records, numbered + RawSeg = "raw" // sources dropped in to be processed into the wiki + CodewikiSeg = "codewiki" // narrated code, citing [path:start-end]() + GlossarySeg = "glossary.md" + StackSeg = "stack.md" // adopted technology; unlisted means undecided + // 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 + // matching those names, which made any other file dropped into wiki/ a page — + // and usually then an orphan finding for a file nobody meant as a page. + WikiPagesSeg = "pages" + WikiIndex = "index.md" // the wiki's entry point; an unreachable page is an orphan + WikiLog = "changelog.md" // what changed in the wiki, and when ) // Config returns the harness's configuration directory under root. @@ -326,6 +332,9 @@ func Docs(root string) string { return filepath.Join(root, DocsSeg) } // Wiki returns docs/wiki/. func Wiki(root string) string { return filepath.Join(root, DocsSeg, WikiSeg) } +// WikiPages returns docs/wiki/pages/, where the pages live. +func WikiPages(root string) string { return filepath.Join(Wiki(root), WikiPagesSeg) } + // ADR returns docs/adr/. func ADR(root string) string { return filepath.Join(root, DocsSeg, ADRSeg) } diff --git a/internal/rtk/rtk.go b/internal/rtk/rtk.go index 522df1f..6b1e1e6 100644 --- a/internal/rtk/rtk.go +++ b/internal/rtk/rtk.go @@ -111,6 +111,19 @@ const ( // has to run it themselves. func InstallCmd() string { return "cargo install --git " + Repo } +// Available says whether the toolchain Install needs is on PATH. Headroom and +// CodeGraph answer the same question about theirs, and a caller that offers to +// install without asking it first ends up putting a prompt to somebody whose only +// possible answer is no. +// +// Cargo is the whole answer here. RTK is distributed as source, so unlike the other +// two there is no second installer to fall back to — which is also why this returns a +// bool rather than the installer the others return. +func Available() bool { + _, err := exec.LookPath("cargo") + return err == nil +} + // Splice returns doc with the block present exactly once, and what it had to do to // get there. // diff --git a/internal/scaffold/update_test.go b/internal/scaffold/update_test.go index 8c0843d..381f82a 100644 --- a/internal/scaffold/update_test.go +++ b/internal/scaffold/update_test.go @@ -251,8 +251,9 @@ func TestApplyUpdateDropsUnmanagedFiles(t *testing.T) { // This is the path a new skill actually ships on, and it is the one with something // to get wrong: the destination is a directory that does not exist yet, so a create // that only wrote files would fail on precisely the workspaces the update is for. -// Nothing here is specific to plan-run — it is read out of assets.WorkflowSkills, so -// the next workflow skill is covered the day it is added. +// Nothing here is specific to any one skill — it is read out of assets.Skills(), which +// is every list scc ships, so the next skill added to any of them is covered the day it +// lands. Reading one list would have left the other two shipping on an untested path. func TestApplyUpdateAddsASkillTheWorkspacePredates(t *testing.T) { for _, h := range paths.Harnesses() { root := t.TempDir() @@ -261,7 +262,7 @@ func TestApplyUpdateAddsASkillTheWorkspacePredates(t *testing.T) { // Roll the workspace back to before the skill existed: the files gone, and // no manifest entry claiming scc ever wrote them. var rels []string - for _, skill := range assets.WorkflowSkills { + for _, skill := range assets.Skills() { rels = append(rels, path.Join(h.Dir, h.SkillsSeg, skill, "SKILL.md")) if h.CommandsSeg != "" { rels = append(rels, path.Join(h.Dir, h.CommandsSeg, "scc-"+skill+".md")) @@ -272,7 +273,16 @@ func TestApplyUpdateAddsASkillTheWorkspacePredates(t *testing.T) { t.Fatalf("%s: Load: %v", h.ID, err) } for _, rel := range rels { - if err := os.RemoveAll(filepath.Dir(filepath.Join(root, filepath.FromSlash(rel)))); err != nil { + p := filepath.Join(root, filepath.FromSlash(rel)) + // A skill is a whole directory that has to be recreated — which is the case + // this test exists for. A command is one file in a directory that stays, so + // removing its parent would take every other command with it and test + // something else. + target := p + if filepath.Base(p) == "SKILL.md" { + target = filepath.Dir(p) + } + if err := os.RemoveAll(target); err != nil { t.Fatalf("%s: %v", h.ID, err) } m.Remove(rel) diff --git a/internal/validate/knowledge_test.go b/internal/validate/knowledge_test.go index f245f5e..5ce0d51 100644 --- a/internal/validate/knowledge_test.go +++ b/internal/validate/knowledge_test.go @@ -49,10 +49,10 @@ func TestKnowledgeValidatorsAreSilentOnAnEmptyWorkspace(t *testing.T) { func TestWikiGraphChecks(t *testing.T) { root := t.TempDir() - wiki := paths.Wiki(root) + wiki, pages := paths.Wiki(root), paths.WikiPages(root) write(t, filepath.Join(wiki, paths.WikiIndex), "# Index\n\n- [[order-total]]\n- [[nowhere]]\n") - write(t, filepath.Join(wiki, "order-total.md"), "# Order total\n\nThe amount charged.\n") - write(t, filepath.Join(wiki, "unlinked.md"), "# Unlinked\n\nNobody links here.\n") + write(t, filepath.Join(pages, "order-total.md"), "# Order total\n\nThe amount charged.\n") + write(t, filepath.Join(pages, "unlinked.md"), "# Unlinked\n\nNobody links here.\n") write(t, filepath.Join(wiki, paths.WikiLog), "# Changelog\n\n- added [[order-total]]\n- added [[deleted-page]]\n") got := runValidator(t, Wiki, root) @@ -75,11 +75,11 @@ func TestWikiGraphChecks(t *testing.T) { // orphan. func TestWikiReachabilityIsTransitive(t *testing.T) { root := t.TempDir() - wiki := paths.Wiki(root) + wiki, pages := paths.Wiki(root), paths.WikiPages(root) write(t, filepath.Join(wiki, paths.WikiIndex), "# Index\n\n- [[one]]\n") - write(t, filepath.Join(wiki, "one.md"), "# One\n\nsee [[two]]\n") - write(t, filepath.Join(wiki, "two.md"), "# Two\n\nsee [[three|the third]]\n") - write(t, filepath.Join(wiki, "three.md"), "# Three\n\nend of the chain\n") + write(t, filepath.Join(pages, "one.md"), "# One\n\nsee [[two]]\n") + write(t, filepath.Join(pages, "two.md"), "# Two\n\nsee [[three|the third]]\n") + write(t, filepath.Join(pages, "three.md"), "# Three\n\nend of the chain\n") write(t, filepath.Join(wiki, paths.WikiLog), "# Changelog\n") if got := runValidator(t, Wiki, root); len(got) != 0 { @@ -87,9 +87,68 @@ func TestWikiReachabilityIsTransitive(t *testing.T) { } } +// A wiki written before pages/ existed still validates as a graph — its pages are read, +// its links resolve, nothing is a false orphan — and each loose page is reported once so +// the author can move it. Seeing the page and reporting the layout are separate answers; +// refusing to see it would report every link into it as broken. +func TestWikiReadsTheLayoutThatPredatesPages(t *testing.T) { + root := t.TempDir() + wiki := paths.Wiki(root) + write(t, filepath.Join(wiki, paths.WikiIndex), "# Index\n\n- [[one]]\n") + write(t, filepath.Join(wiki, "one.md"), "# One\n\nsee [[two]]\n") + write(t, filepath.Join(wiki, "two.md"), "# Two\n\nend\n") + write(t, filepath.Join(wiki, paths.WikiLog), "# Changelog\n") + + set, err := Wiki(root) + if err != nil { + t.Fatal(err) + } + var legacy int + for _, f := range set.Sorted() { + switch f.Rule { + case "wiki.legacy-page": + legacy++ + default: + t.Errorf("%s: a legacy wiki should be graph-clean, got %s", f.File, f.Rule) + } + } + if legacy != 2 { + t.Errorf("wiki.legacy-page fired %d times, want one per loose page (2)", legacy) + } +} + +// Two files claiming one slug makes [[order-total]] ambiguous. The copy already in +// pages/ is the one that keeps the slug, so migrating half a wiki reports the half that +// has not moved rather than the half that has. +func TestWikiDuplicateSlugNamesTheLooseCopy(t *testing.T) { + root := t.TempDir() + wiki, pages := paths.Wiki(root), paths.WikiPages(root) + write(t, filepath.Join(wiki, paths.WikiIndex), "# Index\n\n- [[order-total]]\n") + write(t, filepath.Join(pages, "order-total.md"), "# Order total\n\nmoved\n") + write(t, filepath.Join(wiki, "order-total.md"), "# Order total\n\nnot moved\n") + write(t, filepath.Join(wiki, paths.WikiLog), "# Changelog\n") + + set, err := Wiki(root) + if err != nil { + t.Fatal(err) + } + var dup *finding.Finding + for _, f := range set.Sorted() { + if f.Rule == "wiki.duplicate-page" { + dup = &f + } + } + if dup == nil { + t.Fatalf("no wiki.duplicate-page in %v", runValidator(t, Wiki, root)) + } + if strings.Contains(filepath.ToSlash(dup.File), paths.WikiSeg+"/"+paths.WikiPagesSeg+"/") { + t.Errorf("duplicate reported against %s, want the loose copy", dup.File) + } +} + func TestWikiMissingIndexAndChangelog(t *testing.T) { root := t.TempDir() - write(t, filepath.Join(paths.Wiki(root), "lonely.md"), "# Lonely\n") + write(t, filepath.Join(paths.WikiPages(root), "lonely.md"), "# Lonely\n") got := runValidator(t, Wiki, root) for _, want := range []string{"wiki.missing-index", "wiki.missing-changelog"} { if !contains(got, want) { diff --git a/internal/validate/plan.go b/internal/validate/plan.go index 5691ab5..2237f71 100644 --- a/internal/validate/plan.go +++ b/internal/validate/plan.go @@ -136,7 +136,7 @@ func checkLoopAnswers(set *finding.Set, file string, fm mdscan.Frontmatter) { // checkKickoffAs is checkKickoff with the rule slug the caller's subject uses, so a // plan's finding reads `plan.` and a spec's reads `spec.` for the same defect. func checkKickoffAs(set *finding.Set, file string, fm mdscan.Frontmatter, rule string) { - for _, key := range []string{"autonomy", "ci"} { + for _, key := range []string{"autonomy", "ci", "lang"} { value, ok := fm.Get(key) if !ok { continue diff --git a/internal/validate/plan_test.go b/internal/validate/plan_test.go index 25ef31c..45ccef8 100644 --- a/internal/validate/plan_test.go +++ b/internal/validate/plan_test.go @@ -112,6 +112,20 @@ func TestPlanKickoffAnswers(t *testing.T) { if got := planFindings(t, root, "sweep"); !contains(got, "plan.kickoff-invalid") { t.Errorf("rules = %v, want plan.kickoff-invalid", got) } + + // A plan is the vehicle a whole run is driven from, so it carries the language + // answer on the same terms a spec does — one key, checked when present. + root = t.TempDir() + writePlan(t, root, "wide", "---\nautonomy: auto\nci: wait\nlang: wenyan\n---\n\n# Wide\n\n- [ ] 1.1 (Unit) Do it\n") + if got := planFindings(t, root, "wide"); len(got) != 0 { + t.Errorf("a plan carrying every kickoff answer reported %v", got) + } + + root = t.TempDir() + writePlan(t, root, "narrow", "---\nlang: pt-BR\n---\n\n# Narrow\n\n- [ ] 1.1 (Unit) Do it\n") + if got := planFindings(t, root, "narrow"); !contains(got, "plan.kickoff-invalid") { + t.Errorf("rules = %v, want plan.kickoff-invalid for an undocumented language", got) + } } // The answers `plan-run` writes back before it starts a loop. A wrong value is worth diff --git a/internal/validate/spec.go b/internal/validate/spec.go index 3e8b4d8..cd790a2 100644 --- a/internal/validate/spec.go +++ b/internal/validate/spec.go @@ -37,6 +37,10 @@ var deltaMarkers = map[string]bool{"ADDED": true, "MODIFIED": true, "REMOVED": t var kickoffValues = map[string]map[string]bool{ "autonomy": {"auto": true, "gated": true}, "ci": {"wait": true, "no-wait": true}, + // lang is the register's output language, asked at kickoff with the other two and + // read by caveman.md. Absent means the agent answers in whatever language it is + // addressed in, which is the behavior that predates the question. + "lang": {"en": true, "wenyan": true}, } // requirement is one parsed requirement line. diff --git a/internal/validate/spec_test.go b/internal/validate/spec_test.go index 3581e1d..e4255f2 100644 --- a/internal/validate/spec_test.go +++ b/internal/validate/spec_test.go @@ -3,9 +3,11 @@ package validate import ( "os" "path/filepath" + "regexp" "strings" "testing" + "github.com/protonspy/spec-claude-code/internal/assets" "github.com/protonspy/spec-claude-code/internal/paths" ) @@ -291,6 +293,48 @@ func TestKickoffAnswers(t *testing.T) { } } +// lang is the third kickoff answer, on the same terms as the other two: absent is +// the behavior that predates the question, present is checked. The values are the +// two the rules offer — a language name nobody documented is a typo, and the run it +// would silently change the register for is the whole run. +func TestKickoffLanguage(t *testing.T) { + with := func(value string) string { + return strings.Replace(goodRequirements, "ci: wait\n", "ci: wait\nlang: "+value+"\n", 1) + } + + for _, value := range []string{"en", "wenyan"} { + root := t.TempDir() + writeSpec(t, root, "billing", with(value), goodDesign, goodTasks) + if got := specFindings(t, root, "billing"); len(got) != 0 { + t.Errorf("lang: %s reported %v", value, got) + } + } + + root := t.TempDir() + writeSpec(t, root, "billing", with("classical-chinese"), goodDesign, goodTasks) + if got := specFindings(t, root, "billing"); !contains(got, "spec.kickoff-invalid") { + t.Errorf("rules = %v, want spec.kickoff-invalid", got) + } +} + +// The kickoff answers are asked by a rule and graded here, and the two have to name +// the same values. A value this validator accepts that autonomy.md never offers is one +// no agent will ever write; a value the rule offers under another name is a rollback on +// the command the rule tells the agent to type. +func TestTheRuleOffersEveryKickoffAnswerThisAccepts(t *testing.T) { + rule, err := assets.Content("rules/autonomy.md") + if err != nil { + t.Fatal(err) + } + for key, values := range kickoffValues { + for value := range values { + if !regexp.MustCompile(`\b` + regexp.QuoteMeta(value) + `\b`).MatchString(rule) { + t.Errorf("autonomy.md never offers `%s: %s`", key, value) + } + } + } +} + // The templates ship examples and instructions inside HTML comments and fenced blocks. // A validator that read them would report findings on the file scc just generated. func TestExamplesInCommentsAndFencesAreNotContent(t *testing.T) { diff --git a/internal/validate/wiki.go b/internal/validate/wiki.go index ba3c11e..f013ec6 100644 --- a/internal/validate/wiki.go +++ b/internal/validate/wiki.go @@ -12,15 +12,20 @@ import ( // Wiki validates docs/wiki/ and the drop box beside it. // -// All four checks are graph facts over Markdown — no code is read, and none of them is -// a judgment call: +// Every check is a graph fact over Markdown — no code is read, and none of them is a +// judgment call: // // - a [[wikilink]] resolves to a page // - every page is reachable from index.md, because an orphan is a page nobody will // find again // - the changelog exists and names only pages that exist +// - no two files claim one slug, since [[order-total]] must mean one thing +// - the pages are under wiki/pages/ // - docs/raw/ is empty, because a file still sitting there was collected and never // processed +// +// What this cannot check is whether a page's name names anything — `into-an-engine.md` +// resolves, links, and is reachable. That one is on the wiki skill. func Wiki(root string) (*finding.Set, error) { set := &finding.Set{} if err := checkRaw(set, root); err != nil { @@ -28,7 +33,7 @@ func Wiki(root string) (*finding.Set, error) { } dir := paths.Wiki(root) - pages, err := markdownFiles(dir) + pages, err := wikiPages(dir) if err != nil { return nil, err } @@ -36,48 +41,74 @@ func Wiki(root string) (*finding.Set, error) { return set, nil } - // A page's name is its slug: [[order-total]] resolves to order-total.md. + // A page's name is its slug: [[order-total]] resolves to order-total.md. Pages + // arrive canonical-first, so a slug claimed in both layouts reports the loose copy + // as the duplicate rather than the one already in the right place. slugs := map[string]bool{} - for _, p := range pages { - slugs[strings.TrimSuffix(filepath.Base(p), ".md")] = true + claimed := map[string]string{} + for _, pg := range pages { + if first, dup := claimed[pg.slug]; dup { + set.Addf(rel(root, pg.path), 0, "wiki.duplicate-page", + "[[%s]] already resolves to %s; one slug cannot name two files", + pg.slug, rel(root, first)) + continue + } + claimed[pg.slug] = pg.path + slugs[pg.slug] = true + if pg.legacy { + set.Addf(rel(root, pg.path), 0, "wiki.legacy-page", + "pages live in %s/%s/ now; move it there so the wiki's fixed documents stay distinguishable from its content", + paths.WikiSeg, paths.WikiPagesSeg) + } } - indexSlug := strings.TrimSuffix(paths.WikiIndex, ".md") - if !slugs[indexSlug] { - set.Addf(rel(root, filepath.Join(dir, paths.WikiIndex)), 0, "wiki.missing-index", + indexPath := filepath.Join(dir, paths.WikiIndex) + if !isFile(indexPath) { + set.Addf(rel(root, indexPath), 0, "wiki.missing-index", "the wiki has %d pages and no %s; without an entry point every page is an orphan", len(pages), paths.WikiIndex) } // links[from] = the pages it points at, collected once and used for both the - // broken-link check and the reachability walk. + // broken-link check and the reachability walk. The index is a node in that graph + // without being a page, so it is keyed by something no filename can produce. + const indexNode = "\x00index" links := map[string][]string{} - for _, p := range pages { - slug := strings.TrimSuffix(filepath.Base(p), ".md") + collect := func(node, p string) error { doc, err := read(root, p) if err != nil { if doc == nil { - return nil, err + return err } set.Addf(rel(root, p), 1, "wiki.frontmatter-unreadable", "%v", err) - continue + return nil } for _, link := range doc.Wikilinks { target := strings.TrimSuffix(link.Target, ".md") if !slugs[target] { set.Addf(rel(root, p), link.Line, "wiki.broken-link", - "[[%s]] resolves to no page under %s/", link.Target, paths.WikiSeg) + "[[%s]] resolves to no page under %s/%s/", link.Target, paths.WikiSeg, paths.WikiPagesSeg) continue } - links[slug] = append(links[slug], target) + links[node] = append(links[node], target) + } + return nil + } + if isFile(indexPath) { + if err := collect(indexNode, indexPath); err != nil { + return nil, err + } + } + for _, pg := range pages { + if err := collect(pg.slug, pg.path); err != nil { + return nil, err } } // Reachability from the index. The changelog is a log rather than a page, so it is - // not expected to be linked and is not an orphan when it is not. - logSlug := strings.TrimSuffix(paths.WikiLog, ".md") - reachable := map[string]bool{indexSlug: true, logSlug: true} - walk := []string{indexSlug} + // never walked and is never an orphan. + reachable := map[string]bool{} + walk := []string{indexNode} for len(walk) > 0 { cur := walk[len(walk)-1] walk = walk[:len(walk)-1] @@ -89,14 +120,14 @@ func Wiki(root string) (*finding.Set, error) { } } orphans := make([]string, 0) - for slug := range slugs { - if !reachable[slug] { - orphans = append(orphans, slug) + for _, pg := range pages { + if !reachable[pg.slug] { + orphans = append(orphans, pg.path) } } sort.Strings(orphans) - for _, slug := range orphans { - set.Addf(rel(root, filepath.Join(dir, slug+".md")), 0, "wiki.orphan-page", + for _, p := range orphans { + set.Addf(rel(root, p), 0, "wiki.orphan-page", "not reachable from %s; link it from the index or from a page that is", paths.WikiIndex) } @@ -104,6 +135,46 @@ func Wiki(root string) (*finding.Set, error) { return set, nil } +// page is one wiki page: where it sits, what [[slug]] resolves to it, and whether it +// is still in the layout that predates wiki/pages/. +type page struct { + path string + slug string + legacy bool +} + +// wikiPages lists the pages, the canonical ones first. +// +// A .md file directly in wiki/ that is not one of the two fixed documents is still +// read as a page, and only then reported. Refusing to see it would turn every existing +// wiki into a wall of wiki.broken-link the moment scc was upgraded — pages that are +// really there, reported as missing. A finding that names the layout is an answer the +// author can act on; a finding that says a page does not exist when it does is the +// kind that teaches people to stop believing the validator. +func wikiPages(dir string) ([]page, error) { + canonical, err := markdownFiles(filepath.Join(dir, paths.WikiPagesSeg)) + if err != nil { + return nil, err + } + out := make([]page, 0, len(canonical)) + for _, p := range canonical { + out = append(out, page{path: p, slug: pageSlug(p)}) + } + loose, err := markdownFiles(dir) + if err != nil { + return nil, err + } + for _, p := range loose { + if base := filepath.Base(p); base == paths.WikiIndex || base == paths.WikiLog { + continue + } + out = append(out, page{path: p, slug: pageSlug(p), legacy: true}) + } + return out, nil +} + +func pageSlug(p string) string { return strings.TrimSuffix(filepath.Base(p), ".md") } + // checkChangelog is the index/log desync check, in the form that is actually // structural: the log exists, and every page it names still exists. Comparing the log // against what changed would mean reading history, which scc does not do.