From 374e276d87711374bbc2e992d27e0337b3fa0f8f Mon Sep 17 00:00:00 2001 From: prode Date: Sat, 8 Aug 2026 15:15:27 -0300 Subject: [PATCH] feat(launch): make Headroom's MCP and RTK opt-in, not the default scc launch's only reason to reach for Headroom is its compression proxy; registering MCP servers and wiring up RTK on every launch went beyond that. --headroom-mcp now defaults to none instead of retrieve, and RTK preflight moves from an opt-out (--no-rtk) to an opt-in (--rtk) so a bare `scc launch` starts the agent behind the proxy and nothing else. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01YGMJjeWYoqTdcgedRLjDJX --- CLAUDE.md | 4 ++- internal/cli/launch.go | 22 +++++++----- internal/cli/launch_test.go | 67 +++++++++++++++++++------------------ 3 files changed, 52 insertions(+), 41 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 749a620..f1ed5e6 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -27,10 +27,12 @@ These landed after phase 10, and all are documented in `design/orchestration.md` Two things about `wrap` are load-bearing and were originally documented wrong here: - **It does write to disk.** `headroom wrap` registers MCP servers into the agent's own config (`~/.claude.json` and the Codex/opencode equivalents), and those registrations outlive the session that made them — which is why Headroom ships `unwrap` at all. So `scc launch` defaults to **`--headroom-mcp retrieve`**: Headroom's own retrieve tool stays, because the proxy's compression markers are unactionable without it, and the code-memory server it would otherwise install does not, because code intelligence in an scc workspace is CodeGraph's job. `all` keeps Headroom's defaults; `none` drops the retrieve tool too. + **It does write to disk.** `headroom wrap` registers MCP servers into the agent's own config (`~/.claude.json` and the Codex/opencode equivalents), and those registrations outlive the session that made them — which is why Headroom ships `unwrap` at all. So `scc launch` defaults to **`--headroom-mcp none`**: the only thing a bare `scc launch` wants from Headroom is the compression proxy, so no MCP server is registered on the agent's behalf, not even Headroom's own retrieve tool. The cost of that is real — the proxy's compression markers go unactionable without retrieve — and `--headroom-mcp retrieve` hands that back for anyone who wants the markers expanded; `all` keeps Headroom's defaults, `none` (the default) drops the retrieve tool too. **It also wants the entry file, and scc says no.** `headroom wrap`'s context-tool setup appends RTK guidance to `$PWD/CLAUDE.md` or `$PWD/AGENTS.md` — the same file `scc rtk` splices — behind its own marker pair, ``. Neither marker is a substring of the other, so both tools' idempotency checks pass and both append: an entry file carrying the same RTK instructions twice, in every request of the session. So `scc launch` passes `--no-context-tool` by default. Headroom already gates that injection behind `HEADROOM_RTK`, which makes this belt-and-braces — but only until an environment exports that variable for its own reasons, and `wrap claude` resolves `setup_context_tool = (context_tool or _rtk_opt_in()) and not no_rtk`, so the flag wins over both the env var and `--context-tool`. `--headroom-context-tool` hands it back. + **RTK preflight is opt-in too.** `scc launch` can also make sure the agent it starts has RTK's binary and usage block ready — the same setup `scc rtk` does standalone — but only when asked: `--rtk`. A bare `scc launch` leaves the entry file and PATH untouched on that front, for the same reason MCP and the context tool default off: the only thing Headroom is there for by default is the compression proxy. + **The opt-out flags are discovered, not hardcoded.** `internal/headroom` reads `headroom wrap --help` and picks the spelling that build advertises. This is not defensiveness for its own sake: Headroom renamed this exact control once already (`--no-serena` → `--code-memory none`, and `--no-tokensave` in between), and the harness profiles disagree today — `wrap opencode` still takes `--no-serena` while `wrap claude` and `wrap codex` take `--code-memory`. A flag name compiled into scc turns that kind of release into a launch that dies on `no such option`, which is strictly worse than one unwanted MCP server. A build advertising no opt-out is reported, not overridden. **`--` 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`. diff --git a/internal/cli/launch.go b/internal/cli/launch.go index 9ee367d..2deba54 100644 --- a/internal/cli/launch.go +++ b/internal/cli/launch.go @@ -38,9 +38,15 @@ import ( // // What it does leave behind is MCP registrations in the agent's own config, which // outlive the session that made them. That is why the default is -// --headroom-mcp=retrieve rather than Headroom's own defaults: the proxy needs -// its retrieve tool to make compression markers actionable, and the code-memory -// server it would otherwise install is a job this workspace gives CodeGraph. +// --headroom-mcp=none rather than Headroom's own defaults: the only thing this +// launch wants from Headroom is the compression proxy itself, so no MCP server — +// not even Headroom's own retrieve tool — gets registered into the agent's config +// on its behalf. The cost is that the proxy's compression markers go +// unactionable; `--headroom-mcp retrieve` hands that back for anyone who wants +// the markers expanded again. RTK setup and Headroom's own context-tool are +// opt-in for the same reason: `--rtk` and `--headroom-context-tool` ask for them +// explicitly, rather than a bare `scc launch` doing more than start the agent +// behind the proxy. // // The agent's own exit code is passed straight through, which is the one place // scc's 0/1/2 contract does not apply — and it has to be. A launcher that @@ -58,12 +64,12 @@ func runLaunch(args []string) int { fs.SetOutput(os.Stderr) root := addRoot(fs) noHeadroom := fs.Bool("no-headroom", false, "start the agent directly, without Headroom's compression proxy") - mcp := fs.String("headroom-mcp", headroom.MCPRetrieve.String(), + mcp := fs.String("headroom-mcp", headroom.MCPNone.String(), "which MCP servers Headroom may register: all | retrieve (its own only) | none") contextTool := fs.Bool("headroom-context-tool", false, "let Headroom set up its own CLI context tool (RTK or lean-ctx) and append its guidance to the entry file") noGraph := fs.Bool("no-graph", false, "start the agent without building or refreshing the symbol graph") - noRTK := fs.Bool("no-rtk", false, "start the agent without setting up RTK's binary or its usage block") + rtkFlag := fs.Bool("rtk", false, "also set up RTK's binary and usage block before starting the agent") noInstall := fs.Bool("no-install", false, "never install anything; use Headroom, CodeGraph and RTK only if they are already on PATH") yes := fs.Bool("yes", false, "answer the install prompts with yes, for an unattended run") dryRun := fs.Bool("dry-run", false, "print the command this would run, and run nothing") @@ -128,7 +134,7 @@ func runLaunch(args []string) int { // 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, + disabled: !*rtkFlag, noInstall: *noInstall, yes: *yes, plan: plan, @@ -371,8 +377,8 @@ type rtkLaunchOptions struct { // separate opt-in command precisely because splicing into somebody's CLAUDE.md is not // a thing to do on the side; offering it here is the same decision put where it is // actually actionable — at the moment the session that would benefit is starting. -// Saying no is free, the block is idempotent once written, and --no-rtk is the -// standing answer. +// It stays opt-in here too: --rtk is what asks for it, and a bare `scc launch` +// leaves the entry file untouched. // // It degrades the way the other two do: RTK is an enhancement, so a missing cargo, a // declined install, or a failed build all end in the agent starting anyway. diff --git a/internal/cli/launch_test.go b/internal/cli/launch_test.go index da64b06..c3eef0b 100644 --- a/internal/cli/launch_test.go +++ b/internal/cli/launch_test.go @@ -133,7 +133,7 @@ func TestLaunchWrapsWithHeadroomWhenItIsThere(t *testing.T) { if cmd.Bin != "headroom" { t.Errorf("bin = %q, want headroom", cmd.Bin) } - if got, want := strings.Join(cmd.Args, " "), "wrap claude --code-memory none --no-context-tool"; got != want { + if got, want := strings.Join(cmd.Args, " "), "wrap claude --code-memory none --no-mcp --no-context-tool"; got != want { t.Errorf("args = %q, want %q", got, want) } if cmd.Harness != paths.Claude.ID { @@ -187,11 +187,11 @@ func TestLaunchNoHeadroomSkipsItEntirely(t *testing.T) { } // Headroom's wrap registers MCP servers into the user's agent config, and those -// registrations outlive the session that made them. scc asks for its own retrieve -// tool and nothing else, so a launch does not quietly install a code-memory -// server the workspace has not asked for — this workspace's code intelligence is -// CodeGraph's job. -func TestLaunchTurnsHeadroomsCodeMemoryOffByDefault(t *testing.T) { +// registrations outlive the session that made them. A bare `scc launch` wants +// nothing from Headroom but the compression proxy, so by default no MCP server +// gets registered at all — not the code-memory server (CodeGraph's job in this +// workspace) and not even Headroom's own retrieve tool. +func TestLaunchTurnsHeadroomsMCPOffByDefault(t *testing.T) { root := initWorkspace(t) isolatedPath(t, "headroom", "claude") @@ -199,11 +199,11 @@ func TestLaunchTurnsHeadroomsCodeMemoryOffByDefault(t *testing.T) { if cmd.Headroom == nil { t.Fatal("no headroom report") } - if cmd.Headroom.MCP != "retrieve" { - t.Errorf("mcp = %q, want retrieve", cmd.Headroom.MCP) + if cmd.Headroom.MCP != "none" { + t.Errorf("mcp = %q, want none", cmd.Headroom.MCP) } - if got := strings.Join(cmd.Headroom.Options, " "); !strings.Contains(got, "--code-memory none") { - t.Errorf("options = %q, want them to decline the code-memory server", got) + if got := strings.Join(cmd.Headroom.Options, " "); !strings.Contains(got, "--code-memory none") || !strings.Contains(got, "--no-mcp") { + t.Errorf("options = %q, want them to decline both the code-memory and retrieve servers", got) } } @@ -264,7 +264,7 @@ func TestLaunchPassesArgumentsThroughToTheAgent(t *testing.T) { isolatedPath(t, "headroom", "claude") cmd := launchJSON(t, "launch", "claude", "--json", "--root", root, "--", "--resume", "--model", "opus") - if got, want := strings.Join(cmd.Args, " "), "wrap claude --code-memory none --no-context-tool --resume --model opus"; got != want { + if got, want := strings.Join(cmd.Args, " "), "wrap claude --code-memory none --no-mcp --no-context-tool --resume --model opus"; got != want { t.Errorf("args = %q, want %q", got, want) } @@ -728,24 +728,38 @@ func sameDir(t *testing.T, a, b string) bool { return os.SameFile(fa, fb) } -// RTK joins the preflight the other two integrations already went through, so a -// launch reports all three. --no-rtk is the standing "not in this workspace", and it -// has to leave the field out entirely rather than report a skip: the two are -// different answers, and only one of them means somebody decided. -func TestLaunchReportsRTKAndNoRTKOmitsIt(t *testing.T) { +// RTK preflight is opt-in: a bare `scc launch` wants nothing from Headroom's +// wrap but the compression proxy, and asking scc to also wire up RTK is a +// separate decision, made with --rtk. The two runs have to differ in whether the +// field is present at all, not just in what it says: a launch that never +// considered RTK and a launch that considered it and skipped it are different +// answers, and only one of them means somebody decided. +func TestLaunchRTKIsOptIn(t *testing.T) { root := initWorkspace(t) isolatedPath(t, "claude") - var with launchCommand + var without launchCommand stdout, _, code := run(t, "launch", "--root", root, "--json") if code != ExitOK { t.Fatalf("exit = %d", code) } + if err := json.Unmarshal([]byte(stdout), &without); err != nil { + t.Fatalf("unmarshal: %v\n%s", err, stdout) + } + if without.RTK != nil { + t.Errorf("a bare launch reported %+v, want rtk left out", without.RTK) + } + + var with launchCommand + stdout, _, code = run(t, "launch", "--root", root, "--json", "--rtk") + if code != ExitOK { + t.Fatalf("exit = %d", code) + } if err := json.Unmarshal([]byte(stdout), &with); err != nil { t.Fatalf("unmarshal: %v\n%s", err, stdout) } if with.RTK == nil { - t.Fatal("a launch reported no rtk field at all") + t.Fatal("--rtk reported no rtk field at all") } if with.RTK.Install != installSkipped { t.Errorf("install = %q, want %q with cargo absent", with.RTK.Install, installSkipped) @@ -753,29 +767,18 @@ func TestLaunchReportsRTKAndNoRTKOmitsIt(t *testing.T) { if with.RTK.Reason == "" { t.Error("rtk was skipped without saying why") } - - var without launchCommand - stdout, _, code = run(t, "launch", "--root", root, "--json", "--no-rtk") - if code != ExitOK { - t.Fatalf("exit = %d", code) - } - if err := json.Unmarshal([]byte(stdout), &without); err != nil { - t.Fatalf("unmarshal: %v\n%s", err, stdout) - } - if without.RTK != nil { - t.Errorf("--no-rtk still reported %+v", without.RTK) - } } // --no-install covers RTK too. The flag says "never build anything", and a cargo -// build that takes minutes is the most expensive thing it governs. +// build that takes minutes is the most expensive thing it governs — but only +// once --rtk has asked for RTK to be considered at all. func TestLaunchNoInstallCoversRTK(t *testing.T) { root := initWorkspace(t) isolatedPath(t, "cargo", "claude") withPrompt(t, "\n") withLaunchExec(t, 0) - stdout, stderr, code := run(t, "launch", "--root", root, "--no-install") + stdout, stderr, code := run(t, "launch", "--root", root, "--rtk", "--no-install") if code != ExitOK { t.Fatalf("exit = %d", code) }