Skip to content
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
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
109 changes: 109 additions & 0 deletions src/__tests__/bugfixes.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,115 @@ describe('Agent loop continuation nudge', () => {
})
})

// ---------------------------------------------------------------------------
// Fix 3b: Expanded continuation coverage (PR #1713 review feedback)
// ---------------------------------------------------------------------------
describe('Expanded continuation coverage', () => {
test('newly added verbs trigger continuation', async () => {
const { analyzeContinuationIntent } = await import('../utils/continuation.js')

// Verbs added in the PR: process, download, compile, train, evaluate, etc.
expect(analyzeContinuationIntent("Now I will process the data").shouldNudge).toBe(true)
expect(analyzeContinuationIntent("Let me download the file").shouldNudge).toBe(true)
expect(analyzeContinuationIntent("Time to compile the source").shouldNudge).toBe(true)
expect(analyzeContinuationIntent("I need to train the model").shouldNudge).toBe(true)
expect(analyzeContinuationIntent("So now I will evaluate the results").shouldNudge).toBe(true)
expect(analyzeContinuationIntent("Now I'll test the endpoint").shouldNudge).toBe(true)
expect(analyzeContinuationIntent("Let me extract the archive").shouldNudge).toBe(true)
expect(analyzeContinuationIntent("I will merge the changes").shouldNudge).toBe(true)
expect(analyzeContinuationIntent("Time to deploy to production").shouldNudge).toBe(true)
expect(analyzeContinuationIntent("Now I will install the package").shouldNudge).toBe(true)
expect(analyzeContinuationIntent("I need to configure the server").shouldNudge).toBe(true)
expect(analyzeContinuationIntent("Let me refactor this component").shouldNudge).toBe(true)
expect(analyzeContinuationIntent("Time to optimize the query").shouldNudge).toBe(true)
expect(analyzeContinuationIntent("Now I will upload the artifact").shouldNudge).toBe(true)
expect(analyzeContinuationIntent("I need to convert the format").shouldNudge).toBe(true)
})

test('imperative / declarative patterns trigger continuation', async () => {
const { analyzeContinuationIntent } = await import('../utils/continuation.js')

// "Need to ..." pattern
expect(analyzeContinuationIntent("Need to update the config").shouldNudge).toBe(true)
expect(analyzeContinuationIntent("Need to process these files").shouldNudge).toBe(true)
expect(analyzeContinuationIntent("Need to deploy the changes").shouldNudge).toBe(true)

// "Now ..." pattern (without subject)
expect(analyzeContinuationIntent("Now create the component").shouldNudge).toBe(true)
expect(analyzeContinuationIntent("Now run the tests").shouldNudge).toBe(true)
expect(analyzeContinuationIntent("Now compile everything").shouldNudge).toBe(true)

// "Now ..." should NOT match "Now you ..." (excluded by negative lookahead)
expect(analyzeContinuationIntent("Now you can run the app").shouldNudge).toBe(false)

// "Next I/We ..." pattern
expect(analyzeContinuationIntent("Next I will fix the bug").shouldNudge).toBe(true)
expect(analyzeContinuationIntent("Next we need to add tests").shouldNudge).toBe(true)
expect(analyzeContinuationIntent("Next I should deploy").shouldNudge).toBe(true)
})

test('present-progressive fallback triggers continuation', async () => {
const { analyzeContinuationIntent } = await import('../utils/continuation.js')

// "now processing", "now compiling", "now deploying" with restricted verb list
expect(analyzeContinuationIntent("Task done. Now processing the next batch.").shouldNudge).toBe(true)
expect(analyzeContinuationIntent("Finished step 1. Now compiling the assets.").shouldNudge).toBe(true)
expect(analyzeContinuationIntent("Complete. Now deploying to staging.").shouldNudge).toBe(true)
expect(analyzeContinuationIntent("All set. Now testing the endpoint.").shouldNudge).toBe(true)
expect(analyzeContinuationIntent("Done. Now installing dependencies.").shouldNudge).toBe(true)

// Should NOT match passive/non-action ing-words (regression guard for review feedback)
expect(analyzeContinuationIntent("Now being processed by the system").shouldNudge).toBe(false)
expect(analyzeContinuationIntent("Now waiting for user input").shouldNudge).toBe(false)
expect(analyzeContinuationIntent("Now having some issues").shouldNudge).toBe(false)
})

test('completion marker correctly suppressed by nearby continuation signal', async () => {
const { analyzeContinuationIntent } = await import('../utils/continuation.js')

// "complete" appears mid-sentence before continuation signal — should nudge
expect(analyzeContinuationIntent("The download is complete. Now processing the files.").shouldNudge).toBe(true)
expect(analyzeContinuationIntent("The analysis is done. Let me update the report.").shouldNudge).toBe(true)
expect(analyzeContinuationIntent("Compilation finished. Now deploying the build.").shouldNudge).toBe(true)

// "done" at the very end without continuation signal — no nudge
expect(analyzeContinuationIntent("All tests pass. Task done.").shouldNudge).toBe(false)
expect(analyzeContinuationIntent("The implementation is complete.").shouldNudge).toBe(false)
})

test('shared verb list in continuation.ts avoids duplication', async () => {
const content = await file('utils/continuation.ts').text()

// Should have ACTION_VERBS array and build regexes from it
expect(content).toContain('ACTION_VERBS')
expect(content).toContain('buildContinuationSignals')
expect(content).toContain('VERB_ALT')
expect(content).toContain('VERB_ING')

// The verb list should appear only once as an array definition,
// not repeated across multiple inline regexes
const verbDeclarations = content.match(/ACTION_VERBS\s*=\s*\[/g)
expect(verbDeclarations).toHaveLength(1)
})
})

describe('MAX_CONTINUATION_NUDGES limit', () => {
test('MAX_CONTINUATION_NUDGES is set to 20', async () => {
const content = await file('query.ts').text()

const match = content.match(/MAX_CONTINUATION_NUDGES\s*=\s*(\d+)/)
expect(match).not.toBeNull()
expect(Number(match![1])).toBe(20)
})

test('nudge count is compared to MAX_CONTINUATION_NUDGES', async () => {
const content = await file('query.ts').text()

// The guard must exist: continuationNudgeCount < MAX_CONTINUATION_NUDGES
expect(content).toContain('continuationNudgeCount < MAX_CONTINUATION_NUDGES')
})
})

// ---------------------------------------------------------------------------
// Fix 4: Web search result count improvements
// ---------------------------------------------------------------------------
Expand Down
4 changes: 2 additions & 2 deletions src/cli/print.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1041,7 +1041,7 @@ function runHeadlessStreaming(
const sigintHandler = () => {
logForDiagnosticsNoPII('info', 'shutdown_signal', { signal: 'SIGINT' })
if (abortController && !abortController.signal.aborted) {
abortController.abort()
abortController.abort('interrupt')
}
void gracefulShutdown(0)
}
Expand Down Expand Up @@ -2849,7 +2849,7 @@ function runHeadlessStreaming(
}))
}
if (abortController) {
abortController.abort()
abortController.abort('interrupt')
}
suggestionState.abortController?.abort()
suggestionState.abortController = null
Expand Down
4 changes: 2 additions & 2 deletions src/hooks/toolPermission/PermissionContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ function createPermissionContext(
logForDebugging(
`Aborting: tool=${tool.name} isAbort=${isAbort} hasFeedback=${!!feedback} isSubagent=${sub}`,
)
toolUseContext.abortController.abort()
toolUseContext.abortController.abort('interrupt')
}
return { behavior: 'ask', message, contentBlocks }
},
Expand Down Expand Up @@ -282,7 +282,7 @@ function createPermissionContext(
logForDebugging(
`Hook interrupt: tool=${tool.name} hookMessage=${decision.message}`,
)
toolUseContext.abortController.abort()
toolUseContext.abortController.abort('interrupt')
}
return this.buildDeny(
decision.message || 'Permission denied by hook',
Expand Down
2 changes: 1 addition & 1 deletion src/query.ts
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ function* yieldMissingToolResultBlocks(
* rules, ye will be punished with an entire day of debugging and hair pulling.
*/
const MAX_OUTPUT_TOKENS_RECOVERY_LIMIT = 3
const MAX_CONTINUATION_NUDGES = 3
const MAX_CONTINUATION_NUDGES = 20

function formatAutoCompactRetryDelay(delayMs: number): string {
const totalSeconds = Math.max(1, Math.ceil(delayMs / 1000))
Expand Down
118 changes: 98 additions & 20 deletions src/utils/continuation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,23 +5,97 @@ import { tokenCountWithEstimation } from './tokens.js'
* but stopped (potentially due to truncation or missed tool calls).
*/

export const CONTINUATION_SIGNALS = [
// English: Action-transition phrases (requires intent + action)
/\bso now (i|let me|we) (need to|have to|should|must|will) (do|create|write|edit|update|fix|implement|add|run|check|make|build|set up|start|begin|apply|identify|inspect|analyze|review|search)\b/i,
/\bnow i('ll| will) (do|create|write|edit|update|fix|implement|add|run|check|make|build|set up|go|proceed|start|begin|apply|identify|inspect|analyze|review|search)\b/i,
/\bi (will|shall|now|need to|have to|must|should) (now )?(do|create|write|edit|update|fix|implement|add|run|check|make|build|set up|go|proceed|start|begin|apply|identify|inspect|analyze|review|search)\b/i,
/\blet me (go ahead and |now )?(do|create|write|edit|update|fix|implement|add|run|check|make|build|set up|proceed|start|begin|apply|update|create|identify|inspect|analyze|review|search|summarize)\b/i,
/\btime to (do|create|write|edit|update|fix|implement|add|run|check|make|build|get started|begin|start|inspect|analyze|review|search)\b/i,
/\b(moving on to|next step is to|starting to|proceeding to|applying (the|these) changes|inspecting|analyzing|reviewing|searching)\b/i,
// French: Support for common continuation phrasing (relaxed boundaries for accents and apostrophes)
/(^|\s)(je passe (à|au)|ensuite|l'étape suivante est de|je continue avec|au suivant|passons à|je reviens vers vous|je suis en train d'|je vais maintenant)(\s|$|[a-zà-ÿ])/i,
/(^|\s)(je (vais|dois|dois maintenant|vais maintenant) (faire|créer|écrire|modifier|ajouter|tester|vérifier|lancer|exécuter|procéder|démarrer|commencer|identifier|analyser|inspecter|revoir|chercher))(\s|$|[a-zà-ÿ])/i,
/(^|\s)((lancement|exécution|vérification|modification|mise à jour|analyse|inspection|recherche) de)(\s|$|[a-zà-ÿ])/i,
// Universal: Sentence ending with a colon indicates intent to list/act
/:\s*$/,
// Universal: Open task marker indicates pending work
/◻/,
]
// Shared verb list used across all continuation patterns.
// Build regexes from this array so maintenance stays in one place.
const ACTION_VERBS = [
'do',
'create',
'write',
'edit',
'update',
'fix',
'implement',
'add',
'run',
'check',
'make',
'build',
'set up',
'start',
'begin',
'go',
'proceed',
'apply',
'identify',
'inspect',
'analyze',
'review',
'search',
'process',
'download',
'upload',
'convert',
'compile',
'train',
'evaluate',
'test',
'continue',
'generate',
'extract',
'merge',
'deploy',
'install',
'configure',
'refactor',
'optimize',
'summarize',
] as const

// Base verb alternatives used across most regexes (no "summarize" in older patterns, but harmless)
const VERB_ALT = ACTION_VERBS.join('|')

// Gerund forms for progressive/participle patterns
const VERB_ING = ACTION_VERBS.map(v => {
// Handle special cases: "set up" -> "setting up", "do" -> "doing"
if (v === 'set up') return 'setting up'
if (v === 'do') return 'doing'
if (v === 'go') return 'going'
if (v === 'run') return 'running'
if (v === 'begin') return 'beginning'
if (v === 'make') return 'making'
if (v === 'write') return 'writing'
// Default: add -ing
return v.replace(/e$/, '') + 'ing'
}).join('|')

// Build continuation-signal regexes from the shared verb list.
// (Using function to keep construction readable.)
function buildContinuationSignals(): RegExp[] {
const v = VERB_ALT
return [
// English: Action-transition phrases (requires intent + action)
new RegExp(`\\bso now (i|let me|we) (need to|have to|should|must|will) (${v})\\b`, 'i'),
new RegExp(`\\bnow i('ll| will) (${v})\\b`, 'i'),
new RegExp(`\\bi (will|shall|now|need to|have to|must|should) (now )?(${v})\\b`, 'i'),
new RegExp(`\\blet me (go ahead and |now )?(${v})\\b`, 'i'),
new RegExp(`\\btime to (do|${v.replace(/^do\|/, '')}|get started|begin|start)\\b`, 'i'),
new RegExp(`\\b(moving on to|next step is to|starting to|proceeding to|continuing with|applying (the|these) changes|${VERB_ING})\\b`, 'i'),
// French: Support for common continuation phrasing (relaxed boundaries for accents and apostrophes)
/(^|\s)(je passe (à|au)|ensuite|l'étape suivante est de|je continue avec|au suivant|passons à|je reviens vers vous|je suis en train d'|je vais maintenant)(\s|$|[a-zà-ÿ])/i,
/(^|\s)(je (vais|dois|dois maintenant|vais maintenant) (faire|créer|écrire|modifier|ajouter|tester|vérifier|lancer|exécuter|procéder|démarrer|commencer|identifier|analyser|inspecter|revoir|chercher))(\s|$|[a-zà-ÿ])/i,
/(^|\s)((lancement|exécution|vérification|modification|mise à jour|analyse|inspection|recherche) de)(\s|$|[a-zà-ÿ])/i,
// Universal: Sentence ending with a colon indicates intent to list/act
/:\s*$/,
// Universal: Open task marker indicates pending work
/◻/,
// Imperative/declarative patterns (no subject required)
new RegExp(`\\bneed to (${v})\\b`, 'i'),
new RegExp(`\\bnow (${v})\\b(?!\\s+you\\b)`, 'i'),
new RegExp(`\\bnext (i|we)\\s+(need to|will|shall|should|must)?\\s*(${v})\\b`, 'i'),
]
}

export const CONTINUATION_SIGNALS = buildContinuationSignals()

export const COMPLETION_MARKERS = /\b(done|finished|completed|complete|summary|that's all|that is all|all set|hope this helps|let me know if|no issues|lgtm)\b/i

Expand Down Expand Up @@ -99,10 +173,11 @@ export function analyzeContinuationIntent(
// it's a strong 1st person intent or open tasks are present.
const hasTerminalPunctuation = /[.!??"'`)\]]\s*$/.test(lastText) || lastText.endsWith('`')
if (hasTerminalPunctuation) {
const strongIntent = /\b(i (will|shall|need to|must|should|now)|let (me|us)|je (vais|reviens)|passons à|moving on to|next step is to)\b/i.test(lowerText) ||
const strongIntent = /\b(i (will|shall|need to|must|should|now)|let (me|us)|je (vais|reviens)|passons à|moving on to|continuing with|proceeding to|next step is to)\b/i.test(lowerText) ||
/je suis en train d'/i.test(lowerText) || /◻/.test(lastText)
const presentProgressive = new RegExp(`\\bnow (?:${VERB_ING})\\b`, 'i').test(lateText)
const endsWithColon = /:\s*$/.test(lastText)
if (strongIntent || endsWithColon) {
if (strongIntent || endsWithColon || presentProgressive) {
return { shouldNudge: true, reason: 'continuation_signal' }
}
} else {
Expand All @@ -111,7 +186,10 @@ export function analyzeContinuationIntent(
}

// 3. Completion Marker Guard (Final check for sound, completed messages)
if (COMPLETION_MARKERS.test(lowerText)) {
// Only block continuation if no continuation signal is present (prevents false
// positives when "complete" or "done" appears mid-sentence, e.g. "The download
// is complete. Now processing the files...")
if (COMPLETION_MARKERS.test(lowerText) && !hasLateContinuationSignal && !CONTINUATION_SIGNALS.some(re => re.test(lowerText))) {
return { shouldNudge: false }
}

Expand Down
12 changes: 6 additions & 6 deletions src/utils/permissions/PermissionPromptToolResultSchema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -119,12 +119,12 @@ export function permissionPromptToolResultToPermissionDecision(
updatedInput,
decisionReason,
}
} else if (result.behavior === 'deny' && result.interrupt) {
logForDebugging(
`SDK permission prompt deny+interrupt: tool=${tool.name} message=${result.message}`,
)
toolUseContext.abortController.abort()
}
} else if (result.behavior === 'deny' && result.interrupt) {
logForDebugging(
`SDK permission prompt deny+interrupt: tool=${tool.name} message=${result.message}`,
)
toolUseContext.abortController.abort('interrupt')
}
return {
...result,
decisionReason,
Expand Down
2 changes: 1 addition & 1 deletion src/utils/permissions/permissions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -466,7 +466,7 @@ async function runPermissionRequestHooksForHeadlessAgent(
logForDebugging(
`Hook interrupt: tool=${tool.name} hookMessage=${decision.message}`,
)
context.abortController.abort()
context.abortController.abort('interrupt')
}
return {
behavior: 'deny',
Expand Down