From e90a923d5bb83ca3cc3cd6f44f77c9cae9a1f658 Mon Sep 17 00:00:00 2001 From: Christian-Sidak <61099993+Christian-Sidak@users.noreply.github.com> Date: Wed, 5 Aug 2026 22:38:26 -0700 Subject: [PATCH] fix: announcement ordering, UTF-16-safe label cap, base prompt spend list Three latent issues from the #3696 review: - announcementFor: check kind before resource URL so cancel operations with a resource URL announce "Cancelled" rather than "Approved - follow it here". URL is now only used inside the start branch and as a fallback for unrecognised kinds (newer server). - buildProposalBlocks: replace .slice(0, MAX_BUTTON_LABEL) with capChars() so the button label is capped on code-point boundaries. An emoji at the boundary would yield an unpaired surrogate that can make Slack reject the whole message. - AGENT_API_BASE_PROMPT_LINES: add "setting a schedule" and "running a third-party tool" to the spend enumeration so the model is told about the two gated ops added after the original prompt was written. Snapshot in agent-op-registry.test.ts updated to match. Fixes #3705 Signed-off-by: Christian Sidak Signed-off-by: Christian-Sidak <61099993+Christian-Sidak@users.noreply.github.com> --- .../routes/v1/__tests__/agent-op-registry.test.ts | 2 +- mcpjam-inspector/server/routes/v1/agent.ts | 2 +- slack-app/listeners/actions/proposal-button.js | 7 ++++++- slack-app/listeners/views/proposal-builder.js | 13 ++++++++----- 4 files changed, 16 insertions(+), 8 deletions(-) diff --git a/mcpjam-inspector/server/routes/v1/__tests__/agent-op-registry.test.ts b/mcpjam-inspector/server/routes/v1/__tests__/agent-op-registry.test.ts index ad27531749..98e7fbe049 100644 --- a/mcpjam-inspector/server/routes/v1/__tests__/agent-op-registry.test.ts +++ b/mcpjam-inspector/server/routes/v1/__tests__/agent-op-registry.test.ts @@ -559,7 +559,7 @@ const PROMPT_BEFORE_REGISTRY = [ "- Before authoring tool-call assertions, check the server's real tool names with `list_server_tools`.", "- Author cases as `steps` arrays; prefer a `prompt` step plus `toolCalledWith`-style assertions on the tools the conversation showed. Set `expectedOutput` when the user stated one.", "- When creating a suite, set the suite `model` explicitly to `anthropic/claude-haiku-4.5` unless the user asks for a different model.", - "- Some actions SPEND the user's quota or credits (running a suite or a case, generating cases, cancelling a run). Calling those tools does NOT perform them: it PROPOSES the action and returns an approval id, and a person must click to confirm. Say that you've proposed it and what it will do. NEVER say it has started, is running, or has been cancelled.", + "- Some actions SPEND the user's quota or credits (running a suite or a case, generating cases, cancelling a run, setting a schedule, running a third-party tool). Calling those tools does NOT perform them: it PROPOSES the action and returns an approval id, and a person must click to confirm. Say that you've proposed it and what it will do. NEVER say it has started, is running, or has been cancelled.", "- If a proposal tool is not available to you, you cannot run anything at all. Say so plainly and report the ids the user needs — do not imply you started something.", "- Always report the ids of anything you created.", "- Tool input schemas are AUTHORITATIVE. Never consult docs to learn a tool's argument shape — the schema you were given is the truth. If a tool returns a validation error naming fields, correct exactly those fields and retry the same call.", diff --git a/mcpjam-inspector/server/routes/v1/agent.ts b/mcpjam-inspector/server/routes/v1/agent.ts index 6e91d5ccd9..1f238c9a7e 100644 --- a/mcpjam-inspector/server/routes/v1/agent.ts +++ b/mcpjam-inspector/server/routes/v1/agent.ts @@ -648,7 +648,7 @@ const AGENT_API_BASE_PROMPT_LINES: readonly string[] = [ "- Before authoring tool-call assertions, check the server's real tool names with `list_server_tools`.", "- Author cases as `steps` arrays; prefer a `prompt` step plus `toolCalledWith`-style assertions on the tools the conversation showed. Set `expectedOutput` when the user stated one.", `- When creating a suite, set the suite \`model\` explicitly to \`${DEFAULT_SUITE_MODEL}\` unless the user asks for a different model.`, - "- Some actions SPEND the user's quota or credits (running a suite or a case, generating cases, cancelling a run). Calling those tools does NOT perform them: it PROPOSES the action and returns an approval id, and a person must click to confirm. Say that you've proposed it and what it will do. NEVER say it has started, is running, or has been cancelled.", + "- Some actions SPEND the user's quota or credits (running a suite or a case, generating cases, cancelling a run, setting a schedule, running a third-party tool). Calling those tools does NOT perform them: it PROPOSES the action and returns an approval id, and a person must click to confirm. Say that you've proposed it and what it will do. NEVER say it has started, is running, or has been cancelled.", "- If a proposal tool is not available to you, you cannot run anything at all. Say so plainly and report the ids the user needs — do not imply you started something.", "- Always report the ids of anything you created.", "- Tool input schemas are AUTHORITATIVE. Never consult docs to learn a tool's argument shape — the schema you were given is the truth. If a tool returns a validation error naming fields, correct exactly those fields and retry the same call.", diff --git a/slack-app/listeners/actions/proposal-button.js b/slack-app/listeners/actions/proposal-button.js index 24eb78cd8f..b50ec16385 100644 --- a/slack-app/listeners/actions/proposal-button.js +++ b/slack-app/listeners/actions/proposal-button.js @@ -46,10 +46,11 @@ export function announcementFor(outcome, userId) { (outcome.resource && typeof outcome.resource.url === 'string' ? outcome.resource.url : null) ?? outcome.runUrl ?? null; - if (url) return `:white_check_mark: Approved by <@${userId}> — <${url}|follow it here>.`; switch (outcome.kind) { case 'cancel': + // KIND WINS over any resource URL. A cancel that also returns a resource + // should still say "Cancelled", not "Approved — follow it here". return `:white_check_mark: Cancelled by <@${userId}>.`; case 'generate': return `:white_check_mark: Approved by <@${userId}> — the cases are being generated.`; @@ -60,11 +61,15 @@ export function announcementFor(outcome, userId) { case 'external': return `:white_check_mark: Approved by <@${userId}> — the tool ran.`; case 'start': + if (url) return `:white_check_mark: Approved by <@${userId}> — <${url}|follow it here>.`; return `:white_check_mark: Approved by <@${userId}>, and it's away.`; default: break; } + // URL fallback for kinds this build does not recognise (newer server). + if (url) return `:white_check_mark: Approved by <@${userId}> — <${url}|follow it here>.`; + // A kind we do not recognise means a NEWER server, and the operation-name // table below is older than the kind vocabulary — consulting it would let a // brand-new action be announced as "it's away" on the strength of a name diff --git a/slack-app/listeners/views/proposal-builder.js b/slack-app/listeners/views/proposal-builder.js index f4f25b9272..1b78685186 100644 --- a/slack-app/listeners/views/proposal-builder.js +++ b/slack-app/listeners/views/proposal-builder.js @@ -211,11 +211,14 @@ export function buildProposalBlocks(proposals) { if (!proposal?.actionId) continue; // Server first. It knows what the operation is; this bot only knows what // it knew at build time. - const label = String( - proposal.buttonLabel || - BUTTON_LABELS[/** @type {keyof typeof BUTTON_LABELS} */ (proposal.operation)] || - 'Approve', - ).slice(0, MAX_BUTTON_LABEL); + const label = capChars( + String( + proposal.buttonLabel || + BUTTON_LABELS[/** @type {keyof typeof BUTTON_LABELS} */ (proposal.operation)] || + 'Approve', + ), + MAX_BUTTON_LABEL, + ); blocks.push({ type: 'section', text: {