From 1fe6ea4ee47379007598e163784dbd7ac50b7f79 Mon Sep 17 00:00:00 2001 From: jochen Date: Fri, 9 Oct 2026 02:56:05 +0200 Subject: [PATCH] claude-code: start only the tool servers set at the controller's terminal A server left in the servers state could still be put there by a holder of the runner's credential; render mcp_servers alone and keep the state for listing and unregistering (hq issue 340). Waits for the laptop's server to be set at the terminal first. --- modules/claude-code/README.md | 5 ++-- .../cmd/claude-code/claude_code_test.go | 7 +++--- modules/claude-code/cmd/claude-code/main.go | 2 +- modules/claude-code/cmd/claude-code/node.go | 4 ++- .../cmd/claude-code/registrations.go | 4 +-- .../cmd/claude-code/terminal_only_test.go | 25 ++++++++++++++++--- 6 files changed, 35 insertions(+), 12 deletions(-) diff --git a/modules/claude-code/README.md b/modules/claude-code/README.md index cbdde015..462f23fa 100644 --- a/modules/claude-code/README.md +++ b/modules/claude-code/README.md @@ -46,7 +46,7 @@ must see, a node that joins later included — kept, so it carries no secret eit | what this node holds | the module's `holdings` state, one key for this node — the account, the kind, fingerprints and expiries, never a token — written at start and whenever the credentials file changes (hq ADR 0206) | | a person ran `/login` here | the credentials file gains a refresh token this module never writes; its next report shows it, and the licence manager asks `claude_code_grant` for it, giving its key — the one time a refresh token leaves the node, for the manager to adopt by refreshing it | | what this node should hold | the licence manager's `bindings` state, this node's key; on a newer generation this module asks `anthropic-licence-manager.current` for its token, sealed to the key it sends, and writes it access-token-only — so the agent here never refreshes. A node that was off reads its key when it is back | -| an MCP server registered through this module before registering was refused (hq issue 340) | a key in the module's `servers` state — `all.` for every node, `.` for one; every node watches it and renders what applies to it, a node's own entry over the one for every node. A node that joins later, or was off, reads the whole current set at start; unregistering is a delete. Nothing adds a key any more: a new tool server is the operator's, in the `mcp_servers` setting at the controller's terminal | +| an MCP server registered through this module before registering was refused (hq issue 340) | a key in the module's `servers` state — `all.` for every node, `.` for one; every node watches it and renders what applies to it, a node's own entry over the one for every node. A node that joins later, or was off, reads the whole current set at start; unregistering is a delete. Nothing adds a key any more, and no key is rendered: a tool server is the operator's, in the `mcp_servers` setting at the controller's terminal. What is left is listed and may be unregistered | | the agent's configuration (hq ADR 0216) | a key in the module's `config` state per registration — `mesh..` for every node, `node...` for one, `home...` for one account's own directory — the item and its files in one value, at most 256 KiB. Every node watches it and renders what applies to it, a node item over a mesh item of the same kind and name. A hook, settings, or an item that would run something by itself is never taken, whenever it was put there (hq issue 340) | ## The mesh's tools first (hq ADR 0245) @@ -106,7 +106,8 @@ another node. So these tools refuse, and say why and how it is done instead: reads it into the prompt. A script beside a skill is taken: it runs only when the agent runs it, through the session's own rules. -What was registered before stays: a tool server is still rendered, and may be unregistered; a hook or settings the +What was registered before: a tool server left on the bus is listed and may be unregistered, and no longer +starts in any session (set it in `mcp_servers` at the terminal first, if it is still wanted); a hook or settings the state still holds are never rendered, and `claude_code_hook_unregister` and `claude_code_settings_clear` remove them. `claude_code_registrations` lists everything registered, with who registered it, when, and whether it is rendered, beside what the operator set for this node at the terminal. Its `by` is the node whose tool runner served diff --git a/modules/claude-code/cmd/claude-code/claude_code_test.go b/modules/claude-code/cmd/claude-code/claude_code_test.go index f024c10d..d77f728a 100644 --- a/modules/claude-code/cmd/claude-code/claude_code_test.go +++ b/modules/claude-code/cmd/claude-code/claude_code_test.go @@ -359,8 +359,9 @@ func (b *bus) join(p Paths, w map[string]string) *ServerView { func noOthers() ([]string, error) { return nil, nil } -// A server registered before stays (novox/hq issue 340): every node takes what the state holds for it, a node -// that joins later reads the current set, and an unregistration reaches every node. +// A server registered before is still read from the state, to be listed and unregistered (novox/hq issue 340): +// every node takes what the state holds for it, a node that joins later reads the current set, and an +// unregistration reaches every node. None of it is rendered. func TestEveryNodeRegistrationReachesTheOthersAndALateNodeReadsIt(t *testing.T) { a, wa := node(t, "laptop") s, ws := node(t, "server") @@ -368,7 +369,7 @@ func TestEveryNodeRegistrationReachesTheOthersAndALateNodeReadsIt(t *testing.T) va := b.join(a, wa) b.join(s, ws) b.existing("all.docs", map[string]any{"type": "stdio", "command": "docs-mcp"}) - if Registered(s)["docs"] == nil || !strings.Contains(ws["managed-mcp.json"], "docs-mcp") { + if Registered(s)["docs"] == nil || strings.Contains(ws["managed-mcp.json"], "docs-mcp") { t.Fatalf("the other node did not take it: %v", Registered(s)) } late, wl := node(t, "desktop") diff --git a/modules/claude-code/cmd/claude-code/main.go b/modules/claude-code/cmd/claude-code/main.go index da36b049..12480db0 100644 --- a/modules/claude-code/cmd/claude-code/main.go +++ b/modules/claude-code/cmd/claude-code/main.go @@ -323,7 +323,7 @@ func tools(p Paths, servers ServerState, view *ServerView, config ConfigState, c return AddAPIKey(p, strings.TrimSpace(name), file, useHere, ask) }}, {Name: "claude_code_mcp_list", - Description: "The MCP servers registered through this module: those that apply on this node (beside the console, `mesh`, and those set in the module's settings), and every registration on the mesh, by key — `all.` for every node, `.` for one.", + Description: "The MCP servers registered through this module before that was refused (novox/hq issue 340): those whose key names this node, and every registration on the mesh, by key — `all.` for every node, `.` for one. None of them starts in a session any more: tool servers are set at the controller's terminal, in this module's mcp_servers setting.", Run: func(map[string]any) (any, error) { keys, err := servers.Keys() return map[string]any{"here": Registered(p), "everywhere": keys}, err diff --git a/modules/claude-code/cmd/claude-code/node.go b/modules/claude-code/cmd/claude-code/node.go index f17c9430..ed8d52f2 100644 --- a/modules/claude-code/cmd/claude-code/node.go +++ b/modules/claude-code/cmd/claude-code/node.go @@ -186,7 +186,9 @@ func RenderNow(p Paths, write WriteManaged) ([]string, error) { if mesh != nil { mesh.Refused = RefusedCounts(GuardDataOf(p, mesh).Log) } - files := RenderWithMesh(facts, settings, binding, p.helper(), Registered(p), config, mesh) + // Tool servers come from the operator's `mcp_servers` setting alone, set at the controller's terminal (novox/hq + // issue 340): one left in the `servers` state is listed and may be unregistered, and never starts in a session. + files := RenderWithMesh(facts, settings, binding, p.helper(), nil, config, mesh) tree, _ := json.Marshal(MarketplaceWith(config, guardFiles(p, mesh))) files[MarketplaceDir+"/"] = string(tree) names := make([]string, 0, len(files)) diff --git a/modules/claude-code/cmd/claude-code/registrations.go b/modules/claude-code/cmd/claude-code/registrations.go index 0e275e2a..b4baa51f 100644 --- a/modules/claude-code/cmd/claude-code/registrations.go +++ b/modules/claude-code/cmd/claude-code/registrations.go @@ -35,10 +35,10 @@ func Registrations(p Paths, servers ServerState, config ConfigState) (map[string } row := map[string]any{"entry": entry, "by": "not recorded: a tool server's registration kept only its entry"} if _, name, ok := strings.Cut(key, "."); ok { + row["rendered"] = "no: a tool server is set at the controller's terminal only, in this module's mcp_servers " + + "setting (novox/hq issue 340); unregister this one once it is there, if it is still wanted" if problem := EntryProblem(name, entry); problem != "" { row["rendered"] = "no: " + problem - } else { - row["rendered"] = "yes, on the nodes its key names" } } registered[key] = row diff --git a/modules/claude-code/cmd/claude-code/terminal_only_test.go b/modules/claude-code/cmd/claude-code/terminal_only_test.go index 2445d5a0..0a09193c 100644 --- a/modules/claude-code/cmd/claude-code/terminal_only_test.go +++ b/modules/claude-code/cmd/claude-code/terminal_only_test.go @@ -199,8 +199,8 @@ func TestWhatWasRegisteredBeforeIsListedAndOnlyTextAndServersStillRender(t *test if strings.Contains(hooks, runs) || skill || strings.Contains(w["managed-settings.json"], `"Bash"`) { t.Fatalf("%s: a registered hook, skill or setting was rendered:\n%s\n%s", when, hooks, w["managed-settings.json"]) } - if !strings.Contains(w["managed-mcp.json"], "docs-mcp") || !strings.Contains(w["CLAUDE.md"], "Commit in the imperative.") { - t.Fatalf("%s: what was registered before and is still taken was not rendered:\n%s", when, w["managed-mcp.json"]) + if strings.Contains(w["managed-mcp.json"], "docs-mcp") || !strings.Contains(w["CLAUDE.md"], "Commit in the imperative.") { + t.Fatalf("%s: a tool server from the bus was rendered, or the instructions were not:\n%s", when, w["managed-mcp.json"]) } } rendered("from the module's own file") @@ -223,7 +223,7 @@ func TestWhatWasRegisteredBeforeIsListedAndOnlyTextAndServersStillRender(t *test if ins, _ := cfg["mesh.instructions.conventions"].(map[string]any); !strings.HasPrefix(ins["rendered"].(string), "yes") { t.Errorf("the instructions: %v", ins) } - if srv, _ := srvs["laptop.docs"].(map[string]any); srv == nil || !strings.HasPrefix(srv["rendered"].(string), "yes") { + if srv, _ := srvs["laptop.docs"].(map[string]any); srv == nil || !strings.HasPrefix(srv["rendered"].(string), "no:") { t.Errorf("the tool server: %v", srvs) } @@ -524,3 +524,22 @@ func TestTheOperatorsHomeIsNotChangedThroughTheTools(t *testing.T) { t.Fatal("the agent's own rule was not removed") } } + +// A tool server is the operator's, set at the controller's terminal (novox/hq issue 340): one set there is +// rendered; one left on the bus is not, even for this node. +func TestToolServersComeFromTheTerminalAlone(t *testing.T) { + p, w := node(t, "laptop") + _ = os.WriteFile(p.Settings, []byte(`{"role":"","mcp_servers":{"set-here":{"type":"stdio","command":"set-at-terminal"}}}`), 0o600) + b := &bus{kept: map[string]map[string]any{}} + b.join(p, w) + b.existing("laptop.left", map[string]any{"type": "stdio", "command": "left-on-the-bus"}) + b.existing("all.everywhere", map[string]any{"type": "http", "url": "https://x.example/mcp"}) + if _, err := RenderNow(p, writer(w)); err != nil { + t.Fatal(err) + } + mcp := w["managed-mcp.json"] + if !strings.Contains(mcp, "set-at-terminal") || strings.Contains(mcp, "left-on-the-bus") || strings.Contains(mcp, "x.example") || + !strings.Contains(mcp, `"mesh"`) { + t.Fatalf("managed-mcp.json:\n%s", mcp) + } +}