Skip to content

Commit 3aee515

Browse files
jirispilkaclaude
andauthored
fix: Keep Actor run retries conditional on user intent (#1564)
## Why Run results used to tell the model to start another Actor run after a failed, aborted, or empty run. That could trigger a retry the user did not want. Aborted runs may also have partial dataset items worth checking. Closes #1559 Closes apify/ai-team#255 ## What changed - For SUCCEEDED runs with no dataset items and for FAILED runs, `nextStep` starts with diagnosis. It mentions retrying only if the user wants another attempt. - For ABORTED runs, `nextStep` points out that earlier items remain in the default dataset and leaves another run to the user. - The same text reaches `get-actor-run`, `abort-actor-run`, `call-actor`, and direct Actor tools. It names no additional tool. ## Validation - Updated response tests failed before the wording change and pass afterward. - `pnpm run type-check`, `pnpm run lint`, `pnpm run test:unit` (2631 passed, 1 skipped), `pnpm run format`, and `pnpm run check:agents` pass. AI assistance: Claude Code prepared the original change and mcpc probe. Codex revised the wording, tests, and this description. --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent ea02139 commit 3aee515

3 files changed

Lines changed: 81 additions & 16 deletions

File tree

‎src/tools/actors/actor_run_response.ts‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -599,12 +599,12 @@ function buildSucceededSummaryNextStep(
599599
// so we don't recommend it as `nextStep` — but `kv.summarySuffix` keeps it visible in the summary
600600
// when records exist, so callers can still discover them. Surface the upstream statusMessage so
601601
// a text-only reader sees the actor's own diagnostic (often the only signal here). The nextStep
602-
// stays generic ("re-run the Actor"): this builder is shared by get-actor-run / abort-actor-run,
603-
// which only have a runId and can't know whether the run came from call-actor or a native Actor
604-
// tool — naming a specific tool would mislead callers (see #1007).
602+
// keeps any retry conditional on the user's intent (see #1559).
605603
return {
606604
summary: `SUCCEEDED in ${runTimeSecs}s. No dataset items found.${statusMessageLine(statusMessage)}${kv.summarySuffix}`,
607-
nextStep: `Inspect statusMessage and stats in this response; if the missing output was unexpected, re-run the Actor with adjusted input.`,
605+
nextStep:
606+
'Inspect statusMessage and stats in this response. ' +
607+
'If the user wants another attempt, adjust the input before retrying.',
608608
};
609609
}
610610

@@ -686,12 +686,16 @@ export function buildStatusSummaryNextStep(params: {
686686
case 'FAILED':
687687
return {
688688
summary: `FAILED after ${runTimeSecs}s.${statusMessageLine(statusMessage)}`,
689-
nextStep: `Diagnose using statusMessage and exitCode in this response; re-run the Actor with adjusted input if the cause is fixable.`,
689+
nextStep:
690+
'Diagnose using statusMessage and exitCode in this response. ' +
691+
'If the user wants another attempt, fix the cause before retrying.',
690692
};
691693
case 'ABORTED':
692694
return {
693695
summary: `ABORTED after ${runTimeSecs}s.${statusMessageLine(statusMessage)}`,
694-
nextStep: `Re-run the Actor if you want to retry.`,
696+
nextStep:
697+
`Items written before the abort, if any, remain in the run's default dataset${dataset ? ` (datasetId=${dataset.id})` : ''}. ` +
698+
'Retry only if the user wants another run.',
695699
};
696700
case 'TIMED-OUT':
697701
return buildTimedOutSummaryNextStep(runTimeSecs, dataset, keyValueStore);

‎tests/unit/tools.abort_actor_run.test.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,9 @@ describe('abort-actor-run', () => {
4949
keyValueStores: { default: { id: 'kv-xyz' } },
5050
});
5151
expect(structuredContent.summary).toBeDefined();
52-
expect(structuredContent.nextStep).toBeDefined();
52+
expect(structuredContent.nextStep).toBe(
53+
"Items written before the abort, if any, remain in the run's default dataset (datasetId=dataset-xyz). Retry only if the user wants another run.",
54+
);
5355

5456
// Billing/internal fields must NOT leak.
5557
const dump = JSON.stringify(structuredContent);

‎tests/unit/tools.get_actor_run.response.test.ts‎

Lines changed: 68 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,8 @@ vi.mock('../../src/utils/userid_cache.js', () => ({
2828
* Default mode `get-actor-run` returns: runId, actorId, status, storages, summary, nextStep
2929
* — with no inlined dataset items or KV record bodies, except RAG Web Browser's reserved `TIP` key.
3030
* Tests cover shape invariants and the branching status templates (SUCCEEDED, TIMED-OUT).
31-
* Pure-template states (READY, RUNNING, TIMING-OUT, ABORTING, FAILED, ABORTED) are intentionally
32-
* not asserted here — see the comment above `describe('buildStatusTemplate', ...)` below.
31+
* Pure-template states are asserted only where a rule applies (FAILED and ABORTED only for their
32+
* nextStep) — see the comment above `describe('buildStatusTemplate', ...)` below.
3333
*/
3434

3535
const ACTOR = { username: 'apify', name: 'rag-web-browser' };
@@ -130,6 +130,40 @@ describe('get-actor-run default response', () => {
130130
});
131131
});
132132

133+
it.each([
134+
[
135+
'SUCCEEDED',
136+
{ defaultDatasetId: undefined },
137+
'Inspect statusMessage and stats in this response. If the user wants another attempt, adjust the input before retrying.',
138+
],
139+
[
140+
'FAILED',
141+
{ exitCode: 1 },
142+
'Diagnose using statusMessage and exitCode in this response. If the user wants another attempt, fix the cause before retrying.',
143+
],
144+
[
145+
'ABORTED',
146+
{},
147+
"Items written before the abort, if any, remain in the run's default dataset (datasetId=dataset-xyz). Retry only if the user wants another run.",
148+
],
149+
])(
150+
'returns a nextStep that leaves a retry to the user for %s without dataset items',
151+
async (status, overrides, nextStep) => {
152+
const run = mockSucceededRun({ status, ...overrides });
153+
const result = await (getActorRun as HelperTool).call(
154+
stubToolCallContext(
155+
{ runId: 'run-1', waitSecs: 0 },
156+
stubClient({ run, dataset: mockDataset({ itemCount: 0 }) }),
157+
),
158+
);
159+
160+
const { structuredContent, content } = result as TextToolResult & { structuredContent: RunResponse };
161+
expect(structuredContent.nextStep).toBe(nextStep);
162+
expect(content[1].text).toContain(nextStep);
163+
expect(content[1].text).toContain('the user wants');
164+
},
165+
);
166+
133167
it('surfaces dataset inflatedBytes from stats in structuredContent', async () => {
134168
// The single-dataset GET does not return `inflatedBytes` (only the dataset-list endpoint does);
135169
// this stub injects it to verify the wiring.
@@ -426,12 +460,12 @@ describe('get-actor-run default response', () => {
426460
expect(structuredContent.storages.datasets?.default.itemCount).toBeUndefined();
427461
expect(structuredContent.storages.datasets?.default.fields).toBeUndefined();
428462

429-
// nextStep points at get-dataset-items, not the "no output / re-run" branch.
463+
// nextStep points at get-dataset-items, not the "no output" branch.
430464
expect(structuredContent.nextStep).toContain('get-dataset-items');
431465
expect(structuredContent.nextStep).toContain('datasetId=dataset-xyz');
432-
// Narrative (content[1]) avoids the "re-run" branch wording.
466+
// Narrative (content[1]) avoids the "no output" branch wording.
433467
expect(content[1].text).not.toContain('No dataset items and no key-value records were found');
434-
expect(content[1].text).not.toMatch(/re-run/i);
468+
expect(content[1].text).not.toContain('No dataset items found');
435469
});
436470

437471
it('degrades gracefully when KV listKeys fails: keeps dataset, omits KV', async () => {
@@ -1087,9 +1121,10 @@ const kvWithRecords: RunKeyValueStore = { id: 'kv-1', keys: ['result-a', 'result
10871121
/**
10881122
* Tests below cover real branching in buildSucceededSummaryNextStep / buildTimedOutSummaryNextStep
10891123
* (3 branches each) and the dataset-vs-KV priority + truncation-labelling rules. Pure template
1090-
* re-statements (READY/RUNNING/TIMING-OUT/ABORTING/FAILED/ABORTED) are deliberately not tested
1091-
* here — they would just spell-check the format string and would change in lockstep with any
1092-
* wording tweak. Default-mode integration coverage exercises the dispatch path.
1124+
* re-statements (READY/RUNNING/TIMING-OUT/ABORTING) are deliberately not tested here — they would
1125+
* just spell-check the format string and would change in lockstep with any wording tweak. FAILED
1126+
* and ABORTED are pinned because their nextStep must not ask for a new run (#1559). Default-mode
1127+
* integration coverage exercises the dispatch path.
10931128
*/
10941129
describe('buildStatusTemplate', () => {
10951130
it('SUCCEEDED with dataset items routes to dataset-items nextStep', () => {
@@ -1163,7 +1198,31 @@ describe('buildStatusTemplate', () => {
11631198
const t = buildStatusSummaryNextStep({ run: makeRun('SUCCEEDED') });
11641199
expect(t.summary).toContain('No dataset items found');
11651200
expect(t.summary).not.toContain('Key-value store');
1166-
expect(t.nextStep).toContain('re-run');
1201+
expect(t.nextStep).toBe(
1202+
'Inspect statusMessage and stats in this response. If the user wants another attempt, adjust the input before retrying.',
1203+
);
1204+
});
1205+
1206+
it.each([
1207+
[
1208+
'FAILED',
1209+
'Diagnose using statusMessage and exitCode in this response. If the user wants another attempt, fix the cause before retrying.',
1210+
],
1211+
[
1212+
'ABORTED',
1213+
"Items written before the abort, if any, remain in the run's default dataset (datasetId=ds-1). Retry only if the user wants another run.",
1214+
],
1215+
])('%s nextStep leaves a retry to the user and names no tool', (status, expected) => {
1216+
const t = buildStatusSummaryNextStep({ run: makeRun(status), dataset: datasetWithItems });
1217+
expect(t.nextStep).toBe(expected);
1218+
expect(t.nextStep).toContain('the user wants');
1219+
});
1220+
1221+
it('ABORTED nextStep without a dataset gives no datasetId', () => {
1222+
const t = buildStatusSummaryNextStep({ run: makeRun('ABORTED') });
1223+
expect(t.nextStep).toBe(
1224+
"Items written before the abort, if any, remain in the run's default dataset. Retry only if the user wants another run.",
1225+
);
11671226
});
11681227

11691228
it('SUCCEEDED with both dataset items and KV records picks dataset for nextStep, mentions both in summary', () => {

0 commit comments

Comments
 (0)