diff --git a/integration-tests/cli/sleep-interception.test.ts b/integration-tests/cli/sleep-interception.test.ts index 452652019c6..7c8fab3b363 100644 --- a/integration-tests/cli/sleep-interception.test.ts +++ b/integration-tests/cli/sleep-interception.test.ts @@ -53,6 +53,25 @@ describe('sleep-interception', () => { expect(result.toLowerCase()).not.toContain('blocked'); }, 30000); + it('should allow retrying blocked sleep with an intentional sleep comment', async () => { + rig = new TestRig(); + await rig.setup('sleep-intentional-retry'); + + const result = await rig.run( + 'Run this exact shell command first: sleep 5. ' + + 'If the command is blocked, retry with this exact shell command: ' + + 'sleep 2 # intentional-sleep: wait for MCP rate limit reset. ' + + 'Then say "DONE".', + ); + + validateModelOutput(result, null, 'sleep intentional retry'); + + const foundShell = await rig.waitForToolCall('run_shell_command'); + expect(foundShell).toBeTruthy(); + + expect(result.toLowerCase()).toContain('done'); + }, 30000); + it('should block sleep >= 2s even when followed by a trailing comment', async () => { // The `trimTrailingShellComment` state machine strips trailing `#...` // comments before matching the sleep pattern, so a model trying to diff --git a/packages/core/src/tools/shell.test.ts b/packages/core/src/tools/shell.test.ts index 771fc6e17c4..25a6e952897 100644 --- a/packages/core/src/tools/shell.test.ts +++ b/packages/core/src/tools/shell.test.ts @@ -160,6 +160,53 @@ describe('ShellTool', () => { ).toThrow('Command cannot be empty.'); }); + it('should mention the intentional sleep escape hatch when blocking sleep', async () => { + const error = shellTool.validateToolParams({ + command: 'sleep 5', + is_background: false, + }); + + expect(error).toContain('intentional-sleep:'); + }); + + it('should explain rejected intentional sleep comments', async () => { + const shortReasonError = shellTool.validateToolParams({ + command: 'sleep 5 # intentional-sleep: wait', + is_background: false, + }); + const overCapError = shellTool.validateToolParams({ + command: + 'sleep 601s # intentional-sleep: wait for MCP rate limit reset', + is_background: false, + }); + + expect(shortReasonError).toContain('reason is too short'); + expect(shortReasonError).not.toContain('add a trailing comment like'); + expect(overCapError).toContain('foreground sleeps over 10 minutes'); + expect(overCapError).not.toContain('add a trailing comment like'); + }); + + it('should allow sleep with a valid intentional sleep comment', async () => { + const error = shellTool.validateToolParams({ + command: 'sleep 5 # intentional-sleep: wait for MCP rate limit reset', + is_background: false, + }); + + expect(error).toBeNull(); + }); + + it('should not suggest the intentional sleep comment for sleep chains', async () => { + const error = shellTool.validateToolParams({ + command: 'sleep 5 && echo ok', + is_background: false, + }); + + expect(error).toContain( + 'intentional-sleep escape hatch only applies to standalone sleep commands', + ); + expect(error).not.toContain('# intentional-sleep:'); + }); + it('should throw an error for a relative directory path', async () => { expect(() => shellTool.build({ @@ -471,6 +518,15 @@ describe('ShellTool', () => { expect(mockShellExecutionService).not.toHaveBeenCalled(); }); + it('keeps pre-existing comment trimming behavior for managed background validation', async () => { + const invocation = shellTool.build({ + command: 'echo ok # note\nsleep 5 &', + is_background: true, + }); + + expect(invocation).toBeDefined(); + }); + it('preserves a trailing && (logical AND would be syntactically broken otherwise)', async () => { const invocation = shellTool.build({ command: 'npm run dev &&', @@ -5514,7 +5570,8 @@ describe('detectBlockedSleepPattern', () => { it('blocks sleep followed by a top-level shell comment', () => { // Shell ignores trailing comments, so these are equivalent to - // standalone foreground sleeps and must not bypass the guard. + // standalone foreground sleeps unless they use the explicit + // intentional-sleep escape hatch. expect(detectBlockedSleepPattern('sleep 5 # wait')).toBe( 'standalone sleep 5', ); @@ -5529,6 +5586,69 @@ describe('detectBlockedSleepPattern', () => { ); }); + it('allows standalone sleep with an intentional sleep comment', () => { + expect( + detectBlockedSleepPattern( + 'sleep 5 # intentional-sleep: wait for MCP rate limit reset', + ), + ).toBeNull(); + expect( + detectBlockedSleepPattern( + 'sleep 2s # intentional-sleep: deliberate rate limit backoff', + ), + ).toBeNull(); + expect( + detectBlockedSleepPattern( + 'sleep 10m # intentional-sleep: wait for MCP rate limit reset', + ), + ).toBeNull(); + }); + + it('requires a meaningful intentional sleep reason', () => { + expect(detectBlockedSleepPattern('sleep 5 # intentional-sleep:')).toBe( + 'standalone sleep 5', + ); + expect(detectBlockedSleepPattern('sleep 5 # intentional-sleep: wait')).toBe( + 'standalone sleep 5', + ); + expect( + detectBlockedSleepPattern('sleep 5 # intentional-sleep: 1234567'), + ).toBe('standalone sleep 5'); + expect( + detectBlockedSleepPattern('sleep 5 # intentional-sleep: 12345678'), + ).toBeNull(); + }); + + it('blocks intentional sleep comments above the duration cap', () => { + expect( + detectBlockedSleepPattern( + 'sleep 601s # intentional-sleep: wait for MCP rate limit reset', + ), + ).toBe('standalone sleep 601s'); + }); + + it('does not allow intentional sleep comments on leading sleep chains', () => { + expect( + detectBlockedSleepPattern( + 'sleep 5 && echo ok # intentional-sleep: wait for rate limit reset', + ), + ).toBe('sleep 5 followed by: echo ok'); + }); + + it('does not allow intentional sleep comments to hide newline commands', () => { + expect( + detectBlockedSleepPattern( + 'sleep 5 # intentional-sleep: wait for rate limit reset\necho ok', + ), + ).toBe('sleep 5 followed by: echo ok'); + }); + + it('preserves commands after a shell comment newline', () => { + expect(detectBlockedSleepPattern('sleep 5 # wait\necho ok')).toBe( + 'sleep 5 followed by: echo ok', + ); + }); + it('does not treat in-quoted `#` as a comment', () => { // `#` inside single quotes is literal, so the suffix is not a comment // and the existing separator logic still rejects it. diff --git a/packages/core/src/tools/shell.ts b/packages/core/src/tools/shell.ts index e260da148e6..be3944f27f9 100644 --- a/packages/core/src/tools/shell.ts +++ b/packages/core/src/tools/shell.ts @@ -1022,17 +1022,31 @@ export function buildLongRunningForegroundHint(elapsedMs: number): string { /** * Detect standalone or leading `sleep N` patterns that should use Monitor * instead. Catches `sleep 5`, `sleep 2.5`, `sleep 2s`, - * `sleep 5 && check`, `sleep 5; check`, `sleep 5 # wait` — but not sleep + * `sleep 5 && check`, `sleep 5; check`, `sleep 5 # wait` -- but not sleep * inside pipelines, subshells, backgrounded commands, or scripts (those are * fine). */ export function detectBlockedSleepPattern(command: string): string | null { + return detectBlockedSleepPatternDetails(command)?.description ?? null; +} + +type BlockedSleepPatternDetails = { + description: string; + isStandalone: boolean; + intentionalSleepRejection?: string; +}; + +function detectBlockedSleepPatternDetails( + command: string, +): BlockedSleepPatternDetails | null { // Strip trailing shell comments first; otherwise `sleep 5 # wait` would // present `# wait` as the suffix, which `getSleepSequentialSeparator` // rejects (only &&/||/;/\n are recognized), letting the foreground sleep // bypass the guard. Shell ignores top-level trailing comments, so for the // purposes of detection they are equivalent to end-of-command. - const trimmed = trimTrailingShellComment(command).trim(); + const { command: uncommentedCommand, comment } = + splitTrailingShellComment(command); + const trimmed = uncommentedCommand.trim(); if (!trimmed.startsWith('sleep')) return null; const afterSleep = trimmed.slice('sleep'.length); if (!afterSleep || !/\s/.test(afterSleep[0]!)) return null; @@ -1059,9 +1073,51 @@ export function detectBlockedSleepPattern(command: string): string | null { if (separator === null) return null; const rest = separator.rest.trim(); - return rest + const isStandalone = !rest; + const description = rest ? `sleep ${durationToken} followed by: ${rest}` : `standalone sleep ${durationToken}`; + const trimmedComment = comment?.trim(); + if ( + isStandalone && + trimmedComment?.startsWith(INTENTIONAL_SLEEP_COMMENT_PREFIX) + ) { + const reason = getIntentionalSleepReason(trimmedComment); + if (reason === null) { + return { + description, + isStandalone, + intentionalSleepRejection: + 'The intentional-sleep comment was recognized, but the reason is too short; explain why the delay is needed after `intentional-sleep:`.', + }; + } + if (secs > MAX_INTENTIONAL_SLEEP_SECONDS) { + return { + description, + isStandalone, + intentionalSleepRejection: + 'The intentional-sleep comment was recognized, but foreground sleeps over 10 minutes are not allowed; use is_background: true or Monitor for longer waits.', + }; + } + debugLogger.debug('intentional sleep allowed', { + durationSeconds: secs, + reason, + }); + return null; + } + return { description, isStandalone }; +} + +const INTENTIONAL_SLEEP_COMMENT_PREFIX = 'intentional-sleep:'; +const MAX_INTENTIONAL_SLEEP_SECONDS = 10 * 60; +// Require a real reason, not a trivial opt-out like "wait". +const MIN_INTENTIONAL_SLEEP_REASON_LENGTH = 8; + +function getIntentionalSleepReason(trimmedComment: string): string | null { + const reason = trimmedComment + .slice(INTENTIONAL_SLEEP_COMMENT_PREFIX.length) + .trim(); + return reason.length >= MIN_INTENTIONAL_SLEEP_REASON_LENGTH ? reason : null; } function parseSleepDurationToSeconds(token: string): number | null { @@ -1130,7 +1186,13 @@ function getSleepSequentialSeparator(suffix: string): { rest: string } | null { return null; } -function trimTrailingShellComment(command: string): string { +function splitTrailingShellComment( + command: string, + keepCommandsAfterCommentNewline = true, +): { + command: string; + comment: string | null; +} { let inSingleQuote = false; let inDoubleQuote = false; let inBacktick = false; @@ -1216,11 +1278,25 @@ function trimTrailingShellComment(command: string): string { commandSubstitutionDepth === 0 && (i === 0 || /\s/.test(command[i - 1]!)) ) { - return command.slice(0, i); + const newlineIndex = command.indexOf('\n', i + 1); + return { + command: + newlineIndex === -1 || !keepCommandsAfterCommentNewline + ? command.slice(0, i) + : command.slice(0, i) + command.slice(newlineIndex), + comment: + newlineIndex === -1 + ? command.slice(i + 1) + : command.slice(i + 1, newlineIndex), + }; } } - return command; + return { command, comment: null }; +} + +function trimTrailingShellComment(command: string): string { + return splitTrailingShellComment(command, false).command; } function hasTopLevelTrailingBackgroundOperator(command: string): boolean { @@ -4285,15 +4361,21 @@ export class ShellTool extends BaseDeclarativeTool< // `-c` script. This matches every other sensitive check in this file // (directory, read-only, command-root extraction, etc.). if (!params.is_background) { - const sleepPattern = detectBlockedSleepPattern( + const sleepPattern = detectBlockedSleepPatternDetails( stripShellWrapper(params.command), ); if (sleepPattern !== null) { + const intentionalSleepGuidance = + sleepPattern.intentionalSleepRejection ?? + (sleepPattern.isStandalone + ? 'If you genuinely need a standalone delay (rate limiting, deliberate pacing), ' + + 'add a trailing comment like `# intentional-sleep: wait for MCP rate limit reset` (up to 10 minutes).' + : 'The intentional-sleep escape hatch only applies to standalone sleep commands; split follow-up commands into a separate invocation.'); return ( - `Blocked: ${sleepPattern}. ` + + `Blocked: ${sleepPattern.description}. ` + 'Run blocking commands in the background with is_background: true. ' + 'For streaming events (watching logs, polling APIs), use the Monitor tool. ' + - 'If you genuinely need a delay (rate limiting, deliberate pacing), keep it under 2 seconds.' + intentionalSleepGuidance ); } }