Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down
23 changes: 23 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 `<!-- rtk-instructions -->` 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

Expand Down
21 changes: 20 additions & 1 deletion internal/assets/assets.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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"}
Expand Down
31 changes: 23 additions & 8 deletions internal/assets/assets_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
}
Expand Down Expand Up @@ -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 (`<rules dir>/<name>.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)
}
}
}
Expand Down
105 changes: 51 additions & 54 deletions internal/assets/templates/agents/code-review.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,81 +3,78 @@ 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
git diff --stat main...HEAD
```

Then read what the work was supposed to be: `specs/<feature>/requirements.md`,
`design.md` and `tasks.md`, or the checklist in `plans/<name>.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/<name>.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

Expand All @@ -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
...
Expand All @@ -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.
Loading
Loading