From 4c47b7ad0c70e16c979c5ea5e12f97500ba91872 Mon Sep 17 00:00:00 2001 From: prode Date: Thu, 8 Oct 2026 14:31:09 -0300 Subject: [PATCH] feat(cli): always write RTK's usage block on init and update `scc init` now writes RTK's usage block into CLAUDE.md/AGENTS.md whether or not the rtk binary is on PATH; the cargo build stays a question, and `--rtk` still exits 1 when the build fails (the block is written anyway). `scc update` plans and adds the block to an entry file that lacks one, keeps one already there, and takes `--no-rtk` to skip it; its JSON gains an omitempty "rtk" key in both the dry-run plan and the applied result. The update planner now treats an owned entry file that differs from its template only by the RTK and CodeGraph blocks as current, so a wired workspace reports "nothing to do" instead of listing CLAUDE.md as yours. Plan: plans/rtk-block-always.md --- README.md | 12 ++-- internal/cli/init.go | 48 +++++++------- internal/cli/init_test.go | 2 +- internal/cli/rtk_test.go | 33 ++++++---- internal/cli/update.go | 110 ++++++++++++++++++++++++++++--- internal/cli/update_test.go | 102 +++++++++++++++++++++++++++- internal/scaffold/update.go | 23 +++++++ internal/scaffold/update_test.go | 31 +++++++++ plans/rtk-block-always.md | 45 +++++++++++++ 9 files changed, 350 insertions(+), 56 deletions(-) create mode 100644 plans/rtk-block-always.md diff --git a/README.md b/README.md index fd7c1b4..548bbdf 100644 --- a/README.md +++ b/README.md @@ -134,16 +134,18 @@ npx @protonspy/scc rtk --check # CI: exit 2 when the block is missing The install is the part that is still a decision — cargo, a Rust toolchain, minutes of build — so a bare `init` asks before running it and takes silence for no. -Nothing is written when the binary is absent: guidance naming a command the machine -cannot run is worse than no guidance. `scc launch` does the same thing at the top of -a session, for the workspaces wired before this or scaffolded with `--no-rtk`. +The block is written either way: it travels with the repository, not with this +machine, and RTK passes a command it has no filter for through unchanged. `scc +update` adds it to an entry file that lacks one, and keeps one already there; +`--no-rtk` skips it on both. `scc launch` does the same thing at the top of a +session. The block sits between RTK's own `` markers, which is what makes `rtk init` and `scc rtk` converge on one copy instead of two. `scc rtk` replaces what is there with the block this scc ships — same guidance, roughly a fifth of the bytes, in a file preloaded into every request — and says so when the -one it replaced claimed a newer version; `--keep` leaves it alone. `init` and -`launch` always keep, because replacing somebody's block is a trade-off to make +one it replaced claimed a newer version; `--keep` leaves it alone. `init`, +`update` and `launch` always keep, because replacing somebody's block is a trade-off to make deliberately rather than as a side effect. Everything outside the markers is untouched either way, and `--no-install` writes the block without touching cargo. diff --git a/internal/cli/init.go b/internal/cli/init.go index 448a6ef..5fd27f1 100644 --- a/internal/cli/init.go +++ b/internal/cli/init.go @@ -203,7 +203,7 @@ func mustCwd() string { } // initRTK puts RTK's usage block in the entry file the agent is about to load, -// and makes sure the binary that block names is actually there. +// and offers to build the binary that block names when it is missing. // // Setup is where this belongs. The block is what makes RTK work at all — an agent // that never read it never types the prefix — so a workspace scaffolded without it @@ -214,46 +214,46 @@ func mustCwd() string { // and it means a workspace is wired whether the first session starts through // `scc launch` or by typing `claude`. // -// What stays a decision is the install, and the two halves are settled in that -// order. cargo is a Rust toolchain and minutes of build, so a bare `init` asks -// before running it and takes silence for no; `--rtk` is that consent given in -// advance, and `--no-rtk` declines the whole step. Nothing is written when the -// binary is absent, because guidance naming a command the machine cannot run is -// worse than no guidance: an agent that tries the prefix, watches it fail, and -// then reads the same file's rules learns to discount all of them. +// The block is written whether or not the binary is there. It travels with the +// repository rather than with this machine, so the next clone, the CI image, or +// this machine after a `cargo install` reads a file already wired — and RTK passes +// a command through unchanged when it has no filter for it. `--no-rtk` is the one +// way to scaffold without it. // -// It degrades rather than fails. A missing cargo, a declined prompt, or a failed -// build all end in a scaffolded workspace and one line saying what is not wired — -// except under --rtk, where the user asked for this by name and a failure is an -// answer they need in the exit code. +// What stays a decision is the install. cargo is a Rust toolchain and minutes of +// build, so a bare `init` asks before running it and takes silence for no; `--rtk` +// is that consent given in advance. A missing cargo, a declined prompt, or a failed +// build all end in a scaffolded workspace carrying the block and one line saying +// the binary is not there — except under --rtk, where the user asked for the build +// by name and a failure is an answer they need in the exit code. func initRTK(root string, yes, disabled, quiet bool) (*rtkReport, int) { if disabled { return nil, ExitOK } + failed := false + noInstall := false if _, ok := rtk.Path(); !ok { if reason := rtkInstallOK(rtkAsk{yes: yes, quiet: quiet}); reason != "" { - // --rtk named this step, so not getting it is an error and is said as + // --rtk named the build, so not getting it is an error and is said as // one — on stderr, which is also the only stream left when the caller is - // emitting JSON on stdout. Without the flag it is a status line: the - // workspace is scaffolded and complete, and RTK is the part that is not - // wired yet. + // emitting JSON on stdout. The block is still written below: the build + // is what failed, not the workspace. if yes { render.Err("--rtk: " + reason) render.Detail(" " + rtk.InstallCmd()) - return nil, ExitError + failed = true } - if !quiet { - render.Info("no RTK block: " + reason) - render.Detail(fmt.Sprintf(" wire it in later with: %s rtk", prog())) - } - return nil, ExitOK + noInstall = true } } // keep, because a re-run of `init` is not the moment to replace a block // somebody put there themselves — `scc rtk` is where that trade-off is made // deliberately, and it is the same call `scc launch` makes for the same reason. - report, code := applyRTK(root, rtkOptions{keep: true, quiet: quiet}) - if !yes { + report, code := applyRTK(root, rtkOptions{keep: true, noInstall: noInstall, quiet: quiet}) + switch { + case failed: + return report, ExitError + case !yes: return report, ExitOK } return report, code diff --git a/internal/cli/init_test.go b/internal/cli/init_test.go index a45fd4b..f967cb9 100644 --- a/internal/cli/init_test.go +++ b/internal/cli/init_test.go @@ -96,7 +96,7 @@ func TestInitSeedsTheKnowledgeBaseWithoutManagingIt(t *testing.T) { // rewrite documents the user authored. func TestUpdateIgnoresTheSeededKnowledgeBase(t *testing.T) { root := initWorkspace(t) - stdout, stderr, code := run(t, "update", "--root", root, "--json") + stdout, stderr, code := run(t, "update", "--root", root, "--json", "--no-rtk") if code != ExitOK { t.Fatalf("update: %d (%s)", code, stderr) } diff --git a/internal/cli/rtk_test.go b/internal/cli/rtk_test.go index 42587d8..ce9e999 100644 --- a/internal/cli/rtk_test.go +++ b/internal/cli/rtk_test.go @@ -348,24 +348,24 @@ func TestInitSplicesTheBlockWhenRTKIsAlreadyThere(t *testing.T) { } } -// Nothing is written when the binary is absent and nobody can be asked about -// building it. Guidance naming a command the machine cannot run is worse than no -// guidance: the agent tries the prefix, watches it fail, and discounts the rest of -// the file with it. -func TestInitWritesNoBlockWithoutTheBinary(t *testing.T) { +// The block is written when the binary is absent and nobody can be asked about +// building it. It travels with the repository, not with this machine: the next +// clone, the CI image, or this machine after a `cargo install` reads an entry file +// already wired. +func TestInitWritesTheBlockWithoutTheBinary(t *testing.T) { isolatedPath(t) root := t.TempDir() stdout, stderr, code := run(t, "init", "--claude", "--root", root) if code != ExitOK { t.Fatalf("exit = %d, want a scaffolded workspace anyway (stderr: %s)", code, stderr) } - if got := readEntry(t, root, paths.Claude.EntryFile); strings.Contains(got, "rtk-instructions") { - t.Error("init wrote a block naming a binary that is not there") + if got := readEntry(t, root, paths.Claude.EntryFile); !strings.Contains(got, "\nscc graph explore\n" + codegraph.Markers.Close + "\n" + if err := os.WriteFile(entry, []byte(readEntry(t, root, paths.Claude.EntryFile)+"\n"+graph), 0o644); err != nil { + t.Fatalf("WriteFile: %v", err) + } + stdout, stderr, code = run(t, "update", "--root", root) + if code != ExitOK || !strings.Contains(stdout, "nothing to do") { + t.Errorf("second update: exit = %d, stdout = %q (%s), want nothing to do", code, stdout, stderr) + } +} + +// A block already there is kept, whatever version it claims: it may be `rtk +// init`'s own, and replacing it is `scc rtk`'s call, not an update's side effect. +func TestUpdateKeepsAnExistingRTKBlock(t *testing.T) { + root := initWorkspace(t) + mine := rtk.Markers.Open + " v9 -->\nmine, not scc's\n" + rtk.Markers.Close + "\n" + entry := filepath.Join(root, paths.Claude.EntryFile) + if err := os.WriteFile(entry, []byte(readEntry(t, root, paths.Claude.EntryFile)+"\n"+mine), 0o644); err != nil { + t.Fatalf("WriteFile: %v", err) + } + stdout, stderr, code := run(t, "update", "--root", root, "--yes") + if code != ExitOK { + t.Fatalf("exit = %d (%s)", code, stderr) + } + if strings.Contains(stdout, "RTK") { + t.Errorf("update planned a block that is already there:\n%s", stdout) + } + if got := readEntry(t, root, paths.Claude.EntryFile); !strings.Contains(got, "mine, not scc's") { + t.Errorf("update replaced a block it did not write:\n%s", got) + } +} + +// --no-rtk leaves the entry file exactly as it is. +func TestUpdateNoRTKLeavesTheEntryFileAlone(t *testing.T) { + root := initWorkspace(t) + before := readEntry(t, root, paths.Claude.EntryFile) + stdout, stderr, code := run(t, "update", "--root", root, "--yes", "--no-rtk") + if code != ExitOK { + t.Fatalf("exit = %d (%s)", code, stderr) + } + if !strings.Contains(stdout, "nothing to do") { + t.Errorf("stdout = %q, want nothing to do under --no-rtk", stdout) + } + if readEntry(t, root, paths.Claude.EntryFile) != before { + t.Error("--no-rtk still edited the entry file") + } +} diff --git a/internal/scaffold/update.go b/internal/scaffold/update.go index 8658ccd..3261db6 100644 --- a/internal/scaffold/update.go +++ b/internal/scaffold/update.go @@ -8,8 +8,10 @@ import ( "path/filepath" "github.com/protonspy/spec-claude-code/internal/assets" + "github.com/protonspy/spec-claude-code/internal/codegraph" "github.com/protonspy/spec-claude-code/internal/manifest" "github.com/protonspy/spec-claude-code/internal/paths" + "github.com/protonspy/spec-claude-code/internal/rtk" "github.com/protonspy/spec-claude-code/internal/workspace" ) @@ -138,6 +140,9 @@ func planOne(root string, f assets.File, want string, prior *manifest.Manifest) // decides what may be done about it. if f.Owned { item.Action = UpOwned + if bare, err := withoutEntryBlocks(filepath.Join(root, filepath.FromSlash(f.Rel))); err == nil && manifest.Hash(bare) == manifest.Hash(want) { + item.Action = UpCurrent + } return item, nil } if e, ok := prior.Get(f.Rel); ok && got == e.Hash { @@ -306,3 +311,21 @@ func carry(next, prior *manifest.Manifest, rel string) { next.Set(rel, e.Hash, e.Version) } } + +// withoutEntryBlocks reads an owned file with the usage blocks scc splices into an +// entry file taken out. +// +// Those blocks are scc's own writes — `init` and `update` add RTK's, `launch` adds +// CodeGraph's — so a file that differs from its template only by them was never +// authored into. Counting them as an edit would report every wired workspace as +// pending forever, and "nothing to do" would be a message nobody ever sees. +func withoutEntryBlocks(path string) (string, error) { + raw, err := os.ReadFile(path) + if err != nil { + return "", err + } + doc := string(raw) + doc, _ = rtk.Markers.Remove(doc) + doc, _ = codegraph.Markers.Remove(doc) + return doc, nil +} diff --git a/internal/scaffold/update_test.go b/internal/scaffold/update_test.go index d996b3f..9bd569e 100644 --- a/internal/scaffold/update_test.go +++ b/internal/scaffold/update_test.go @@ -4,11 +4,14 @@ import ( "os" "path" "path/filepath" + "strings" "testing" "github.com/protonspy/spec-claude-code/internal/assets" + "github.com/protonspy/spec-claude-code/internal/codegraph" "github.com/protonspy/spec-claude-code/internal/manifest" "github.com/protonspy/spec-claude-code/internal/paths" + "github.com/protonspy/spec-claude-code/internal/rtk" "github.com/protonspy/spec-claude-code/internal/workspace" ) @@ -402,3 +405,31 @@ func TestCountAndWritesDescribeThePlanTheUserIsShown(t *testing.T) { t.Errorf("Count on an unknown action = %d", n) } } + +// The usage blocks scc splices into an entry file are scc's own writes, so an entry +// file that differs from its template only by them is current — CRLF included. One +// real edit beside them is still the user's file. +func TestPlanTreatsAnEntryFileWithOnlyUsageBlocksAsCurrent(t *testing.T) { + blocks := rtk.Markers.Open + " v2 -->\n## RTK\n" + rtk.Markers.Close + "\n\n" + + codegraph.Markers.Open + " v1 -->\n## CodeGraph\n" + codegraph.Markers.Close + "\n" + for _, h := range paths.Harnesses() { + root := t.TempDir() + applyTo(t, root, h, false) + entry := read(t, root, h.EntryFile) + + writeManaged(t, root, h, h.EntryFile, entry+"\n"+blocks, false) + if got := action(t, planFor(t, root, h), h.EntryFile); got != UpCurrent { + t.Errorf("%s: entry file with both blocks = %s, want %s", h.ID, got, UpCurrent) + } + + writeManaged(t, root, h, h.EntryFile, strings.ReplaceAll(entry+"\n"+blocks, "\n", "\r\n"), false) + if got := action(t, planFor(t, root, h), h.EntryFile); got != UpCurrent { + t.Errorf("%s: CRLF entry file with both blocks = %s, want %s", h.ID, got, UpCurrent) + } + + writeManaged(t, root, h, h.EntryFile, entry+"\nmine\n\n"+blocks, false) + if got := action(t, planFor(t, root, h), h.EntryFile); got != UpOwned { + t.Errorf("%s: edited entry file with blocks = %s, want %s", h.ID, got, UpOwned) + } + } +} diff --git a/plans/rtk-block-always.md b/plans/rtk-block-always.md new file mode 100644 index 0000000..22302ba --- /dev/null +++ b/plans/rtk-block-always.md @@ -0,0 +1,45 @@ +--- +autonomy: auto +ci: no-wait +--- + +# Rtk block always + +Make `scc init` and `scc update` always leave RTK's usage block in every entry file, +whether or not the `rtk` binary is on PATH; only `--no-rtk` opts out. + +## Why + +Today `init` writes the block only when the binary is present or gets built, and +`update` never touches it, so a workspace scaffolded on a machine without RTK — or +created before the block existed — has an entry file that never mentions the prefix. +The block passes through unchanged when no filter matches, and the binary stays a +separate question (`init` still asks before running cargo). Done when a bare `init` +and an `update` both leave the block in place without a binary, an existing block is +kept as it is, `--no-rtk` skips it on both, and the suite and lint pass. + +## Paths + +- `internal/cli/init.go` +- `internal/cli/update.go` +- `internal/cli/rtk.go` +- `internal/cli/rtk_test.go` +- `internal/cli/update_test.go` + +## Out of scope + +- Replacing an existing block on `update`: an existing one is kept; `scc rtk` replaces on demand. +- Installing the binary from `update`: it splices the block and builds nothing. + +## Tasks + +- [x] 1.1 (Unit) Write the block on `init` even when the binary is absent or its install is declined +- [x] 1.2 (Unit) Plan and apply a missing block on `update`, with `--no-rtk` to skip it + _Depends 1.1_ +- [x] 1.3 (Unit) Compare an owned entry file to its template without the RTK and + CodeGraph blocks, so `update` reports a current workspace as current + +## Done when + +- `go test ./...` and `golangci-lint run` exit 0. +- `scc validate --checks --pr` exits 0.