fix(discord): row 25 approvers are user names, never Discord ids in tool text (#1509)
Jason's live check after the 20:58Z restart posted no Approve button. The
Discord Sage wrote DEC-009's required_approvers as names; the SetSpark
service stores approvers as discord:<id> and accepted the names, and the
connector correctly refused the approval request ("bad approver id").
- binding.mjs derives setspark.approvers from the binding's users (name to
id); a binding-set approvers key and duplicate names are refused. With
setspark set, a user id or name change refuses the reload (pi's approvers
are fixed at start).
- setspark.mjs: record_create/record_update map required_approvers names to
discord:<id> and refuse unknown names, ids, duplicates and non-lists
before any request, without echoing the value. hideIds turns mentions,
discord: values and standalone 17-20 digit runs into the user's name or
"unknown user" in every verb's text and refusal, including the service
message and code before they are cut. The connector's approval request
keeps the bare ids.
- tests: boundary test over nested, keyed, numeric, mention and cut ids;
a local contract fixture from create through validateRequest, with the
old name-stored shape still refused.
Rocko: R1 revise, R2 revise, R3 approve (81379830..., report da75219f...).
Suites on an index export: 24/90/43/17/14/15/63/18; Discord node tests 173/173.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
This commit is contained in:
@@ -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:<id>,
|
||||
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.
|
||||
@@ -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.
|
||||
@@ -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:<id> 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.
|
||||
@@ -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:<id>` 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:<id>` 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:<id>`; 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})>|(?<![0-9A-Za-z])(?:discord:)?([0-9]{17,20})(?![0-9A-Za-z])/g;
|
||||
+
|
||||
+export function hideIds(config, text) {
|
||||
+ const byId = new Map(Object.entries((config && config.approvers) || {}).map(([n, id]) => [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, /(?<![0-9A-Za-z])[0-9]{12,16}(?![0-9A-Za-z])/, `no part of an id survives a cut: ${text}`);
|
||||
+ // the connector's request keeps the service's bare ids; only text is hidden
|
||||
+ assert.deepEqual([...opened.details.request.approvers], ["100000000000000002", "100000000000000004"]);
|
||||
+ assert.equal(hideIds(null, "<@100000000000000100> 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:<id>), 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);
|
||||
@@ -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:<id>` 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:<id>` 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:<id>`; 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})>|(?<![0-9A-Za-z])(?:discord:)?([0-9]{17,20})(?![0-9A-Za-z])/g;
|
||||
+
|
||||
+export function hideIds(config, text) {
|
||||
+ const byId = new Map(Object.entries((config && config.approvers) || {}).map(([n, id]) => [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, /(?<![0-9A-Za-z])[0-9]{12,16}(?![0-9A-Za-z])/, `no part of an id survives a cut: ${text}`);
|
||||
+ // the connector's request keeps the service's bare ids; only text is hidden
|
||||
+ assert.deepEqual([...opened.details.request.approvers], ["100000000000000002", "100000000000000004"]);
|
||||
+ assert.equal(hideIds(null, "<@100000000000000100> 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:<id>), 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);
|
||||
@@ -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:<id>` 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:<id>` 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:<id>`; 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);
|
||||
@@ -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:<id>` 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
|
||||
|
||||
@@ -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 } } } : {}) };
|
||||
}
|
||||
|
||||
@@ -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:<id>` 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:<id>`; 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})>|(?<![0-9A-Za-z])(?:discord:)?([0-9]{17,20})(?![0-9A-Za-z])/g;
|
||||
|
||||
export function hideIds(config, text) {
|
||||
const byId = new Map(Object.entries((config && config.approvers) || {}).map(([n, id]) => [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: {
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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, /(?<![0-9A-Za-z])[0-9]{12,16}(?![0-9A-Za-z])/, `no part of an id survives a cut: ${text}`);
|
||||
// the connector's request keeps the service's bare ids; only text is hidden
|
||||
assert.deepEqual([...opened.details.request.approvers], ["100000000000000002", "100000000000000004"]);
|
||||
assert.equal(hideIds(null, "<@100000000000000100> 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:<id>), 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);
|
||||
|
||||
Reference in New Issue
Block a user