fix(discord): row 25 SetSpark key reaches pi and the connector (#1509)

resolveToolRoots dropped tools.setspark, so the record verbs never reached
pi's --tools list and the connector never built its SetSpark client. The
resolved config now carries only baseUrl, keyFile, principal and timeoutMs,
the keys the extension's loadSetsparkConfig accepts; the extension restores
the response cap. The connector's client uses the validated binding object,
which keeps the cap. README: principal is required, timeoutMs is optional.

Test (failing first): binding -> resolveToolRoots -> JSON -> loadToolsConfig
gives the binding's config, and the verbs reach enabledToolNames. Eight
suites green on an index export. Rocko approved
(agents/rocko/work/row25-setspark-fix-review-2026-09-26.md, a28df89e).

Co-Authored-By: Claude Opus 5.5 <[email protected]>

agents/rocko/work/row25-setspark-fix-review-2026-09-26.md

@@ -0,0 +1,59 @@
+# Row 25 SetSpark configuration fix review
+
+Verdict: **approve**. Rocko for Sage, 2026-09-26.
+
+Packet SHA-256:
+`4dec189844f0f77f3fcfca1bf97731f015f41535e97d0b6a47635311900ba7a5`.
+The complete staged diff hashes to that same value. The four candidate
+working files have no unstaged difference from the index.
+
+No blocking findings.
+
+1. **The trimmed object is correct for the extension boundary.**
+   resolveToolRoots now emits exactly baseUrl, keyFile, principal and
+   timeoutMs. cli.mjs JSON-serializes toolRoots into TOOLS_ENV. The actual
+   extension at packages/discord/extension/tools.mjs parses that JSON,
+   calls loadToolsConfig and only then creates the tool set.
+   loadSetsparkConfig restores the fixed 262144-byte cap. enabledToolNames
+   only needs SetSpark presence before this validation; it does not consume
+   the omitted cap. The check display only consumes baseUrl/principal.
+   Other inspected SetSpark consumers use the validated binding or
+   extension config, not the trimmed wire object.
+
+2. **The connector retains the response cap.** cli.mjs now constructs
+   createSetsparkApi from binding.tools.setspark, the full validated object.
+   That API delegates to callApi, whose response-size comparison therefore
+   receives maxResponseBytes. The factory itself performs no key read or
+   HTTP call. Principal use in connector/context still comes from the
+   validated binding. Absent SetSpark produces no API and no record verbs.
+
+3. **SetSpark check-time validation reads metadata only.**
+   loadSetsparkConfig calls checkPrivateFile, which uses lstatSync to check
+   symlink, regular-file, mode and nonzero size. Key contents are read in
+   readKey, reached by callApi at request time, not by check or preparation.
+   Check prints service origin and principal, not the key contents. Failure
+   diagnostics can name the key path; that is distinct from printing the
+   secret. The existing check command DOES read the separate Discord bot
+   token and contact Discord; this approval's metadata-only statement is
+   specifically about the SetSpark key. I did not run a live check or read
+   either credential.
+
+4. **The regression test proves the repaired serialization path.**
+   It starts from validateBinding, passes through resolveToolRoots and a
+   real JSON round trip into loadToolsConfig, asserts every SetSpark verb,
+   deep-compares the reconstructed config including the cap, and checks
+   absence behavior. This catches both dropping the field and passing the
+   internal cap into the strict input schema. It does not execute CLI
+   startup or directly regression-test the CLI factory argument; that
+   one-line wiring is verified by source inspection here. That test scope
+   is sufficient for this small fix and is not a full live-service claim.
+
+Independent verification:
+`timeout 40s node --test packages/discord/tests/setspark.test.mjs`
+returned 12 passed, zero failed, exit 0. Sage's eight-suite index-export
+results remain author evidence, not an independent rerun. README now
+matches principal being required and timeoutMs being optional.
+
+Only this report was written. No source/index edits, commit, restart,
+credential access or external service call. Approval applies to the pinned
+diff; Sage owns integration and the authorized restart.

packages/discord/README.md

@@ -139,7 +139,7 @@ is `src/binding.mjs`.
 | `engine` | `provider`, `model`, `thinking` for pi |
 | `limits` | `turnsPerDay` (200), `turnTimeoutSeconds` (180), `replyChunkChars` (1900), `inboundMaxChars` (4000) |
 | `context.files[]` | files appended to pi's system prompt in order, repository-relative and inside the repository (no absolute paths, `..` or symlinks); the Discord block is added after them |
-| `tools` | optional. `roots[]` of `{name, path, write?, git?}`: absolute directories the seat may read through `list_dir`, `read_file` and `search`; a root with `"write": true` may also be written through `write_file` and `edit_file`; a writable root that is a git work tree may carry `git` `{branch, identity, tokenFile, author, protocol?}` and gains `git_status`, `git_commit`, `git_pull` and `git_push` (`protocol: "vault"` adds `reserve_id`); `maxFileBytes` (262144), `maxCallsPerTurn` (8); `web` (optional) `{searxng, maxFetchBytes}` enables `web_fetch` and `web_search` through the named SearXNG instance (https, or http on loopback; `maxFetchBytes` 1048576); `setspark` (optional) `{baseUrl, keyFile, principal?}` names the SetSpark record service (https origin, or http on loopback; key file absolute, 0600, read per call, never printed) and turns on connector-verified approvals. Absent means no tools and a pi launch with `--no-tools`. A root may not be `/`, the home directory, a symlink, a path with a dot-prefixed segment, or anything inside or above the data root |
+| `tools` | optional. `roots[]` of `{name, path, write?, git?}`: absolute directories the seat may read through `list_dir`, `read_file` and `search`; a root with `"write": true` may also be written through `write_file` and `edit_file`; a writable root that is a git work tree may carry `git` `{branch, identity, tokenFile, author, protocol?}` and gains `git_status`, `git_commit`, `git_pull` and `git_push` (`protocol: "vault"` adds `reserve_id`); `maxFileBytes` (262144), `maxCallsPerTurn` (8); `web` (optional) `{searxng, maxFetchBytes}` enables `web_fetch` and `web_search` through the named SearXNG instance (https, or http on loopback; `maxFetchBytes` 1048576); `setspark` (optional) `{baseUrl, keyFile, principal, timeoutMs?}` names the SetSpark record service (https origin, or http on loopback; key file absolute, 0600, read per call, never printed) and turns on connector-verified approvals. Absent means no tools and a pi launch with `--no-tools`. A root may not be `/`, the home directory, a symlink, a path with a dot-prefixed segment, or anything inside or above the data root |
 
 Unknown keys, missing fields, wrong types, empty allowlists, a user channel
 that is not listed and a bot listed as a user all refuse with exit 2. A

packages/discord/src/binding.mjs

@@ -360,5 +360,8 @@ export function resolveToolRoots(binding, { dataRoot }) {
     if (real === data || real.startsWith(data + sep) || data.startsWith(real + sep)) throw new DiscordError(`tool root ${r.name} overlaps the data root: ${r.path}`);
     return { name: r.name, path: real, write: r.write, ...(r.git ? { git: { branch: r.git.branch, identity: r.git.identity, tokenFile: r.git.tokenFile, author: `${r.git.author.name} <${r.git.author.email}>`, ...(r.git.protocol ? { protocol: r.git.protocol } : {}) } } : {}) };
   });
