diff --git a/CLAUDE.md b/CLAUDE.md index 03d1712..9fdf870 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -18,6 +18,7 @@ Three things landed after phase 10 and all are documented in `design/orchestrati - **Three harnesses, one template set.** `scc init --claude|--codex|--opencode` (Claude Code is the default, and a terminal with no flag gets a picker). One prose source: paths come from a `paths.Harness` profile and the header each loader parses is synthesized at render time — YAML frontmatter for Claude Code and opencode, a TOML agent role file for Codex. - **`scc update` (phase 11), as replace-or-keep rather than the planned three-way merge.** It hashes every managed file against this build and against the manifest, prints the plan grouped by outcome, asks, and then replaces what is safe to replace. An edited file is kept and named; `--force` is the separate decision. `internal/merge` is still unbuilt. +- **`scc rtk`, and `scc init --rtk`.** Wires in [RTK](https://github.com/rtk-ai/rtk), the CLI proxy that filters command output: `cargo install` when the binary is missing, plus a splice of RTK's marker-delimited usage block into the entry file. Insert-only — a block already between the markers is left alone whatever version it claims, because RTK owns that text and `rtk init` refreshes it; `--force` is the separate decision. Opt-in in both places, since the block tells the agent to prefix every command with a binary the machine may not have. `--check` reports without writing and exits 2 when the block is missing. - **The four seeded `docs/` anchors** (`assets.Seeds`). `init` writes `glossary.md`, `stack.md`, `wiki/index.md`, and `wiki/changelog.md` — the knowledge base's only fixed-name documents, each holding the format its validator checks. A seed is written once and tracked nowhere: not in the manifest, not by `scc update`. `scc` is a redesign of `csdd` (`github.com/protonspy/csdd`), narrowed to spec-driven development and deliberately leaner. When reaching for something from there, port the *decision*, not the file. Already decided against: a TUI, an embedded web dashboard, an MCP server, a devcontainer. @@ -79,6 +80,7 @@ cmd/scc/main.go os.Exit(cli.Run(os.Args[1:])) | `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/ears` | EARS requirement parsing, all five patterns plus complex. | | `internal/validate` | The eight validators, one file each, sharing `mdscan` and `finding`. The exception is `stack_manifests.go`: the seven dependency-file readers age on their own schedule, so they sit beside the rule rather than inside it. | +| `internal/rtk` | RTK's marker pair and the idempotent splice of its block into the entry file, plus finding or `cargo install`ing the binary. The only package that shells out to another program — keep that boundary here rather than in a command handler. | | `internal/cli` | The dispatcher and every command handler. | `go.mod` is stdlib-only. Keep it that way unless a dependency earns its place — the binary is distributed to six platforms and every dep is a supply-chain surface. diff --git a/README.md b/README.md index 3c074fc..3dfb418 100644 --- a/README.md +++ b/README.md @@ -33,6 +33,29 @@ Installed globally (`npm i -g @protonspy/scc`) the same commands are just `scc i | `plan new\|list\|delete\|validate` | One file, for everything else: a checklist, a decomposition into specs, or both. | | `skill validate` | Conformance to the published [Agent Skills](https://agentskills.io/specification) spec. | | `validate` | Every applicable validator, one exit code, one JSON document. | +| `rtk` | Wires in [RTK](https://github.com/rtk-ai/rtk): installs it if missing, then splices its usage block into the entry file. | + +### RTK, optionally + +[RTK](https://github.com/rtk-ai/rtk) is a CLI proxy that filters command output down +to what is worth spending context on. `scc rtk` — or `scc init --rtk` in one step — +installs it with cargo when it is not on PATH, and puts its usage block into +`CLAUDE.md`/`AGENTS.md` so the agent knows to prefix commands with it: + +```bash +npx @protonspy/scc init --rtk # scaffold, then wire RTK in +npx @protonspy/scc rtk # wire it into a workspace that already exists +npx @protonspy/scc rtk --check # CI: exit 2 when the block is missing +``` + +The block sits between RTK's own `` markers, and scc inserts +one only where there is none: a block already in the file is left exactly as it is, +whatever version it claims, because RTK writes that block and `rtk init` is what +refreshes it. `--force` replaces it with the copy this scc ships. Everything outside +the markers is untouched either way. + +Opt-in on purpose: it tells the agent to prefix every command with a binary the +machine may not have. `--no-install` writes the block and never touches cargo. ### Three harnesses, one methodology diff --git a/internal/assets/assets.go b/internal/assets/assets.go index 63b8c7f..9d96690 100644 --- a/internal/assets/assets.go +++ b/internal/assets/assets.go @@ -64,7 +64,10 @@ import ( // 6: the knowledge base states the stack check without naming an ecosystem. // 7: the plan-run workflow skill and its command — running a whole plan, group by // group, is a procedure with an entry point rather than a rule read in passing. -const Version = "7" +// 8: the entry file names *when* to read each rule instead of tabulating all nine +// as equals — five read on their own trigger, four looked up by name — and the two +// review agents are tightened in the same pass. +const Version = "8" // The embedded tree. "all:" so nothing is silently dropped for having a name the // default embed pattern skips. @@ -221,6 +224,22 @@ func Seeds() []Seed { } } +// RTKTemplate is the embedded name of the RTK usage block. +const RTKTemplate = "rtk.md" + +// RTKBlock returns the marker-delimited RTK instructions `scc rtk` splices into +// the entry file. +// +// A fragment, which is a fourth category and the only one that is not a file: it +// lands *inside* a document the user owns, so the markers rather than a path are +// what make it replaceable. RTK stamps its own version into the opening marker and +// rewrites between the two, so scc shipping the block verbatim means `rtk init` and +// `scc rtk` converge on one copy instead of racing to append a second. +// +// Data-free like every workspace template, and for a stronger reason than usual: +// the same block has to be recognizable to a tool that is not scc. +func RTKBlock() (string, error) { return Content(RTKTemplate) } + // ReviewAgents names the two subagents scc ships. Both read and neither writes: // review is where a cold context is worth paying for, and authorship is not. var ReviewAgents = []string{"code-review", "security-review"} diff --git a/internal/assets/assets_test.go b/internal/assets/assets_test.go index 67fbf4f..52f1225 100644 --- a/internal/assets/assets_test.go +++ b/internal/assets/assets_test.go @@ -88,9 +88,13 @@ func TestWorkspaceSetAndTreeAgree(t *testing.T) { } referenced[s.Name] = true } + if _, err := RTKBlock(); err != nil { + t.Errorf("RTKBlock(): %v", err) + } + referenced[RTKTemplate] = true for name := range inTree { if !referenced[name] { - t.Errorf("embedded template %q is in no harness's Workspace(), Seeds(), and is not an artifact template", name) + t.Errorf("embedded template %q is in no harness's Workspace(), Seeds(), and is neither an artifact template nor a fragment", name) } } } @@ -254,21 +258,32 @@ func TestScaffoldedEntryFileStaysShort(t *testing.T) { } } -// Every rule the scaffolded entry file points at has to exist under this -// harness's own rules directory, or the entry file sends the agent to a file that -// isn't there. -func TestEntryFileLinksResolve(t *testing.T) { +// Every rule scc scaffolds has to be named in the entry file, and the entry file +// has to say where the rules live. Together those are the whole path from a cold +// session to a rule: a rule nobody is told about is a file the agent never opens, +// and a name with no directory is a lookup that fails. +// +// The check is the rule's own name rather than a Markdown link, because the entry +// file addresses the set by pattern (`/.md`) instead of listing +// nine links. What it still guarantees is the thing that actually breaks: a rule +// added to the template set without being announced fails here. +func TestEntryFileNamesEveryRule(t *testing.T) { for _, h := range paths.Harnesses() { raw, err := Render(h, entryFile(t, h)) if err != nil { t.Fatalf("%s: %v", h.ID, err) } + rules := h.Dir + "/" + h.RulesSeg + if !strings.Contains(raw, rules) { + t.Errorf("%s: %s never says the rules are in %s/", h.ID, h.EntryFile, rules) + } for _, f := range Workspace(h) { - if !strings.HasPrefix(f.Rel, h.Dir+"/"+h.RulesSeg+"/") { + if !strings.HasPrefix(f.Rel, rules+"/") { continue } - if !strings.Contains(raw, "("+f.Rel+")") { - t.Errorf("%s: %s does not link to %s", h.ID, h.EntryFile, f.Rel) + name := strings.TrimSuffix(path.Base(f.Rel), ".md") + if !strings.Contains(raw, name) { + t.Errorf("%s: %s never names the %s rule", h.ID, h.EntryFile, name) } } } diff --git a/internal/assets/templates/agents/code-review.md b/internal/assets/templates/agents/code-review.md index d0906a1..2581898 100644 --- a/internal/assets/templates/agents/code-review.md +++ b/internal/assets/templates/agents/code-review.md @@ -3,17 +3,17 @@ name: code-review description: Reviews a diff for correctness and quality against the task list, re-runs the feature's tests and lint, and reports. Use it after the last task of a spec or plan is verified and before opening the PR — never on your own work as the author. --- -You review a diff. You do not write code, and you do not fix what you find — you +You review a diff. You do not write code and you do not fix what you find — you report it. The orchestrator decides what to do with the report. You exist because **the author of a change is its worst reader**: they see what they -meant. A cold context is the entire value you add, so do not ask the author what they -intended, and do not take "it's done" as evidence. Read the diff, read the task list, -and run the checks yourself. +meant. A cold context is your entire value. Do not ask the author what they intended, +do not take "it's done" as evidence. Read the diff, read the task list, run the checks +yourself. ## Scope -The changed lines, plus enough of the surrounding code to judge them. Start with: +The changed lines, plus enough surrounding code to judge them. ```bash git diff main...HEAD # or the base branch the work targets @@ -21,63 +21,60 @@ git diff --stat main...HEAD ``` Then read what the work was supposed to be: `specs//requirements.md`, -`design.md` and `tasks.md`, or the checklist in `plans/.md`. **The artifact is -the standard the code is held to** — not the implementation's own apparent intent. -Read `{{.Rules}}/project.md` for this project's build, test, and lint commands. +`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 five gates — run every one, in this order -Run all five even after one fails. Bailing at the first red gate gives the -orchestrator one problem when there were four, and it pays for a second review round -to learn the rest. +Run all five even after one fails. Bailing at the first red gate reports one problem +when there were four, and buys a second review round to learn the rest. -**1 · The ticked boxes are true.** For every task marked `[x]`, find the code that -implements it in the diff. A box ticked with nothing behind it is the most expensive -defect you can find here, because everything downstream — the PR body, the spec, the -next session's assumptions — is built on it. Report the reverse too: code in the diff -that no task claims, and unticked boxes that are in fact implemented. +**1 · The ticked boxes are true.** For every `[x]`, find the code in the diff. A box +ticked with nothing behind it is the most expensive defect here — the PR body, the +spec, and the next session's assumptions are all built on it. Report the reverse too: +diff with no task, tasks implemented but unticked. **2 · The code does what the task says.** Not something adjacent, not a superset. -Trace each changed behavior back to a requirement or a task line, and read the -acceptance criteria as written. Scope the author added on their own is a finding even -when it works: nobody reviewed the decision to build it. +Trace each changed behavior to a requirement or task line and read the acceptance +criteria as written. Scope the author added on their own is a finding even when it +works: nobody reviewed the decision to build it. -**3 · The feature's tests run green — because you ran them.** Use this project's test -command, scoped to what the diff touches, and quote the exact command and the tail of -its output. Then judge the tests themselves: +**3 · The feature's tests run green — because you ran them.** Project test command, +scoped to what the diff touches; quote the exact command and the tail of its output. +Then judge the tests: -- **Tests that assert the implementation instead of the requirement.** A test that - would still pass if the bug were intentional is worth less than no test — it locks - the bug in. This is the most common defect in agent-written tests: look for - assertions that read like a transcript of the code. -- **Missing tests.** Unit tasks owe a test per function; TDD tasks owe a test that was - seen to fail first. Untested new functions are a finding. +- **Tests asserting the implementation instead of the requirement.** A test that would + still pass if the bug were intentional is worth less than no test — it locks the bug + in. Most common defect in agent-written tests: assertions that read like a + transcript of the code. +- **Missing tests.** Unit tasks owe a test per function; TDD tasks owe a test seen to + fail first. Untested new functions are a finding. -**4 · The lint runs clean — because you ran it.** The project's linter, quoted the -same way. Lint is the automated half of the best-practices check: unused code, -unchecked errors, shadowed variables, unsafe conversions. Do not re-derive by eye what -the linter already answers. +**4 · The lint runs clean — because you ran it.** Same quoting. Lint is the automated +half of best practices: unused code, unchecked errors, shadowed variables, unsafe +conversions. Do not re-derive by eye what the linter already answers. -**5 · Best practices, read by hand.** The half no linter has an opinion about, in -rough order of what actually bites: +**5 · Best practices, by hand.** The half no linter has an opinion about, in rough +order of what bites: - **Correctness at the edges** — empty, zero, nil, one, many, concurrent, the error - path. Errors dropped, wrapped without context, or returned but not handled. -- **Behavior changed by accident.** A modified function whose existing callers were - not looked at. -- **Consistency with the codebase.** A new way of doing something the project already - does one way is a cost paid by everyone who reads it next. -- **Complexity that is not paying for itself** — an abstraction with one caller, a - layer that only forwards, a config knob nobody asked for. + path. Errors dropped, wrapped without context, or returned but unhandled. +- **Behavior changed by accident** — a modified function whose existing callers were + never looked at. +- **Consistency** — a new way of doing what the project already does one way is a cost + paid by every future reader. +- **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 no comment. + worse than none. -If a gate cannot be run — no test command, a suite that needs a service you do not -have — report it as `not-run` with the reason. **Never report a gate you skipped as -passing**, and never infer green from the author having said so. +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 +infer green from the author saying so. -Security is a different lens and has its own reviewer. Note anything alarming in one -line, but do not try to be that reviewer. +Security has its own reviewer. Note anything alarming in one line; do not try to be +that reviewer. ## The report @@ -98,8 +95,8 @@ End with this, and nothing after it: ## Findings ### 1 · blocker — path/to/file.go:118 -What is wrong, and what specifically makes it wrong: the input, the state, or the -caller that breaks. Which task or requirement it violates. What to do instead. +What is wrong and what makes it wrong: the input, state, or caller that breaks. Which +task or requirement it violates. What to do instead. ### 2 · major — path/to/other.go:40 ... @@ -108,10 +105,10 @@ caller that breaks. Which task or requirement it violates. What to do instead. Anything the author may reasonably ignore, one line each. ``` -Severity is `blocker` (do not open the PR), `major` (fix before merge), or `minor` -(the author's call). A red gate 1, 3, or 4 is always a blocker. +Severity: `blocker` (do not open the PR), `major` (fix before merge), `minor` (author's +call). A red gate 1, 3, or 4 is always a blocker. **A finding the author cannot act on is noise.** If you are not sure something is a -defect, put it under Notes with what would make it one, rather than reporting it as -one. `clean` with an empty Findings section is a legitimate and useful answer; padding -a review with style preferences is how a reviewer stops being read. +defect, put it under Notes with what would make it one. `clean` with an empty Findings +section is a legitimate answer; padding a review with style preferences is how a +reviewer stops being read. diff --git a/internal/assets/templates/agents/security-review.md b/internal/assets/templates/agents/security-review.md index 205c04c..a7849e8 100644 --- a/internal/assets/templates/agents/security-review.md +++ b/internal/assets/templates/agents/security-review.md @@ -3,14 +3,13 @@ name: security-review description: Reviews a diff for exploitable weaknesses, attack-class agnostic — traces attacker-controlled input to effect and reports reachable paths. Use it alongside code-review before opening a PR — the two are deliberately separate lenses. --- -You review a diff for security defects, and for nothing else. You do not write code -and you do not fix what you find — you report it, and the orchestrator decides. +You review a diff for security defects and nothing else. You do not write code and you +do not fix what you find — you report it, and the orchestrator decides. -This is a separate agent from `code-review` on purpose. **A single reviewer asked for -"everything" reliably under-weights security**, because correctness findings are -easier to produce and crowd it out. Your narrow scope is the point: do not report -style, naming, or design taste, and do not re-report what a correctness reviewer would -obviously catch. +Separate from `code-review` on purpose: **one reviewer asked for "everything" +reliably under-weights security**, because correctness findings are easier to produce +and crowd it out. The narrow scope is the point — no style, naming, or design taste, +and do not re-report what a correctness reviewer obviously catches. **You are not a checklist runner.** The question is not "does this diff contain any of the ten bugs I know the names of" — it is **"what can someone make this code do that @@ -25,61 +24,56 @@ git diff main...HEAD # or the base branch the work targets 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. +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. ## The method — four passes, in this order **1 · Map what the change adds to the attack surface.** Before judging anything, list -it: new inputs, new outputs, new files or paths touched, new privileges or -capabilities exercised, new state that persists, new dependencies, new network calls, -new places a secret can flow. This list is what the rest of the review works through — -if it is empty, say so and stop. - -**2 · Find the trust boundaries.** For each input on that list: who controls it, and -what is assumed about it. A boundary is anywhere data crosses from someone else's -control into yours — request bodies, CLI arguments, filenames, environment, file -contents, database rows written by users, anything over the network, and the output of -any other system. **The vulnerability is almost always an assumption that holds on one -side of a boundary and is enforced nowhere.** - -**3 · Trace reachability, source to effect.** For each boundary crossing, follow the -value until it either gets validated or reaches something that acts on it: a shell, a -query, a path, a template, a deserializer, an allocation, a permission check, a -redirect, a model prompt. Write the path down — file to file, call to call. **A -finding without a path from an attacker-controlled source to an effect is a -hypothesis, and you must label it as one.** - -**4 · Attack it deliberately.** Ask what you would try if you wanted this code to -misbehave, and answer concretely: the input, the sequence, the race, the state you -would put it in first. Consider order of operations (check before use, use before -check), what happens on the error path, what happens twice, and what happens when a -value is at its boundary — empty, huge, negative, encoded, or a lookalike. +it: new inputs, outputs, files or paths touched, privileges exercised, persisted state, +dependencies, network calls, places a secret can flow. That list is what the rest of +the review works through — if it is empty, say so and stop. + +**2 · Find the trust boundaries.** For each input: who controls it, and what is assumed +about it. A boundary is anywhere data crosses from someone else's control into yours — +request bodies, CLI arguments, filenames, environment, file contents, user-written +database rows, anything over the network, the output of any other system. **The +vulnerability is almost always an assumption that holds on one side of a boundary and +is enforced nowhere.** + +**3 · Trace reachability, source to effect.** Follow each crossing value until it is +validated or reaches something that acts on it: a shell, query, path, template, +deserializer, allocation, permission check, redirect, or model prompt. Write the path +down, file to file, call to call. **A finding without a path from an +attacker-controlled source to an effect is a hypothesis, and you must label it one.** + +**4 · Attack it deliberately.** Ask what you would try to make this code misbehave, and +answer concretely: the input, the sequence, the race, the state you would set up first. +Consider order of operations (check before use, use before check), the error path, what +happens twice, and values at their boundary — empty, huge, negative, encoded, or a +lookalike. ### Known classes, as prompts and not as a scope -Use these to jog the passes above — never as the definition of "done". Absence of -every class below is not evidence of safety. +Jog the passes above with these — never treat them as the definition of "done". +Absence of every class below is not evidence of safety. - **Injection into any interpreter** — SQL, shell, template, path, URL, regex, serialization format, or a prompt an agent will act on. -- **Path traversal.** A caller-supplied name that becomes a path segment without being - validated first: `..`, absolute paths, separators, symlinks, Windows device names. - This is the one that turns a delete command into deleting the project. -- **Authorization.** A new endpoint, command, or branch that skips the check its +- **Path traversal.** A caller-supplied name becoming a path segment unvalidated: + `..`, absolute paths, separators, symlinks, Windows device names. This is the one + that turns a delete command into deleting the project. +- **Authorization.** A new endpoint, command, or branch skipping the check its neighbors make. Missing authorization is far more common than broken authentication. -- **Secrets.** Credentials, tokens, or keys added to source, config, a test fixture, a - log line, an error message, or a cache. +- **Secrets** in source, config, a test fixture, a log line, an error message, a cache. - **Crypto and randomness.** `math/rand` where unpredictability matters, a hand-rolled - comparison of secrets, a hash chosen for speed where it needed to be slow. + secret comparison, a hash chosen for speed where it needed to be slow. - **Resource exhaustion** reachable from input: unbounded reads or allocations, decompression, quadratic regexes over attacker-controlled strings. -- **Time-of-check to time-of-use**, and anything else where two operations assume the - world did not move between them. -- **Dependencies added in this diff.** New attack surface and a new maintainer to - trust. Say whether it earned that, and run the project's vulnerability scanner if it - has one. +- **Time-of-check to time-of-use**, and anything else assuming the world did not move + between two operations. +- **Dependencies added in this diff.** New surface and a new maintainer to trust. Say + whether it earned that, and run the project's vulnerability scanner if it has one. - **What the change loosens** — a widened permission, a disabled check, a suppression comment, a TLS verification skipped "for now". @@ -105,18 +99,18 @@ line. **Fix:** what to do instead. ## Hypotheses -Suspicions with no reachable path yet, and what would confirm each one. +Suspicions with no reachable path yet, and what would confirm each. ## Pre-existing Anything alarming in untouched code, one line each — not this diff's problem. ``` -Severity is `critical` (reachable now, high impact), `high` (reachable, bounded -impact), `medium` (needs a precondition an attacker may well have), or `low` -(defense-in-depth). A finding whose path you could not complete belongs under -Hypotheses, whatever it would score if it were real. +Severity: `critical` (reachable now, high impact), `high` (reachable, bounded impact), +`medium` (needs a precondition an attacker may well have), `low` (defense-in-depth). A +finding whose path you could not complete belongs under Hypotheses, whatever it would +score if it were real. **Do not inflate severity to be heard.** One wrong high-severity finding costs the -author's trust in every finding after it. "No security findings in this diff" is a -real result; report it plainly, with the surface table showing what you actually -looked at, so the orchestrator can tell a clean review from a shallow one. +author's trust in every finding after it. "No security findings in this diff" is a real +result; report it plainly, with the surface table showing what you actually looked at, +so the orchestrator can tell a clean review from a shallow one. diff --git a/internal/assets/templates/entry.md b/internal/assets/templates/entry.md index d8899fd..6f4f937 100644 --- a/internal/assets/templates/entry.md +++ b/internal/assets/templates/entry.md @@ -1,60 +1,41 @@ # {{.Entry}} -This repo practises spec-driven development, scaffolded and checked by `scc`. - -**This file is deliberately short, and keeping it short is a requirement.** Every -session pays for it in context, and model accuracy degrades as context grows — -non-uniformly, well before any documented limit. The methodology lives in -`{{.Rules}}/`, one file per concern, read when the concern is live. Do not -inline those rules here. - -## The rules - -| Read | When | -|---|---| -| [routing.md]({{.Rules}}/routing.md) | Work arrives and needs a vehicle — a spec, or a plan. | -| [autonomy.md]({{.Rules}}/autonomy.md) | At kickoff, before writing anything. | -| [methodology.md]({{.Rules}}/methodology.md) | Starting any task: which cycle, and what to run first. | -| [tasks.md]({{.Rules}}/tasks.md) | Writing or reading a task list. | -| [verification.md]({{.Rules}}/verification.md) | A task's code is written and you think it is done. | -| [delivery.md]({{.Rules}}/delivery.md) | The last task is done: branch, review, PR. | -| [specs.md]({{.Rules}}/specs.md) | Writing or changing anything under `specs/`. | -| [knowledge-base.md]({{.Rules}}/knowledge-base.md) | Something learned is worth keeping, or a decision was made. | -| [project.md]({{.Rules}}/project.md) | You need this project's build, test, or lint commands. | - -## The layout +Spec-driven development, scaffolded and checked by `scc`. Keep this file short — +the methodology lives in `{{.Rules}}/`, read when the concern is live. Never inline it here. + +## Rules — `{{.Rules}}/.md` + +Read at these moments, without being asked: + +- autonomy — at kickoff, before writing anything +- routing — work arrives and needs a vehicle: a spec, or a plan +- methodology — starting a task: which cycle, what to run first +- verification — code is written and you think it is done +- delivery — last task done: branch, review, PR + +Read by name when you're in that territory: tasks, specs, project (build/test/lint +commands), knowledge-base (something learned, or a decision made). + +## Layout ``` specs// requirements.md · design.md · tasks.md -plans/.md one file: structure, plus a checklist and/or spec references -docs/ the knowledge base — wiki, adr, codewiki, glossary, stack +plans/.md structure, plus a checklist and/or spec references +docs/ knowledge base — wiki, adr, codewiki, glossary, stack {{.Rules}}/ — the methodology above -{{.Skills}}/ — how to author each part of docs/, and how to run a plan to the end -{{.Agents}}/ — code-review and security-review, run before the PR +{{.Skills}}/ — authoring each part of docs/, and running a plan group by group {{- if .HasCommands}} -{{.Commands}}/ — the same skills on demand, as /scc-plan-run, /scc-wiki, /scc-adr, … +{{.Commands}}/ — the same skills on demand: /scc-plan-run, /scc-wiki, /scc-adr, … {{- end}} ``` ## Checking your work -``` -scc validate # every applicable validator; exit 2 means findings -scc update # take a newer scc's rules and agents; shows the plan, then asks -``` - -If `scc` is not installed on this machine, run it with no install — same binary, -same exit codes: - -``` -npx @protonspy/scc validate # always the latest -npx @protonspy/scc@ ... # pin a version (CI) -``` +`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 codes are the contract: `0` ok, `1` the command could not run, `2` it ran and -found something. A finding is an answer, not a crash. +Exit `0` ok · `1` could not run · `2` ran and found something. A finding is an answer, not a crash. -`scc` checks the *shape* of these artifacts — that a decision was made and written -down. It never reads your source, so it cannot tell you the code honors what the -artifact says. That part is on you. +`scc` checks artifact *shape* only; it never reads source, so whether the code honors +the artifact is on you. diff --git a/internal/assets/templates/rtk.md b/internal/assets/templates/rtk.md new file mode 100644 index 0000000..7cd4c5d --- /dev/null +++ b/internal/assets/templates/rtk.md @@ -0,0 +1,18 @@ + +## RTK +Prefix EVERY command with `rtk`, including each link in a `&&` chain (`rtk git add . && rtk git commit -m "x"`). +No dedicated filter means it passes through unchanged — always safe. + +Covered: +- cargo build/check/clippy/test, go test, tsc, lint, prettier, next build +- jest, vitest, playwright, pytest, rspec, rake test, test `` +- git (all subcommands) +- gh pr view/checks, gh run list, gh issue list, gh api +- pnpm, npm run, npx, prisma, uv run +- ls, read, grep, find +- err, log, json, deps, env, summary, diff +- docker, kubectl, curl, wget + +Meta: `rtk gain [--history]`, `rtk discover`, `rtk proxy ` (no filtering), `rtk init [--global]` +Caveat: `rtk grep` with `-c -l -L -o -Z` runs raw. + diff --git a/internal/cli/cli.go b/internal/cli/cli.go index a841a76..5fbaee1 100644 --- a/internal/cli/cli.go +++ b/internal/cli/cli.go @@ -60,6 +60,8 @@ func Run(args []string) int { return runInit(args[1:]) case "update": return runUpdate(args[1:]) + case "rtk": + return runRTK(args[1:]) case "spec": return runSpec(args[1:]) case "plan": @@ -104,6 +106,7 @@ Usage: Commands: init Scaffold a workspace: rules, agents, skills, commands, layout, manifest update Bring the managed files onto this build's templates, after showing the plan + rtk Install RTK if missing and put its usage block in the entry file spec Create and inspect specs — new | list | show | delete | validate plan Create and inspect plans — new | list | delete | validate skill Agent Skills conformance — validate diff --git a/internal/cli/init.go b/internal/cli/init.go index f3c5e67..d7e7e76 100644 --- a/internal/cli/init.go +++ b/internal/cli/init.go @@ -30,6 +30,7 @@ func runInit(args []string) int { fs.SetOutput(os.Stderr) root := addRoot(fs) force := fs.Bool("force", false, "overwrite existing files, naming every edited file it clobbers") + withRTK := fs.Bool("rtk", false, "also wire in RTK: install it if missing, and put its usage block in the entry file") picks := map[string]*bool{} for _, h := range paths.Harnesses() { picks[h.ID] = fs.Bool(h.ID, false, "scaffold for "+h.ID+" ("+h.EntryFile+", "+h.Dir+"/)") @@ -60,8 +61,21 @@ func runInit(args []string) int { return ExitError } + // RTK is opt-in, and stays opt-in: wiring it in can shell out to cargo for + // minutes and it tells the agent to prefix every command with a binary this + // machine may not have. A default that does either would be scc making a + // decision about someone else's toolchain. + rtkCode := ExitOK + var rtkOut *rtkReport + if *withRTK { + rtkOut, rtkCode = applyRTK(target, rtkOptions{quiet: *jsonOut}) + } + if *jsonOut { - return emitJSON(res) + if c := emitJSON(initReport{Result: res, RTK: rtkOut}); c != ExitOK { + return c + } + return rtkCode } // Clobbered files come first: it is the only outcome that destroyed something, @@ -84,7 +98,15 @@ func runInit(args []string) int { render.Info(fmt.Sprintf("read %s, then fill in %s/project.md with this project's commands", harness.EntryFile, path.Join(harness.Dir, harness.RulesSeg))) } - return ExitOK + return rtkCode +} + +// initReport is init's JSON document. The scaffold result is embedded, so its shape +// is unchanged for every caller already parsing it, and "rtk" appears only in the +// runs that asked for RTK. +type initReport struct { + *scaffold.Result + RTK *rtkReport `json:"rtk,omitempty"` } // mustCwd is only ever used to make a path friendlier to print, so a failure diff --git a/internal/cli/rtk.go b/internal/cli/rtk.go new file mode 100644 index 0000000..34c6c7b --- /dev/null +++ b/internal/cli/rtk.go @@ -0,0 +1,260 @@ +package cli + +import ( + "flag" + "fmt" + "os" + "path/filepath" + "strings" + + "github.com/protonspy/spec-claude-code/internal/assets" + "github.com/protonspy/spec-claude-code/internal/render" + "github.com/protonspy/spec-claude-code/internal/rtk" + "github.com/protonspy/spec-claude-code/internal/workspace" +) + +// runRTK wires RTK into this workspace: the binary on PATH, and its usage block in +// the entry file every harness here loads. +// +// Two halves on purpose, because they fail independently. The block is scc's kind +// of work — a marker-delimited splice into a file the user owns, idempotent, safe +// to re-run. The install is not: it shells out to cargo, needs a network and a Rust +// toolchain, and takes minutes. --no-install is the seam for anyone who wants only +// the half scc is actually good at. +func runRTK(args []string) int { + fs := flag.NewFlagSet("rtk", flag.ContinueOnError) + fs.SetOutput(os.Stderr) + root := addRoot(fs) + check := fs.Bool("check", false, "report only, write nothing; exit 2 when the block is missing") + noInstall := fs.Bool("no-install", false, "never run cargo; only write the block") + force := fs.Bool("force", false, "replace an existing block with the one this scc ships (default: leave it alone)") + jsonOut := addJSON(fs) + rest, err := parseFlags(fs, args) + if err != nil { + return ExitError + } + if !noPositionals(rest, "rtk") { + return ExitError + } + + target, ok := resolveRoot(*root) + if !ok { + return ExitError + } + if !requireWorkspace(target) { + return ExitError + } + + report, code := applyRTK(target, rtkOptions{check: *check, noInstall: *noInstall, force: *force, quiet: *jsonOut}) + if *jsonOut { + if c := emitJSON(report); c != ExitOK { + return c + } + return code + } + return code +} + +// rtkOptions are the knobs both entry points share — the `rtk` command and +// `init --rtk`. +type rtkOptions struct { + check bool + noInstall bool + // force replaces a block that is already there. Off by default: the block + // between RTK's markers is RTK's, and `rtk init` is what refreshes it. + force bool + // quiet suppresses the human status lines because the caller is emitting JSON + // on stdout, which nothing else may touch. + quiet bool +} + +// rtkReport is the frozen JSON shape of a run, and the same values the human lines +// are printed from, so the two cannot describe different outcomes. +type rtkReport struct { + Installed bool `json:"installed"` + Path string `json:"path,omitempty"` + Version string `json:"version,omitempty"` + Install string `json:"install"` // present | installed | skipped | failed + Files []rtkFile `json:"files"` + Changed int `json:"changed"` + Error string `json:"error,omitempty"` +} + +type rtkFile struct { + Path string `json:"path"` + Action string `json:"action"` // added | present | replaced | missing + // Block is the version the opening marker claims, for a file that had one. + // Reported, never compared: which version supersedes which is RTK's to say. + Block string `json:"block,omitempty"` +} + +// The values rtkReport.Install takes. Not rtk.Action: these describe the binary, +// which is either already there, newly built, deliberately left alone, or a failed +// build — a different question from what happened to the block. +const ( + installPresent = "present" + installInstalled = "installed" + installSkipped = "skipped" + installFailed = "failed" +) + +// rtkMissing is the action for an entry file that is not on disk. scc writes the +// block into a document it does not own, so it declines to bring that document into +// existence: an entry file holding nothing but RTK's block would be a workspace +// missing its methodology while looking configured. +const rtkMissing = "missing" + +// applyRTK does the work and returns the report plus the exit code, so `init --rtk` +// gets the same behavior without re-deriving any of it. +func applyRTK(root string, opts rtkOptions) (*rtkReport, int) { + report := &rtkReport{Files: []rtkFile{}} + code := ExitOK + + if err := ensureBinary(report, opts); err != nil { + report.Error = err.Error() + render.Err(err.Error()) + code = ExitError + } + + block, err := assets.RTKBlock() + if err != nil { + report.Error = err.Error() + render.Err(err.Error()) + return report, ExitError + } + + findings := false + for _, entry := range entryFiles(root) { + file, err := spliceEntry(root, entry, block, opts) + if err != nil { + report.Error = err.Error() + render.Err(fmt.Sprintf("%s: %v", entry, err)) + code = ExitError + continue + } + report.Files = append(report.Files, file) + switch action := file.Action; action { + case string(rtk.Present): + // Already wired. Whose block it is and which version it claims is RTK's + // business, so this reports and stops there. + if !opts.quiet { + render.Info(strings.TrimSpace(fmt.Sprintf("%s — RTK block already there %s", entry, file.Block))) + } + case rtkMissing: + // An initialized workspace whose entry file is gone: --check calls it a + // finding, and a run that was asked to write the block reports that it + // could not, rather than exiting 0 having done nothing. + findings = true + if !opts.check { + code = ExitError + } + render.Warn(fmt.Sprintf("%s does not exist; run `%s init` first", entry, prog())) + default: + findings = true + if opts.check { + render.Warn(fmt.Sprintf("%s — RTK block would be %s", entry, action)) + continue + } + report.Changed++ + if !opts.quiet { + render.OK(fmt.Sprintf("%s — RTK block %s", entry, action)) + } + } + } + + // --check answers one question — is this workspace's entry file wired for RTK — + // and answers it with the findings code, so CI branches on it the way it branches + // on `scc validate`. An existing block is an answer of yes whatever version it + // claims, so only a missing one is a finding. The binary is reported but + // deliberately not counted either: a CI checkout has no reason to have RTK + // installed, and failing there would train people to ignore the check. + if opts.check && findings && code == ExitOK { + return report, ExitFindings + } + return report, code +} + +// ensureBinary finds RTK, or builds it when it is missing and the user did not opt +// out. Returns an error only when an install was attempted and failed — everything +// else is a state, not a failure. +func ensureBinary(report *rtkReport, opts rtkOptions) error { + if p, ok := rtk.Path(); ok { + report.Installed, report.Path, report.Version, report.Install = true, p, rtk.Version(p), installPresent + if !opts.quiet { + render.Info(strings.TrimSpace("rtk found: " + p + " " + report.Version)) + } + return nil + } + if opts.check || opts.noInstall { + report.Install = installSkipped + render.Warn(fmt.Sprintf("rtk is not on PATH; install it with: %s", rtk.InstallCmd())) + return nil + } + + render.Info(fmt.Sprintf("rtk is not on PATH; running %s — this takes a few minutes", rtk.InstallCmd())) + // cargo's own output goes to stderr in both streams when the caller is emitting + // JSON: stdout carries the document and nothing else. + out := os.Stdout + if opts.quiet { + out = os.Stderr + } + if err := rtk.Install(out, os.Stderr); err != nil { + report.Install = installFailed + return err + } + p, ok := rtk.Path() + if !ok { + report.Install = installFailed + return fmt.Errorf("cargo reported success but %s is still not on PATH; check that cargo's bin directory is in PATH", rtk.Bin) + } + report.Installed, report.Path, report.Version, report.Install = true, p, rtk.Version(p), installInstalled + if !opts.quiet { + render.OK(strings.TrimSpace("rtk installed: " + p + " " + report.Version)) + } + return nil +} + +// entryFiles lists the entry file of every harness this workspace was scaffolded +// for, deduplicated: Codex and opencode both read AGENTS.md, and splicing the same +// block into one file twice would append a second copy on the second pass. +func entryFiles(root string) []string { + var out []string + seen := map[string]bool{} + for _, h := range workspace.Harnesses(root) { + if seen[h.EntryFile] { + continue + } + seen[h.EntryFile] = true + out = append(out, h.EntryFile) + } + return out +} + +func spliceEntry(root, entry, block string, opts rtkOptions) (rtkFile, error) { + path := filepath.Join(root, entry) + raw, err := os.ReadFile(path) + if os.IsNotExist(err) { + return rtkFile{Path: entry, Action: rtkMissing}, nil + } + if err != nil { + return rtkFile{}, err + } + next, action, err := rtk.Splice(string(raw), block, opts.force) + if err != nil { + return rtkFile{}, err + } + // The version reported is the one that ends up in the file: what was already + // there when the block was left alone, what scc wrote when it did the writing. + found := rtk.BlockVersion(string(raw)) + if action != rtk.Present { + found = rtk.BlockVersion(block) + } + file := rtkFile{Path: entry, Action: string(action), Block: found} + if action == rtk.Present || opts.check { + return file, nil + } + if err := workspace.AtomicWrite(path, []byte(next), 0o644); err != nil { + return rtkFile{}, err + } + return file, nil +} diff --git a/internal/cli/rtk_test.go b/internal/cli/rtk_test.go new file mode 100644 index 0000000..3a08025 --- /dev/null +++ b/internal/cli/rtk_test.go @@ -0,0 +1,288 @@ +package cli + +import ( + "encoding/json" + "os" + "path/filepath" + "runtime" + "strings" + "testing" + + "github.com/protonspy/spec-claude-code/internal/paths" +) + +// Every test here passes --no-install or puts a stub on PATH. A test that let the +// real path run would shell out to `cargo install` and spend minutes of CI on a +// network build — and would behave differently depending on whether the machine +// running it already had RTK. +func readEntry(t *testing.T, root, entry string) string { + t.Helper() + b, err := os.ReadFile(filepath.Join(root, entry)) + if err != nil { + t.Fatalf("ReadFile %s: %v", entry, err) + } + return string(b) +} + +// stubRTK puts a do-nothing `rtk` on PATH so the install branch is never taken. +func stubRTK(t *testing.T) { + t.Helper() + dir := t.TempDir() + script, name := "#!/bin/sh\necho 'rtk 0.0.0-stub'\n", "rtk" + if runtime.GOOS == "windows" { + script, name = "@echo rtk 0.0.0-stub\r\n", "rtk.bat" + } + if err := os.WriteFile(filepath.Join(dir, name), []byte(script), 0o755); err != nil { + t.Fatalf("stub rtk: %v", err) + } + t.Setenv("PATH", dir+string(os.PathListSeparator)+os.Getenv("PATH")) +} + +func TestRTKAddsTheBlockToTheEntryFile(t *testing.T) { + root := initWorkspace(t) + before := readEntry(t, root, paths.Claude.EntryFile) + + stdout, stderr, code := run(t, "rtk", "--root", root, "--no-install") + if code != ExitOK { + t.Fatalf("exit = %d, want %d (stderr: %s)", code, ExitOK, stderr) + } + got := readEntry(t, root, paths.Claude.EntryFile) + if !strings.HasPrefix(got, before) { + t.Error("the scaffolded entry file was rewritten rather than appended to") + } + if !strings.Contains(got, "") { + t.Errorf("%s did not get the block:\n%s", paths.Claude.EntryFile, got) + } + if !strings.Contains(stdout, paths.Claude.EntryFile) { + t.Errorf("stdout does not name the file it changed: %q", stdout) + } +} + +// The command an agent runs at the top of a session has to be free to re-run: a +// second pass reports the block as current and leaves the file byte-identical. +func TestRTKIsIdempotent(t *testing.T) { + root := initWorkspace(t) + if _, stderr, code := run(t, "rtk", "--root", root, "--no-install"); code != ExitOK { + t.Fatalf("first run: exit = %d (stderr: %s)", code, stderr) + } + once := readEntry(t, root, paths.Claude.EntryFile) + + stdout, stderr, code := run(t, "rtk", "--root", root, "--no-install", "--json") + if code != ExitOK { + t.Fatalf("second run: exit = %d (stderr: %s)", code, stderr) + } + if got := readEntry(t, root, paths.Claude.EntryFile); got != once { + t.Error("a second run changed the entry file") + } + var report rtkReport + if err := json.Unmarshal([]byte(stdout), &report); err != nil { + t.Fatalf("stdout is not valid JSON (%v): %q", err, stdout) + } + if report.Changed != 0 { + t.Errorf("changed = %d, want 0", report.Changed) + } + if len(report.Files) != 1 || report.Files[0].Action != "present" { + t.Errorf("files = %+v, want one file reported as present", report.Files) + } + if report.Files[0].Block == "" { + t.Error("the report does not say which block version is in the file") + } +} + +// The block between RTK's markers is RTK's. scc inserts one where there is none and +// otherwise keeps its hands off — a newer block, or one the user edited, survives +// every re-run. --force is the separate, explicit decision. +func TestRTKLeavesAnExistingBlockAlone(t *testing.T) { + root := initWorkspace(t) + entry := filepath.Join(root, paths.Claude.EntryFile) + base := readEntry(t, root, paths.Claude.EntryFile) + theirs := base + "\n\n## RTK\nnewer text\n\n" + if err := os.WriteFile(entry, []byte(theirs), 0o644); err != nil { + t.Fatalf("WriteFile: %v", err) + } + + stdout, stderr, code := run(t, "rtk", "--root", root, "--no-install", "--json") + if code != ExitOK { + t.Fatalf("exit = %d (stderr: %s)", code, stderr) + } + if got := readEntry(t, root, paths.Claude.EntryFile); got != theirs { + t.Errorf("a newer block was rewritten:\n%s", got) + } + var report rtkReport + if err := json.Unmarshal([]byte(stdout), &report); err != nil { + t.Fatalf("stdout is not valid JSON (%v): %q", err, stdout) + } + if len(report.Files) != 1 || report.Files[0].Block != "v9" { + t.Errorf("files = %+v, want the v9 block reported as found", report.Files) + } + // And --check calls it wired, because it is: the version is RTK's to judge. + if _, _, code := run(t, "rtk", "--root", root, "--check"); code != ExitOK { + t.Errorf("--check exit = %d, want %d with a block already in place", code, ExitOK) + } + + // --force is the explicit "use the one this scc ships". + if _, stderr, code := run(t, "rtk", "--root", root, "--no-install", "--force"); code != ExitOK { + t.Fatalf("--force exit = %d (stderr: %s)", code, stderr) + } + got := readEntry(t, root, paths.Claude.EntryFile) + if strings.Contains(got, "newer text") || strings.Contains(got, "v9") { + t.Error("--force did not replace the block") + } + if !strings.HasPrefix(got, base) { + t.Error("--force rewrote the document above the block") + } +} + +// --check is the CI question: is this workspace's entry file wired for RTK. It +// answers with the findings code and writes nothing. +func TestRTKCheckReportsFindingsAndWritesNothing(t *testing.T) { + root := initWorkspace(t) + before := readEntry(t, root, paths.Claude.EntryFile) + + if _, _, code := run(t, "rtk", "--root", root, "--check"); code != ExitFindings { + t.Errorf("exit = %d, want %d on a workspace with no block", code, ExitFindings) + } + if got := readEntry(t, root, paths.Claude.EntryFile); got != before { + t.Error("--check wrote to the entry file") + } + + if _, stderr, code := run(t, "rtk", "--root", root, "--no-install"); code != ExitOK { + t.Fatalf("apply: exit = %d (stderr: %s)", code, stderr) + } + if _, stderr, code := run(t, "rtk", "--root", root, "--check"); code != ExitOK { + t.Errorf("exit = %d, want %d once the block is in place (stderr: %s)", code, ExitOK, stderr) + } +} + +// A repo worked on from Codex and opencode has two manifests and one AGENTS.md. +// Splicing per harness rather than per file would append the block twice. +func TestRTKWritesOneBlockForASharedEntryFile(t *testing.T) { + root := t.TempDir() + for _, h := range []string{"--codex", "--opencode"} { + if _, stderr, code := run(t, "init", h, "--root", root); code != ExitOK { + t.Fatalf("init %s: exit = %d (stderr: %s)", h, code, stderr) + } + } + stdout, stderr, code := run(t, "rtk", "--root", root, "--no-install", "--json") + if code != ExitOK { + t.Fatalf("exit = %d (stderr: %s)", code, stderr) + } + var report rtkReport + if err := json.Unmarshal([]byte(stdout), &report); err != nil { + t.Fatalf("stdout is not valid JSON (%v): %q", err, stdout) + } + if len(report.Files) != 1 { + t.Errorf("files = %+v, want the shared %s reported once", report.Files, paths.Codex.EntryFile) + } + got := readEntry(t, root, paths.Codex.EntryFile) + if n := strings.Count(got, "` … ``) and +// rewrites between them, so scc addressing the block by those same markers is what +// keeps `rtk init` and `scc rtk` converging on one copy instead of each appending +// its own. Everything outside the markers is untouched, always. +package rtk + +import ( + "fmt" + "io" + "os/exec" + "strings" + + "github.com/protonspy/spec-claude-code/internal/textutil" +) + +// Repo is where the binary is built from. RTK is a Rust program distributed as +// source, so cargo is the install path rather than a release download. +const Repo = "https://github.com/rtk-ai/rtk" + +// Bin is the executable's name, as it appears on PATH and as it prefixes every +// command in the block. +const Bin = "rtk" + +// The markers RTK itself writes. The opening one carries a version, so it is +// matched by prefix: a block stamped v1 or v9 is still the block, and replacing it +// with the one this build ships is exactly what an update means. +const ( + openPrefix = "" +) + +// Action is what splicing the block did to a document. +type Action string + +const ( + // Added: the document carried no block, so one was appended. + Added Action = "added" + // Present: it already carried a block, which was left exactly as it was. + // + // This is the default outcome for an existing block whatever its version says, + // and the reason is ownership: RTK writes that block, stamps its own version + // into the opening marker, and `rtk init` is what refreshes it. An scc that + // rewrote it on every run would silently downgrade a v3 block to whatever this + // build happens to ship, and it would do it to a file the user owns. + Present Action = "present" + // Replaced: the caller passed force, so an existing block was overwritten with + // the one this build ships. + Replaced Action = "replaced" +) + +// InstallCmd is the command Install runs, as a string, so the CLI can name it +// before running it and can print it verbatim when cargo is missing and the user +// has to run it themselves. +func InstallCmd() string { return "cargo install --git " + Repo } + +// Splice returns doc with the block present exactly once, and what it had to do to +// get there. +// +// Insert only when the marker is absent. A document that already carries a block is +// returned untouched and reported Present, because that block is RTK's — see the +// Action constants. force is the explicit "replace it with what this scc ships", +// and it is the only path that ever overwrites one. +// +// It preserves the document's own line endings: an entry file checked out CRLF +// stays CRLF, because scc is a guest in this file and rewriting every line of +// someone else's document is not a change they asked for. +// +// A document carrying an opening marker with no closing one is malformed rather +// than blockless, and it is an error: appending a second block there would leave +// the file with two openings and one close, which no tool could then update. +func Splice(doc, block string, force bool) (string, Action, error) { + block = strings.TrimRight(textutil.NormalizeNewlines(block), "\n") + eol := "\n" + if strings.Contains(doc, "\r\n") { + eol = "\r\n" + block = strings.ReplaceAll(block, "\n", "\r\n") + } + + start := strings.Index(doc, openPrefix) + if start < 0 { + if strings.Contains(doc, closeTag) { + return "", "", fmt.Errorf("found %s with no opening marker", closeTag) + } + trimmed := strings.TrimRight(doc, " \t\r\n") + if trimmed == "" { + return block + eol, Added, nil + } + return trimmed + eol + eol + block + eol, Added, nil + } + + rest := doc[start:] + end := strings.Index(rest, closeTag) + if end < 0 { + return "", "", fmt.Errorf("found %s with no closing %s", openPrefix+" …", closeTag) + } + end += len(closeTag) + if !force || rest[:end] == block { + return doc, Present, nil + } + return doc[:start] + block + doc[start+end:], Replaced, nil +} + +// BlockVersion reports what the opening marker in doc claims — "v2" for +// `` — or "" when doc carries no block. +// +// Advisory: it is printed so a run that left a block alone says which one it left, +// and never compared. Version ordering is RTK's to define, not scc's to guess. +func BlockVersion(doc string) string { + start := strings.Index(doc, openPrefix) + if start < 0 { + return "" + } + rest := doc[start+len(openPrefix):] + end := strings.Index(rest, "-->") + if end < 0 { + return "" + } + return strings.TrimSpace(rest[:end]) +} + +// Path reports where the rtk binary is, and whether it is on PATH at all. +func Path() (string, bool) { + p, err := exec.LookPath(Bin) + if err != nil { + return "", false + } + return p, true +} + +// Version reports what `rtk --version` says, or "" when the binary cannot answer. +// Advisory only: it is printed, never branched on, so a build of RTK that words +// its version differently costs nothing. +func Version(bin string) string { + out, err := exec.Command(bin, "--version").Output() + if err != nil { + return "" + } + return strings.TrimSpace(string(out)) +} + +// Install builds and installs RTK with cargo, streaming the build's own output to +// stdout and stderr — it takes minutes, and a silent command that long reads as a +// hang. +// +// Missing cargo is reported as itself rather than as a failed build: the user has +// to install a Rust toolchain, which is a different problem from a build that +// broke, and telling them "cargo install failed" would send them looking in the +// wrong place. +func Install(stdout, stderr io.Writer) error { + cargo, err := exec.LookPath("cargo") + if err != nil { + return fmt.Errorf("cargo is not on PATH; install a Rust toolchain (https://rustup.rs), then run: %s", InstallCmd()) + } + cmd := exec.Command(cargo, "install", "--git", Repo) + cmd.Stdout = stdout + cmd.Stderr = stderr + if err := cmd.Run(); err != nil { + return fmt.Errorf("%s: %w", InstallCmd(), err) + } + return nil +} diff --git a/internal/rtk/rtk_test.go b/internal/rtk/rtk_test.go new file mode 100644 index 0000000..8aeef35 --- /dev/null +++ b/internal/rtk/rtk_test.go @@ -0,0 +1,186 @@ +package rtk + +import ( + "strings" + "testing" + + "github.com/protonspy/spec-claude-code/internal/assets" +) + +func block(t *testing.T) string { + t.Helper() + b, err := assets.RTKBlock() + if err != nil { + t.Fatalf("RTKBlock: %v", err) + } + return b +} + +// The block scc ships has to carry both markers, or nothing — not scc and not RTK +// itself — can ever replace it in place. +func TestShippedBlockIsMarkerDelimited(t *testing.T) { + b := block(t) + if !strings.HasPrefix(b, openPrefix) { + t.Errorf("the block does not open with %q: %q", openPrefix, firstLine(b)) + } + if !strings.Contains(b, closeTag) { + t.Errorf("the block does not carry %q", closeTag) + } + if !strings.Contains(b, "Prefix EVERY command with `rtk`") { + t.Error("the block lost the instruction it exists for") + } +} + +func TestSpliceAppendsToADocumentWithoutABlock(t *testing.T) { + doc := "# CLAUDE.md\n\nSome rules.\n" + got, action, err := Splice(doc, block(t), false) + if err != nil { + t.Fatalf("Splice: %v", err) + } + if action != Added { + t.Errorf("action = %q, want %q", action, Added) + } + if !strings.HasPrefix(got, doc) { + t.Error("splicing rewrote the document above the block") + } + if !strings.Contains(got, "## RTK") { + t.Error("the block was not appended") + } + if !strings.HasSuffix(got, closeTag+"\n") { + t.Errorf("the result does not end with the closing marker and one newline: %q", tail(got)) + } +} + +// Re-running has to be free. `scc rtk` is the kind of command an agent runs at the +// top of a session, and a splice that appended every time would grow the entry file +// without bound. +func TestSpliceIsIdempotent(t *testing.T) { + b := block(t) + once, _, err := Splice("# CLAUDE.md\n\nSome rules.\n", b, false) + if err != nil { + t.Fatalf("Splice: %v", err) + } + twice, action, err := Splice(once, b, false) + if err != nil { + t.Fatalf("Splice: %v", err) + } + if action != Present { + t.Errorf("action = %q, want %q", action, Present) + } + if twice != once { + t.Error("a second splice changed the document") + } +} + +// The block between the markers is RTK's: `rtk init` writes it and stamps its own +// version into the opening marker. scc inserts one only where there is none, so a +// block this build does not recognize — a newer one, or one the user edited — +// survives contact. +func TestSpliceLeavesAnExistingBlockAlone(t *testing.T) { + doc := "# CLAUDE.md\n\n\n## RTK\nnewer text\n\n" + got, action, err := Splice(doc, block(t), false) + if err != nil { + t.Fatalf("Splice: %v", err) + } + if action != Present { + t.Errorf("action = %q, want %q", action, Present) + } + if got != doc { + t.Errorf("an existing block was rewritten without force:\n%s", got) + } + if v := BlockVersion(doc); v != "v9" { + t.Errorf("BlockVersion = %q, want %q", v, "v9") + } + if v := BlockVersion("# CLAUDE.md\n\nno block here.\n"); v != "" { + t.Errorf("BlockVersion of a blockless document = %q, want empty", v) + } +} + +// force is the explicit "use the one this scc ships". It replaces the block where +// it stands, between the markers and nowhere else. +func TestSpliceReplacesAnOlderBlockInPlaceWithForce(t *testing.T) { + doc := "# CLAUDE.md\n\nAbove.\n\n\n## RTK\nold text\n\n\nBelow.\n" + got, action, err := Splice(doc, block(t), true) + if err != nil { + t.Fatalf("Splice: %v", err) + } + if action != Replaced { + t.Errorf("action = %q, want %q", action, Replaced) + } + if strings.Contains(got, "old text") || strings.Contains(got, "v1") { + t.Error("the old block survived") + } + if !strings.Contains(got, "# CLAUDE.md\n\nAbove.\n") || !strings.HasSuffix(got, "\nBelow.\n") { + t.Errorf("the user's own prose did not survive: %q", got) + } + if strings.Count(got, openPrefix) != 1 { + t.Errorf("the document carries %d opening markers, want 1", strings.Count(got, openPrefix)) + } +} + +// scc is a guest in this file. A CRLF entry file stays CRLF, so `scc rtk` shows up +// in the diff as the block it added and not as every line of the document. +func TestSplicePreservesCRLF(t *testing.T) { + doc := "# CLAUDE.md\r\n\r\nSome rules.\r\n" + got, _, err := Splice(doc, block(t), false) + if err != nil { + t.Fatalf("Splice: %v", err) + } + if strings.Contains(strings.ReplaceAll(got, "\r\n", ""), "\n") { + t.Error("the spliced block carries bare LF inside a CRLF document") + } + // And it is recognized on the way back in, rather than appended a second time. + again, action, err := Splice(got, block(t), false) + if err != nil { + t.Fatalf("Splice: %v", err) + } + if action != Present || again != got { + t.Errorf("a CRLF document was not recognized as current: action = %q", action) + } +} + +func TestSpliceIntoAnEmptyDocument(t *testing.T) { + got, action, err := Splice("", block(t), false) + if err != nil { + t.Fatalf("Splice: %v", err) + } + if action != Added { + t.Errorf("action = %q, want %q", action, Added) + } + if !strings.HasPrefix(got, openPrefix) { + t.Errorf("an empty document got leading blank lines: %q", firstLine(got)) + } +} + +// A half-block is malformed, not blockless. Appending a second one there would +// leave two openings and one close, which nothing could update afterwards. +func TestSpliceRefusesAHalfBlock(t *testing.T) { + for name, doc := range map[string]string{ + "no close": "# CLAUDE.md\n\n\n## RTK\nsomething\n", + "no open": "# CLAUDE.md\n\n## RTK\nsomething\n\n", + } { + if _, _, err := Splice(doc, block(t), false); err == nil { + t.Errorf("%s: Splice returned no error", name) + } + } +} + +func TestInstallCmdNamesTheRepo(t *testing.T) { + if !strings.Contains(InstallCmd(), Repo) { + t.Errorf("InstallCmd() = %q, want it to name %s", InstallCmd(), Repo) + } +} + +func firstLine(s string) string { + if i := strings.Index(s, "\n"); i >= 0 { + return s[:i] + } + return s +} + +func tail(s string) string { + if len(s) > 60 { + return s[len(s)-60:] + } + return s +}