Skip to content

fix: announcement ordering, UTF-16-safe label cap, base prompt spend list #3747

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Christian-Sidak wants to merge 1 commit into
base: main
Choose a base branch
from
+16 −8
Open

fix: announcement ordering, UTF-16-safe label cap, base prompt spend list #3747

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion mcpjam-inspector/server/routes/v1/__tests__/agent-op-registry.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.",

@cubic-dev-ai cubic-dev-ai Bot Aug 6, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: The updated base prompt line now lists 'running a third-party tool' alongside quota-spending actions and describes schedule changes generically as spend. If call_server_tool is actually gated for external side effects rather than quota consumption, and only enabling a schedule (not disabling one) creates recurring spend, this wording could mislead the agent/user about what each action actually costs. Consider splitting the description so call_server_tool is described as an external action requiring approval, and only 'enabling' a schedule is called out as spend.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At mcpjam-inspector/server/routes/v1/__tests__/agent-op-registry.test.ts, line 562:

<comment>The updated base prompt line now lists 'running a third-party tool' alongside quota-spending actions and describes schedule changes generically as spend. If call_server_tool is actually gated for external side effects rather than quota consumption, and only enabling a schedule (not disabling one) creates recurring spend, this wording could mislead the agent/user about what each action actually costs. Consider splitting the description so call_server_tool is described as an external action requiring approval, and only 'enabling' a schedule is called out as spend.</comment>

<file context>
@@ -559,7 +559,7 @@ const PROMPT_BEFORE_REGISTRY = [
   "- 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.",
</file context>

"- 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.",
Expand Down
2 changes: 1 addition & 1 deletion mcpjam-inspector/server/routes/v1/agent.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.",

@coderabbitai coderabbitai Bot Aug 6, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Correct the shared base-prompt classification.

The prompt currently presents call_server_tool as quota spending and treats all schedule changes as spending. The registry shows that call_server_tool is gated for external side effects, while disabling a schedule does not spend quota.

  • mcpjam-inspector/server/routes/v1/agent.ts#L651-L651: Describe only enabling a schedule as recurring spend, and describe call_server_tool as an external action that still requires approval.
  • mcpjam-inspector/server/routes/v1/__tests__/agent-op-registry.test.ts#L562-L562: Update the frozen prompt literal to match the corrected production wording.
📍 Affects 2 files
  • mcpjam-inspector/server/routes/v1/agent.ts#L651-L651 (this comment)
  • mcpjam-inspector/server/routes/v1/__tests__/agent-op-registry.test.ts#L562-L562
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@mcpjam-inspector/server/routes/v1/agent.ts` at line 651, Correct the shared
base prompt wording near the action-classification text in
mcpjam-inspector/server/routes/v1/agent.ts:651-651 so only enabling a schedule
is described as recurring quota or credit spending, while call_server_tool is
described as an external action requiring approval; preserve the instruction
that these tools propose actions rather than execute them. Update the
corresponding frozen prompt literal in
mcpjam-inspector/server/routes/v1/__tests__/agent-op-registry.test.ts:562-562 to
exactly match the production wording.

"- 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.",
Expand Down
7 changes: 6 additions & 1 deletion slack-app/listeners/actions/proposal-button.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.`;
Expand All @@ -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>.`;

@cubic-dev-ai cubic-dev-ai Bot Aug 6, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: Reordering kind-vs-URL to fix the cancel-with-resource case is correct, but it also silently drops the server-built "follow it here" link for the other known kinds. Before this change the URL was consulted before the switch, so a generate, schedule, or external outcome that carries a resource URL surfaced the link; now the URL is used only for start and unknown kinds, so those outcomes lose the deep link even when the server supplied one. Two concrete knock-ons: the JSDoc just above this function still states "A URL wins over the copy when there is one," which no longer matches the implementation, and the characterization-test comment "A URL outranks the copy, whatever the kind" encodes the old semantics. If URL-scoped-to-start is intended, update the doc comment (and re-check whether generate/schedule/external ops are expected to return resource URLs whose links should not regress); otherwise restore the URL handling for those kinds.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At slack-app/listeners/actions/proposal-button.js, line 64:

<comment>Reordering kind-vs-URL to fix the cancel-with-resource case is correct, but it also silently drops the server-built "follow it here" link for the other known kinds. Before this change the URL was consulted before the switch, so a `generate`, `schedule`, or `external` outcome that carries a resource URL surfaced the link; now the URL is used only for `start` and unknown kinds, so those outcomes lose the deep link even when the server supplied one. Two concrete knock-ons: the JSDoc just above this function still states "A URL wins over the copy when there is one," which no longer matches the implementation, and the characterization-test comment "A URL outranks the copy, whatever the kind" encodes the old semantics. If URL-scoped-to-start is intended, update the doc comment (and re-check whether generate/schedule/external ops are expected to return resource URLs whose links should not regress); otherwise restore the URL handling for those kinds.</comment>

<file context>
@@ -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:
</file context>

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
Expand Down
13 changes: 8 additions & 5 deletions slack-app/listeners/views/proposal-builder.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand Down