diff --git a/CLAUDE.md b/CLAUDE.md index 15fd27b..749a620 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -35,7 +35,25 @@ These landed after phase 10, and all are documented in `design/orchestration.md` **`--` reaches `wrap`, not only the agent.** `headroom wrap` parses every flag it recognizes out of the tail and forwards only the rest, so a pass-through argument that collides with one of Headroom's — `--verbose`, which both Claude Code and `wrap` define — is silently eaten. `WrapArgs` therefore takes scc's options and the pass-through as separate parameters and puts scc's first, so a colliding argument the user typed lands last and wins. To force something past `wrap` to the agent, use a second terminator: `scc launch claude -- -- -p`. -- **`scc graph`, and the launch-time index.** A wrap over [CodeGraph](https://github.com/colbymchenry/codegraph) — `build | sync | status | query | explore` — plus the same index run automatically by `scc launch`: `codegraph init` when `.codegraph/` is absent, `sync` when it is there. Launch is the one moment where indexing is free, and it degrades exactly the way Headroom does, for the same reason: a graph is an enhancement, so a missing binary or a failed index still starts the agent. `--no-graph` opts out and a plan-only run (`--json`/`--dry-run`) reports without indexing. +- **`scc graph`, the launch-time index, and its usage block.** A wrap over [CodeGraph](https://github.com/colbymchenry/codegraph) — `build | sync | status | query | explore` — plus the same index run automatically by `scc launch`: `codegraph init` when `.codegraph/` is absent, `sync` when it is there. **`scc launch` also splices a CodeGraph usage block into the entry file**, the way it does RTK's, because an index nobody knows how to query is an index nobody queries. + + Three things about that block, and each is the reason it is not written at `init` time instead. It goes in **only once the binary is present** — guidance naming a command the machine cannot run is worse than none, since an agent that tries `scc graph explore` and watches it fail discounts the whole file — and CodeGraph is usually not installed when `init` runs. It is **written before the index**, so a failed index still leaves the agent knowing the command that rebuilds one. And a **plan-only run writes nothing**: `--dry-run` that edited a file the user owns would be the one flag nobody expects to change anything doing exactly that. + + **Its markers are scc's own** — `` — which is deliberately the opposite of the RTK decision above and for the opposite reason. Sharing RTK's markers is what makes `rtk init` and `scc rtk` converge on one copy of a block they *both* write. CodeGraph writes nothing into the entry file, and this block is scc's account of `scc graph` rather than CodeGraph's account of itself, so namespacing it leaves a future CodeGraph release free to add its own without either tool clobbering the other. The splice itself is `internal/mdblock`, shared by both: choosing the markers is the integration's decision, and everything after that choice — find, compare, replace, append, preserve the file's line endings — is one implementation rather than two sets of edge cases. Launch is the one moment where indexing is free, and it degrades exactly the way Headroom does, for the same reason: a graph is an enhancement, so a missing binary or a failed index still starts the agent. `--no-graph` opts out and a plan-only run (`--json`/`--dry-run`) reports without indexing. + + **How to actually use it.** The graph answers relationship questions — who calls this, what breaks if I change it, where does this concept live — in one command where reading files costs a dozen: + + ```bash + scc graph explore "how does a plan get validated" # start here: relevant symbols' source plus the call paths between them + scc graph query renderTask --kind function # find one symbol by name (--limit N) + scc graph status # what the graph holds (--check exits 2 when there is none) + scc graph sync # re-index after writing code you then need to search + scc graph build --force # full rebuild, for a graph that has gone wrong rather than stale + ``` + + Reach for `explore` before `Read` when the question is about relationships, and for `Read` when you already know the file. `explore` takes a sentence and needs no quoting discipline — the positionals are joined, so an unquoted question still arrives as the sentence it was typed as. It emits no `--json` and that is CodeGraph's design, not an omission: it is the CLI face of the `codegraph_explore` MCP tool and returns the same agent-shaped text. + + The index goes stale as you edit. `scc launch` syncs before the session starts, so it is current at turn one; after that, `sync` before searching for something you just wrote. What scc adds over typing `codegraph` directly is the two things it already knows: the workspace root, so `scc graph build` from `specs/` indexes the repo rather than a subtree, and whether the binary is there at all. The graph itself is *not* an scc artifact — not in the manifest, never touched by `scc update`, and `.codegraph/` stays CodeGraph's directory on CodeGraph's schedule. Unlike the launch path, a missing binary in `scc graph` is a hard error: the whole command is the binary. @@ -49,6 +67,8 @@ These landed after phase 10, and all are documented in `design/orchestration.md` Three consequences had to land together or the result is worse than before: `Task.End` covers the flags (so `map show` returns them and `patch rm` removes them), `Detail` excludes them (so the searcher does not index `_Priority 2_` as prose), and **`renderTask` re-emits them plus the continuation** — without that, `patch task --method TDD` was a data-loss command that deleted a sixty-line description and every dependency the task declared. **The reading surface is what gives "never read the plan" its authority.** `map brief` is the header, `map tasks` is the checklist, and no command returns both — so a session pays `brief` once and `--next` per task instead of ~14k tokens per reread. Forbidding the read without offering the equivalent query produces an agent that disobeys the rule, correctly — so the surface shipped in the phase before the rule did. `--next` is now determined (eligible → priority ascending, absent last → number compared *numerically*, which is also the fix for `1.10` sorting before `1.9`), and `--ready`/`--blocked`/`--deps` share that one implementation, because two notions of eligibility would be two answers to "what do I work on". + + **`--next` prints the task whole; the listings clip.** Every task in a real plan runs past one line and the line below the checkbox is usually where the decision sits, so a `--next` that stopped at the line break sent the reader to the file — the exact cost this surface exists to remove. It therefore ignores `--width` and prints the continuation under the checkbox, as it sits in the file. The listings still clip to one line, because a list of sixty-one-line tasks is not a list. `Task.Continuation()` is the raw lines with the flags removed; `Task.Detail` is the same text collapsed to one line, which is right for a row in a table and wrong for anything meant to be read. - **`scc plan approve|reseal|migrate`, and the seal.** `approve` validates, then writes `status: approved` and a `checksum:` over the file minus its own checksum line, LF-normalized. It is **tamper-evidence, not prevention** — `reseal --force` is one command away and sha256 is public — and it is recorded that way here so nobody builds a guarantee on it later. The check runs before an edit is applied, which is the whole value: a harness that edited by hand and then ran `patch check` would otherwise have its edit resealed by the command that should have reported it. A plan with no `status:` is never checked, which is what makes every pre-existing plan keep working. After approval the work is fixed and only discovery moves: `add` allocates the number (high-water mark including removed tasks, so nothing is stored anywhere) and demands `--reason`; `rm` strikes the task out where it stands rather than deleting it; rewriting a task or the prose is refused. What discovery can never touch is guaranteed structurally rather than by instruction — `Why`, `Out of scope`, `Done when` and the title are reachable only through `append`/`prepend`/`replace`, and those are exactly the three refused. @@ -126,6 +146,7 @@ Three packages sit off to the side of that tree — `rtk`, `headroom`, `codegrap | `internal/workspace` | Resolves the root by walking up for *any* harness's `scc-manifest.json` marker; `Harnesses(root)` says which trees exist. Owns `KebabCheck`, `SafeName`, `AtomicWrite`. Knows nothing about specs or wikis. | | `internal/render` | CLI terminal output (`✓ ✗ ! •`, `NO_COLOR`/TTY aware), split across stdout/stderr. | | `internal/textutil` | Line-ending and BOM normalization, in exactly one place. | +| `internal/mdblock` | The marker-delimited splice: keeping one generated block current inside a Markdown file somebody else owns. `Markers.Splice/Block/Version`, idempotent, CRLF-preserving, and everything outside the markers untouched. Which markers is the integration's decision — `rtk.Markers` are RTK's own, `codegraph.Markers` are namespaced as scc's; what happens after that choice lives here once. | | `internal/finding` | One finding type and one frozen JSON shape (`{findings, count}`) for every validator, plus the grouped human report. | | `internal/manifest` | `/scc-manifest.json`: `{path, hash, version}` per managed file plus the harness, deterministic serialization, `Status → pristine\|edited\|missing`. Unknown fields are preserved. Every call takes the `paths.Harness` whose manifest it means. | | `internal/assets` | The embedded template set — rules, review agents, skills, slash commands, artifact templates. **Workspace templates are data-free except for the harness profile** (a `(version, harness)` pair still renders byte-identically everywhere, and the manifest records both, so the future three-way merge can still reconstruct the old side); **artifact templates take data** (`spec new` renders them and the user owns the result); **seeds are the `docs/` anchors** — data-free like a workspace file, untracked like an artifact. `Render(h, file)` is the only way to get a workspace file's bytes: it expands paths and synthesizes the per-harness header for agents and commands. `Version` is the template-set version and must be bumped whenever a workspace template changes. | @@ -134,9 +155,9 @@ Three packages sit off to the side of that tree — `rtk`, `headroom`, `codegrap | `internal/artifact` | The navigable model of one artifact, layered on `mdscan`: sections (two ends — the subtree, and the body before the first child), tasks with their continuation *and their flags*, requirements, spec-reference leaves, paragraph blocks. Owns **every grammar** (task, requirement, spec reference, flag), `Find` for address resolution, `Editor` for line splices resolved against the original and applied bottom-up, `Search`, the schedule (`Ready`/`BlockedTasks`/`Next`/`Cycles`, one implementation shared by `--next`, `--ready` and `--blocked`), and the seal. Knows nothing about findings or exit codes. | | `internal/ears` | EARS requirement parsing, all five patterns plus complex. | | `internal/validate` | The eight validators, one file each, sharing `mdscan` and `finding`. The exception is `stack_manifests.go`: the seven dependency-file readers age on their own schedule, so they sit beside the rule rather than inside it. | -| `internal/rtk` | RTK's marker pair and the idempotent splice of its block into the entry file, plus finding or `cargo install`ing the binary. | +| `internal/rtk` | RTK's marker pair (`rtk.Markers`, spliced by `internal/mdblock`), the foreign-block detection that names Headroom's copy, and finding or `cargo install`ing the binary. | | `internal/headroom` | Headroom's agent-slug table, the `wrap` argument vector, the MCP opt-out discovered from `wrap --help`, and finding or installing the binary (uv, then pip — never npm, which ships the SDK and no CLI). The slugs live here rather than on `paths.Harness` because they are Headroom's vocabulary, not scc's layout. | -| `internal/codegraph` | CodeGraph's argument vectors (`init`/`sync`/`index`/`status`/`query`/`explore`), the `.codegraph/` presence test, and finding or `npm install -g`ing the binary. Composes command lines and reads nothing inside the graph — the database is CodeGraph's schema on CodeGraph's schedule. | +| `internal/codegraph` | CodeGraph's argument vectors (`init`/`sync`/`index`/`status`/`query`/`explore`), the `.codegraph/` presence test, `codegraph.Markers` for the usage block `scc launch` splices, and finding or `npm install -g`ing the binary. Composes command lines and reads nothing inside the graph — the database is CodeGraph's schema on CodeGraph's schedule. | | `internal/cli` | The dispatcher and every command handler. | `internal/rtk`, `internal/headroom`, and `internal/codegraph` are the only packages that shell out to another program. Keep that boundary there rather than in a command handler: a third party's binary name, install command, and argument vocabulary all age on that third party's schedule, and one package per integration is what keeps a version bump from touching the dispatcher. Headroom's renamed MCP flag is the worked example — the fix stayed inside `internal/headroom`, and nothing else in the tree knows the flag exists. diff --git a/internal/artifact/parse.go b/internal/artifact/parse.go index a990aaf..b92344a 100644 --- a/internal/artifact/parse.go +++ b/internal/artifact/parse.go @@ -98,7 +98,24 @@ func (t Task) Group() string { // Summary is the task's description clipped to n runes, for a listing where one // task is one line. It is the first line only: a 61-line task exists, and printing // it in a list would defeat the point of the list. -func (t Task) Summary(n int) string { return clip(t.Text, n) } +// +// A non-positive n is the whole line, for the caller that is printing one task +// rather than a list of them. +func (t Task) Summary(n int) string { + if n <= 0 { + return strings.TrimSpace(t.Text) + } + return clip(t.Text, n) +} + +// Continuation is the rest of the description as it sits in the file, flag lines +// excluded — the lines a listing drops and a single-task answer must not. +// +// It is the raw lines rather than Detail because a task's continuation carries the +// decision, and often carries it as an indented list or a fenced example; Detail is +// the same text collapsed to one line, which is right for a row in a table and wrong +// for anything meant to be read. +func (t Task) Continuation() []string { return t.detail } // Requirement is one numbered EARS requirement. type Requirement struct { diff --git a/internal/assets/assets.go b/internal/assets/assets.go index 8a73a88..8dc5d5b 100644 --- a/internal/assets/assets.go +++ b/internal/assets/assets.go @@ -279,6 +279,21 @@ const RTKTemplate = "rtk.md" // the same block has to be recognizable to a tool that is not scc. func RTKBlock() (string, error) { return Content(RTKTemplate) } +// CodeGraphTemplate is the embedded name of the CodeGraph usage block. +const CodeGraphTemplate = "codegraph.md" + +// CodeGraphBlock returns the marker-delimited CodeGraph instructions `scc launch` +// splices into the entry file. +// +// A fragment like the RTK one, and delimited by markers of scc's own — which is the +// opposite of that decision, for the opposite reason. `rtk init` writes an RTK block +// into this same file, so sharing its markers is what makes the two tools converge +// on one copy. CodeGraph writes nothing into the entry file at all: this block is +// scc's account of `scc graph`, not CodeGraph's account of itself, so namespacing it +// leaves a future CodeGraph release free to add its own without either clobbering +// the other. +func CodeGraphBlock() (string, error) { return Content(CodeGraphTemplate) } + // 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 4835025..88313b6 100644 --- a/internal/assets/assets_test.go +++ b/internal/assets/assets_test.go @@ -92,6 +92,10 @@ func TestWorkspaceSetAndTreeAgree(t *testing.T) { t.Errorf("RTKBlock(): %v", err) } referenced[RTKTemplate] = true + if _, err := CodeGraphBlock(); err != nil { + t.Errorf("CodeGraphBlock(): %v", err) + } + referenced[CodeGraphTemplate] = true for name := range inTree { if !referenced[name] { t.Errorf("embedded template %q is in no harness's Workspace(), Seeds(), and is neither an artifact template nor a fragment", name) diff --git a/internal/assets/templates/codegraph.md b/internal/assets/templates/codegraph.md new file mode 100644 index 0000000..4b9092a --- /dev/null +++ b/internal/assets/templates/codegraph.md @@ -0,0 +1,15 @@ + +## CodeGraph +Ask the symbol graph before reading files. "Who calls this", "what breaks if I change it", +"where does this concept live" are one command here and a dozen reads otherwise. + +- `scc graph explore ""` — the relevant symbols' source plus the call paths between them. Start here. +- `scc graph query [--kind function|class] [--limit N]` — find a symbol by name. +- `scc graph status` — what the graph holds. `--check` exits 2 when there is none. +- `scc graph sync` — re-index after you have written code you then need to search. +- `scc graph build [--force]` — first index, or a full rebuild when the graph has gone wrong. + +`scc launch` indexes before the session starts, so the graph is current at turn one. +It goes stale as you edit: sync before searching for something you just wrote. +The graph is CodeGraph's — never edit `.codegraph/`, and never commit it. + diff --git a/internal/cli/entryblock.go b/internal/cli/entryblock.go new file mode 100644 index 0000000..78a527f --- /dev/null +++ b/internal/cli/entryblock.go @@ -0,0 +1,106 @@ +package cli + +import ( + "os" + "path/filepath" + + "github.com/protonspy/spec-claude-code/internal/mdblock" + "github.com/protonspy/spec-claude-code/internal/workspace" +) + +// blockFile is what happened to one marker-delimited block in one entry file. +// +// One shape for both integrations that write into that file, because it is the same +// question twice: which file, what happened to the block, and how many bytes it now +// costs in every request of the session. The JSON is `scc rtk`'s frozen shape, which +// is why the fields are named for a block rather than for RTK. +type blockFile 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. + Block string `json:"block,omitempty"` + // Was is the version of the block that got replaced, when it differed from the + // one scc ships. This is the honest half of preferring scc's block by default: + // between two v2 blocks the smaller one simply wins, but a v3 replaced by a v2 + // is a downgrade and has to be visible rather than inferred. + Was string `json:"was,omitempty"` + // Bytes and WasBytes size the block now in the file against the one it + // replaced — the whole argument for replacing it, stated in the unit that + // matters. + Bytes int `json:"bytes,omitempty"` + WasBytes int `json:"was_bytes,omitempty"` + // Foreign names another tool's block found in the same file — Headroom writes + // RTK guidance behind its own markers. Reported and never touched: scc does not + // own that block, and the file would carry the same instructions twice. + Foreign string `json:"foreign,omitempty"` +} + +// blockMissing is the action for an entry file that is not on disk. scc writes these +// blocks into a document it does not own, so it declines to bring that document into +// existence: an entry file holding nothing but a usage block would be a workspace +// missing its methodology while looking configured. +const blockMissing = "missing" + +// spliceEntryBlock keeps one block current in one entry file, and reports what that +// took. check reports without writing. +// +// Everything outside the markers is untouched, and the file is written atomically: +// the entry file is loaded on every launch, and a half-written one is a workspace +// that has lost its methodology. +func spliceEntryBlock(root, entry string, m mdblock.Markers, block string, keep, check bool) (blockFile, error) { + path := filepath.Join(root, entry) + raw, err := os.ReadFile(path) + if os.IsNotExist(err) { + return blockFile{Path: entry, Action: blockMissing}, nil + } + if err != nil { + return blockFile{}, err + } + doc := string(raw) + next, action, err := m.Splice(doc, block, keep) + if err != nil { + return blockFile{}, 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 := m.Version(doc) + if action != mdblock.Present { + found = m.Version(block) + } + file := blockFile{Path: entry, Action: string(action), Block: found, Bytes: len(block)} + if action == mdblock.Present { + file.Bytes = len(m.Block(doc)) + } + if action == mdblock.Replaced { + was := m.Block(doc) + file.WasBytes = len(was) + // Only when it differs: naming the version on every replacement would bury + // the one case that actually needs reading. + if v := m.Version(was); v != found { + file.Was = v + } + } + if action == mdblock.Present || check { + return file, nil + } + if err := workspace.AtomicWrite(path, []byte(next), 0o644); err != nil { + return blockFile{}, err + } + return file, 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 +} diff --git a/internal/cli/launch.go b/internal/cli/launch.go index a0deaea..9ee367d 100644 --- a/internal/cli/launch.go +++ b/internal/cli/launch.go @@ -8,8 +8,10 @@ import ( "os/exec" "strings" + "github.com/protonspy/spec-claude-code/internal/assets" "github.com/protonspy/spec-claude-code/internal/codegraph" "github.com/protonspy/spec-claude-code/internal/headroom" + "github.com/protonspy/spec-claude-code/internal/mdblock" "github.com/protonspy/spec-claude-code/internal/paths" "github.com/protonspy/spec-claude-code/internal/render" "github.com/protonspy/spec-claude-code/internal/rtk" @@ -119,9 +121,12 @@ func runLaunch(args []string) int { plan: plan, quiet: *jsonOut, }) - // RTK last of the three, because it is the only one that writes to a file the - // user owns: a run the user aborts at the Headroom or CodeGraph prompt should not - // already have edited their entry file. + // RTK last of the three, because it is the only one whose *install* is a + // decision — a Rust toolchain and a build that takes minutes — and a run the user + // aborts at the Headroom or CodeGraph prompt should not already have edited their + // entry file. Both it and CodeGraph write a usage block there, and both do so only + // once their binary is known to be present: guidance naming a command the machine + // cannot run is worse than no guidance, because it costs the file its credibility. cmd.RTK = resolveRTK(target, rtkLaunchOptions{ disabled: *noRTK, noInstall: *noInstall, @@ -427,6 +432,12 @@ type graphReport struct { Indexed bool `json:"indexed"` Path string `json:"path,omitempty"` Version string `json:"version,omitempty"` + // Blocks is what happened to the CodeGraph usage block in each entry file: + // added | present | replaced | missing. Empty when nothing was written, which + // is every run where the binary is not there — a block telling the agent to run + // `scc graph explore` in a workspace with no CodeGraph is guidance that fails on + // first use, and an agent that has been lied to once discounts the whole file. + Blocks []blockFile `json:"blocks,omitempty"` // Reason names why nothing was built, for the run where that is a surprise. Reason string `json:"reason,omitempty"` } @@ -508,13 +519,19 @@ func resolveGraph(root string, opts graphOptions) *graphReport { report.Path, report.Version = bin, codegraph.Version(bin) // A plan-only run reports what it would do and indexes nothing: --json has to - // leave stdout clean for the document, and --dry-run means what it says. + // leave stdout clean for the document, and --dry-run means what it says. It + // writes no block either — --dry-run that edited the entry file would be the one + // flag nobody expects to change anything doing exactly that. if opts.plan { report.Action = graphSkipped report.Reason = "plan-only run" return report } + // The block before the index rather than after it, so a failed index still + // leaves the agent knowing the command that rebuilds one. + report.Blocks = spliceGraphBlock(root, opts) + args, action, doing := codegraph.InitArgs(), graphBuilt, "building the symbol graph — the first index takes a while" if report.Indexed { args, action, doing = codegraph.SyncArgs(), graphSynced, "refreshing the symbol graph" @@ -543,6 +560,50 @@ func resolveGraph(root string, opts graphOptions) *graphReport { return report } +// spliceGraphBlock keeps the CodeGraph usage block current in every entry file this +// workspace has, and returns what that took. +// +// It runs at launch and nowhere else, which is the same reasoning that puts the +// index here: this is the moment the guidance is about to be read, and the binary +// has just been proven to exist. `scc init` cannot write it — CodeGraph may not be +// installed then, and often is not — and a block promising `scc graph explore` in a +// workspace where that command fails teaches the agent to distrust the whole file. +// +// A write failure is not fatal. The agent is about to start either way, and the +// block is an enhancement exactly as the graph is; refusing to launch over a +// Markdown splice would be scc putting its own tidiness above what was asked for. +func spliceGraphBlock(root string, opts graphOptions) []blockFile { + block, err := assets.CodeGraphBlock() + if err != nil { + if !opts.quiet { + render.Warn("could not read the CodeGraph usage block: " + err.Error()) + } + return nil + } + var out []blockFile + for _, entry := range entryFiles(root) { + // keep=false: scc owns this block behind its own markers, so replacing it is + // how a workspace picks up a newer one. There is no other author to defer to. + file, err := spliceEntryBlock(root, entry, codegraph.Markers, block, false, false) + if err != nil { + if !opts.quiet { + render.Warn(fmt.Sprintf("%s: %v", entry, err)) + } + continue + } + out = append(out, file) + if opts.quiet || file.Action == string(mdblock.Present) { + continue + } + if file.Action == blockMissing { + render.Warn(fmt.Sprintf("%s does not exist; run `%s init` first", entry, prog())) + continue + } + render.OK(fmt.Sprintf("%s — CodeGraph block %s", entry, file.Action)) + } + return out +} + // warnNoGraph says, once, why the agent is starting without a fresh graph. func warnNoGraph(report *graphReport, opts graphOptions) { if opts.quiet { diff --git a/internal/cli/launch_test.go b/internal/cli/launch_test.go index 483f482..da64b06 100644 --- a/internal/cli/launch_test.go +++ b/internal/cli/launch_test.go @@ -8,6 +8,7 @@ import ( "strings" "testing" + "github.com/protonspy/spec-claude-code/internal/codegraph" "github.com/protonspy/spec-claude-code/internal/paths" ) @@ -310,6 +311,86 @@ func TestLaunchBuildsTheGraphAndThenRefreshesIt(t *testing.T) { } } +// An index nobody knows how to query is an index nobody queries. Launch writes the +// CodeGraph usage block into the entry file for the same reason it runs the index +// there: the session is about to read that file, and the binary has just been proven +// to exist. +func TestLaunchWritesTheCodeGraphBlockIntoTheEntryFile(t *testing.T) { + root := initWorkspace(t) + dir := isolatedPath(t, "claude") + recordingStub(t, dir, "codegraph") + withLaunchExec(t, 0) + + if _, stderr, code := run(t, "launch", "--root", root, "--no-headroom"); code != ExitOK { + t.Fatalf("exit = %d (stderr: %s)", code, stderr) + } + entry := filepath.Join(root, paths.Claude.EntryFile) + first, err := os.ReadFile(entry) + if err != nil { + t.Fatalf("ReadFile: %v", err) + } + if !strings.Contains(string(first), "scc graph explore") { + t.Errorf("the entry file does not carry the CodeGraph block:\n%s", first) + } + + // And it is a splice, not an append: a second launch leaves one copy. + if _, stderr, code := run(t, "launch", "--root", root, "--no-headroom"); code != ExitOK { + t.Fatalf("second launch: exit = %d (stderr: %s)", code, stderr) + } + second, err := os.ReadFile(entry) + if err != nil { + t.Fatalf("ReadFile: %v", err) + } + if n := strings.Count(string(second), codegraph.Markers.Open); n != 1 { + t.Errorf("the entry file carries %d CodeGraph blocks, want 1", n) + } + if string(second) != string(first) { + t.Error("the second launch rewrote a block that was already current") + } +} + +// Guidance naming a command the machine cannot run is worse than no guidance: the +// agent tries `scc graph explore`, it fails, and the whole file loses its +// credibility. So the block goes in only once the binary is there. +func TestLaunchWritesNoCodeGraphBlockWithoutTheBinary(t *testing.T) { + root := initWorkspace(t) + isolatedPath(t, "claude") + withoutTerminal(t) + withLaunchExec(t, 0) + + if _, stderr, code := run(t, "launch", "--root", root, "--no-headroom"); code != ExitOK { + t.Fatalf("exit = %d (stderr: %s)", code, stderr) + } + entry, err := os.ReadFile(filepath.Join(root, paths.Claude.EntryFile)) + if err != nil { + t.Fatalf("ReadFile: %v", err) + } + if strings.Contains(string(entry), codegraph.Markers.Open) { + t.Error("the entry file was told to use a binary this machine does not have") + } +} + +// --dry-run and --json report what a launch would do. A flag nobody expects to +// change anything must not edit a file the user owns. +func TestLaunchPlanOnlyWritesNoCodeGraphBlock(t *testing.T) { + root := initWorkspace(t) + dir := isolatedPath(t, "claude") + recordingStub(t, dir, "codegraph") + + for _, flag := range []string{"--json", "--dry-run"} { + if _, stderr, code := run(t, "launch", "--root", root, "--no-headroom", flag); code != ExitOK { + t.Fatalf("%s: exit = %d (stderr: %s)", flag, code, stderr) + } + entry, err := os.ReadFile(filepath.Join(root, paths.Claude.EntryFile)) + if err != nil { + t.Fatalf("ReadFile: %v", err) + } + if strings.Contains(string(entry), codegraph.Markers.Open) { + t.Errorf("%s edited the entry file", flag) + } + } +} + // A graph is an enhancement — the agent can still read files — so every way of not // getting one ends in the agent starting anyway, with a line saying what happened. // A launch that refused to run because an index was stale would put scc's diff --git a/internal/cli/map.go b/internal/cli/map.go index 92b26c8..03bb844 100644 --- a/internal/cli/map.go +++ b/internal/cli/map.go @@ -320,7 +320,7 @@ func runMapTasks(args []string) int { blocked := fs.Bool("blocked", false, "open tasks that are not eligible, each naming what it waits on") deps := fs.Bool("deps", false, "the dependency edges alone, one line per task that has any") removed := fs.Bool("removed", false, "the tasks discovery struck out, with their reasons") - width := fs.Int("width", 96, "clip each description to this many `runes`") + width := fs.Int("width", 96, "clip each description to this many `runes` (--next prints whole)") noVerify := addNoVerify(fs) jsonOut := addJSON(fs) rest, err := parseFlags(fs, args) @@ -350,7 +350,7 @@ func runMapTasks(args []string) int { switch { case *next: - return runMapNext(arts, *jsonOut, *width) + return runMapNext(arts, *jsonOut) case *ready, *blocked, *deps: return runMapSchedule(arts, scheduleView{ready: *ready, blocked: *blocked, deps: *deps}, *jsonOut, *width) @@ -441,7 +441,7 @@ type blockedRow struct { // The reason is the part that had to be designed rather than fallen into. A loop // that got an empty answer could not tell "the plan is finished" from "everything // left is waiting on something", and those call for opposite next moves. -func runMapNext(arts []*artifact.Artifact, jsonOut bool, width int) int { +func runMapNext(arts []*artifact.Artifact, jsonOut bool) int { if code := reportUnrunnable(arts); code != ExitOK { return code } @@ -454,7 +454,16 @@ func runMapNext(arts []*artifact.Artifact, jsonOut bool, width int) int { Task *taskRow `json:"task"` }{&row}) } - render.Info(taskLine(row, width)) + // The listings clip to one line because they are lists. This is not a + // list: it is the task the session is about to do, and a description + // that stops mid-sentence at the line break sends the reader to the + // file — which is the cost the whole reading surface exists to avoid. + // So --next ignores --width and prints the checkbox line whole, then + // the continuation under it as it sits in the file. + render.Info(taskLine(row, 0)) + for _, line := range t.Continuation() { + fmt.Println(" " + line) + } return ExitOK } blocked = append(blocked, blockedRowsFor(a)...) diff --git a/internal/cli/map_test.go b/internal/cli/map_test.go index cbe9e36..db6c3e2 100644 --- a/internal/cli/map_test.go +++ b/internal/cli/map_test.go @@ -32,6 +32,7 @@ What this is for, and what done means for the whole of it. - [x] 1.1 (Unit) Build the parser, and prove with a test that it reads a fenced block without treating the example inside it as a task - [ ] 1.2 (TDD) Guard the credential before the provider client lands + so the secret is never the thing a first run discovers is missing ## Notes @@ -124,6 +125,30 @@ func TestMapTasksNextIsTheFirstOpenOne(t *testing.T) { } } +// Every task in a real plan runs past one line, and the line below the checkbox is +// where the decision usually sits. A listing clips it because a listing is a list; +// --next is one task, and clipping there sends the reader to the file — the exact +// cost `map` exists to remove. +func TestMapTasksNextPrintsTheWholeDescription(t *testing.T) { + root := mapWorkspace(t) + stdout, _, code := run(t, "map", "tasks", "sample", "--next", "--root", root) + if code != ExitOK { + t.Fatalf("exit %d", code) + } + if !strings.Contains(stdout, "so the secret is never the thing a first run discovers is missing") { + t.Errorf("--next dropped the continuation line:\n%s", stdout) + } + + // And the listing still clips, or the reading surface has no cheap view left. + list, _, code := run(t, "map", "tasks", "sample", "--root", root) + if code != ExitOK { + t.Fatalf("exit %d", code) + } + if strings.Contains(list, "so the secret is never") { + t.Errorf("the listing printed a continuation:\n%s", list) + } +} + func TestMapShowReturnsOnlyThatPiece(t *testing.T) { root := mapWorkspace(t) stdout, stderr, code := run(t, "map", "show", "sample", "notes:1", "--root", root) diff --git a/internal/cli/rtk.go b/internal/cli/rtk.go index 557c01d..ce7f65e 100644 --- a/internal/cli/rtk.go +++ b/internal/cli/rtk.go @@ -10,7 +10,6 @@ import ( "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 @@ -72,34 +71,13 @@ type rtkOptions struct { // 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. - Block string `json:"block,omitempty"` - // Was is the version of the block that got replaced, when it differed from the - // one scc ships. This is the honest half of preferring scc's block by default: - // between two v2 blocks the smaller one simply wins, but a v3 replaced by a v2 - // is a downgrade and has to be visible rather than inferred. - Was string `json:"was,omitempty"` - // Bytes and WasBytes size the block now in the file against the one it - // replaced — the whole argument for replacing it, stated in the unit that - // matters. - Bytes int `json:"bytes,omitempty"` - WasBytes int `json:"was_bytes,omitempty"` - // Foreign names another tool's RTK block found in the same file — Headroom - // writes one behind its own markers. Reported and never touched: scc does not - // own that block, and the file would carry the same instructions twice. - Foreign string `json:"foreign,omitempty"` + Installed bool `json:"installed"` + Path string `json:"path,omitempty"` + Version string `json:"version,omitempty"` + Install string `json:"install"` // present | installed | skipped | failed + Files []blockFile `json:"files"` + Changed int `json:"changed"` + Error string `json:"error,omitempty"` } // The values rtkReport.Install takes. Not rtk.Action: these describe the binary, @@ -112,16 +90,10 @@ const ( 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{}} + report := &rtkReport{Files: []blockFile{}} code := ExitOK if err := ensureBinary(report, opts); err != nil { @@ -154,7 +126,7 @@ func applyRTK(root string, opts rtkOptions) (*rtkReport, int) { if !opts.quiet { render.Info(strings.TrimSpace(fmt.Sprintf("%s — RTK block already there %s", entry, file.Block))) } - case rtkMissing: + case blockMissing: // 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. @@ -198,7 +170,7 @@ func applyRTK(root string, opts rtkOptions) (*rtkReport, int) { // sizeNote is the whole argument for replacing a block, in the unit that makes it: // bytes the entry file no longer spends in every request of the session. Silent // when nothing was replaced, and when the replacement was not actually smaller. -func sizeNote(file rtkFile) string { +func sizeNote(file blockFile) string { if file.WasBytes <= file.Bytes { return "" } @@ -245,64 +217,27 @@ func ensureBinary(report *rtkReport, opts rtkOptions) error { 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.keep) +// spliceEntry keeps RTK's block current in one entry file, and says so when another +// tool has written the same guidance behind its own markers. +// +// The foreign check is the half that is RTK's alone. Headroom's context-tool setup +// appends RTK instructions to this same file behind ``; +// neither marker is a substring of the other, so both tools' idempotency checks pass +// and the file ends up telling the agent the same thing twice in every request. +// Detected, named, and left exactly where it is — that block belongs to Headroom. +func spliceEntry(root, entry, block string, opts rtkOptions) (blockFile, error) { + raw, err := os.ReadFile(filepath.Join(root, entry)) + if err != nil && !os.IsNotExist(err) { + return blockFile{}, err + } + file, err := spliceEntryBlock(root, entry, rtk.Markers, block, opts.keep, opts.check) 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, Bytes: len(block)} - if action == rtk.Present { - file.Bytes = len(rtk.Block(string(raw))) - } - if action == rtk.Replaced { - was := rtk.Block(string(raw)) - file.WasBytes = len(was) - // Only when it differs: naming the version on every replacement would bury - // the one case that actually needs reading. - if v := rtk.BlockVersion(was); v != found { - file.Was = v - } + return blockFile{}, err } if foreign, ok := rtk.ForeignBlock(string(raw)); ok { file.Foreign = foreign.Tool render.Warn(fmt.Sprintf("%s also carries %s's RTK block; the agent will read these instructions twice", entry, foreign.Tool)) render.Detail(" remove that one with: " + foreign.Fix) } - 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/codegraph/codegraph.go b/internal/codegraph/codegraph.go index 8373349..1b020f3 100644 --- a/internal/codegraph/codegraph.go +++ b/internal/codegraph/codegraph.go @@ -21,6 +21,8 @@ import ( "path/filepath" "strconv" "strings" + + "github.com/protonspy/spec-claude-code/internal/mdblock" ) // Repo is where CodeGraph is developed, for the error that has to send somebody @@ -39,6 +41,20 @@ const Pkg = "@colbymchenry/codegraph" // schedule, not a format scc has any business knowing. const Dir = ".codegraph" +// Markers delimit the usage block scc keeps current in the entry file — the one +// that tells the agent to ask the graph before it starts reading files. +// +// Namespaced as scc's own, which is deliberately the opposite of the RTK decision. +// There, sharing RTK's markers is what makes `rtk init` and `scc rtk` converge on +// one copy of a block they both write. CodeGraph writes nothing into the entry file, +// and the block is scc's account of `scc graph` rather than CodeGraph's account of +// itself — so a marker of scc's own is what leaves a future CodeGraph release free +// to add its own without either tool clobbering the other. +var Markers = mdblock.Markers{ + Open: "", +} + // Indexed reports whether root has a graph. func Indexed(root string) bool { info, err := os.Stat(filepath.Join(root, Dir)) diff --git a/internal/mdblock/mdblock.go b/internal/mdblock/mdblock.go new file mode 100644 index 0000000..862e669 --- /dev/null +++ b/internal/mdblock/mdblock.go @@ -0,0 +1,129 @@ +// Package mdblock is the marker-delimited splice: keeping one block of generated +// guidance current inside a Markdown file that somebody else owns. +// +// It exists because two integrations need the same thing and neither owns it. RTK's +// block is addressed by RTK's own markers, so that `rtk init` and `scc rtk` converge +// on one copy; CodeGraph writes nothing into the entry file, so scc's block there is +// namespaced as scc's. Those are different decisions about *which* markers, and they +// belong in the integration packages. What is identical is everything after the +// markers are chosen — find, compare, replace, append, preserve the line endings — +// and a second copy of that would be a second set of edge cases to get right. +// +// Everything outside the markers is untouched, always. The document belongs to the +// user; scc is a guest in it. +package mdblock + +import ( + "fmt" + "strings" + + "github.com/protonspy/spec-claude-code/internal/textutil" +) + +// Markers is the HTML comment pair that delimits one tool's block. +// +// Open is matched by prefix rather than exactly, because the marker carries a +// version: 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. +type Markers struct { + // Open is the opening marker up to but not including its version, e.g. + // "` — 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 belongs to whoever defines the block. +func (m Markers) Version(doc string) string { + start := strings.Index(doc, m.Open) + if start < 0 { + return "" + } + rest := doc[start+len(m.Open):] + end := strings.Index(rest, "-->") + if end < 0 { + return "" + } + return strings.TrimSpace(rest[:end]) +} diff --git a/internal/mdblock/mdblock_test.go b/internal/mdblock/mdblock_test.go new file mode 100644 index 0000000..3361c73 --- /dev/null +++ b/internal/mdblock/mdblock_test.go @@ -0,0 +1,84 @@ +package mdblock + +import ( + "strings" + "testing" +) + +var ( + alpha = Markers{Open: ""} + beta = Markers{Open: ""} +) + +const ( + alphaBlock = "\nAlpha says this.\n" + betaBlock = "\nBeta says this.\n" +) + +// The reason this package exists as its own thing: one entry file now carries two +// blocks written by two integrations, and each has to be able to update its own +// without touching the other. Neither marker is a substring of the other, which is +// the property that makes that true — and the property nobody would notice breaking +// until a real workspace lost a block. +func TestTwoBlocksCoexistInOneDocument(t *testing.T) { + doc := "# CLAUDE.md\n\nThe user's own prose.\n" + + doc, action, err := alpha.Splice(doc, alphaBlock, false) + if err != nil || action != Added { + t.Fatalf("alpha: action = %q, err = %v", action, err) + } + doc, action, err = beta.Splice(doc, betaBlock, false) + if err != nil || action != Added { + t.Fatalf("beta: action = %q, err = %v", action, err) + } + + // Now rewrite alpha's, and beta's must come through untouched. + next := "\nAlpha says something else.\n" + doc, action, err = alpha.Splice(doc, next, false) + if err != nil || action != Replaced { + t.Fatalf("alpha rewrite: action = %q, err = %v", action, err) + } + if !strings.Contains(doc, betaBlock) { + t.Errorf("rewriting alpha's block damaged beta's:\n%s", doc) + } + if !strings.Contains(doc, "The user's own prose.") { + t.Errorf("the user's own prose did not survive:\n%s", doc) + } + if got := alpha.Version(doc); got != "v2" { + t.Errorf("alpha version = %q, want v2", got) + } + if got := beta.Version(doc); got != "v1" { + t.Errorf("beta version = %q, want v1 — it read alpha's marker", got) + } + if got := beta.Block(doc); got != betaBlock { + t.Errorf("beta block = %q, want its own", got) + } +} + +// An idempotent splice is what lets a launch run the same write on every session +// without the entry file growing a block each time. +func TestSpliceIsIdempotent(t *testing.T) { + once, _, err := alpha.Splice("# CLAUDE.md\n", alphaBlock, false) + if err != nil { + t.Fatalf("Splice: %v", err) + } + twice, action, err := alpha.Splice(once, alphaBlock, false) + if err != nil { + t.Fatalf("Splice: %v", err) + } + if action != Present || twice != once { + t.Errorf("action = %q and the document changed; want %q and no change", action, Present) + } +} + +// A document with an opening marker and no close is malformed rather than +// blockless: appending would leave two openings and one close, which no tool could +// then update. +func TestSpliceRefusesAnUnclosedBlock(t *testing.T) { + if _, _, err := alpha.Splice("\ndangling\n", alphaBlock, false); err == nil { + t.Error("Splice accepted a document whose block never closes") + } + if _, _, err := alpha.Splice("\n", alphaBlock, false); err == nil { + t.Error("Splice accepted a closing marker with no opening one") + } +} diff --git a/internal/rtk/rtk.go b/internal/rtk/rtk.go index 6b1e1e6..a52a70a 100644 --- a/internal/rtk/rtk.go +++ b/internal/rtk/rtk.go @@ -16,7 +16,7 @@ import ( "os/exec" "strings" - "github.com/protonspy/spec-claude-code/internal/textutil" + "github.com/protonspy/spec-claude-code/internal/mdblock" ) // Repo is where the binary is built from. RTK is a Rust program distributed as @@ -27,9 +27,9 @@ const Repo = "https://github.com/rtk-ai/rtk" // 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. +// Markers are the ones 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. // // Sharing RTK's markers rather than namespacing scc's own is the load-bearing // choice here, and it is worth naming what it buys: `rtk init` writes this exact @@ -37,10 +37,10 @@ const Bin = "rtk" // what makes `rtk init` and `scc rtk` converge on one copy. A marker of scc's own // — `scc:rtk-instructions`, say — would make each tool blind to the other's block // and leave the file carrying both. -const ( - openPrefix = "" -) +var Markers = mdblock.Markers{ + Open: "", +} // Foreign is a marker some other tool writes for the same guidance. // @@ -81,29 +81,27 @@ func ForeignBlock(doc string) (Foreign, bool) { } // Action is what splicing the block did to a document. -type Action string +type Action = mdblock.Action +// The three outcomes, re-exported so a caller working in RTK's vocabulary does not +// have to reach past it. +// +// Replaced is the default here, and the reason is size. `rtk init` and scc both +// stamp v2 and give the agent the same instruction, but RTK's own block spends +// roughly five times the bytes doing it — and the entry file is preloaded into every +// request of the session, so the difference is paid continuously rather than once. +// Between two blocks of the same version, the condensed one is simply better, and +// leaving the larger one in place because it got there first is not deference, it is +// a standing cost. +// +// What that does give up is version ordering: a future `rtk init` writing v3 would +// be overwritten by scc's v2. Splice therefore keeps the replaced block's version +// visible so the caller can say so, and keep is the standing answer for anyone who +// has deliberately curated their own. const ( - // Added: the document carried no block, so one was appended. - Added Action = "added" - // Present: the block in the document is already the one scc ships, byte for - // byte, or the caller asked for an existing block to be kept. - Present Action = "present" - // Replaced: the document carried a different block, and scc's replaced it. - // - // This is the default, and the reason is size. `rtk init` and scc both stamp - // v2 and give the agent the same instruction, but RTK's own block spends - // roughly five times the bytes doing it — and the entry file is preloaded into - // every request of the session, so the difference is paid continuously rather - // than once. Between two blocks of the same version, the condensed one is - // simply better, and leaving the larger one in place because it got there first - // is not deference, it is a standing cost. - // - // What that does give up is version ordering: a future `rtk init` writing v3 - // would be overwritten by scc's v2. Splice therefore keeps the replaced block's - // version visible so the caller can say so, and Keep is the standing answer for - // anyone who has deliberately curated their own. - Replaced Action = "replaced" + Added = mdblock.Added + Present = mdblock.Present + Replaced = mdblock.Replaced ) // InstallCmd is the command Install runs, as a string, so the CLI can name it @@ -140,35 +138,7 @@ func Available() bool { // 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, keep 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 keep || rest[:end] == block { - return doc, Present, nil - } - return doc[:start] + block + doc[start+end:], Replaced, nil + return Markers.Splice(doc, block, keep) } // Block returns the marker-delimited block in doc, markers included, or "" when @@ -179,36 +149,14 @@ func Splice(doc, block string, keep bool) (string, Action, error) { // both stamp v2 and say the same thing, but RTK's own block spends roughly five // times the bytes doing it, and the entry file is preloaded into every request of // the session. Version ordering cannot separate those two — only size can. -func Block(doc string) string { - start := strings.Index(doc, openPrefix) - if start < 0 { - return "" - } - rest := doc[start:] - end := strings.Index(rest, closeTag) - if end < 0 { - return "" - } - return rest[:end+len(closeTag)] -} +func Block(doc string) string { return Markers.Block(doc) } // 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]) -} +func BlockVersion(doc string) string { return Markers.Version(doc) } // Path reports where the rtk binary is, and whether it is on PATH at all. func Path() (string, bool) { diff --git a/internal/rtk/rtk_test.go b/internal/rtk/rtk_test.go index 5d91066..8bea17b 100644 --- a/internal/rtk/rtk_test.go +++ b/internal/rtk/rtk_test.go @@ -16,15 +16,15 @@ func block(t *testing.T) string { 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. +// 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.HasPrefix(b, Markers.Open) { + t.Errorf("the block does not open with %q: %q", Markers.Open, firstLine(b)) } - if !strings.Contains(b, closeTag) { - t.Errorf("the block does not carry %q", closeTag) + if !strings.Contains(b, Markers.Close) { + t.Errorf("the block does not carry %q", Markers.Close) } if !strings.Contains(b, "Prefix EVERY command with `rtk`") { t.Error("the block lost the instruction it exists for") @@ -36,8 +36,8 @@ func TestShippedBlockIsMarkerDelimited(t *testing.T) { // exact pair into the project's entry file. A marker of scc's own would make each // tool blind to the other's block and leave the file carrying both. func TestTheMarkersAreRTKsOwn(t *testing.T) { - if openPrefix != "" { - t.Errorf("markers are %q / %q, which is not what `rtk init` writes", openPrefix, closeTag) + if Markers.Open != "" { + t.Errorf("markers are %q / %q, which is not what `rtk init` writes", Markers.Open, Markers.Close) } // And the namespaced variant somebody will eventually propose must not match, // or scc would claim a block it does not own. @@ -47,7 +47,7 @@ func TestTheMarkersAreRTKsOwn(t *testing.T) { } // Headroom writes the same guidance behind its own marker pair, into the same -// entry file. scc cannot address that block — it is Headroom's — but a file +// entry file. scc cannot address that block — it is Headroom's — but a file // carrying both tells the agent the same thing twice in every request, so the one // thing scc must not do is fail to notice. func TestForeignBlockFindsHeadroomsCopy(t *testing.T) { @@ -71,7 +71,7 @@ func TestForeignBlockFindsHeadroomsCopy(t *testing.T) { // Neither marker is a substring of the other, which is why both tools' idempotency // checks pass and both append. Splice must leave Headroom's block exactly where it -// is and add scc's alongside — anything else would be scc editing a document it +// is and add scc's alongside — anything else would be scc editing a document it // does not own. func TestSpliceLeavesAForeignBlockAlone(t *testing.T) { foreign := "\nuse rtk\n" @@ -87,7 +87,7 @@ func TestSpliceLeavesAForeignBlockAlone(t *testing.T) { if !strings.Contains(got, foreign) { t.Error("Splice modified Headroom's block") } - if !strings.Contains(got, openPrefix) { + if !strings.Contains(got, Markers.Open) { t.Error("Splice did not add scc's own block") } } @@ -107,7 +107,7 @@ func TestSpliceAppendsToADocumentWithoutABlock(t *testing.T) { if !strings.Contains(got, "## RTK") { t.Error("the block was not appended") } - if !strings.HasSuffix(got, closeTag+"\n") { + if !strings.HasSuffix(got, Markers.Close+"\n") { t.Errorf("the result does not end with the closing marker and one newline: %q", tail(got)) } } @@ -134,7 +134,7 @@ func TestSpliceIsIdempotent(t *testing.T) { } // The block scc ships wins by default, replacing whatever is between the markers -// — in place, between the markers and nowhere else. +// — in place, between the markers and nowhere else. func TestSpliceReplacesAnExistingBlockInPlace(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), false) @@ -150,13 +150,13 @@ func TestSpliceReplacesAnExistingBlockInPlace(t *testing.T) { 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)) + if strings.Count(got, Markers.Open) != 1 { + t.Errorf("the document carries %d opening markers, want 1", strings.Count(got, Markers.Open)) } } // keep is the standing "leave whatever is already there", for a block somebody -// curated on purpose — or one whose version is ahead of what this build ships. +// curated on purpose — or one whose version is ahead of what this build ships. func TestSpliceKeepsAnExistingBlockWhenAsked(t *testing.T) { doc := "# CLAUDE.md\n\n\n## RTK\nnewer text\n\n" got, action, err := Splice(doc, block(t), true) @@ -224,7 +224,7 @@ func TestSpliceIntoAnEmptyDocument(t *testing.T) { if action != Added { t.Errorf("action = %q, want %q", action, Added) } - if !strings.HasPrefix(got, openPrefix) { + if !strings.HasPrefix(got, Markers.Open) { t.Errorf("an empty document got leading blank lines: %q", firstLine(got)) } }