-  return { roots, maxFileBytes: binding.tools.maxFileBytes, maxCallsPerTurn: binding.tools.maxCallsPerTurn, ...(binding.tools.web ? { web: { searxng: binding.tools.web.searxng, maxFetchBytes: binding.tools.web.maxFetchBytes } } : {}) };
+  // setspark carries only the keys loadSetsparkConfig accepts; the extension
+  // re-validates it and adds the response cap itself.
+  const ss = binding.tools.setspark;
+  return { roots, maxFileBytes: binding.tools.maxFileBytes, maxCallsPerTurn: binding.tools.maxCallsPerTurn, ...(binding.tools.web ? { web: { searxng: binding.tools.web.searxng, maxFetchBytes: binding.tools.web.maxFetchBytes } } : {}), ...(ss ? { setspark: { baseUrl: ss.baseUrl, keyFile: ss.keyFile, principal: ss.principal, timeoutMs: ss.timeoutMs } } : {}) };
 }

packages/discord/src/cli.mjs

@@ -214,7 +214,7 @@ async function run(opts) {
     // The connector's own SetSpark client (bind and approvals) uses the same
     // key as the model's verbs; without a setspark key it has none, and an
     // approval request from the model is refused.
-    const api = toolRoots && toolRoots.setspark ? createSetsparkApi(toolRoots.setspark) : null;
+    const api = binding.tools && binding.tools.setspark ? createSetsparkApi(binding.tools.setspark) : null;
     const connector = createConnector({ binding, journalDir, rest, gateway, engine, api, log: warn });
     let shuttingDown = false;
     let exitCode = 0;

packages/discord/tests/setspark.test.mjs

@@ -12,7 +12,7 @@ import {
   loadSetsparkConfig, readKey, idempotencyKey, connectorKey, callApi, renderRefusal, createSetsparkApi, renderRecord,
 } from "../src/setspark.mjs";
 import { loadToolsConfig, createToolSet, enabledToolNames } from "../src/tools.mjs";
-import { validateBinding } from "../src/binding.mjs";
+import { validateBinding, resolveToolRoots } from "../src/binding.mjs";
 import { makeRoot, rawBinding } from "./helpers.mjs";
 
 const KEY_A = "ssk_" + "a".repeat(40);
@@ -120,6 +120,24 @@ test("setspark config: reaches the tools config and the binding as a fixed key",
   assert.throws(() => validateBinding(rawBinding({ tools: { roots: [{ name: "docs", path: docs }], setspark: { baseUrl: "https://api.setspark.io", keyFile: kf, principal: "sage", nope: 1 } } })), /unknown key/);
 });
 
