diff --git a/agents/rocko/work/row25-approver-names-r2-review-2026-09-26.md b/agents/rocko/work/row25-approver-names-r2-review-2026-09-26.md new file mode 100644 index 00000000..0e87e3f1 --- /dev/null +++ b/agents/rocko/work/row25-approver-names-r2-review-2026-09-26.md @@ -0,0 +1,78 @@ +# Row 25 approver names R2 — review + +Verdict: **revise**, one remaining medium finding. Rocko, 2026-09-26. + +Packet and staged diff verified as +`7da44a5975e6261dc524431df2a3d3f9093e064d26d67e9b1f5926467d9c00f9`. + +## 1. Medium — two error paths still cut identities before sanitization + +The fix correctly hides service messages before their 400-character cut, +and hides successful output deeply before its field/render limits. But: + +- callApi still takes `err.code.slice(0, 64)` before renderRefusal hides + identities (setspark.mjs line 265). +- needObject and record_list's filter-name refusal still interpolate + `k.slice(0, 32)` before renderRefusal (lines 342 and 427). + +**Executed reproductions:** with synthetic id `100000000000000100`: + +1. A local fake service returns HTTP 422 with code consisting of 49 x's, + a space and the id. The 64-character cut retains 14 digits. Final text: + +```text +refused: the record service refused the request (code xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx 10000000000000) +refused +``` + +2. record_create receives an invalid property name consisting of 17 x's, + a space and the id. No request is sent. Final refusal: + +```text +refused: the call's arguments are not valid +record has a property name that is not allowed: xxxxxxxxxxxxxxxxx 10000000000000 +``` + +The filter-name diagnostic has the same cut-before-hide pattern. These +14-digit fragments survive because hideIds correctly targets complete +17–20 digit tokens. This is the same truncation problem R2 explicitly fixes +for service messages and tests for 12–16 digit remnants; the other early +cuts were missed. + +**Fix:** sanitize the service code before its length cap, while retaining +any raw code needed privately for reason classification. For invalid +property/filter names, prefer a fixed or position-based diagnostic that +does not echo the invalid value, as already done for approvers. Alternatively +sanitize the complete value before slicing. Add regression cases for both +code and property/filter-name cut boundaries. Do not widen the final regex +to redact arbitrary shorter numbers; fix the operation order. + +## Previous findings and requested checks + +The rejected-approver echo is closed. All eight success renderers share the +new boundary, including resolve/document output. Embedded mentions, nested +keys/values and numeric values are handled before successful output is cut. +The deliberate longer-token exception is documented; this is a text-format +rule, not semantic recognition of every possible identity encoding. + +The contract fixture now connects name-based create, stored discord:, +bare-id approval view and validateRequest. It also retains the malformed +legacy-name refusal. The raw connector request remains intact; sanitization +operates on the rendered copy. That separation is correct. Existing bad +records still need a separately authorized correction. + +The restart requirement for changed name/id mappings remains acceptable: +Pi's map is fixed at startup and hot-reloading only the connector would +split authority views. Channel changes remain reloadable. + +## Verification + +`timeout 30s node --test packages/discord/tests/setspark.test.mjs`: +16 passed, zero failed, exit 0. Both additional failures above were executed +independently, using a disposable local HTTP server and fake key for the +service case and a direct pre-send refusal for the property-name case. +Temporary fixtures were removed. No live token or service was accessed. + +Only this report was written in the repository. No source/index edits, +commit, push or restart. This remaining finding should be fixed before the +approval requested for R2. diff --git a/agents/rocko/work/row25-approver-names-r3-review-2026-09-26.md b/agents/rocko/work/row25-approver-names-r3-review-2026-09-26.md new file mode 100644 index 00000000..c0aca503 --- /dev/null +++ b/agents/rocko/work/row25-approver-names-r3-review-2026-09-26.md @@ -0,0 +1,40 @@ +# Row 25 approver names R3 — review + +Verdict: **approve**. Rocko for Sage, 2026-09-26. + +Packet and complete staged diff both verified as +`81379830715f48ff3c8e8dac1c1556925aa6b599f5ffc8a0ae231eb7a3b1c2a9`. +The reviewed source/test working files match the index. Compared R3 with +the pinned R2 packet: changes are limited to the three truncation fixes +and their test fixtures/assertions. + +The remaining R2 finding is closed. Service error codes now pass through +hideIds before the 64-character limit. Invalid record-property and filter +names pass through it before the 32-character limit. Their null config +correctly yields unknown user rather than echoing an identity. The final +render/refusal passes remain, and the regex was not broadened to hide +arbitrary shorter numbers. The new tests exercise all three early-cut +paths and check the resulting tool text for full ids and digit remnants. + +The other slice sites do not reopen this finding: list/match slices limit +array counts; successful text limits follow hideDeep; service message cuts +follow hideIds. The approver-name config validation diagnostic is a startup +configuration error, not a model-facing tool return. The documented +longer-token exception remains the scope of the text-hiding rule. + +The previous name mapping, fixed-config reload restriction, raw internal +approval request and connected service-contract fixture remain acceptable. +This approval does not repair existing malformed records automatically or +prove a live service round trip; Jason's planned acceptance check supplies +that evidence after the authorized integration/restart. + +Independent verification: +`timeout 30s node --test packages/discord/tests/setspark.test.mjs` +returned 16 passed, zero failed, exit 0. This includes boundary and approval +contract fixtures. Sage's 173-test run, index-export suites and live check +are author evidence, not independent reruns. The disclosed Docker pool +failure and cleanup do not change the candidate reviewed here. + +Only this report was written. No source/index edits, commit, push, live +credential access, Docker action or restart. Approval applies to the pinned +R3 diff; Sage owns the authorized integration and restart. diff --git a/agents/rocko/work/row25-approver-names-review-2026-09-26.md b/agents/rocko/work/row25-approver-names-review-2026-09-26.md new file mode 100644 index 00000000..ea028ee2 --- /dev/null +++ b/agents/rocko/work/row25-approver-names-review-2026-09-26.md @@ -0,0 +1,104 @@ +# Row 25 approver names fix — review + +Verdict: **revise**. Rocko for Sage, 2026-09-26. + +Packet and complete staged diff both verified as: +`c17e15cd2a7e398e72a1d4c5e69e35f42255facb621bde7aa786163a4f00c9ed`. +The name-to-identity write mapping addresses the reported malformed +approver failure, but the stated model-visible ID boundary is incomplete. + +## 1. Medium — rejected raw IDs are echoed back to the model + +`approverIds` embeds the rejected value in the bad-argument message. +`renderRefusal` prints that message. Executed with a synthetic id: + +```text +refused: the call's arguments are not valid +required_approvers: "discord:100000000000000100" is not a known user; use names from: jason +``` + +No request or key read is needed for this reproduction. The existing raw-id +test asserts only `ok === false`, not that the returned text omits the id. +Server refusal messages, codes and changed_fields are also rendered without +this new mapping, so a service rejection that echoes an approver can expose +it even when the model supplied only a name. + +**Fix:** never echo a raw identity in the invalid-name diagnostic; provide +known names and a fixed reason. Apply the intended model-facing identity +policy to all rendered refusal fields, including service error text. Test +the final tool text, not only the verb's rejection status. + +## 2. Medium — namesFor matches whole string values only, and only two verbs use it + +The anchored regex converts exact bare or discord-prefixed string values. +It does not remove an id inside prose, a Discord mention, an object key, +or a numeric JSON value. A local fake service returned a record containing +an exact required_approvers value plus nested text and a body mention. +`record_get` produced: + +```text +nested: {"text":"approver discord:100000000000000100"} +required_approvers: jason +body: +Ask <@100000000000000100> +``` + +Thus the happy-path assertion for the simple required_approvers field does +not establish “no Discord id reaches the model.” `resolve_id` renders match +titles without mapping, and create_document renders title/URL without it. +Approval view rendering also interpolates service fields directly. Record +create/update return terse receipts, so their unconverted raw records are +not themselves proof of a current approver-list text leak; assess the final +rendered surface instead of merely the intermediate object. + +**Fix:** centralize model-facing rendering/sanitization across all SetSpark +success and refusal text, with a defined rule for known identities, +unknown identities and embedded Discord mentions/prefixed values. Cover +nested fields and keys, numeric identity values, embedded text, resolve +results, document receipts and service errors in boundary tests. If the +intended promise is narrower than every Discord id in arbitrary service +text, state that narrower promise explicitly instead of claiming complete +ID concealment. Do not sanitize the connector's internal approval request +ids into names: that would break its authorization check. + +## Requested checks that are fine + +**Reload refusal is acceptable.** The derived map is part of tools, which +Pi receives once at startup. Allowing name/id edits only on the connector +side would split the identity views. Refusing those reloads until restart +is conservative and documented; channel-only changes remain reloadable. +JSON equality can also refuse user reordering or equivalent map insertion +orders, and case-only normalized names may compare unchanged. Those are +availability/normalization details, not authority widening. An urgent user +removal requires an operator stop/restart under this policy, not an assumed +successful hot reload. + +**Keep the bare-id connector path.** Under the stated service contract, +create/update sends discord: in record properties and the service's +approval-request view returns bare snowflakes. open_approval_request copies +those into request.approvers; setsparkDetails carries that request through +tool details to the engine/connector, and validateRequest requires 1..16 +unique bare snowflakes. That is the correct internal representation. +Model-facing prose must be handled separately. If the service returns +names or prefixed ids instead, refusing is correct; do not weaken +validateRequest to make malformed views pass. + +The added tests prove name mapping, rejection before requests, round-trip +configuration and reload restrictions. They do not prove the whole +create-decision → service view → connector validation/button path against +the service normalization contract. Add a local contract fixture connecting +those stages, while retaining malformed-view negatives. Existing malformed +records such as DEC-009 will not be repaired automatically by this code; +a separately authorized update can now express their approvers as names. + +## Evidence and boundaries + +Independent `timeout 30s node --test packages/discord/tests/setspark.test.mjs`: +14 passed, zero failed, exit 0. The two leaks above were independently +executed with synthetic ids and a temporary local HTTP fixture. Only a +throwaway fake key was read; no live credential, service or pane was used. +The temporary fixture was removed. Staged diff hash remained unchanged. + +Only this report was written in the repository. No source/index edits, +commit, push or restart. The approval/restart gate should remain held for +these output-boundary fixes or an explicit narrower requirement. diff --git a/agents/sage/work/row25-approver-names-r2.diff b/agents/sage/work/row25-approver-names-r2.diff new file mode 100644 index 00000000..3fc3b0bb --- /dev/null +++ b/agents/sage/work/row25-approver-names-r2.diff @@ -0,0 +1,436 @@ +diff --git a/packages/discord/README.md b/packages/discord/README.md +index 4703fa86..c84b6afe 100644 +--- a/packages/discord/README.md ++++ b/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, 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 | ++| `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 (approver names come from `users`, lower-cased; a decision's `required_approvers` are written as `discord:` from those names; SetSpark tool text and refusals never carry a Discord user id: a mention, a `discord:` value or a standalone 17 to 20 digit run reads as the user's name or `unknown user`, while the connector's approval request keeps the bare ids; with `setspark` set, a change to a user's id or name refuses the reload, since pi's approvers are fixed at start) (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 +diff --git a/packages/discord/src/binding.mjs b/packages/discord/src/binding.mjs +index c9349b81..19ed0a5d 100644 +--- a/packages/discord/src/binding.mjs ++++ b/packages/discord/src/binding.mjs +@@ -217,7 +217,7 @@ export function validateBinding(raw, where = "binding") { + maxFileBytes: requireInteger(mergedTools, "maxFileBytes", `${where}.tools`, { min: 1024, max: 4 * 1024 * 1024 }), + maxCallsPerTurn: requireInteger(mergedTools, "maxCallsPerTurn", `${where}.tools`, { min: 1, max: 64 }), + web: raw.tools.web === undefined ? null : webConfig(raw.tools.web, `${where}.tools.web`), +- setspark: raw.tools.setspark === undefined ? null : setsparkConfig(raw.tools.setspark, `${where}.tools.setspark`), ++ setspark: raw.tools.setspark === undefined ? null : setsparkConfig(raw.tools.setspark, `${where}.tools.setspark`, users), + }); + } + +@@ -328,9 +328,16 @@ export function resolveContextFiles(binding, repo) { + // Tool roots must exist as real directories on this host, not symlinks, and + // must not sit inside the data root (bindings, tokens, journals) or contain + // it. Returns the resolved config the engine hands the extension. +-function setsparkConfig(raw, where) { ++function setsparkConfig(raw, where, users) { ++ if (raw !== null && typeof raw === "object" && Object.hasOwn(raw, "approvers")) throw new DiscordError(`${where}.approvers: not allowed; approvers come from users`); ++ const approvers = {}; ++ for (const u of users) { ++ const name = u.name.trim().toLowerCase(); ++ if (Object.hasOwn(approvers, name)) throw new DiscordError(`${where}: two users named ${name}; approver names must be distinct`); ++ approvers[name] = u.id; ++ } + try { +- return loadSetsparkConfig(raw, where); ++ return loadSetsparkConfig(raw !== null && typeof raw === "object" && !Array.isArray(raw) ? { ...raw, approvers } : raw, where); + } catch (err) { + throw new DiscordError(err.message); + } +@@ -363,5 +370,5 @@ export function resolveToolRoots(binding, { dataRoot }) { + // 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 } } : {}) }; ++ 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, approvers: { ...ss.approvers } } } : {}) }; + } +diff --git a/packages/discord/src/setspark.mjs b/packages/discord/src/setspark.mjs +index e33c84b3..69e4d010 100644 +--- a/packages/discord/src/setspark.mjs ++++ b/packages/discord/src/setspark.mjs +@@ -69,7 +69,7 @@ const isObject = (v) => v !== null && typeof v === "object" && !Array.isArray(v) + export function loadSetsparkConfig(raw, where = "setspark") { + if (!isObject(raw)) throw new Error(`${where}: not an object`); + for (const k of Object.keys(raw)) { +- if (!["baseUrl", "keyFile", "principal", "timeoutMs"].includes(k)) throw new Error(`${where}: unknown key ${JSON.stringify(k)}`); ++ if (!["baseUrl", "keyFile", "principal", "timeoutMs", "approvers"].includes(k)) throw new Error(`${where}: unknown key ${JSON.stringify(k)}`); + } + if (typeof raw.baseUrl !== "string") throw new Error(`${where}.baseUrl: must be a url string`); + let u; +@@ -86,7 +86,21 @@ export function loadSetsparkConfig(raw, where = "setspark") { + if (typeof raw.principal !== "string" || !PRINCIPAL.test(raw.principal)) throw new Error(`${where}.principal: must match ${PRINCIPAL}`); + const timeoutMs = raw.timeoutMs === undefined ? SETSPARK_DEFAULTS.timeoutMs : raw.timeoutMs; + if (!Number.isInteger(timeoutMs) || timeoutMs < 1000 || timeoutMs > 60000) throw new Error(`${where}.timeoutMs: must be an integer between 1000 and 60000`); +- return Object.freeze({ baseUrl: u.origin, keyFile: raw.keyFile, principal: raw.principal, timeoutMs, maxResponseBytes: SETSPARK_DEFAULTS.maxResponseBytes }); ++ const approvers = loadApprovers(raw.approvers === undefined ? {} : raw.approvers, `${where}.approvers`); ++ return Object.freeze({ baseUrl: u.origin, keyFile: raw.keyFile, principal: raw.principal, timeoutMs, maxResponseBytes: SETSPARK_DEFAULTS.maxResponseBytes, approvers }); ++} ++ ++// Lower-case user name to Discord user id. The binding derives it from its ++// users; the verbs turn a decision's required_approvers from names into ++// `discord:` and back, so the model never handles an id. ++function loadApprovers(raw, where) { ++ if (!isObject(raw) || Object.keys(raw).length > 64) throw new Error(`${where}: must be an object of at most 64 names`); ++ for (const [name, id] of Object.entries(raw)) { ++ if (name.length === 0 || name.length > 64 || name !== name.trim().toLowerCase() || /[\u0000-\u001f\u007f]/.test(name)) throw new Error(`${where}: ${JSON.stringify(name.slice(0, 64))} must be a trimmed lower-case name`); ++ if (typeof id !== "string" || !SNOWFLAKE.test(id)) throw new Error(`${where}.${name}: must be a Discord user id`); ++ } ++ if (new Set(Object.values(raw)).size !== Object.keys(raw).length) throw new Error(`${where}: one Discord user id under two names`); ++ return Object.freeze({ ...raw }); + } + + function checkPrivateFile(path, what) { +@@ -249,7 +263,7 @@ export async function callApi(config, { method, path, body, idempotencyKey: key + } + const err = isObject(json) ? json : {}; + const code = typeof err.code === "string" ? err.code.slice(0, 64) : null; +- const extra = { status, code, message: cutMessage(err.message) }; ++ const extra = { status, code, message: cutMessage(typeof err.message === "string" ? hideIds(config, err.message) : err.message) }; + if (status === 409) { + if (err.current_revision !== undefined) extra.currentRevision = err.current_revision; + if (Array.isArray(err.changed_fields)) extra.changedFields = err.changed_fields.filter((f) => typeof f === "string").slice(0, 32); +@@ -263,14 +277,15 @@ export async function callApi(config, { method, path, body, idempotencyKey: key + } + + // How a refusal reads to the model and in the turn record: the fixed +-// reason, the code, and on 409 the fields that changed. Never the raw body. +-export function renderRefusal(err) { ++// reason, the code, and on 409 the fields that changed. Never the raw body, ++// and never a Discord user id (hideIds). ++export function renderRefusal(err, config = null) { + let s = `refused: ${err.reason}`; + if (err.code) s += ` (code ${err.code})`; + if (err.currentRevision !== undefined) s += `; current revision ${err.currentRevision}`; + if (Array.isArray(err.changedFields) && err.changedFields.length > 0) s += `; changed: ${err.changedFields.join(", ")}`; + if (err.message && err.message !== err.reason) s += `\n${err.message}`; +- return s; ++ return hideIds(config, s); + } + + // --- the verbs (contract: shared-signals stack/api/openapi.json at a5425a2) --- +@@ -354,6 +369,52 @@ function record(body) { + return isObject(body) ? body : {}; + } + ++// A decision's required_approvers as the model gives them: names of the ++// binding's users. Each becomes `discord:`; anything else is refused ++// before a request, since the service checks an approval's author against ++// these values and a stored name could never be approved. ++function approverIds(config, v) { ++ const map = config.approvers || {}; ++ const names = Object.keys(map).join(", ") || "(none)"; ++ if (!Array.isArray(v) || v.length === 0 || v.length > 16) throw bad(`required_approvers must be a list of 1 to 16 names; use names from: ${names}`); ++ // The refusal names the entry's position, never its value: a model that ++ // wrote an id must not get it echoed back. ++ const ids = v.map((a, i) => { ++ const k = typeof a === "string" ? a.trim().replace(/^@/, "").toLowerCase() : ""; ++ if (k.length > 0 && Object.hasOwn(map, k)) return `discord:${map[k]}`; ++ throw bad(`required_approvers: entry ${i + 1} is not a known user name; use names from: ${names}`); ++ }); ++ if (new Set(ids).size !== ids.length) throw bad("required_approvers names the same person twice"); ++ return ids; ++} ++ ++function withApprovers(config, props) { ++ return props.required_approvers === undefined ? props : { ...props, required_approvers: approverIds(config, props.required_approvers) }; ++} ++ ++// The way back, for everything the model reads: a Discord user id never ++// appears in SetSpark tool text. A mention (<@id>), a `discord:` value or a ++// 17 to 20 digit run standing alone becomes the binding's name for that id, ++// or "unknown user". A digit run inside a longer token (a hex digest, SS-027) ++// is left alone. The connector's own request keeps the bare ids; only the ++// rendered text and refusals go through this. ++const DISCORD_ID_TEXT = /<@!?([0-9]{17,20})>|(? [id, n])); ++ return String(text).replace(DISCORD_ID_TEXT, (_, a, b) => byId.get(a || b) || "unknown user"); ++} ++ ++// The same rule over a service value before it is rendered, so a field cut ++// at its length limit cannot leave part of an id. Keys and big integers too. ++function hideDeep(config, v) { ++ if (typeof v === "string") return hideIds(config, v); ++ if (typeof v === "number") return /^[0-9]{17,20}$/.test(String(v)) ? hideIds(config, String(v)) : v; ++ if (Array.isArray(v)) return v.map((x) => hideDeep(config, x)); ++ if (isObject(v)) return Object.fromEntries(Object.entries(v).map(([k, x]) => [hideIds(config, k), hideDeep(config, x)])); ++ return v; ++} ++ + export const setsparkVerbs = Object.freeze({ + async record_list(config, params, state, deps) { + const type = needType(params); +@@ -379,7 +440,7 @@ export const setsparkVerbs = Object.freeze({ + }, + async record_create(config, params, state, deps) { + const type = needType(params); +- const rec = needObject(params, "record"); ++ const rec = withApprovers(config, needObject(params, "record")); + if (typeof rec.title !== "string" || rec.title.trim().length === 0) throw bad("record.title is required"); + const r = await write(config, state, "POST", "/v1/records", { record_type: type, record: rec }, deps); + return { verb: "record_create", key: r.key, recordType: type, record: record(r.body) }; +@@ -387,7 +448,7 @@ export const setsparkVerbs = Object.freeze({ + async record_update(config, params, state, deps) { + const id = needId(params); + const revision = needInt(params, "revision", 1, 1000000000); +- const fields = needObject(params, "fields"); ++ const fields = withApprovers(config, needObject(params, "fields")); + if (Object.keys(fields).length === 0) throw bad("fields must name at least one property"); + const r = await write(config, state, "PATCH", `/v1/records/${id}`, { revision, fields }, deps); + return { verb: "record_update", key: r.key, id, from: revision, record: record(r.body) }; +@@ -476,7 +537,13 @@ function renderView(v) { + return `request ${scalar(v.request_id)} for ${scalar(v.decision_id)} version ${scalar(v.proposal_version)}: ${scalar(v.state) || "?"}; ${approvals.length} of ${who} approvals recorded${v.message_id ? "; bound to a Discord message" : "; no message bound yet"}`; + } + +-export function renderSetspark(name, out) { ++// Tool text for the model. `out` is hidden (hideDeep) before any field is ++// cut, and the whole text once more after. ++export function renderSetspark(name, out, config = null) { ++ return hideIds(config, renderOut(name, hideDeep(config, out))); ++} ++ ++function renderOut(name, out) { + if (name === "record_list") { + const body = out.items.map(summary).join("\n"); + return `${out.items.length} ${out.recordType} record(s) from offset ${out.offset} (limit ${out.limit})\n${body || "(none)"}`; +@@ -520,7 +587,7 @@ export const SETSPARK_TOOL_DESCRIPTIONS = Object.freeze({ + }, + record_create: { + label: "Create record", +- description: "Create one SetSpark record; the service allocates the id. Give record_type and the record's properties (title required). Only when the user asked for a record to be created.", ++ description: "Create one SetSpark record; the service allocates the id. Give record_type and the record's properties (title required). A decision's required_approvers is a list of user names (such as jason); the connector turns each into that user's Discord identity and refuses a name it does not know. Only when the user asked for a record to be created.", + snippet: "record_create creates one SetSpark record", + }, + record_update: { +diff --git a/packages/discord/src/tools.mjs b/packages/discord/src/tools.mjs +index c66365be..5596101b 100644 +--- a/packages/discord/src/tools.mjs ++++ b/packages/discord/src/tools.mjs +@@ -567,8 +567,8 @@ const TOOL_FNS = Object.freeze({ + }); + const SETSPARK_SET = new Set(SETSPARK_TOOL_NAMES); + +-function render(name, out) { +- if (SETSPARK_SET.has(name)) return renderSetspark(name, out); ++function render(name, out, config) { ++ if (SETSPARK_SET.has(name)) return renderSetspark(name, out, config.setspark); + if (name === "list_dir") { + const head = `${out.root}/${out.path}`.replace(/\/$/, ""); + const body = out.entries.map((e) => (e.type === "dir" ? `${e.name}/` : `${e.name} (${e.bytes} bytes)`)).join("\n"); +@@ -648,11 +648,11 @@ export function createToolSet(config) { + : name === "git_status" ? { branch: out.branch } + : name === "reserve_id" ? { id: out.id } + : { path: out.path }; +- return { ok: true, text: render(name, out), details: { ...base, ok: true, ...extra, ...(bytes === undefined ? {} : { bytes }), ms: Date.now() - t0 } }; ++ return { ok: true, text: render(name, out, config), details: { ...base, ok: true, ...extra, ...(bytes === undefined ? {} : { bytes }), ms: Date.now() - t0 } }; + }; + const refused = (err) => { + if (err instanceof SetsparkRefusal) { +- return { ok: false, text: renderSetsparkRefusal(err), details: { ...base, ok: false, reason: err.reason, ...(err.code ? { code: err.code } : {}), ...(err.status ? { status: err.status } : {}), ms: Date.now() - t0 } }; ++ return { ok: false, text: renderSetsparkRefusal(err, config.setspark), details: { ...base, ok: false, reason: err.reason, ...(err.code ? { code: err.code } : {}), ...(err.status ? { status: err.status } : {}), ms: Date.now() - t0 } }; + } + if (!(err instanceof Refusal) && !(err instanceof WebRefusal) && !(err instanceof GitRefusal)) throw err; + const reason = err instanceof GitRefusal ? err.message : err.reason; +diff --git a/packages/discord/tests/setspark.test.mjs b/packages/discord/tests/setspark.test.mjs +index f88dfb23..609e1545 100644 +--- a/packages/discord/tests/setspark.test.mjs ++++ b/packages/discord/tests/setspark.test.mjs +@@ -9,10 +9,11 @@ import { chmodSync, mkdirSync, symlinkSync, writeFileSync } from "node:fs"; + import { join } from "node:path"; + import { + SETSPARK_REFUSAL, SetsparkRefusal, IDEMPOTENCY_HEADER, MESSAGE_MAX_CHARS, USER_AGENT, SETSPARK_TOOL_NAMES, LIST_MAX, +- loadSetsparkConfig, readKey, idempotencyKey, connectorKey, callApi, renderRefusal, createSetsparkApi, renderRecord, ++ loadSetsparkConfig, readKey, idempotencyKey, connectorKey, callApi, renderRefusal, createSetsparkApi, renderRecord, hideIds, + } from "../src/setspark.mjs"; + import { loadToolsConfig, createToolSet, enabledToolNames } from "../src/tools.mjs"; +-import { validateBinding, resolveToolRoots } from "../src/binding.mjs"; ++import { validateBinding, resolveToolRoots, reloadDiff } from "../src/binding.mjs"; ++import { validateRequest } from "../src/approvals.mjs"; + import { makeRoot, rawBinding } from "./helpers.mjs"; + + const KEY_A = "ssk_" + "a".repeat(40); +@@ -46,6 +47,14 @@ const server = createServer((req, res) => { + if (path === "/v1/records" && req.method === "GET") return json(200, { record_type: "work_item", items: [{ id: "WI-7", record_type: "work_item", revision: 1, title: "Ship it", status: "active" }, { id: "WI-8", record_type: "work_item", revision: 4, title: "Later", status: "active", priority: "low" }], limit: 20, offset: 0 }); + if (path === "/v1/records/WI-7" && req.method === "GET") return json(200, { id: "WI-7", record_type: "work_item", revision: 3, title: "Ship it", status: "active", owner: "Jason", tags: ["a", "b"], accepted_snapshot: { hidden: true }, body: "Two lines.\nOf body." }); + if (path === "/v1/records/WI-7" && req.method === "PATCH") return json(200, { id: "WI-7", record_type: "work_item", revision: parsed.revision + 1, title: "Ship it", ...parsed.fields }); ++ if (path === "/v1/records/DEC-9" && req.method === "GET") return json(200, { id: "DEC-9", record_type: "decision", revision: 2, title: "Pick one", status: "Proposed", required_approvers: ["discord:100000000000000100", "discord:199999999999999999"], approvals: [{ approver: "100000000000000100", at: "t" }] }); ++ if (path === "/v1/records/DEC-9" && req.method === "PATCH") return json(200, { id: "DEC-9", record_type: "decision", revision: parsed.revision + 1, title: "Pick one", ...parsed.fields }); ++ // Discord ids where the model could read them (the id-hiding boundary) ++ if (path === "/v1/records/DEC-10" && req.method === "GET") return json(200, { id: "DEC-10", record_type: "decision", revision: 1, title: "Ask 100000000000000100", required_approvers: ["discord:100000000000000100", "100000000000000101"], proposal_digest: "12345678901234567890abcdef0123456789abcdef0123456789abcdef012345", nested: { text: "approver discord:100000000000000100", "199999999999999999": "keyed" }, owner_id: 100000000000000100, note: `${"x".repeat(485)} 100000000000000101`, body: "Ask <@100000000000000100> and <@!199999999999999999>." }); ++ if (path === "/v1/records/DEC-10" && req.method === "PATCH") return json(409, { code: "stale_revision", message: "approver discord:100000000000000101 changed", current_revision: 2, changed_fields: ["required_approvers", "100000000000000100"] }); ++ if (path === "/v1/records/DEC-11" && req.method === "PATCH") return json(422, { code: "validation", message: `required_approvers: 199999999999999999 is not a user ${"z".repeat(332)} 100000000000000100` }); ++ if (path === "/v1/resolve" && new URL(req.url, "http://x").searchParams.get("q") === "leak") return json(200, { query: "leak", matches: [{ id: "DEC-10", record_type: "decision", title: "Ask <@100000000000000101>", exact: false }] }); ++ if (path === "/v1/documents" && req.method === "POST" && parsed.collection === "leak") return json(201, { id: "doc-2", title: "Notes for 100000000000000100", url: "https://outline.example.test/doc/199999999999999999" }); + if (path === "/v1/records/WI-9" && req.method === "PATCH") return json(409, { code: "stale_revision", message: "behind", current_revision: 5, changed_fields: ["status"] }); + if (path === "/v1/resolve") return json(200, { query: "ship", matches: [{ id: "WI-7", record_type: "work_item", title: "Ship it", exact: false }] }); + if (path === "/v1/approval-requests" && req.method === "POST") return json(201, view); +@@ -83,7 +92,7 @@ test("setspark config: a bare https or loopback origin, a private key file, a pr + const root = makeRoot(); + const kf = keyFile(root); + const c = loadSetsparkConfig({ baseUrl: "https://api.setspark.io", keyFile: kf, principal: "sage" }); +- assert.deepEqual(c, { baseUrl: "https://api.setspark.io", keyFile: kf, principal: "sage", timeoutMs: 15000, maxResponseBytes: 262144 }); ++ assert.deepEqual(c, { baseUrl: "https://api.setspark.io", keyFile: kf, principal: "sage", timeoutMs: 15000, maxResponseBytes: 262144, approvers: {} }); + assert.equal(loadSetsparkConfig({ baseUrl: "https://api.setspark.io/", keyFile: kf, principal: "sage" }).baseUrl, "https://api.setspark.io"); + assert.throws(() => loadSetsparkConfig(null), /not an object/); + assert.throws(() => loadSetsparkConfig({ baseUrl: "https://api.setspark.io", keyFile: kf, principal: "sage", extra: 1 }), /unknown key/); +@@ -129,7 +138,7 @@ test("setspark config: the binding's key survives resolveToolRoots and the engin + 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 }); ++ assert.deepEqual(resolved.setspark, { baseUrl: "https://api.setspark.io", keyFile: kf, principal: "sage", timeoutMs: 5000, approvers: { owner: "100000000000000100" } }); + 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"); +@@ -138,6 +147,144 @@ test("setspark config: the binding's key survives resolveToolRoots and the engin + assert.ok(!enabledToolNames(plain).some((n) => SETSPARK_TOOL_NAMES.includes(n))); + }); + ++test("setspark config: approvers come from the binding's users, never from the binding's setspark key", () => { ++ const root = makeRoot(); ++ const docs = join(root, "docs"); ++ mkdirSync(docs); ++ const kf = keyFile(root); ++ const users = [{ id: "100000000000000100", name: "Jason" }, { id: "100000000000000101", name: "carmen" }]; ++ const b = validateBinding(rawBinding({ users, tools: { roots: [{ name: "docs", path: docs }], setspark: { baseUrl: "https://api.setspark.io", keyFile: kf, principal: "sage" } } })); ++ assert.deepEqual(b.tools.setspark.approvers, { jason: "100000000000000100", carmen: "100000000000000101" }); ++ assert.throws(() => validateBinding(rawBinding({ users, tools: { roots: [{ name: "docs", path: docs }], setspark: { baseUrl: "https://api.setspark.io", keyFile: kf, principal: "sage", approvers: { mallory: "100000000000000199" } } } })), /approvers come from users/); ++ assert.throws(() => validateBinding(rawBinding({ users: [{ id: "100000000000000100", name: "Jason" }, { id: "100000000000000101", name: "jason" }], tools: { roots: [{ name: "docs", path: docs }], setspark: { baseUrl: "https://api.setspark.io", keyFile: kf, principal: "sage" } } })), /two users named jason/); ++ const withCarmenOut = validateBinding(rawBinding({ users: [users[0]], tools: { roots: [{ name: "docs", path: docs }], setspark: { baseUrl: "https://api.setspark.io", keyFile: kf, principal: "sage" } } })); ++ assert.throws(() => reloadDiff(b, withCarmenOut), /tools cannot change/, "pi's approvers are fixed at start, so a user change needs a restart"); ++ const channelsOnly = validateBinding(rawBinding({ users: [{ ...users[0], channels: [rawBinding().channels[0].id] }, users[1]], tools: { roots: [{ name: "docs", path: docs }], setspark: { baseUrl: "https://api.setspark.io", keyFile: kf, principal: "sage" } } })); ++ assert.doesNotThrow(() => reloadDiff(b, channelsOnly), "a user's channels still reload"); ++ assert.throws(() => loadSetsparkConfig({ baseUrl: "https://api.setspark.io", keyFile: kf, principal: "sage", approvers: { jason: "not-an-id" } }), /approvers/); ++ assert.throws(() => loadSetsparkConfig({ baseUrl: "https://api.setspark.io", keyFile: kf, principal: "sage", approvers: { jason: "100000000000000100", jay: "100000000000000100" } }), /approvers/); ++}); ++ ++test("setspark verbs: required_approvers go out as discord ids from names and come back as names", async () => { ++ const root = makeRoot(); ++ const named = createToolSet(loadToolsConfig({ roots: [{ name: "docs", path: root }], maxCallsPerTurn: 12, setspark: { baseUrl: base, keyFile: keyFile(root), principal: "sage", timeoutMs: 1000, approvers: { jason: "100000000000000100", carmen: "100000000000000101" } } })); ++ named.setTurn(TURN); ++ seen.length = 0; ++ const created = await named.call("record_create", { record_type: "decision", record: { title: "Pick one", status: "Proposed", work_item: "SS-1", required_approvers: ["Jason", "@carmen"] } }); ++ assert.equal(created.ok, true, created.text); ++ assert.deepEqual(JSON.parse(seen[0].body).record.required_approvers, ["discord:100000000000000100", "discord:100000000000000101"]); ++ const unknown = await named.call("record_create", { record_type: "decision", record: { title: "Pick one", required_approvers: ["Jason", "Mallory"] } }); ++ assert.equal(unknown.ok, false); ++ assert.match(unknown.text, /entry 2 is not a known user name; use names from: jason, carmen/); ++ const raw = await named.call("record_create", { record_type: "decision", record: { title: "Pick one", required_approvers: ["discord:100000000000000100"] } }); ++ assert.equal(raw.ok, false, "an id is not a name"); ++ assert.doesNotMatch(raw.text, /100000000000000100/, "a refused id is not echoed back"); ++ const twice = await named.call("record_create", { record_type: "decision", record: { title: "Pick one", required_approvers: ["jason", "Jason"] } }); ++ assert.match(twice.text, /the same person twice/); ++ const notList = await named.call("record_update", { id: "DEC-9", revision: 2, fields: { required_approvers: "jason" } }); ++ assert.equal(notList.ok, false); ++ assert.equal(seen.length, 1, "refused calls send nothing"); ++ const updated = await named.call("record_update", { id: "DEC-9", revision: 2, fields: { required_approvers: ["carmen"] } }); ++ assert.equal(updated.ok, true, updated.text); ++ assert.deepEqual(JSON.parse(seen[1].body).fields.required_approvers, ["discord:100000000000000101"]); ++ const got = await named.call("record_get", { id: "DEC-9" }); ++ assert.match(got.text, /required_approvers: jason, unknown user/); ++ assert.doesNotMatch(got.text, /1000000000000001|1999999999/, "no discord id reaches the model"); ++ const none = createToolSet(loadToolsConfig({ roots: [{ name: "docs", path: root }], setspark: { baseUrl: base, keyFile: keyFile(root), principal: "sage", timeoutMs: 1000 } })); ++ none.setTurn(TURN); ++ const noUsers = await none.call("record_create", { record_type: "decision", record: { title: "Pick one", required_approvers: ["jason"] } }); ++ assert.match(noUsers.text, /use names from: \(none\)/); ++}); ++ ++test("setspark verbs: no Discord user id reaches tool text, whatever shape the service returns it in", async () => { ++ const root = makeRoot(); ++ const ids = ["100000000000000100", "100000000000000101", "199999999999999999"]; ++ const t = createToolSet(loadToolsConfig({ roots: [{ name: "docs", path: root }], maxCallsPerTurn: 12, setspark: { baseUrl: base, keyFile: keyFile(root), principal: "sage", timeoutMs: 1000, approvers: { jason: "100000000000000100", carmen: "100000000000000101" } } })); ++ t.setTurn(TURN); ++ const texts = []; ++ const got = await t.call("record_get", { id: "DEC-10" }); ++ assert.equal(got.ok, true, got.text); ++ texts.push(got.text); ++ assert.match(got.text, /title: Ask jason/); ++ assert.match(got.text, /required_approvers: jason, carmen/); ++ assert.match(got.text, /nested: \{"text":"approver jason","unknown user":"keyed"\}/); ++ assert.match(got.text, /Ask jason and unknown user\./); ++ assert.match(got.text, /proposal_digest: 12345678901234567890abcdef/, "a digit run inside a digest is not an id"); ++ const stale = await t.call("record_update", { id: "DEC-10", revision: 1, fields: { status: "Proposed" } }); ++ assert.equal(stale.ok, false); ++ assert.match(stale.text, /changed: required_approvers, jason/); ++ assert.match(stale.text, /approver carmen changed/); ++ texts.push(stale.text); ++ const rejected = await t.call("record_update", { id: "DEC-11", revision: 1, fields: { status: "Proposed" } }); ++ assert.equal(rejected.ok, false); ++ assert.match(rejected.text, /unknown user is not a user/); ++ texts.push(rejected.text); ++ const found = await t.call("resolve_id", { query: "leak" }); ++ assert.match(found.text, /Ask carmen/); ++ texts.push(found.text); ++ const doc = await t.call("create_document", { collection: "leak", title: "Notes", text: "x" }); ++ assert.equal(doc.ok, true, doc.text); ++ texts.push(doc.text); ++ const opened = await t.call("open_approval_request", { decision_id: "DEC-012", proposal_version: 2, proposal_digest: "0123456789abcdef0123456789abcdef" }); ++ texts.push(opened.text); ++ for (const text of texts) for (const id of ids) assert.equal(text.includes(id), false, `id ${id} in: ${text}`); ++ for (const text of texts) assert.doesNotMatch(text, /(? discord:100000000000000100 SS-027 v2"), "unknown user unknown user SS-027 v2"); ++}); ++ ++test("setspark contract: a decision made with names opens a request the connector accepts; names stored by an old record still refuse", async () => { ++ // A local stand-in for the service's contract: records keep ++ // required_approvers as written (discord:), and the approval-request ++ // view returns them as bare Discord ids, as shared-signals documents. ++ const store = new Map(); ++ let next = 1; ++ const svc = createServer((req, res) => { ++ const chunks = []; ++ req.on("data", (c) => chunks.push(c)); ++ req.on("end", () => { ++ const parsed = chunks.length ? JSON.parse(Buffer.concat(chunks).toString("utf8")) : null; ++ const json = (status, obj) => { ++ res.writeHead(status, { "content-type": "application/json" }); ++ res.end(JSON.stringify(obj)); ++ }; ++ if (req.url === "/v1/records" && req.method === "POST") { ++ const id = `DEC-${next++}`; ++ store.set(id, { id, record_type: parsed.record_type, revision: 1, ...parsed.record }); ++ return json(201, store.get(id)); ++ } ++ if (req.url === "/v1/approval-requests" && req.method === "POST") { ++ const d = store.get(parsed.decision_id); ++ if (!d) return json(404, { code: "not_found", message: "no decision" }); ++ const bare = d.required_approvers.map((a) => (typeof a === "string" && a.startsWith("discord:") ? a.slice(8) : a)); ++ return json(201, { request_id: next++, decision_id: d.id, state: "open", proposal_version: parsed.proposal_version, proposal_digest: parsed.proposal_digest, required_approvers: bare, approvals: [], message_id: null }); ++ } ++ return json(404, { code: "not_found", message: "no route" }); ++ }); ++ }); ++ svc.listen(0, "127.0.0.1"); ++ await once(svc, "listening"); ++ try { ++ const root = makeRoot(); ++ const t = createToolSet(loadToolsConfig({ roots: [{ name: "docs", path: root }], maxCallsPerTurn: 12, setspark: { baseUrl: `http://127.0.0.1:${svc.address().port}`, keyFile: keyFile(root), principal: "sage", timeoutMs: 1000, approvers: { jason: "100000000000000100", carmen: "100000000000000101" } } })); ++ t.setTurn(TURN); ++ const made = await t.call("record_create", { record_type: "decision", record: { title: "Pick one", status: "Proposed", required_approvers: ["jason", "carmen"] } }); ++ assert.equal(made.ok, true, made.text); ++ const opened = await t.call("open_approval_request", { decision_id: "DEC-1", proposal_version: 1, proposal_digest: "0123456789abcdef" }); ++ assert.equal(opened.ok, true, opened.text); ++ const request = validateRequest(opened.details.request); ++ assert.deepEqual([...request.approvers], ["100000000000000100", "100000000000000101"]); ++ // DEC-009's shape: names written before this fix. The connector refuses. ++ store.set("DEC-9", { id: "DEC-9", record_type: "decision", revision: 1, title: "Old", required_approvers: ["Jason", "Carmen"] }); ++ const old = await t.call("open_approval_request", { decision_id: "DEC-9", proposal_version: 1, proposal_digest: "0123456789abcdef" }); ++ assert.equal(old.ok, true, old.text); ++ assert.throws(() => validateRequest(old.details.request), /bad approver id/); ++ } finally { ++ svc.close(); ++ } ++}); ++ + test("setspark keys: read per call, one printable token per file, rotation without a restart", async () => { + const root = makeRoot(); + const c = config(root); diff --git a/agents/sage/work/row25-approver-names-r3.diff b/agents/sage/work/row25-approver-names-r3.diff new file mode 100644 index 00000000..3421ae1e --- /dev/null +++ b/agents/sage/work/row25-approver-names-r3.diff @@ -0,0 +1,468 @@ +diff --git a/packages/discord/README.md b/packages/discord/README.md +index 4703fa86..c84b6afe 100644 +--- a/packages/discord/README.md ++++ b/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, 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 | ++| `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 (approver names come from `users`, lower-cased; a decision's `required_approvers` are written as `discord:` from those names; SetSpark tool text and refusals never carry a Discord user id: a mention, a `discord:` value or a standalone 17 to 20 digit run reads as the user's name or `unknown user`, while the connector's approval request keeps the bare ids; with `setspark` set, a change to a user's id or name refuses the reload, since pi's approvers are fixed at start) (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 +diff --git a/packages/discord/src/binding.mjs b/packages/discord/src/binding.mjs +index c9349b81..19ed0a5d 100644 +--- a/packages/discord/src/binding.mjs ++++ b/packages/discord/src/binding.mjs +@@ -217,7 +217,7 @@ export function validateBinding(raw, where = "binding") { + maxFileBytes: requireInteger(mergedTools, "maxFileBytes", `${where}.tools`, { min: 1024, max: 4 * 1024 * 1024 }), + maxCallsPerTurn: requireInteger(mergedTools, "maxCallsPerTurn", `${where}.tools`, { min: 1, max: 64 }), + web: raw.tools.web === undefined ? null : webConfig(raw.tools.web, `${where}.tools.web`), +- setspark: raw.tools.setspark === undefined ? null : setsparkConfig(raw.tools.setspark, `${where}.tools.setspark`), ++ setspark: raw.tools.setspark === undefined ? null : setsparkConfig(raw.tools.setspark, `${where}.tools.setspark`, users), + }); + } + +@@ -328,9 +328,16 @@ export function resolveContextFiles(binding, repo) { + // Tool roots must exist as real directories on this host, not symlinks, and + // must not sit inside the data root (bindings, tokens, journals) or contain + // it. Returns the resolved config the engine hands the extension. +-function setsparkConfig(raw, where) { ++function setsparkConfig(raw, where, users) { ++ if (raw !== null && typeof raw === "object" && Object.hasOwn(raw, "approvers")) throw new DiscordError(`${where}.approvers: not allowed; approvers come from users`); ++ const approvers = {}; ++ for (const u of users) { ++ const name = u.name.trim().toLowerCase(); ++ if (Object.hasOwn(approvers, name)) throw new DiscordError(`${where}: two users named ${name}; approver names must be distinct`); ++ approvers[name] = u.id; ++ } + try { +- return loadSetsparkConfig(raw, where); ++ return loadSetsparkConfig(raw !== null && typeof raw === "object" && !Array.isArray(raw) ? { ...raw, approvers } : raw, where); + } catch (err) { + throw new DiscordError(err.message); + } +@@ -363,5 +370,5 @@ export function resolveToolRoots(binding, { dataRoot }) { + // 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 } } : {}) }; ++ 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, approvers: { ...ss.approvers } } } : {}) }; + } +diff --git a/packages/discord/src/setspark.mjs b/packages/discord/src/setspark.mjs +index e33c84b3..25c5a688 100644 +--- a/packages/discord/src/setspark.mjs ++++ b/packages/discord/src/setspark.mjs +@@ -69,7 +69,7 @@ const isObject = (v) => v !== null && typeof v === "object" && !Array.isArray(v) + export function loadSetsparkConfig(raw, where = "setspark") { + if (!isObject(raw)) throw new Error(`${where}: not an object`); + for (const k of Object.keys(raw)) { +- if (!["baseUrl", "keyFile", "principal", "timeoutMs"].includes(k)) throw new Error(`${where}: unknown key ${JSON.stringify(k)}`); ++ if (!["baseUrl", "keyFile", "principal", "timeoutMs", "approvers"].includes(k)) throw new Error(`${where}: unknown key ${JSON.stringify(k)}`); + } + if (typeof raw.baseUrl !== "string") throw new Error(`${where}.baseUrl: must be a url string`); + let u; +@@ -86,7 +86,21 @@ export function loadSetsparkConfig(raw, where = "setspark") { + if (typeof raw.principal !== "string" || !PRINCIPAL.test(raw.principal)) throw new Error(`${where}.principal: must match ${PRINCIPAL}`); + const timeoutMs = raw.timeoutMs === undefined ? SETSPARK_DEFAULTS.timeoutMs : raw.timeoutMs; + if (!Number.isInteger(timeoutMs) || timeoutMs < 1000 || timeoutMs > 60000) throw new Error(`${where}.timeoutMs: must be an integer between 1000 and 60000`); +- return Object.freeze({ baseUrl: u.origin, keyFile: raw.keyFile, principal: raw.principal, timeoutMs, maxResponseBytes: SETSPARK_DEFAULTS.maxResponseBytes }); ++ const approvers = loadApprovers(raw.approvers === undefined ? {} : raw.approvers, `${where}.approvers`); ++ return Object.freeze({ baseUrl: u.origin, keyFile: raw.keyFile, principal: raw.principal, timeoutMs, maxResponseBytes: SETSPARK_DEFAULTS.maxResponseBytes, approvers }); ++} ++ ++// Lower-case user name to Discord user id. The binding derives it from its ++// users; the verbs turn a decision's required_approvers from names into ++// `discord:` and back, so the model never handles an id. ++function loadApprovers(raw, where) { ++ if (!isObject(raw) || Object.keys(raw).length > 64) throw new Error(`${where}: must be an object of at most 64 names`); ++ for (const [name, id] of Object.entries(raw)) { ++ if (name.length === 0 || name.length > 64 || name !== name.trim().toLowerCase() || /[\u0000-\u001f\u007f]/.test(name)) throw new Error(`${where}: ${JSON.stringify(name.slice(0, 64))} must be a trimmed lower-case name`); ++ if (typeof id !== "string" || !SNOWFLAKE.test(id)) throw new Error(`${where}.${name}: must be a Discord user id`); ++ } ++ if (new Set(Object.values(raw)).size !== Object.keys(raw).length) throw new Error(`${where}: one Discord user id under two names`); ++ return Object.freeze({ ...raw }); + } + + function checkPrivateFile(path, what) { +@@ -248,8 +262,8 @@ export async function callApi(config, { method, path, body, idempotencyKey: key + return finish(resolve, { status, body: json }); + } + const err = isObject(json) ? json : {}; +- const code = typeof err.code === "string" ? err.code.slice(0, 64) : null; +- const extra = { status, code, message: cutMessage(err.message) }; ++ const code = typeof err.code === "string" ? hideIds(config, err.code).slice(0, 64) : null; ++ const extra = { status, code, message: cutMessage(typeof err.message === "string" ? hideIds(config, err.message) : err.message) }; + if (status === 409) { + if (err.current_revision !== undefined) extra.currentRevision = err.current_revision; + if (Array.isArray(err.changed_fields)) extra.changedFields = err.changed_fields.filter((f) => typeof f === "string").slice(0, 32); +@@ -263,14 +277,15 @@ export async function callApi(config, { method, path, body, idempotencyKey: key + } + + // How a refusal reads to the model and in the turn record: the fixed +-// reason, the code, and on 409 the fields that changed. Never the raw body. +-export function renderRefusal(err) { ++// reason, the code, and on 409 the fields that changed. Never the raw body, ++// and never a Discord user id (hideIds). ++export function renderRefusal(err, config = null) { + let s = `refused: ${err.reason}`; + if (err.code) s += ` (code ${err.code})`; + if (err.currentRevision !== undefined) s += `; current revision ${err.currentRevision}`; + if (Array.isArray(err.changedFields) && err.changedFields.length > 0) s += `; changed: ${err.changedFields.join(", ")}`; + if (err.message && err.message !== err.reason) s += `\n${err.message}`; +- return s; ++ return hideIds(config, s); + } + + // --- the verbs (contract: shared-signals stack/api/openapi.json at a5425a2) --- +@@ -324,7 +339,7 @@ function needObject(params, name) { + if (!isObject(v)) throw bad(`${name} must be an object`); + const size = Buffer.byteLength(JSON.stringify(v), "utf8"); + if (size > RECORD_MAX_BYTES) throw bad(`${name} is over ${RECORD_MAX_BYTES} bytes`); +- for (const k of Object.keys(v)) if (!PROP_NAME.test(k)) throw bad(`${name} has a property name that is not allowed: ${k.slice(0, 32)}`); ++ for (const k of Object.keys(v)) if (!PROP_NAME.test(k)) throw bad(`${name} has a property name that is not allowed: ${hideIds(null, k).slice(0, 32)}`); + return v; + } + +@@ -354,6 +369,52 @@ function record(body) { + return isObject(body) ? body : {}; + } + ++// A decision's required_approvers as the model gives them: names of the ++// binding's users. Each becomes `discord:`; anything else is refused ++// before a request, since the service checks an approval's author against ++// these values and a stored name could never be approved. ++function approverIds(config, v) { ++ const map = config.approvers || {}; ++ const names = Object.keys(map).join(", ") || "(none)"; ++ if (!Array.isArray(v) || v.length === 0 || v.length > 16) throw bad(`required_approvers must be a list of 1 to 16 names; use names from: ${names}`); ++ // The refusal names the entry's position, never its value: a model that ++ // wrote an id must not get it echoed back. ++ const ids = v.map((a, i) => { ++ const k = typeof a === "string" ? a.trim().replace(/^@/, "").toLowerCase() : ""; ++ if (k.length > 0 && Object.hasOwn(map, k)) return `discord:${map[k]}`; ++ throw bad(`required_approvers: entry ${i + 1} is not a known user name; use names from: ${names}`); ++ }); ++ if (new Set(ids).size !== ids.length) throw bad("required_approvers names the same person twice"); ++ return ids; ++} ++ ++function withApprovers(config, props) { ++ return props.required_approvers === undefined ? props : { ...props, required_approvers: approverIds(config, props.required_approvers) }; ++} ++ ++// The way back, for everything the model reads: a Discord user id never ++// appears in SetSpark tool text. A mention (<@id>), a `discord:` value or a ++// 17 to 20 digit run standing alone becomes the binding's name for that id, ++// or "unknown user". A digit run inside a longer token (a hex digest, SS-027) ++// is left alone. The connector's own request keeps the bare ids; only the ++// rendered text and refusals go through this. ++const DISCORD_ID_TEXT = /<@!?([0-9]{17,20})>|(? [id, n])); ++ return String(text).replace(DISCORD_ID_TEXT, (_, a, b) => byId.get(a || b) || "unknown user"); ++} ++ ++// The same rule over a service value before it is rendered, so a field cut ++// at its length limit cannot leave part of an id. Keys and big integers too. ++function hideDeep(config, v) { ++ if (typeof v === "string") return hideIds(config, v); ++ if (typeof v === "number") return /^[0-9]{17,20}$/.test(String(v)) ? hideIds(config, String(v)) : v; ++ if (Array.isArray(v)) return v.map((x) => hideDeep(config, x)); ++ if (isObject(v)) return Object.fromEntries(Object.entries(v).map(([k, x]) => [hideIds(config, k), hideDeep(config, x)])); ++ return v; ++} ++ + export const setsparkVerbs = Object.freeze({ + async record_list(config, params, state, deps) { + const type = needType(params); +@@ -363,7 +424,7 @@ export const setsparkVerbs = Object.freeze({ + if (params.filters !== undefined) { + if (!isObject(params.filters) || Object.keys(params.filters).length > FILTERS_MAX) throw bad(`filters must be an object of at most ${FILTERS_MAX} properties`); + for (const [k, v] of Object.entries(params.filters)) { +- if (!PROP_NAME.test(k) || ["record_type", "type", "limit", "offset"].includes(k)) throw bad(`filters: property name not allowed: ${k.slice(0, 32)}`); ++ if (!PROP_NAME.test(k) || ["record_type", "type", "limit", "offset"].includes(k)) throw bad(`filters: property name not allowed: ${hideIds(null, k).slice(0, 32)}`); + if (typeof v !== "string" || v.length === 0 || v.length > 200) throw bad(`filters.${k} must be a short string`); + q.set(k, v); + } +@@ -379,7 +440,7 @@ export const setsparkVerbs = Object.freeze({ + }, + async record_create(config, params, state, deps) { + const type = needType(params); +- const rec = needObject(params, "record"); ++ const rec = withApprovers(config, needObject(params, "record")); + if (typeof rec.title !== "string" || rec.title.trim().length === 0) throw bad("record.title is required"); + const r = await write(config, state, "POST", "/v1/records", { record_type: type, record: rec }, deps); + return { verb: "record_create", key: r.key, recordType: type, record: record(r.body) }; +@@ -387,7 +448,7 @@ export const setsparkVerbs = Object.freeze({ + async record_update(config, params, state, deps) { + const id = needId(params); + const revision = needInt(params, "revision", 1, 1000000000); +- const fields = needObject(params, "fields"); ++ const fields = withApprovers(config, needObject(params, "fields")); + if (Object.keys(fields).length === 0) throw bad("fields must name at least one property"); + const r = await write(config, state, "PATCH", `/v1/records/${id}`, { revision, fields }, deps); + return { verb: "record_update", key: r.key, id, from: revision, record: record(r.body) }; +@@ -476,7 +537,13 @@ function renderView(v) { + return `request ${scalar(v.request_id)} for ${scalar(v.decision_id)} version ${scalar(v.proposal_version)}: ${scalar(v.state) || "?"}; ${approvals.length} of ${who} approvals recorded${v.message_id ? "; bound to a Discord message" : "; no message bound yet"}`; + } + +-export function renderSetspark(name, out) { ++// Tool text for the model. `out` is hidden (hideDeep) before any field is ++// cut, and the whole text once more after. ++export function renderSetspark(name, out, config = null) { ++ return hideIds(config, renderOut(name, hideDeep(config, out))); ++} ++ ++function renderOut(name, out) { + if (name === "record_list") { + const body = out.items.map(summary).join("\n"); + return `${out.items.length} ${out.recordType} record(s) from offset ${out.offset} (limit ${out.limit})\n${body || "(none)"}`; +@@ -520,7 +587,7 @@ export const SETSPARK_TOOL_DESCRIPTIONS = Object.freeze({ + }, + record_create: { + label: "Create record", +- description: "Create one SetSpark record; the service allocates the id. Give record_type and the record's properties (title required). Only when the user asked for a record to be created.", ++ description: "Create one SetSpark record; the service allocates the id. Give record_type and the record's properties (title required). A decision's required_approvers is a list of user names (such as jason); the connector turns each into that user's Discord identity and refuses a name it does not know. Only when the user asked for a record to be created.", + snippet: "record_create creates one SetSpark record", + }, + record_update: { +diff --git a/packages/discord/src/tools.mjs b/packages/discord/src/tools.mjs +index c66365be..5596101b 100644 +--- a/packages/discord/src/tools.mjs ++++ b/packages/discord/src/tools.mjs +@@ -567,8 +567,8 @@ const TOOL_FNS = Object.freeze({ + }); + const SETSPARK_SET = new Set(SETSPARK_TOOL_NAMES); + +-function render(name, out) { +- if (SETSPARK_SET.has(name)) return renderSetspark(name, out); ++function render(name, out, config) { ++ if (SETSPARK_SET.has(name)) return renderSetspark(name, out, config.setspark); + if (name === "list_dir") { + const head = `${out.root}/${out.path}`.replace(/\/$/, ""); + const body = out.entries.map((e) => (e.type === "dir" ? `${e.name}/` : `${e.name} (${e.bytes} bytes)`)).join("\n"); +@@ -648,11 +648,11 @@ export function createToolSet(config) { + : name === "git_status" ? { branch: out.branch } + : name === "reserve_id" ? { id: out.id } + : { path: out.path }; +- return { ok: true, text: render(name, out), details: { ...base, ok: true, ...extra, ...(bytes === undefined ? {} : { bytes }), ms: Date.now() - t0 } }; ++ return { ok: true, text: render(name, out, config), details: { ...base, ok: true, ...extra, ...(bytes === undefined ? {} : { bytes }), ms: Date.now() - t0 } }; + }; + const refused = (err) => { + if (err instanceof SetsparkRefusal) { +- return { ok: false, text: renderSetsparkRefusal(err), details: { ...base, ok: false, reason: err.reason, ...(err.code ? { code: err.code } : {}), ...(err.status ? { status: err.status } : {}), ms: Date.now() - t0 } }; ++ return { ok: false, text: renderSetsparkRefusal(err, config.setspark), details: { ...base, ok: false, reason: err.reason, ...(err.code ? { code: err.code } : {}), ...(err.status ? { status: err.status } : {}), ms: Date.now() - t0 } }; + } + if (!(err instanceof Refusal) && !(err instanceof WebRefusal) && !(err instanceof GitRefusal)) throw err; + const reason = err instanceof GitRefusal ? err.message : err.reason; +diff --git a/packages/discord/tests/setspark.test.mjs b/packages/discord/tests/setspark.test.mjs +index f88dfb23..7fe2999b 100644 +--- a/packages/discord/tests/setspark.test.mjs ++++ b/packages/discord/tests/setspark.test.mjs +@@ -9,10 +9,11 @@ import { chmodSync, mkdirSync, symlinkSync, writeFileSync } from "node:fs"; + import { join } from "node:path"; + import { + SETSPARK_REFUSAL, SetsparkRefusal, IDEMPOTENCY_HEADER, MESSAGE_MAX_CHARS, USER_AGENT, SETSPARK_TOOL_NAMES, LIST_MAX, +- loadSetsparkConfig, readKey, idempotencyKey, connectorKey, callApi, renderRefusal, createSetsparkApi, renderRecord, ++ loadSetsparkConfig, readKey, idempotencyKey, connectorKey, callApi, renderRefusal, createSetsparkApi, renderRecord, hideIds, + } from "../src/setspark.mjs"; + import { loadToolsConfig, createToolSet, enabledToolNames } from "../src/tools.mjs"; +-import { validateBinding, resolveToolRoots } from "../src/binding.mjs"; ++import { validateBinding, resolveToolRoots, reloadDiff } from "../src/binding.mjs"; ++import { validateRequest } from "../src/approvals.mjs"; + import { makeRoot, rawBinding } from "./helpers.mjs"; + + const KEY_A = "ssk_" + "a".repeat(40); +@@ -46,6 +47,15 @@ const server = createServer((req, res) => { + if (path === "/v1/records" && req.method === "GET") return json(200, { record_type: "work_item", items: [{ id: "WI-7", record_type: "work_item", revision: 1, title: "Ship it", status: "active" }, { id: "WI-8", record_type: "work_item", revision: 4, title: "Later", status: "active", priority: "low" }], limit: 20, offset: 0 }); + if (path === "/v1/records/WI-7" && req.method === "GET") return json(200, { id: "WI-7", record_type: "work_item", revision: 3, title: "Ship it", status: "active", owner: "Jason", tags: ["a", "b"], accepted_snapshot: { hidden: true }, body: "Two lines.\nOf body." }); + if (path === "/v1/records/WI-7" && req.method === "PATCH") return json(200, { id: "WI-7", record_type: "work_item", revision: parsed.revision + 1, title: "Ship it", ...parsed.fields }); ++ if (path === "/v1/records/DEC-9" && req.method === "GET") return json(200, { id: "DEC-9", record_type: "decision", revision: 2, title: "Pick one", status: "Proposed", required_approvers: ["discord:100000000000000100", "discord:199999999999999999"], approvals: [{ approver: "100000000000000100", at: "t" }] }); ++ if (path === "/v1/records/DEC-9" && req.method === "PATCH") return json(200, { id: "DEC-9", record_type: "decision", revision: parsed.revision + 1, title: "Pick one", ...parsed.fields }); ++ // Discord ids where the model could read them (the id-hiding boundary) ++ if (path === "/v1/records/DEC-10" && req.method === "GET") return json(200, { id: "DEC-10", record_type: "decision", revision: 1, title: "Ask 100000000000000100", required_approvers: ["discord:100000000000000100", "100000000000000101"], proposal_digest: "12345678901234567890abcdef0123456789abcdef0123456789abcdef012345", nested: { text: "approver discord:100000000000000100", "199999999999999999": "keyed" }, owner_id: 100000000000000100, note: `${"x".repeat(485)} 100000000000000101`, body: "Ask <@100000000000000100> and <@!199999999999999999>." }); ++ if (path === "/v1/records/DEC-10" && req.method === "PATCH") return json(409, { code: "stale_revision", message: "approver discord:100000000000000101 changed", current_revision: 2, changed_fields: ["required_approvers", "100000000000000100"] }); ++ if (path === "/v1/records/DEC-12" && req.method === "PATCH") return json(400, { code: `${"x".repeat(49)} 100000000000000100`, message: "no" }); ++ if (path === "/v1/records/DEC-11" && req.method === "PATCH") return json(422, { code: "validation", message: `required_approvers: 199999999999999999 is not a user ${"z".repeat(332)} 100000000000000100` }); ++ if (path === "/v1/resolve" && new URL(req.url, "http://x").searchParams.get("q") === "leak") return json(200, { query: "leak", matches: [{ id: "DEC-10", record_type: "decision", title: "Ask <@100000000000000101>", exact: false }] }); ++ if (path === "/v1/documents" && req.method === "POST" && parsed.collection === "leak") return json(201, { id: "doc-2", title: "Notes for 100000000000000100", url: "https://outline.example.test/doc/199999999999999999" }); + if (path === "/v1/records/WI-9" && req.method === "PATCH") return json(409, { code: "stale_revision", message: "behind", current_revision: 5, changed_fields: ["status"] }); + if (path === "/v1/resolve") return json(200, { query: "ship", matches: [{ id: "WI-7", record_type: "work_item", title: "Ship it", exact: false }] }); + if (path === "/v1/approval-requests" && req.method === "POST") return json(201, view); +@@ -83,7 +93,7 @@ test("setspark config: a bare https or loopback origin, a private key file, a pr + const root = makeRoot(); + const kf = keyFile(root); + const c = loadSetsparkConfig({ baseUrl: "https://api.setspark.io", keyFile: kf, principal: "sage" }); +- assert.deepEqual(c, { baseUrl: "https://api.setspark.io", keyFile: kf, principal: "sage", timeoutMs: 15000, maxResponseBytes: 262144 }); ++ assert.deepEqual(c, { baseUrl: "https://api.setspark.io", keyFile: kf, principal: "sage", timeoutMs: 15000, maxResponseBytes: 262144, approvers: {} }); + assert.equal(loadSetsparkConfig({ baseUrl: "https://api.setspark.io/", keyFile: kf, principal: "sage" }).baseUrl, "https://api.setspark.io"); + assert.throws(() => loadSetsparkConfig(null), /not an object/); + assert.throws(() => loadSetsparkConfig({ baseUrl: "https://api.setspark.io", keyFile: kf, principal: "sage", extra: 1 }), /unknown key/); +@@ -129,7 +139,7 @@ test("setspark config: the binding's key survives resolveToolRoots and the engin + 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 }); ++ assert.deepEqual(resolved.setspark, { baseUrl: "https://api.setspark.io", keyFile: kf, principal: "sage", timeoutMs: 5000, approvers: { owner: "100000000000000100" } }); + 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"); +@@ -138,6 +148,155 @@ test("setspark config: the binding's key survives resolveToolRoots and the engin + assert.ok(!enabledToolNames(plain).some((n) => SETSPARK_TOOL_NAMES.includes(n))); + }); + ++test("setspark config: approvers come from the binding's users, never from the binding's setspark key", () => { ++ const root = makeRoot(); ++ const docs = join(root, "docs"); ++ mkdirSync(docs); ++ const kf = keyFile(root); ++ const users = [{ id: "100000000000000100", name: "Jason" }, { id: "100000000000000101", name: "carmen" }]; ++ const b = validateBinding(rawBinding({ users, tools: { roots: [{ name: "docs", path: docs }], setspark: { baseUrl: "https://api.setspark.io", keyFile: kf, principal: "sage" } } })); ++ assert.deepEqual(b.tools.setspark.approvers, { jason: "100000000000000100", carmen: "100000000000000101" }); ++ assert.throws(() => validateBinding(rawBinding({ users, tools: { roots: [{ name: "docs", path: docs }], setspark: { baseUrl: "https://api.setspark.io", keyFile: kf, principal: "sage", approvers: { mallory: "100000000000000199" } } } })), /approvers come from users/); ++ assert.throws(() => validateBinding(rawBinding({ users: [{ id: "100000000000000100", name: "Jason" }, { id: "100000000000000101", name: "jason" }], tools: { roots: [{ name: "docs", path: docs }], setspark: { baseUrl: "https://api.setspark.io", keyFile: kf, principal: "sage" } } })), /two users named jason/); ++ const withCarmenOut = validateBinding(rawBinding({ users: [users[0]], tools: { roots: [{ name: "docs", path: docs }], setspark: { baseUrl: "https://api.setspark.io", keyFile: kf, principal: "sage" } } })); ++ assert.throws(() => reloadDiff(b, withCarmenOut), /tools cannot change/, "pi's approvers are fixed at start, so a user change needs a restart"); ++ const channelsOnly = validateBinding(rawBinding({ users: [{ ...users[0], channels: [rawBinding().channels[0].id] }, users[1]], tools: { roots: [{ name: "docs", path: docs }], setspark: { baseUrl: "https://api.setspark.io", keyFile: kf, principal: "sage" } } })); ++ assert.doesNotThrow(() => reloadDiff(b, channelsOnly), "a user's channels still reload"); ++ assert.throws(() => loadSetsparkConfig({ baseUrl: "https://api.setspark.io", keyFile: kf, principal: "sage", approvers: { jason: "not-an-id" } }), /approvers/); ++ assert.throws(() => loadSetsparkConfig({ baseUrl: "https://api.setspark.io", keyFile: kf, principal: "sage", approvers: { jason: "100000000000000100", jay: "100000000000000100" } }), /approvers/); ++}); ++ ++test("setspark verbs: required_approvers go out as discord ids from names and come back as names", async () => { ++ const root = makeRoot(); ++ const named = createToolSet(loadToolsConfig({ roots: [{ name: "docs", path: root }], maxCallsPerTurn: 12, setspark: { baseUrl: base, keyFile: keyFile(root), principal: "sage", timeoutMs: 1000, approvers: { jason: "100000000000000100", carmen: "100000000000000101" } } })); ++ named.setTurn(TURN); ++ seen.length = 0; ++ const created = await named.call("record_create", { record_type: "decision", record: { title: "Pick one", status: "Proposed", work_item: "SS-1", required_approvers: ["Jason", "@carmen"] } }); ++ assert.equal(created.ok, true, created.text); ++ assert.deepEqual(JSON.parse(seen[0].body).record.required_approvers, ["discord:100000000000000100", "discord:100000000000000101"]); ++ const unknown = await named.call("record_create", { record_type: "decision", record: { title: "Pick one", required_approvers: ["Jason", "Mallory"] } }); ++ assert.equal(unknown.ok, false); ++ assert.match(unknown.text, /entry 2 is not a known user name; use names from: jason, carmen/); ++ const raw = await named.call("record_create", { record_type: "decision", record: { title: "Pick one", required_approvers: ["discord:100000000000000100"] } }); ++ assert.equal(raw.ok, false, "an id is not a name"); ++ assert.doesNotMatch(raw.text, /100000000000000100/, "a refused id is not echoed back"); ++ const twice = await named.call("record_create", { record_type: "decision", record: { title: "Pick one", required_approvers: ["jason", "Jason"] } }); ++ assert.match(twice.text, /the same person twice/); ++ const notList = await named.call("record_update", { id: "DEC-9", revision: 2, fields: { required_approvers: "jason" } }); ++ assert.equal(notList.ok, false); ++ assert.equal(seen.length, 1, "refused calls send nothing"); ++ const updated = await named.call("record_update", { id: "DEC-9", revision: 2, fields: { required_approvers: ["carmen"] } }); ++ assert.equal(updated.ok, true, updated.text); ++ assert.deepEqual(JSON.parse(seen[1].body).fields.required_approvers, ["discord:100000000000000101"]); ++ const got = await named.call("record_get", { id: "DEC-9" }); ++ assert.match(got.text, /required_approvers: jason, unknown user/); ++ assert.doesNotMatch(got.text, /1000000000000001|1999999999/, "no discord id reaches the model"); ++ const none = createToolSet(loadToolsConfig({ roots: [{ name: "docs", path: root }], setspark: { baseUrl: base, keyFile: keyFile(root), principal: "sage", timeoutMs: 1000 } })); ++ none.setTurn(TURN); ++ const noUsers = await none.call("record_create", { record_type: "decision", record: { title: "Pick one", required_approvers: ["jason"] } }); ++ assert.match(noUsers.text, /use names from: \(none\)/); ++}); ++ ++test("setspark verbs: no Discord user id reaches tool text, whatever shape the service returns it in", async () => { ++ const root = makeRoot(); ++ const ids = ["100000000000000100", "100000000000000101", "199999999999999999"]; ++ const t = createToolSet(loadToolsConfig({ roots: [{ name: "docs", path: root }], maxCallsPerTurn: 12, setspark: { baseUrl: base, keyFile: keyFile(root), principal: "sage", timeoutMs: 1000, approvers: { jason: "100000000000000100", carmen: "100000000000000101" } } })); ++ t.setTurn(TURN); ++ const texts = []; ++ const got = await t.call("record_get", { id: "DEC-10" }); ++ assert.equal(got.ok, true, got.text); ++ texts.push(got.text); ++ assert.match(got.text, /title: Ask jason/); ++ assert.match(got.text, /required_approvers: jason, carmen/); ++ assert.match(got.text, /nested: \{"text":"approver jason","unknown user":"keyed"\}/); ++ assert.match(got.text, /Ask jason and unknown user\./); ++ assert.match(got.text, /proposal_digest: 12345678901234567890abcdef/, "a digit run inside a digest is not an id"); ++ const stale = await t.call("record_update", { id: "DEC-10", revision: 1, fields: { status: "Proposed" } }); ++ assert.equal(stale.ok, false); ++ assert.match(stale.text, /changed: required_approvers, jason/); ++ assert.match(stale.text, /approver carmen changed/); ++ texts.push(stale.text); ++ const rejected = await t.call("record_update", { id: "DEC-11", revision: 1, fields: { status: "Proposed" } }); ++ assert.equal(rejected.ok, false); ++ assert.match(rejected.text, /unknown user is not a user/); ++ texts.push(rejected.text); ++ // a field capped before it is shown: the service's code, a bad property ++ // name, a bad filter name ++ const code = await t.call("record_update", { id: "DEC-12", revision: 1, fields: { status: "Proposed" } }); ++ assert.equal(code.ok, false); ++ texts.push(code.text); ++ const prop = await t.call("record_create", { record_type: "decision", record: { title: "x", [`${"x".repeat(17)} 100000000000000101`]: "v" } }); ++ assert.match(prop.text, /property name that is not allowed/); ++ texts.push(prop.text); ++ const filter = await t.call("record_list", { record_type: "decision", filters: { [`${"x".repeat(17)} 199999999999999999`]: "v" } }); ++ assert.match(filter.text, /property name not allowed/); ++ texts.push(filter.text); ++ const found = await t.call("resolve_id", { query: "leak" }); ++ assert.match(found.text, /Ask carmen/); ++ texts.push(found.text); ++ const doc = await t.call("create_document", { collection: "leak", title: "Notes", text: "x" }); ++ assert.equal(doc.ok, true, doc.text); ++ texts.push(doc.text); ++ const opened = await t.call("open_approval_request", { decision_id: "DEC-012", proposal_version: 2, proposal_digest: "0123456789abcdef0123456789abcdef" }); ++ texts.push(opened.text); ++ for (const text of texts) for (const id of ids) assert.equal(text.includes(id), false, `id ${id} in: ${text}`); ++ for (const text of texts) assert.doesNotMatch(text, /(? discord:100000000000000100 SS-027 v2"), "unknown user unknown user SS-027 v2"); ++}); ++ ++test("setspark contract: a decision made with names opens a request the connector accepts; names stored by an old record still refuse", async () => { ++ // A local stand-in for the service's contract: records keep ++ // required_approvers as written (discord:), and the approval-request ++ // view returns them as bare Discord ids, as shared-signals documents. ++ const store = new Map(); ++ let next = 1; ++ const svc = createServer((req, res) => { ++ const chunks = []; ++ req.on("data", (c) => chunks.push(c)); ++ req.on("end", () => { ++ const parsed = chunks.length ? JSON.parse(Buffer.concat(chunks).toString("utf8")) : null; ++ const json = (status, obj) => { ++ res.writeHead(status, { "content-type": "application/json" }); ++ res.end(JSON.stringify(obj)); ++ }; ++ if (req.url === "/v1/records" && req.method === "POST") { ++ const id = `DEC-${next++}`; ++ store.set(id, { id, record_type: parsed.record_type, revision: 1, ...parsed.record }); ++ return json(201, store.get(id)); ++ } ++ if (req.url === "/v1/approval-requests" && req.method === "POST") { ++ const d = store.get(parsed.decision_id); ++ if (!d) return json(404, { code: "not_found", message: "no decision" }); ++ const bare = d.required_approvers.map((a) => (typeof a === "string" && a.startsWith("discord:") ? a.slice(8) : a)); ++ return json(201, { request_id: next++, decision_id: d.id, state: "open", proposal_version: parsed.proposal_version, proposal_digest: parsed.proposal_digest, required_approvers: bare, approvals: [], message_id: null }); ++ } ++ return json(404, { code: "not_found", message: "no route" }); ++ }); ++ }); ++ svc.listen(0, "127.0.0.1"); ++ await once(svc, "listening"); ++ try { ++ const root = makeRoot(); ++ const t = createToolSet(loadToolsConfig({ roots: [{ name: "docs", path: root }], maxCallsPerTurn: 12, setspark: { baseUrl: `http://127.0.0.1:${svc.address().port}`, keyFile: keyFile(root), principal: "sage", timeoutMs: 1000, approvers: { jason: "100000000000000100", carmen: "100000000000000101" } } })); ++ t.setTurn(TURN); ++ const made = await t.call("record_create", { record_type: "decision", record: { title: "Pick one", status: "Proposed", required_approvers: ["jason", "carmen"] } }); ++ assert.equal(made.ok, true, made.text); ++ const opened = await t.call("open_approval_request", { decision_id: "DEC-1", proposal_version: 1, proposal_digest: "0123456789abcdef" }); ++ assert.equal(opened.ok, true, opened.text); ++ const request = validateRequest(opened.details.request); ++ assert.deepEqual([...request.approvers], ["100000000000000100", "100000000000000101"]); ++ // DEC-009's shape: names written before this fix. The connector refuses. ++ store.set("DEC-9", { id: "DEC-9", record_type: "decision", revision: 1, title: "Old", required_approvers: ["Jason", "Carmen"] }); ++ const old = await t.call("open_approval_request", { decision_id: "DEC-9", proposal_version: 1, proposal_digest: "0123456789abcdef" }); ++ assert.equal(old.ok, true, old.text); ++ assert.throws(() => validateRequest(old.details.request), /bad approver id/); ++ } finally { ++ svc.close(); ++ } ++}); ++ + test("setspark keys: read per call, one printable token per file, rotation without a restart", async () => { + const root = makeRoot(); + const c = config(root); diff --git a/agents/sage/work/row25-approver-names.diff b/agents/sage/work/row25-approver-names.diff new file mode 100644 index 00000000..42a40f32 --- /dev/null +++ b/agents/sage/work/row25-approver-names.diff @@ -0,0 +1,266 @@ +diff --git a/packages/discord/README.md b/packages/discord/README.md +index 4703fa86..d37c62f2 100644 +--- a/packages/discord/README.md ++++ b/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, 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 | ++| `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 (approver names come from `users`, lower-cased; a decision's `required_approvers` are written as `discord:` from those names and shown back as names; with `setspark` set, a change to a user's id or name refuses the reload, since pi's approvers are fixed at start) (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 +diff --git a/packages/discord/src/binding.mjs b/packages/discord/src/binding.mjs +index c9349b81..19ed0a5d 100644 +--- a/packages/discord/src/binding.mjs ++++ b/packages/discord/src/binding.mjs +@@ -217,7 +217,7 @@ export function validateBinding(raw, where = "binding") { + maxFileBytes: requireInteger(mergedTools, "maxFileBytes", `${where}.tools`, { min: 1024, max: 4 * 1024 * 1024 }), + maxCallsPerTurn: requireInteger(mergedTools, "maxCallsPerTurn", `${where}.tools`, { min: 1, max: 64 }), + web: raw.tools.web === undefined ? null : webConfig(raw.tools.web, `${where}.tools.web`), +- setspark: raw.tools.setspark === undefined ? null : setsparkConfig(raw.tools.setspark, `${where}.tools.setspark`), ++ setspark: raw.tools.setspark === undefined ? null : setsparkConfig(raw.tools.setspark, `${where}.tools.setspark`, users), + }); + } + +@@ -328,9 +328,16 @@ export function resolveContextFiles(binding, repo) { + // Tool roots must exist as real directories on this host, not symlinks, and + // must not sit inside the data root (bindings, tokens, journals) or contain + // it. Returns the resolved config the engine hands the extension. +-function setsparkConfig(raw, where) { ++function setsparkConfig(raw, where, users) { ++ if (raw !== null && typeof raw === "object" && Object.hasOwn(raw, "approvers")) throw new DiscordError(`${where}.approvers: not allowed; approvers come from users`); ++ const approvers = {}; ++ for (const u of users) { ++ const name = u.name.trim().toLowerCase(); ++ if (Object.hasOwn(approvers, name)) throw new DiscordError(`${where}: two users named ${name}; approver names must be distinct`); ++ approvers[name] = u.id; ++ } + try { +- return loadSetsparkConfig(raw, where); ++ return loadSetsparkConfig(raw !== null && typeof raw === "object" && !Array.isArray(raw) ? { ...raw, approvers } : raw, where); + } catch (err) { + throw new DiscordError(err.message); + } +@@ -363,5 +370,5 @@ export function resolveToolRoots(binding, { dataRoot }) { + // 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 } } : {}) }; ++ 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, approvers: { ...ss.approvers } } } : {}) }; + } +diff --git a/packages/discord/src/setspark.mjs b/packages/discord/src/setspark.mjs +index e33c84b3..7b58a19a 100644 +--- a/packages/discord/src/setspark.mjs ++++ b/packages/discord/src/setspark.mjs +@@ -69,7 +69,7 @@ const isObject = (v) => v !== null && typeof v === "object" && !Array.isArray(v) + export function loadSetsparkConfig(raw, where = "setspark") { + if (!isObject(raw)) throw new Error(`${where}: not an object`); + for (const k of Object.keys(raw)) { +- if (!["baseUrl", "keyFile", "principal", "timeoutMs"].includes(k)) throw new Error(`${where}: unknown key ${JSON.stringify(k)}`); ++ if (!["baseUrl", "keyFile", "principal", "timeoutMs", "approvers"].includes(k)) throw new Error(`${where}: unknown key ${JSON.stringify(k)}`); + } + if (typeof raw.baseUrl !== "string") throw new Error(`${where}.baseUrl: must be a url string`); + let u; +@@ -86,7 +86,21 @@ export function loadSetsparkConfig(raw, where = "setspark") { + if (typeof raw.principal !== "string" || !PRINCIPAL.test(raw.principal)) throw new Error(`${where}.principal: must match ${PRINCIPAL}`); + const timeoutMs = raw.timeoutMs === undefined ? SETSPARK_DEFAULTS.timeoutMs : raw.timeoutMs; + if (!Number.isInteger(timeoutMs) || timeoutMs < 1000 || timeoutMs > 60000) throw new Error(`${where}.timeoutMs: must be an integer between 1000 and 60000`); +- return Object.freeze({ baseUrl: u.origin, keyFile: raw.keyFile, principal: raw.principal, timeoutMs, maxResponseBytes: SETSPARK_DEFAULTS.maxResponseBytes }); ++ const approvers = loadApprovers(raw.approvers === undefined ? {} : raw.approvers, `${where}.approvers`); ++ return Object.freeze({ baseUrl: u.origin, keyFile: raw.keyFile, principal: raw.principal, timeoutMs, maxResponseBytes: SETSPARK_DEFAULTS.maxResponseBytes, approvers }); ++} ++ ++// Lower-case user name to Discord user id. The binding derives it from its ++// users; the verbs turn a decision's required_approvers from names into ++// `discord:` and back, so the model never handles an id. ++function loadApprovers(raw, where) { ++ if (!isObject(raw) || Object.keys(raw).length > 64) throw new Error(`${where}: must be an object of at most 64 names`); ++ for (const [name, id] of Object.entries(raw)) { ++ if (name.length === 0 || name.length > 64 || name !== name.trim().toLowerCase() || /[\u0000-\u001f\u007f]/.test(name)) throw new Error(`${where}: ${JSON.stringify(name.slice(0, 64))} must be a trimmed lower-case name`); ++ if (typeof id !== "string" || !SNOWFLAKE.test(id)) throw new Error(`${where}.${name}: must be a Discord user id`); ++ } ++ if (new Set(Object.values(raw)).size !== Object.keys(raw).length) throw new Error(`${where}: one Discord user id under two names`); ++ return Object.freeze({ ...raw }); + } + + function checkPrivateFile(path, what) { +@@ -354,6 +368,43 @@ function record(body) { + return isObject(body) ? body : {}; + } + ++// A decision's required_approvers as the model gives them: names of the ++// binding's users. Each becomes `discord:`; anything else is refused ++// before a request, since the service checks an approval's author against ++// these values and a stored name could never be approved. ++function approverIds(config, v) { ++ const map = config.approvers || {}; ++ const names = Object.keys(map).join(", ") || "(none)"; ++ if (!Array.isArray(v) || v.length === 0 || v.length > 16) throw bad(`required_approvers must be a list of 1 to 16 names; use names from: ${names}`); ++ const ids = v.map((a) => { ++ const k = typeof a === "string" ? a.trim().replace(/^@/, "").toLowerCase() : ""; ++ if (k.length > 0 && Object.hasOwn(map, k)) return `discord:${map[k]}`; ++ throw bad(`required_approvers: ${JSON.stringify(String(a).slice(0, 64))} is not a known user; use names from: ${names}`); ++ }); ++ if (new Set(ids).size !== ids.length) throw bad("required_approvers names the same person twice"); ++ return ids; ++} ++ ++function withApprovers(config, props) { ++ return props.required_approvers === undefined ? props : { ...props, required_approvers: approverIds(config, props.required_approvers) }; ++} ++ ++// The way back: any Discord user id in a record, bare or `discord:`, is ++// shown as the binding's name for it or "unknown user". ++function namesFor(config, value) { ++ const byId = new Map(Object.entries(config.approvers || {}).map(([n, id]) => [id, n])); ++ const walk = (v) => { ++ if (typeof v === "string") { ++ const m = /^(?:discord:)?([0-9]{17,20})$/.exec(v); ++ return m ? byId.get(m[1]) || "unknown user" : v; ++ } ++ if (Array.isArray(v)) return v.map(walk); ++ if (isObject(v)) return Object.fromEntries(Object.entries(v).map(([k, x]) => [k, walk(x)])); ++ return v; ++ }; ++ return walk(value); ++} ++ + export const setsparkVerbs = Object.freeze({ + async record_list(config, params, state, deps) { + const type = needType(params); +@@ -369,17 +420,17 @@ export const setsparkVerbs = Object.freeze({ + } + } + const r = await callApi(config, { method: "GET", path: `/v1/records?${q}` }, deps); +- const items = Array.isArray(record(r.body).items) ? record(r.body).items.filter(isObject) : []; ++ const items = Array.isArray(record(r.body).items) ? record(r.body).items.filter(isObject).map((x) => namesFor(config, x)) : []; + return { verb: "record_list", recordType: type, items, limit, offset }; + }, + async record_get(config, params, state, deps) { + const id = needId(params); + const r = await callApi(config, { method: "GET", path: `/v1/records/${id}` }, deps); +- return { verb: "record_get", id, record: record(r.body) }; ++ return { verb: "record_get", id, record: namesFor(config, record(r.body)) }; + }, + async record_create(config, params, state, deps) { + const type = needType(params); +- const rec = needObject(params, "record"); ++ const rec = withApprovers(config, needObject(params, "record")); + if (typeof rec.title !== "string" || rec.title.trim().length === 0) throw bad("record.title is required"); + const r = await write(config, state, "POST", "/v1/records", { record_type: type, record: rec }, deps); + return { verb: "record_create", key: r.key, recordType: type, record: record(r.body) }; +@@ -387,7 +438,7 @@ export const setsparkVerbs = Object.freeze({ + async record_update(config, params, state, deps) { + const id = needId(params); + const revision = needInt(params, "revision", 1, 1000000000); +- const fields = needObject(params, "fields"); ++ const fields = withApprovers(config, needObject(params, "fields")); + if (Object.keys(fields).length === 0) throw bad("fields must name at least one property"); + const r = await write(config, state, "PATCH", `/v1/records/${id}`, { revision, fields }, deps); + return { verb: "record_update", key: r.key, id, from: revision, record: record(r.body) }; +@@ -520,7 +571,7 @@ export const SETSPARK_TOOL_DESCRIPTIONS = Object.freeze({ + }, + record_create: { + label: "Create record", +- description: "Create one SetSpark record; the service allocates the id. Give record_type and the record's properties (title required). Only when the user asked for a record to be created.", ++ description: "Create one SetSpark record; the service allocates the id. Give record_type and the record's properties (title required). A decision's required_approvers is a list of user names (such as jason); the connector turns each into that user's Discord identity and refuses a name it does not know. Only when the user asked for a record to be created.", + snippet: "record_create creates one SetSpark record", + }, + record_update: { +diff --git a/packages/discord/tests/setspark.test.mjs b/packages/discord/tests/setspark.test.mjs +index f88dfb23..1f3bebb7 100644 +--- a/packages/discord/tests/setspark.test.mjs ++++ b/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, resolveToolRoots } from "../src/binding.mjs"; ++import { validateBinding, resolveToolRoots, reloadDiff } from "../src/binding.mjs"; + import { makeRoot, rawBinding } from "./helpers.mjs"; + + const KEY_A = "ssk_" + "a".repeat(40); +@@ -46,6 +46,8 @@ const server = createServer((req, res) => { + if (path === "/v1/records" && req.method === "GET") return json(200, { record_type: "work_item", items: [{ id: "WI-7", record_type: "work_item", revision: 1, title: "Ship it", status: "active" }, { id: "WI-8", record_type: "work_item", revision: 4, title: "Later", status: "active", priority: "low" }], limit: 20, offset: 0 }); + if (path === "/v1/records/WI-7" && req.method === "GET") return json(200, { id: "WI-7", record_type: "work_item", revision: 3, title: "Ship it", status: "active", owner: "Jason", tags: ["a", "b"], accepted_snapshot: { hidden: true }, body: "Two lines.\nOf body." }); + if (path === "/v1/records/WI-7" && req.method === "PATCH") return json(200, { id: "WI-7", record_type: "work_item", revision: parsed.revision + 1, title: "Ship it", ...parsed.fields }); ++ if (path === "/v1/records/DEC-9" && req.method === "GET") return json(200, { id: "DEC-9", record_type: "decision", revision: 2, title: "Pick one", status: "Proposed", required_approvers: ["discord:100000000000000100", "discord:199999999999999999"], approvals: [{ approver: "100000000000000100", at: "t" }] }); ++ if (path === "/v1/records/DEC-9" && req.method === "PATCH") return json(200, { id: "DEC-9", record_type: "decision", revision: parsed.revision + 1, title: "Pick one", ...parsed.fields }); + if (path === "/v1/records/WI-9" && req.method === "PATCH") return json(409, { code: "stale_revision", message: "behind", current_revision: 5, changed_fields: ["status"] }); + if (path === "/v1/resolve") return json(200, { query: "ship", matches: [{ id: "WI-7", record_type: "work_item", title: "Ship it", exact: false }] }); + if (path === "/v1/approval-requests" && req.method === "POST") return json(201, view); +@@ -83,7 +85,7 @@ test("setspark config: a bare https or loopback origin, a private key file, a pr + const root = makeRoot(); + const kf = keyFile(root); + const c = loadSetsparkConfig({ baseUrl: "https://api.setspark.io", keyFile: kf, principal: "sage" }); +- assert.deepEqual(c, { baseUrl: "https://api.setspark.io", keyFile: kf, principal: "sage", timeoutMs: 15000, maxResponseBytes: 262144 }); ++ assert.deepEqual(c, { baseUrl: "https://api.setspark.io", keyFile: kf, principal: "sage", timeoutMs: 15000, maxResponseBytes: 262144, approvers: {} }); + assert.equal(loadSetsparkConfig({ baseUrl: "https://api.setspark.io/", keyFile: kf, principal: "sage" }).baseUrl, "https://api.setspark.io"); + assert.throws(() => loadSetsparkConfig(null), /not an object/); + assert.throws(() => loadSetsparkConfig({ baseUrl: "https://api.setspark.io", keyFile: kf, principal: "sage", extra: 1 }), /unknown key/); +@@ -129,7 +131,7 @@ test("setspark config: the binding's key survives resolveToolRoots and the engin + 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 }); ++ assert.deepEqual(resolved.setspark, { baseUrl: "https://api.setspark.io", keyFile: kf, principal: "sage", timeoutMs: 5000, approvers: { owner: "100000000000000100" } }); + 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"); +@@ -138,6 +140,54 @@ test("setspark config: the binding's key survives resolveToolRoots and the engin + assert.ok(!enabledToolNames(plain).some((n) => SETSPARK_TOOL_NAMES.includes(n))); + }); + ++test("setspark config: approvers come from the binding's users, never from the binding's setspark key", () => { ++ const root = makeRoot(); ++ const docs = join(root, "docs"); ++ mkdirSync(docs); ++ const kf = keyFile(root); ++ const users = [{ id: "100000000000000100", name: "Jason" }, { id: "100000000000000101", name: "carmen" }]; ++ const b = validateBinding(rawBinding({ users, tools: { roots: [{ name: "docs", path: docs }], setspark: { baseUrl: "https://api.setspark.io", keyFile: kf, principal: "sage" } } })); ++ assert.deepEqual(b.tools.setspark.approvers, { jason: "100000000000000100", carmen: "100000000000000101" }); ++ assert.throws(() => validateBinding(rawBinding({ users, tools: { roots: [{ name: "docs", path: docs }], setspark: { baseUrl: "https://api.setspark.io", keyFile: kf, principal: "sage", approvers: { mallory: "100000000000000199" } } } })), /approvers come from users/); ++ assert.throws(() => validateBinding(rawBinding({ users: [{ id: "100000000000000100", name: "Jason" }, { id: "100000000000000101", name: "jason" }], tools: { roots: [{ name: "docs", path: docs }], setspark: { baseUrl: "https://api.setspark.io", keyFile: kf, principal: "sage" } } })), /two users named jason/); ++ const withCarmenOut = validateBinding(rawBinding({ users: [users[0]], tools: { roots: [{ name: "docs", path: docs }], setspark: { baseUrl: "https://api.setspark.io", keyFile: kf, principal: "sage" } } })); ++ assert.throws(() => reloadDiff(b, withCarmenOut), /tools cannot change/, "pi's approvers are fixed at start, so a user change needs a restart"); ++ const channelsOnly = validateBinding(rawBinding({ users: [{ ...users[0], channels: [rawBinding().channels[0].id] }, users[1]], tools: { roots: [{ name: "docs", path: docs }], setspark: { baseUrl: "https://api.setspark.io", keyFile: kf, principal: "sage" } } })); ++ assert.doesNotThrow(() => reloadDiff(b, channelsOnly), "a user's channels still reload"); ++ assert.throws(() => loadSetsparkConfig({ baseUrl: "https://api.setspark.io", keyFile: kf, principal: "sage", approvers: { jason: "not-an-id" } }), /approvers/); ++ assert.throws(() => loadSetsparkConfig({ baseUrl: "https://api.setspark.io", keyFile: kf, principal: "sage", approvers: { jason: "100000000000000100", jay: "100000000000000100" } }), /approvers/); ++}); ++ ++test("setspark verbs: required_approvers go out as discord ids from names and come back as names", async () => { ++ const root = makeRoot(); ++ const named = createToolSet(loadToolsConfig({ roots: [{ name: "docs", path: root }], maxCallsPerTurn: 12, setspark: { baseUrl: base, keyFile: keyFile(root), principal: "sage", timeoutMs: 1000, approvers: { jason: "100000000000000100", carmen: "100000000000000101" } } })); ++ named.setTurn(TURN); ++ seen.length = 0; ++ const created = await named.call("record_create", { record_type: "decision", record: { title: "Pick one", status: "Proposed", work_item: "SS-1", required_approvers: ["Jason", "@carmen"] } }); ++ assert.equal(created.ok, true, created.text); ++ assert.deepEqual(JSON.parse(seen[0].body).record.required_approvers, ["discord:100000000000000100", "discord:100000000000000101"]); ++ const unknown = await named.call("record_create", { record_type: "decision", record: { title: "Pick one", required_approvers: ["Jason", "Mallory"] } }); ++ assert.equal(unknown.ok, false); ++ assert.match(unknown.text, /"Mallory" is not a known user; use names from: jason, carmen/); ++ const raw = await named.call("record_create", { record_type: "decision", record: { title: "Pick one", required_approvers: ["discord:100000000000000100"] } }); ++ assert.equal(raw.ok, false, "an id is not a name"); ++ const twice = await named.call("record_create", { record_type: "decision", record: { title: "Pick one", required_approvers: ["jason", "Jason"] } }); ++ assert.match(twice.text, /the same person twice/); ++ const notList = await named.call("record_update", { id: "DEC-9", revision: 2, fields: { required_approvers: "jason" } }); ++ assert.equal(notList.ok, false); ++ assert.equal(seen.length, 1, "refused calls send nothing"); ++ const updated = await named.call("record_update", { id: "DEC-9", revision: 2, fields: { required_approvers: ["carmen"] } }); ++ assert.equal(updated.ok, true, updated.text); ++ assert.deepEqual(JSON.parse(seen[1].body).fields.required_approvers, ["discord:100000000000000101"]); ++ const got = await named.call("record_get", { id: "DEC-9" }); ++ assert.match(got.text, /required_approvers: jason, unknown user/); ++ assert.doesNotMatch(got.text, /1000000000000001|1999999999/, "no discord id reaches the model"); ++ const none = createToolSet(loadToolsConfig({ roots: [{ name: "docs", path: root }], setspark: { baseUrl: base, keyFile: keyFile(root), principal: "sage", timeoutMs: 1000 } })); ++ none.setTurn(TURN); ++ const noUsers = await none.call("record_create", { record_type: "decision", record: { title: "Pick one", required_approvers: ["jason"] } }); ++ assert.match(noUsers.text, /use names from: \(none\)/); ++}); ++ + test("setspark keys: read per call, one printable token per file, rotation without a restart", async () => { + const root = makeRoot(); + const c = config(root); diff --git a/packages/discord/README.md b/packages/discord/README.md index 4703fa86..c84b6afe 100644 --- a/packages/discord/README.md +++ b/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, 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 | +| `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 (approver names come from `users`, lower-cased; a decision's `required_approvers` are written as `discord:` from those names; SetSpark tool text and refusals never carry a Discord user id: a mention, a `discord:` value or a standalone 17 to 20 digit run reads as the user's name or `unknown user`, while the connector's approval request keeps the bare ids; with `setspark` set, a change to a user's id or name refuses the reload, since pi's approvers are fixed at start) (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 diff --git a/packages/discord/src/binding.mjs b/packages/discord/src/binding.mjs index c9349b81..19ed0a5d 100644 --- a/packages/discord/src/binding.mjs +++ b/packages/discord/src/binding.mjs @@ -217,7 +217,7 @@ export function validateBinding(raw, where = "binding") { maxFileBytes: requireInteger(mergedTools, "maxFileBytes", `${where}.tools`, { min: 1024, max: 4 * 1024 * 1024 }), maxCallsPerTurn: requireInteger(mergedTools, "maxCallsPerTurn", `${where}.tools`, { min: 1, max: 64 }), web: raw.tools.web === undefined ? null : webConfig(raw.tools.web, `${where}.tools.web`), - setspark: raw.tools.setspark === undefined ? null : setsparkConfig(raw.tools.setspark, `${where}.tools.setspark`), + setspark: raw.tools.setspark === undefined ? null : setsparkConfig(raw.tools.setspark, `${where}.tools.setspark`, users), }); } @@ -328,9 +328,16 @@ export function resolveContextFiles(binding, repo) { // Tool roots must exist as real directories on this host, not symlinks, and // must not sit inside the data root (bindings, tokens, journals) or contain // it. Returns the resolved config the engine hands the extension. -function setsparkConfig(raw, where) { +function setsparkConfig(raw, where, users) { + if (raw !== null && typeof raw === "object" && Object.hasOwn(raw, "approvers")) throw new DiscordError(`${where}.approvers: not allowed; approvers come from users`); + const approvers = {}; + for (const u of users) { + const name = u.name.trim().toLowerCase(); + if (Object.hasOwn(approvers, name)) throw new DiscordError(`${where}: two users named ${name}; approver names must be distinct`); + approvers[name] = u.id; + } try { - return loadSetsparkConfig(raw, where); + return loadSetsparkConfig(raw !== null && typeof raw === "object" && !Array.isArray(raw) ? { ...raw, approvers } : raw, where); } catch (err) { throw new DiscordError(err.message); } @@ -363,5 +370,5 @@ export function resolveToolRoots(binding, { dataRoot }) { // 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 } } : {}) }; + 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, approvers: { ...ss.approvers } } } : {}) }; } diff --git a/packages/discord/src/setspark.mjs b/packages/discord/src/setspark.mjs index e33c84b3..25c5a688 100644 --- a/packages/discord/src/setspark.mjs +++ b/packages/discord/src/setspark.mjs @@ -69,7 +69,7 @@ const isObject = (v) => v !== null && typeof v === "object" && !Array.isArray(v) export function loadSetsparkConfig(raw, where = "setspark") { if (!isObject(raw)) throw new Error(`${where}: not an object`); for (const k of Object.keys(raw)) { - if (!["baseUrl", "keyFile", "principal", "timeoutMs"].includes(k)) throw new Error(`${where}: unknown key ${JSON.stringify(k)}`); + if (!["baseUrl", "keyFile", "principal", "timeoutMs", "approvers"].includes(k)) throw new Error(`${where}: unknown key ${JSON.stringify(k)}`); } if (typeof raw.baseUrl !== "string") throw new Error(`${where}.baseUrl: must be a url string`); let u; @@ -86,7 +86,21 @@ export function loadSetsparkConfig(raw, where = "setspark") { if (typeof raw.principal !== "string" || !PRINCIPAL.test(raw.principal)) throw new Error(`${where}.principal: must match ${PRINCIPAL}`); const timeoutMs = raw.timeoutMs === undefined ? SETSPARK_DEFAULTS.timeoutMs : raw.timeoutMs; if (!Number.isInteger(timeoutMs) || timeoutMs < 1000 || timeoutMs > 60000) throw new Error(`${where}.timeoutMs: must be an integer between 1000 and 60000`); - return Object.freeze({ baseUrl: u.origin, keyFile: raw.keyFile, principal: raw.principal, timeoutMs, maxResponseBytes: SETSPARK_DEFAULTS.maxResponseBytes }); + const approvers = loadApprovers(raw.approvers === undefined ? {} : raw.approvers, `${where}.approvers`); + return Object.freeze({ baseUrl: u.origin, keyFile: raw.keyFile, principal: raw.principal, timeoutMs, maxResponseBytes: SETSPARK_DEFAULTS.maxResponseBytes, approvers }); +} + +// Lower-case user name to Discord user id. The binding derives it from its +// users; the verbs turn a decision's required_approvers from names into +// `discord:` and back, so the model never handles an id. +function loadApprovers(raw, where) { + if (!isObject(raw) || Object.keys(raw).length > 64) throw new Error(`${where}: must be an object of at most 64 names`); + for (const [name, id] of Object.entries(raw)) { + if (name.length === 0 || name.length > 64 || name !== name.trim().toLowerCase() || /[\u0000-\u001f\u007f]/.test(name)) throw new Error(`${where}: ${JSON.stringify(name.slice(0, 64))} must be a trimmed lower-case name`); + if (typeof id !== "string" || !SNOWFLAKE.test(id)) throw new Error(`${where}.${name}: must be a Discord user id`); + } + if (new Set(Object.values(raw)).size !== Object.keys(raw).length) throw new Error(`${where}: one Discord user id under two names`); + return Object.freeze({ ...raw }); } function checkPrivateFile(path, what) { @@ -248,8 +262,8 @@ export async function callApi(config, { method, path, body, idempotencyKey: key return finish(resolve, { status, body: json }); } const err = isObject(json) ? json : {}; - const code = typeof err.code === "string" ? err.code.slice(0, 64) : null; - const extra = { status, code, message: cutMessage(err.message) }; + const code = typeof err.code === "string" ? hideIds(config, err.code).slice(0, 64) : null; + const extra = { status, code, message: cutMessage(typeof err.message === "string" ? hideIds(config, err.message) : err.message) }; if (status === 409) { if (err.current_revision !== undefined) extra.currentRevision = err.current_revision; if (Array.isArray(err.changed_fields)) extra.changedFields = err.changed_fields.filter((f) => typeof f === "string").slice(0, 32); @@ -263,14 +277,15 @@ export async function callApi(config, { method, path, body, idempotencyKey: key } // How a refusal reads to the model and in the turn record: the fixed -// reason, the code, and on 409 the fields that changed. Never the raw body. -export function renderRefusal(err) { +// reason, the code, and on 409 the fields that changed. Never the raw body, +// and never a Discord user id (hideIds). +export function renderRefusal(err, config = null) { let s = `refused: ${err.reason}`; if (err.code) s += ` (code ${err.code})`; if (err.currentRevision !== undefined) s += `; current revision ${err.currentRevision}`; if (Array.isArray(err.changedFields) && err.changedFields.length > 0) s += `; changed: ${err.changedFields.join(", ")}`; if (err.message && err.message !== err.reason) s += `\n${err.message}`; - return s; + return hideIds(config, s); } // --- the verbs (contract: shared-signals stack/api/openapi.json at a5425a2) --- @@ -324,7 +339,7 @@ function needObject(params, name) { if (!isObject(v)) throw bad(`${name} must be an object`); const size = Buffer.byteLength(JSON.stringify(v), "utf8"); if (size > RECORD_MAX_BYTES) throw bad(`${name} is over ${RECORD_MAX_BYTES} bytes`); - for (const k of Object.keys(v)) if (!PROP_NAME.test(k)) throw bad(`${name} has a property name that is not allowed: ${k.slice(0, 32)}`); + for (const k of Object.keys(v)) if (!PROP_NAME.test(k)) throw bad(`${name} has a property name that is not allowed: ${hideIds(null, k).slice(0, 32)}`); return v; } @@ -354,6 +369,52 @@ function record(body) { return isObject(body) ? body : {}; } +// A decision's required_approvers as the model gives them: names of the +// binding's users. Each becomes `discord:`; anything else is refused +// before a request, since the service checks an approval's author against +// these values and a stored name could never be approved. +function approverIds(config, v) { + const map = config.approvers || {}; + const names = Object.keys(map).join(", ") || "(none)"; + if (!Array.isArray(v) || v.length === 0 || v.length > 16) throw bad(`required_approvers must be a list of 1 to 16 names; use names from: ${names}`); + // The refusal names the entry's position, never its value: a model that + // wrote an id must not get it echoed back. + const ids = v.map((a, i) => { + const k = typeof a === "string" ? a.trim().replace(/^@/, "").toLowerCase() : ""; + if (k.length > 0 && Object.hasOwn(map, k)) return `discord:${map[k]}`; + throw bad(`required_approvers: entry ${i + 1} is not a known user name; use names from: ${names}`); + }); + if (new Set(ids).size !== ids.length) throw bad("required_approvers names the same person twice"); + return ids; +} + +function withApprovers(config, props) { + return props.required_approvers === undefined ? props : { ...props, required_approvers: approverIds(config, props.required_approvers) }; +} + +// The way back, for everything the model reads: a Discord user id never +// appears in SetSpark tool text. A mention (<@id>), a `discord:` value or a +// 17 to 20 digit run standing alone becomes the binding's name for that id, +// or "unknown user". A digit run inside a longer token (a hex digest, SS-027) +// is left alone. The connector's own request keeps the bare ids; only the +// rendered text and refusals go through this. +const DISCORD_ID_TEXT = /<@!?([0-9]{17,20})>|(? [id, n])); + return String(text).replace(DISCORD_ID_TEXT, (_, a, b) => byId.get(a || b) || "unknown user"); +} + +// The same rule over a service value before it is rendered, so a field cut +// at its length limit cannot leave part of an id. Keys and big integers too. +function hideDeep(config, v) { + if (typeof v === "string") return hideIds(config, v); + if (typeof v === "number") return /^[0-9]{17,20}$/.test(String(v)) ? hideIds(config, String(v)) : v; + if (Array.isArray(v)) return v.map((x) => hideDeep(config, x)); + if (isObject(v)) return Object.fromEntries(Object.entries(v).map(([k, x]) => [hideIds(config, k), hideDeep(config, x)])); + return v; +} + export const setsparkVerbs = Object.freeze({ async record_list(config, params, state, deps) { const type = needType(params); @@ -363,7 +424,7 @@ export const setsparkVerbs = Object.freeze({ if (params.filters !== undefined) { if (!isObject(params.filters) || Object.keys(params.filters).length > FILTERS_MAX) throw bad(`filters must be an object of at most ${FILTERS_MAX} properties`); for (const [k, v] of Object.entries(params.filters)) { - if (!PROP_NAME.test(k) || ["record_type", "type", "limit", "offset"].includes(k)) throw bad(`filters: property name not allowed: ${k.slice(0, 32)}`); + if (!PROP_NAME.test(k) || ["record_type", "type", "limit", "offset"].includes(k)) throw bad(`filters: property name not allowed: ${hideIds(null, k).slice(0, 32)}`); if (typeof v !== "string" || v.length === 0 || v.length > 200) throw bad(`filters.${k} must be a short string`); q.set(k, v); } @@ -379,7 +440,7 @@ export const setsparkVerbs = Object.freeze({ }, async record_create(config, params, state, deps) { const type = needType(params); - const rec = needObject(params, "record"); + const rec = withApprovers(config, needObject(params, "record")); if (typeof rec.title !== "string" || rec.title.trim().length === 0) throw bad("record.title is required"); const r = await write(config, state, "POST", "/v1/records", { record_type: type, record: rec }, deps); return { verb: "record_create", key: r.key, recordType: type, record: record(r.body) }; @@ -387,7 +448,7 @@ export const setsparkVerbs = Object.freeze({ async record_update(config, params, state, deps) { const id = needId(params); const revision = needInt(params, "revision", 1, 1000000000); - const fields = needObject(params, "fields"); + const fields = withApprovers(config, needObject(params, "fields")); if (Object.keys(fields).length === 0) throw bad("fields must name at least one property"); const r = await write(config, state, "PATCH", `/v1/records/${id}`, { revision, fields }, deps); return { verb: "record_update", key: r.key, id, from: revision, record: record(r.body) }; @@ -476,7 +537,13 @@ function renderView(v) { return `request ${scalar(v.request_id)} for ${scalar(v.decision_id)} version ${scalar(v.proposal_version)}: ${scalar(v.state) || "?"}; ${approvals.length} of ${who} approvals recorded${v.message_id ? "; bound to a Discord message" : "; no message bound yet"}`; } -export function renderSetspark(name, out) { +// Tool text for the model. `out` is hidden (hideDeep) before any field is +// cut, and the whole text once more after. +export function renderSetspark(name, out, config = null) { + return hideIds(config, renderOut(name, hideDeep(config, out))); +} + +function renderOut(name, out) { if (name === "record_list") { const body = out.items.map(summary).join("\n"); return `${out.items.length} ${out.recordType} record(s) from offset ${out.offset} (limit ${out.limit})\n${body || "(none)"}`; @@ -520,7 +587,7 @@ export const SETSPARK_TOOL_DESCRIPTIONS = Object.freeze({ }, record_create: { label: "Create record", - description: "Create one SetSpark record; the service allocates the id. Give record_type and the record's properties (title required). Only when the user asked for a record to be created.", + description: "Create one SetSpark record; the service allocates the id. Give record_type and the record's properties (title required). A decision's required_approvers is a list of user names (such as jason); the connector turns each into that user's Discord identity and refuses a name it does not know. Only when the user asked for a record to be created.", snippet: "record_create creates one SetSpark record", }, record_update: { diff --git a/packages/discord/src/tools.mjs b/packages/discord/src/tools.mjs index c66365be..5596101b 100644 --- a/packages/discord/src/tools.mjs +++ b/packages/discord/src/tools.mjs @@ -567,8 +567,8 @@ const TOOL_FNS = Object.freeze({ }); const SETSPARK_SET = new Set(SETSPARK_TOOL_NAMES); -function render(name, out) { - if (SETSPARK_SET.has(name)) return renderSetspark(name, out); +function render(name, out, config) { + if (SETSPARK_SET.has(name)) return renderSetspark(name, out, config.setspark); if (name === "list_dir") { const head = `${out.root}/${out.path}`.replace(/\/$/, ""); const body = out.entries.map((e) => (e.type === "dir" ? `${e.name}/` : `${e.name} (${e.bytes} bytes)`)).join("\n"); @@ -648,11 +648,11 @@ export function createToolSet(config) { : name === "git_status" ? { branch: out.branch } : name === "reserve_id" ? { id: out.id } : { path: out.path }; - return { ok: true, text: render(name, out), details: { ...base, ok: true, ...extra, ...(bytes === undefined ? {} : { bytes }), ms: Date.now() - t0 } }; + return { ok: true, text: render(name, out, config), details: { ...base, ok: true, ...extra, ...(bytes === undefined ? {} : { bytes }), ms: Date.now() - t0 } }; }; const refused = (err) => { if (err instanceof SetsparkRefusal) { - return { ok: false, text: renderSetsparkRefusal(err), details: { ...base, ok: false, reason: err.reason, ...(err.code ? { code: err.code } : {}), ...(err.status ? { status: err.status } : {}), ms: Date.now() - t0 } }; + return { ok: false, text: renderSetsparkRefusal(err, config.setspark), details: { ...base, ok: false, reason: err.reason, ...(err.code ? { code: err.code } : {}), ...(err.status ? { status: err.status } : {}), ms: Date.now() - t0 } }; } if (!(err instanceof Refusal) && !(err instanceof WebRefusal) && !(err instanceof GitRefusal)) throw err; const reason = err instanceof GitRefusal ? err.message : err.reason; diff --git a/packages/discord/tests/setspark.test.mjs b/packages/discord/tests/setspark.test.mjs index f88dfb23..7fe2999b 100644 --- a/packages/discord/tests/setspark.test.mjs +++ b/packages/discord/tests/setspark.test.mjs @@ -9,10 +9,11 @@ import { chmodSync, mkdirSync, symlinkSync, writeFileSync } from "node:fs"; import { join } from "node:path"; import { SETSPARK_REFUSAL, SetsparkRefusal, IDEMPOTENCY_HEADER, MESSAGE_MAX_CHARS, USER_AGENT, SETSPARK_TOOL_NAMES, LIST_MAX, - loadSetsparkConfig, readKey, idempotencyKey, connectorKey, callApi, renderRefusal, createSetsparkApi, renderRecord, + loadSetsparkConfig, readKey, idempotencyKey, connectorKey, callApi, renderRefusal, createSetsparkApi, renderRecord, hideIds, } from "../src/setspark.mjs"; import { loadToolsConfig, createToolSet, enabledToolNames } from "../src/tools.mjs"; -import { validateBinding, resolveToolRoots } from "../src/binding.mjs"; +import { validateBinding, resolveToolRoots, reloadDiff } from "../src/binding.mjs"; +import { validateRequest } from "../src/approvals.mjs"; import { makeRoot, rawBinding } from "./helpers.mjs"; const KEY_A = "ssk_" + "a".repeat(40); @@ -46,6 +47,15 @@ const server = createServer((req, res) => { if (path === "/v1/records" && req.method === "GET") return json(200, { record_type: "work_item", items: [{ id: "WI-7", record_type: "work_item", revision: 1, title: "Ship it", status: "active" }, { id: "WI-8", record_type: "work_item", revision: 4, title: "Later", status: "active", priority: "low" }], limit: 20, offset: 0 }); if (path === "/v1/records/WI-7" && req.method === "GET") return json(200, { id: "WI-7", record_type: "work_item", revision: 3, title: "Ship it", status: "active", owner: "Jason", tags: ["a", "b"], accepted_snapshot: { hidden: true }, body: "Two lines.\nOf body." }); if (path === "/v1/records/WI-7" && req.method === "PATCH") return json(200, { id: "WI-7", record_type: "work_item", revision: parsed.revision + 1, title: "Ship it", ...parsed.fields }); + if (path === "/v1/records/DEC-9" && req.method === "GET") return json(200, { id: "DEC-9", record_type: "decision", revision: 2, title: "Pick one", status: "Proposed", required_approvers: ["discord:100000000000000100", "discord:199999999999999999"], approvals: [{ approver: "100000000000000100", at: "t" }] }); + if (path === "/v1/records/DEC-9" && req.method === "PATCH") return json(200, { id: "DEC-9", record_type: "decision", revision: parsed.revision + 1, title: "Pick one", ...parsed.fields }); + // Discord ids where the model could read them (the id-hiding boundary) + if (path === "/v1/records/DEC-10" && req.method === "GET") return json(200, { id: "DEC-10", record_type: "decision", revision: 1, title: "Ask 100000000000000100", required_approvers: ["discord:100000000000000100", "100000000000000101"], proposal_digest: "12345678901234567890abcdef0123456789abcdef0123456789abcdef012345", nested: { text: "approver discord:100000000000000100", "199999999999999999": "keyed" }, owner_id: 100000000000000100, note: `${"x".repeat(485)} 100000000000000101`, body: "Ask <@100000000000000100> and <@!199999999999999999>." }); + if (path === "/v1/records/DEC-10" && req.method === "PATCH") return json(409, { code: "stale_revision", message: "approver discord:100000000000000101 changed", current_revision: 2, changed_fields: ["required_approvers", "100000000000000100"] }); + if (path === "/v1/records/DEC-12" && req.method === "PATCH") return json(400, { code: `${"x".repeat(49)} 100000000000000100`, message: "no" }); + if (path === "/v1/records/DEC-11" && req.method === "PATCH") return json(422, { code: "validation", message: `required_approvers: 199999999999999999 is not a user ${"z".repeat(332)} 100000000000000100` }); + if (path === "/v1/resolve" && new URL(req.url, "http://x").searchParams.get("q") === "leak") return json(200, { query: "leak", matches: [{ id: "DEC-10", record_type: "decision", title: "Ask <@100000000000000101>", exact: false }] }); + if (path === "/v1/documents" && req.method === "POST" && parsed.collection === "leak") return json(201, { id: "doc-2", title: "Notes for 100000000000000100", url: "https://outline.example.test/doc/199999999999999999" }); if (path === "/v1/records/WI-9" && req.method === "PATCH") return json(409, { code: "stale_revision", message: "behind", current_revision: 5, changed_fields: ["status"] }); if (path === "/v1/resolve") return json(200, { query: "ship", matches: [{ id: "WI-7", record_type: "work_item", title: "Ship it", exact: false }] }); if (path === "/v1/approval-requests" && req.method === "POST") return json(201, view); @@ -83,7 +93,7 @@ test("setspark config: a bare https or loopback origin, a private key file, a pr const root = makeRoot(); const kf = keyFile(root); const c = loadSetsparkConfig({ baseUrl: "https://api.setspark.io", keyFile: kf, principal: "sage" }); - assert.deepEqual(c, { baseUrl: "https://api.setspark.io", keyFile: kf, principal: "sage", timeoutMs: 15000, maxResponseBytes: 262144 }); + assert.deepEqual(c, { baseUrl: "https://api.setspark.io", keyFile: kf, principal: "sage", timeoutMs: 15000, maxResponseBytes: 262144, approvers: {} }); assert.equal(loadSetsparkConfig({ baseUrl: "https://api.setspark.io/", keyFile: kf, principal: "sage" }).baseUrl, "https://api.setspark.io"); assert.throws(() => loadSetsparkConfig(null), /not an object/); assert.throws(() => loadSetsparkConfig({ baseUrl: "https://api.setspark.io", keyFile: kf, principal: "sage", extra: 1 }), /unknown key/); @@ -129,7 +139,7 @@ test("setspark config: the binding's key survives resolveToolRoots and the engin 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 }); + assert.deepEqual(resolved.setspark, { baseUrl: "https://api.setspark.io", keyFile: kf, principal: "sage", timeoutMs: 5000, approvers: { owner: "100000000000000100" } }); 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"); @@ -138,6 +148,155 @@ test("setspark config: the binding's key survives resolveToolRoots and the engin assert.ok(!enabledToolNames(plain).some((n) => SETSPARK_TOOL_NAMES.includes(n))); }); +test("setspark config: approvers come from the binding's users, never from the binding's setspark key", () => { + const root = makeRoot(); + const docs = join(root, "docs"); + mkdirSync(docs); + const kf = keyFile(root); + const users = [{ id: "100000000000000100", name: "Jason" }, { id: "100000000000000101", name: "carmen" }]; + const b = validateBinding(rawBinding({ users, tools: { roots: [{ name: "docs", path: docs }], setspark: { baseUrl: "https://api.setspark.io", keyFile: kf, principal: "sage" } } })); + assert.deepEqual(b.tools.setspark.approvers, { jason: "100000000000000100", carmen: "100000000000000101" }); + assert.throws(() => validateBinding(rawBinding({ users, tools: { roots: [{ name: "docs", path: docs }], setspark: { baseUrl: "https://api.setspark.io", keyFile: kf, principal: "sage", approvers: { mallory: "100000000000000199" } } } })), /approvers come from users/); + assert.throws(() => validateBinding(rawBinding({ users: [{ id: "100000000000000100", name: "Jason" }, { id: "100000000000000101", name: "jason" }], tools: { roots: [{ name: "docs", path: docs }], setspark: { baseUrl: "https://api.setspark.io", keyFile: kf, principal: "sage" } } })), /two users named jason/); + const withCarmenOut = validateBinding(rawBinding({ users: [users[0]], tools: { roots: [{ name: "docs", path: docs }], setspark: { baseUrl: "https://api.setspark.io", keyFile: kf, principal: "sage" } } })); + assert.throws(() => reloadDiff(b, withCarmenOut), /tools cannot change/, "pi's approvers are fixed at start, so a user change needs a restart"); + const channelsOnly = validateBinding(rawBinding({ users: [{ ...users[0], channels: [rawBinding().channels[0].id] }, users[1]], tools: { roots: [{ name: "docs", path: docs }], setspark: { baseUrl: "https://api.setspark.io", keyFile: kf, principal: "sage" } } })); + assert.doesNotThrow(() => reloadDiff(b, channelsOnly), "a user's channels still reload"); + assert.throws(() => loadSetsparkConfig({ baseUrl: "https://api.setspark.io", keyFile: kf, principal: "sage", approvers: { jason: "not-an-id" } }), /approvers/); + assert.throws(() => loadSetsparkConfig({ baseUrl: "https://api.setspark.io", keyFile: kf, principal: "sage", approvers: { jason: "100000000000000100", jay: "100000000000000100" } }), /approvers/); +}); + +test("setspark verbs: required_approvers go out as discord ids from names and come back as names", async () => { + const root = makeRoot(); + const named = createToolSet(loadToolsConfig({ roots: [{ name: "docs", path: root }], maxCallsPerTurn: 12, setspark: { baseUrl: base, keyFile: keyFile(root), principal: "sage", timeoutMs: 1000, approvers: { jason: "100000000000000100", carmen: "100000000000000101" } } })); + named.setTurn(TURN); + seen.length = 0; + const created = await named.call("record_create", { record_type: "decision", record: { title: "Pick one", status: "Proposed", work_item: "SS-1", required_approvers: ["Jason", "@carmen"] } }); + assert.equal(created.ok, true, created.text); + assert.deepEqual(JSON.parse(seen[0].body).record.required_approvers, ["discord:100000000000000100", "discord:100000000000000101"]); + const unknown = await named.call("record_create", { record_type: "decision", record: { title: "Pick one", required_approvers: ["Jason", "Mallory"] } }); + assert.equal(unknown.ok, false); + assert.match(unknown.text, /entry 2 is not a known user name; use names from: jason, carmen/); + const raw = await named.call("record_create", { record_type: "decision", record: { title: "Pick one", required_approvers: ["discord:100000000000000100"] } }); + assert.equal(raw.ok, false, "an id is not a name"); + assert.doesNotMatch(raw.text, /100000000000000100/, "a refused id is not echoed back"); + const twice = await named.call("record_create", { record_type: "decision", record: { title: "Pick one", required_approvers: ["jason", "Jason"] } }); + assert.match(twice.text, /the same person twice/); + const notList = await named.call("record_update", { id: "DEC-9", revision: 2, fields: { required_approvers: "jason" } }); + assert.equal(notList.ok, false); + assert.equal(seen.length, 1, "refused calls send nothing"); + const updated = await named.call("record_update", { id: "DEC-9", revision: 2, fields: { required_approvers: ["carmen"] } }); + assert.equal(updated.ok, true, updated.text); + assert.deepEqual(JSON.parse(seen[1].body).fields.required_approvers, ["discord:100000000000000101"]); + const got = await named.call("record_get", { id: "DEC-9" }); + assert.match(got.text, /required_approvers: jason, unknown user/); + assert.doesNotMatch(got.text, /1000000000000001|1999999999/, "no discord id reaches the model"); + const none = createToolSet(loadToolsConfig({ roots: [{ name: "docs", path: root }], setspark: { baseUrl: base, keyFile: keyFile(root), principal: "sage", timeoutMs: 1000 } })); + none.setTurn(TURN); + const noUsers = await none.call("record_create", { record_type: "decision", record: { title: "Pick one", required_approvers: ["jason"] } }); + assert.match(noUsers.text, /use names from: \(none\)/); +}); + +test("setspark verbs: no Discord user id reaches tool text, whatever shape the service returns it in", async () => { + const root = makeRoot(); + const ids = ["100000000000000100", "100000000000000101", "199999999999999999"]; + const t = createToolSet(loadToolsConfig({ roots: [{ name: "docs", path: root }], maxCallsPerTurn: 12, setspark: { baseUrl: base, keyFile: keyFile(root), principal: "sage", timeoutMs: 1000, approvers: { jason: "100000000000000100", carmen: "100000000000000101" } } })); + t.setTurn(TURN); + const texts = []; + const got = await t.call("record_get", { id: "DEC-10" }); + assert.equal(got.ok, true, got.text); + texts.push(got.text); + assert.match(got.text, /title: Ask jason/); + assert.match(got.text, /required_approvers: jason, carmen/); + assert.match(got.text, /nested: \{"text":"approver jason","unknown user":"keyed"\}/); + assert.match(got.text, /Ask jason and unknown user\./); + assert.match(got.text, /proposal_digest: 12345678901234567890abcdef/, "a digit run inside a digest is not an id"); + const stale = await t.call("record_update", { id: "DEC-10", revision: 1, fields: { status: "Proposed" } }); + assert.equal(stale.ok, false); + assert.match(stale.text, /changed: required_approvers, jason/); + assert.match(stale.text, /approver carmen changed/); + texts.push(stale.text); + const rejected = await t.call("record_update", { id: "DEC-11", revision: 1, fields: { status: "Proposed" } }); + assert.equal(rejected.ok, false); + assert.match(rejected.text, /unknown user is not a user/); + texts.push(rejected.text); + // a field capped before it is shown: the service's code, a bad property + // name, a bad filter name + const code = await t.call("record_update", { id: "DEC-12", revision: 1, fields: { status: "Proposed" } }); + assert.equal(code.ok, false); + texts.push(code.text); + const prop = await t.call("record_create", { record_type: "decision", record: { title: "x", [`${"x".repeat(17)} 100000000000000101`]: "v" } }); + assert.match(prop.text, /property name that is not allowed/); + texts.push(prop.text); + const filter = await t.call("record_list", { record_type: "decision", filters: { [`${"x".repeat(17)} 199999999999999999`]: "v" } }); + assert.match(filter.text, /property name not allowed/); + texts.push(filter.text); + const found = await t.call("resolve_id", { query: "leak" }); + assert.match(found.text, /Ask carmen/); + texts.push(found.text); + const doc = await t.call("create_document", { collection: "leak", title: "Notes", text: "x" }); + assert.equal(doc.ok, true, doc.text); + texts.push(doc.text); + const opened = await t.call("open_approval_request", { decision_id: "DEC-012", proposal_version: 2, proposal_digest: "0123456789abcdef0123456789abcdef" }); + texts.push(opened.text); + for (const text of texts) for (const id of ids) assert.equal(text.includes(id), false, `id ${id} in: ${text}`); + for (const text of texts) assert.doesNotMatch(text, /(? discord:100000000000000100 SS-027 v2"), "unknown user unknown user SS-027 v2"); +}); + +test("setspark contract: a decision made with names opens a request the connector accepts; names stored by an old record still refuse", async () => { + // A local stand-in for the service's contract: records keep + // required_approvers as written (discord:), and the approval-request + // view returns them as bare Discord ids, as shared-signals documents. + const store = new Map(); + let next = 1; + const svc = createServer((req, res) => { + const chunks = []; + req.on("data", (c) => chunks.push(c)); + req.on("end", () => { + const parsed = chunks.length ? JSON.parse(Buffer.concat(chunks).toString("utf8")) : null; + const json = (status, obj) => { + res.writeHead(status, { "content-type": "application/json" }); + res.end(JSON.stringify(obj)); + }; + if (req.url === "/v1/records" && req.method === "POST") { + const id = `DEC-${next++}`; + store.set(id, { id, record_type: parsed.record_type, revision: 1, ...parsed.record }); + return json(201, store.get(id)); + } + if (req.url === "/v1/approval-requests" && req.method === "POST") { + const d = store.get(parsed.decision_id); + if (!d) return json(404, { code: "not_found", message: "no decision" }); + const bare = d.required_approvers.map((a) => (typeof a === "string" && a.startsWith("discord:") ? a.slice(8) : a)); + return json(201, { request_id: next++, decision_id: d.id, state: "open", proposal_version: parsed.proposal_version, proposal_digest: parsed.proposal_digest, required_approvers: bare, approvals: [], message_id: null }); + } + return json(404, { code: "not_found", message: "no route" }); + }); + }); + svc.listen(0, "127.0.0.1"); + await once(svc, "listening"); + try { + const root = makeRoot(); + const t = createToolSet(loadToolsConfig({ roots: [{ name: "docs", path: root }], maxCallsPerTurn: 12, setspark: { baseUrl: `http://127.0.0.1:${svc.address().port}`, keyFile: keyFile(root), principal: "sage", timeoutMs: 1000, approvers: { jason: "100000000000000100", carmen: "100000000000000101" } } })); + t.setTurn(TURN); + const made = await t.call("record_create", { record_type: "decision", record: { title: "Pick one", status: "Proposed", required_approvers: ["jason", "carmen"] } }); + assert.equal(made.ok, true, made.text); + const opened = await t.call("open_approval_request", { decision_id: "DEC-1", proposal_version: 1, proposal_digest: "0123456789abcdef" }); + assert.equal(opened.ok, true, opened.text); + const request = validateRequest(opened.details.request); + assert.deepEqual([...request.approvers], ["100000000000000100", "100000000000000101"]); + // DEC-009's shape: names written before this fix. The connector refuses. + store.set("DEC-9", { id: "DEC-9", record_type: "decision", revision: 1, title: "Old", required_approvers: ["Jason", "Carmen"] }); + const old = await t.call("open_approval_request", { decision_id: "DEC-9", proposal_version: 1, proposal_digest: "0123456789abcdef" }); + assert.equal(old.ok, true, old.text); + assert.throws(() => validateRequest(old.details.request), /bad approver id/); + } finally { + svc.close(); + } +}); + test("setspark keys: read per call, one printable token per file, rotation without a restart", async () => { const root = makeRoot(); const c = config(root);