Skip to content

Commit 7fa47b4

Browse files
committed
feat: add invalid input as a nudge case for report tool
1 parent f217207 commit 7fa47b4

3 files changed

Lines changed: 67 additions & 45 deletions

File tree

src/tools/report_problem/report_problem.ts

Lines changed: 25 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -26,23 +26,33 @@ const ACKNOWLEDGEMENT = `Problem reported. Thank you — this helps the Apify te
2626
export const REPORT_PROBLEM_NUDGE = `If you cannot resolve this yourself, report it to the Apify team by calling ${HELPER_TOOLS.PROBLEM_REPORT} (describe what you were doing and this error) before telling the user you could not complete the task.`;
2727

2828
/**
29-
* Failure categories that represent expected, user-resolvable control-flow states (fix input, pay,
30-
* approve, fix token) rather than genuine Apify defects. The nudge is suppressed for these so it does
31-
* not flood the channel with paywalls/approvals or steer the agent to give up instead of surfacing the
32-
* payment/approval URL. `INTERNAL_ERROR` and an absent/unknown category still get the nudge.
29+
* Softer nudge for INVALID_INPUT failures (payment/standby short-circuits, actor input rejected at
30+
* runtime, etc.). Most of these are the agent's own mistake, so this doesn't push it to report before
31+
* giving up — but some are a genuine schema/Actor mismatch the agent would otherwise never think to
32+
* report, so it still surfaces the tool's existence.
33+
*/
34+
export const REPORT_PROBLEM_INVALID_INPUT_NUDGE = `If this looks like a bug rather than something fixable by adjusting your request, report it by calling ${HELPER_TOOLS.PROBLEM_REPORT} (describe what you were doing and this error).`;
35+
36+
/**
37+
* Failure categories that represent expected, user-resolvable control-flow states (pay, approve, fix
38+
* token) rather than potential Apify defects. The nudge is suppressed for these so it does not flood
39+
* the channel with paywalls/approvals or steer the agent to give up instead of surfacing the
40+
* payment/approval URL. `INVALID_INPUT` gets the softer {@link REPORT_PROBLEM_INVALID_INPUT_NUDGE}
41+
* instead of being suppressed outright. `INTERNAL_ERROR` and an absent/unknown category still get the
42+
* full {@link REPORT_PROBLEM_NUDGE}.
3343
*/
3444
const NON_NUDGE_FAILURE_CATEGORIES = new Set<string>([
35-
FAILURE_CATEGORY.INVALID_INPUT,
3645
FAILURE_CATEGORY.PERMISSION_APPROVAL_REQUIRED,
3746
FAILURE_CATEGORY.AUTH,
3847
]);
3948

4049
/**
41-
* Append {@link REPORT_PROBLEM_NUDGE} to a failed tool result's text content, returning a shallow
42-
* copy with a new `content` array (never mutates the input). Returns the original unchanged unless the
43-
* result is an error with a text `content[]`, `report-problem` is actually served (`available`), the
44-
* failing tool is not `report-problem` itself, and `failureCategory` is not an expected,
45-
* user-resolvable state (see {@link NON_NUDGE_FAILURE_CATEGORIES}).
50+
* Append a report-problem nudge to a failed tool result's text content, returning a shallow copy with
51+
* a new `content` array (never mutates the input). Returns the original unchanged unless the result is
52+
* an error with a text `content[]`, `report-problem` is actually served (`available`), the failing tool
53+
* is not `report-problem` itself, and `failureCategory` is not an expected, user-resolvable state (see
54+
* {@link NON_NUDGE_FAILURE_CATEGORIES}). Uses {@link REPORT_PROBLEM_INVALID_INPUT_NUDGE} for
55+
* `INVALID_INPUT` and {@link REPORT_PROBLEM_NUDGE} otherwise.
4656
*/
4757
export function appendReportProblemNudge<T>(
4858
result: T,
@@ -53,7 +63,11 @@ export function appendReportProblemNudge<T>(
5363
if (opts.failureCategory !== undefined && NON_NUDGE_FAILURE_CATEGORIES.has(opts.failureCategory)) return result;
5464
const r = result as { isError?: unknown; content?: unknown };
5565
if (r?.isError !== true || !Array.isArray(r.content)) return result;
56-
return { ...r, content: [...r.content, { type: 'text', text: REPORT_PROBLEM_NUDGE }] } as T;
66+
const nudge =
67+
opts.failureCategory === FAILURE_CATEGORY.INVALID_INPUT
68+
? REPORT_PROBLEM_INVALID_INPUT_NUDGE
69+
: REPORT_PROBLEM_NUDGE;
70+
return { ...r, content: [...r.content, { type: 'text', text: nudge }] } as T;
5771
}
5872

5973
export const reportProblemArgsSchema = z.object({

tests/integration/suite.ts

Lines changed: 31 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,19 @@ import { assertStatusMessagePropagated, captureInflightActorRunId, waitForRunAbo
2727

2828
const AUTO_INJECTED_TOOL_NAMES = AUTO_INJECTED_TOOLS.map((t) => t.name);
2929

30+
// report-problem is served only when telemetry is enabled — its sole function is forwarding
31+
// submissions via telemetry (see isToolServable in mcp/server.ts). This suite runs with telemetry
32+
// off (the helper default, so runs emit no Sentry/Segment data), so report-problem is absent from
33+
// the served default set here. Its served/hidden/acknowledge behavior is covered by the unit tests
34+
// (tests/unit/mcp.server.report_problem_gating.test.ts, tests/unit/tools.report_problem.test.ts).
35+
// These helpers give the default set as this suite actually sees it, with report-problem excluded.
36+
function servedDefaultTools(): ToolEntry[] {
37+
return getDefaultTools('default').filter((tool) => tool.name !== HELPER_TOOLS.PROBLEM_REPORT);
38+
}
39+
function servedDefaultToolNames(): string[] {
40+
return getDefaultToolNames().filter((name) => name !== HELPER_TOOLS.PROBLEM_REPORT);
41+
}
42+
3043
// Helper to find tool by name, resolving categories for the given mode on each call.
3144
// This ensures we always validate against the correct mode-specific tool definition
3245
// (e.g. outputSchema may diverge between modes in the future).
@@ -240,10 +253,10 @@ export function createIntegrationTestsSuite(options: IntegrationTestsSuiteOption
240253
it('should list all default tools and Actors', async () => {
241254
client = await createClientFn();
242255
const tools = await client.listTools();
243-
expect(tools.tools.length).toEqual(getDefaultTools('default').length + defaults.actors.length + 4);
256+
expect(tools.tools.length).toEqual(servedDefaultTools().length + defaults.actors.length + 4);
244257

245258
const names = getToolNames(tools);
246-
expectToolNamesToContain(names, getDefaultToolNames());
259+
expectToolNamesToContain(names, servedDefaultToolNames());
247260
expectToolNamesToContain(names, DEFAULT_ACTOR_NAMES);
248261
// get-actor-run + storage/abort helpers are auto-injected alongside call-actor.
249262
expect(names).toContain(HELPER_TOOLS.ACTOR_RUNS_GET);
@@ -258,21 +271,17 @@ export function createIntegrationTestsSuite(options: IntegrationTestsSuiteOption
258271

259272
// Should be equivalent to tools=actors,docs,apify/rag-web-browser
260273
// Note: UI tools (search-actors-widget, fetch-actor-details-widget) are only available in apps mode
274+
// report-problem is telemetry-gated and telemetry is off in this suite, so it is not listed.
261275
const expectedActorsTools = ['fetch-actor-details', 'search-actors', 'call-actor'];
262276
const expectedDocsTools = ['search-apify-docs', 'fetch-apify-docs'];
263-
const expectedReportProblemTools = ['report-problem'];
264277
const expectedActors = [actorNameToToolName('apify/rag-web-browser')];
265278

266-
const expectedTotal = expectedActorsTools.concat(
267-
expectedDocsTools,
268-
expectedReportProblemTools,
269-
expectedActors,
270-
);
279+
const expectedTotal = expectedActorsTools.concat(expectedDocsTools, expectedActors);
271280
expect(names).toHaveLength(expectedTotal.length + 4);
272281

273282
expectToolNamesToContain(names, expectedActorsTools);
274283
expectToolNamesToContain(names, expectedDocsTools);
275-
expectToolNamesToContain(names, expectedReportProblemTools);
284+
expect(names).not.toContain(HELPER_TOOLS.PROBLEM_REPORT);
276285
expectToolNamesToContain(names, expectedActors);
277286
expectToolNamesToContain(names, AUTO_INJECTED_TOOL_NAMES);
278287
// get-actor-run should be automatically included when call-actor is present
@@ -282,27 +291,14 @@ export function createIntegrationTestsSuite(options: IntegrationTestsSuiteOption
282291
});
283292

284293
describe('report-problem', () => {
285-
it('is listed by default and acknowledges a submission', async () => {
294+
// report-problem is served only when telemetry is enabled; this suite runs with telemetry
295+
// off, so end-to-end it must be absent. That gating is what we can verify here without
296+
// emitting telemetry. The served path (listed for non-Anthropic clients, hidden from
297+
// Anthropic clients, acknowledges a submission) is covered by the unit tests
298+
// tests/unit/mcp.server.report_problem_gating.test.ts and tests/unit/tools.report_problem.test.ts.
299+
it('is not served when telemetry is disabled', async () => {
286300
client = await createClientFn();
287301
const names = getToolNames(await client.listTools());
288-
expect(names).toContain(HELPER_TOOLS.PROBLEM_REPORT);
289-
290-
const result = await client.callTool({
291-
name: HELPER_TOOLS.PROBLEM_REPORT,
292-
arguments: {
293-
message: 'fetch-actor-details did not explain the input schema clearly.',
294-
relatedTools: ['fetch-actor-details'],
295-
},
296-
});
297-
298-
expect(result.isError).not.toBe(true);
299-
const content = result.content as { type: string; text: string }[];
300-
expect(content[0].text).toContain('Problem reported');
301-
});
302-
303-
it('is hidden from Anthropic clients', async () => {
304-
client = await createClientFn({ clientName: 'claude-ai' });
305-
const names = getToolNames(await client.listTools());
306302
expect(names).not.toContain(HELPER_TOOLS.PROBLEM_REPORT);
307303
});
308304
});
@@ -336,9 +332,9 @@ export function createIntegrationTestsSuite(options: IntegrationTestsSuiteOption
336332
it('should list all default tools and Actors when enableAddingActors is false', async () => {
337333
client = await createClientFn({ enableAddingActors: false });
338334
const names = getToolNames(await client.listTools());
339-
expect(names.length).toEqual(getDefaultTools('default').length + defaults.actors.length + 4);
335+
expect(names.length).toEqual(servedDefaultTools().length + defaults.actors.length + 4);
340336

341-
expectToolNamesToContain(names, getDefaultToolNames());
337+
expectToolNamesToContain(names, servedDefaultToolNames());
342338
expectToolNamesToContain(names, DEFAULT_ACTOR_NAMES);
343339
expectToolNamesToContain(names, AUTO_INJECTED_TOOL_NAMES);
344340
// get-actor-run should be automatically included when call-actor is present
@@ -1755,7 +1751,9 @@ export function createIntegrationTestsSuite(options: IntegrationTestsSuiteOption
17551751
validateStructuredOutputForTool(result, HELPER_TOOLS.DOCS_FETCH, 'default');
17561752
});
17571753

1758-
it.for(Object.keys(getCategoryTools('default')))(
1754+
// report-problem is telemetry-gated (off in this suite), so it can't load standalone here;
1755+
// its gating is covered by unit tests. Exclude it from the category sweep.
1756+
it.for(Object.keys(getCategoryTools('default')).filter((category) => category !== 'report-problem'))(
17591757
'should load correct tools for %s category',
17601758
async (category) => {
17611759
client = await createClientFn({
@@ -1929,9 +1927,9 @@ export function createIntegrationTestsSuite(options: IntegrationTestsSuiteOption
19291927
// Test with enableAddingActors = false via env var
19301928
client = await createClientFn({ enableAddingActors: false, useEnv: true });
19311929
const names = getToolNames(await client.listTools());
1932-
expect(names.length).toEqual(getDefaultTools('default').length + defaults.actors.length + 4);
1930+
expect(names.length).toEqual(servedDefaultTools().length + defaults.actors.length + 4);
19331931

1934-
expectToolNamesToContain(names, getDefaultToolNames());
1932+
expectToolNamesToContain(names, servedDefaultToolNames());
19351933
expectToolNamesToContain(names, DEFAULT_ACTOR_NAMES);
19361934
expectToolNamesToContain(names, AUTO_INJECTED_TOOL_NAMES);
19371935
// get-actor-run should be automatically included when call-actor is present

tests/unit/tools.report_problem.test.ts

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { describe, expect, it } from 'vitest';
22

33
import { FAILURE_CATEGORY, HELPER_TOOLS } from '../../src/const.js';
44
import {
5+
REPORT_PROBLEM_INVALID_INPUT_NUDGE,
56
REPORT_PROBLEM_NUDGE,
67
appendReportProblemNudge,
78
reportProblem,
@@ -98,7 +99,6 @@ describe('reportProblem', () => {
9899
});
99100

100101
it.each([
101-
['INVALID_INPUT (payment/standby)', FAILURE_CATEGORY.INVALID_INPUT],
102102
['PERMISSION_APPROVAL_REQUIRED', FAILURE_CATEGORY.PERMISSION_APPROVAL_REQUIRED],
103103
['AUTH', FAILURE_CATEGORY.AUTH],
104104
])('does not append for the user-resolvable category %s', (_label, failureCategory) => {
@@ -110,6 +110,16 @@ describe('reportProblem', () => {
110110
expect(nudgeCount(result)).toBe(0);
111111
});
112112

113+
it('appends the softer nudge for INVALID_INPUT (payment/standby, or a genuine input bug)', () => {
114+
const result = appendReportProblemNudge(errorResult(), {
115+
failingToolName: HELPER_TOOLS.ACTOR_CALL,
116+
available: true,
117+
failureCategory: FAILURE_CATEGORY.INVALID_INPUT,
118+
});
119+
expect(result.content.filter((c) => c.text === REPORT_PROBLEM_INVALID_INPUT_NUDGE)).toHaveLength(1);
120+
expect(nudgeCount(result)).toBe(0);
121+
});
122+
113123
it('appends for a genuine INTERNAL_ERROR failure', () => {
114124
const result = appendReportProblemNudge(errorResult(), {
115125
failingToolName: HELPER_TOOLS.ACTOR_CALL,

0 commit comments

Comments
 (0)