+test("setspark config: the binding's key survives resolveToolRoots and the engine's JSON hand-off to the extension", () => {
+  const root = makeRoot();
+  const docs = join(root, "docs");
+  mkdirSync(docs);
+  const dataRoot = join(root, "data");
+  mkdirSync(dataRoot);
+  const kf = keyFile(root);
+  const b = validateBinding(rawBinding({ tools: { roots: [{ name: "docs", path: docs }], setspark: { baseUrl: "https://api.setspark.io", keyFile: kf, principal: "sage", timeoutMs: 5000 } } }));
+  const resolved = resolveToolRoots(b, { dataRoot });
+  assert.deepEqual(resolved.setspark, { baseUrl: "https://api.setspark.io", keyFile: kf, principal: "sage", timeoutMs: 5000 });
+  for (const name of SETSPARK_TOOL_NAMES) assert.ok(enabledToolNames(resolved).includes(name), `${name} reaches pi's --tools list`);
+  const ext = loadToolsConfig(JSON.parse(JSON.stringify(resolved)));
+  assert.deepEqual(ext.setspark, b.tools.setspark, "the extension rebuilds the same config, response cap included");
+  const plain = resolveToolRoots(validateBinding(rawBinding({ tools: { roots: [{ name: "docs", path: docs }] } })), { dataRoot });
+  assert.equal("setspark" in plain, false);
+  assert.ok(!enabledToolNames(plain).some((n) => SETSPARK_TOOL_NAMES.includes(n)));
+});
+
 test("setspark keys: read per call, one printable token per file, rotation without a restart", async () => {
   const root = makeRoot();
   const c = config(root);