fix(agent): prevent session context poisoning and infinite loops on interrupted turns - #29397

Import the BENIGN_INTERRUPTION_REPLACEMENT constant from the sanitizer module so it can be used to check for sanitized interruption placeholders.

The current implementation of isInterruptionContent only checks for the raw interruption placeholder (INTERRUPTED_RESPONSE_TEXT). However, if the stream was interrupted, the turn is sanitized and stored as BENIGN_INTERRUPTION_REPLACEMENT ("Continuing.").

To ensure the short-circuit logic correctly identifies both raw and sanitized interruption placeholders (as intended by the comment), we should also check if any part of the last model message matches BENIGN_INTERRUPTION_REPLACEMENT.

  if (
    lastComprehensiveMessage &&
    (isInterruptionContent(lastComprehensiveMessage) ||
      lastComprehensiveMessage.parts?.some(
        (part) => typeof part.text === 'string' && part.text === BENIGN_INTERRUPTION_REPLACEMENT
      ))
  ) {

Update the test case to verify that the next speaker checker correctly short-circuits and returns { next_speaker: 'model' } for both the raw interruption placeholder and the sanitized interruption placeholder. Note that in tests, we prefer using hardcoded literal values (such as 'Continuing.') instead of importing constants to ensure tests are self-contained and less brittle.

  it.each([
    ['raw placeholder', INTERRUPTED_RESPONSE_PLACEHOLDER],
    ['sanitized placeholder', 'Continuing.'],
  ])("should return { next_speaker: 'model' } when last message is %s", async (_, placeholder) => {
    (chatInstance.getHistory as Mock).mockImplementation(
      (curated?: boolean) => {
        const turn = [
          {
            role: 'model',
            parts: [{ text: placeholder }],
          },
        ] as Content[];
        return turn;
      },
    );
    const result = await checkNextSpeaker(
      chatInstance,
      mockBaseLlmClient,
      abortSignal,
      promptId,
    );
    expect(result).toEqual({
      reasoning:
        'The last model response was an interruption placeholder, so the model should speak next to provide a real response.',
      next_speaker: 'model',
    });
    // Should NOT make an LLM call — this is a short-circuit
    expect(mockBaseLlmClient.generateJson).not.toHaveBeenCalled();
  });

To prevent potential runtime TypeError exceptions, we should defensively guard against content being null/undefined, and ensure that each part in content.parts is also checked for null/undefined before accessing its properties. Since parts is an optional property, we should handle the undefined case using the nullish coalescing operator (??) to code against the interface contract.

export function isInterruptionContent(content: Content): boolean {
  if (!content || content.role !== 'model') return false;
  const parts = content.parts ?? [];
  return parts.some(
    (part) =>
      part && typeof part.text === 'string' && isInterruptionPlaceholder(part.text),
  );
}

Defensively guard against part being null or undefined before attempting to access part.text. This prevents runtime crashes if the parts array contains nullish values.

Add a defensive check to ensure content is not null or undefined before processing. Since parts is an optional property, use the nullish coalescing operator (??) to handle the undefined case.

Defensively guard against turns being null/undefined, and ensure that each turn and its content property are checked for null/undefined before processing.

Defensively guard against history being null or undefined before mapping over it.

The current implementation of isSanitizedInterruption uses parts.some() to check if any part of the last model message matches BENIGN_INTERRUPTION_REPLACEMENT ("Continuing."). This creates a critical bug: if the model outputs a multi-part response that contains the word "Continuing." (e.g., as a transition phrase) followed by a tool call, isSanitizedInterruption will evaluate to true. This triggers the short-circuit logic, returning { next_speaker: 'model' } immediately and bypassing the tool execution entirely, which breaks the agentic loop. To prevent this, we should ensure that isSanitizedInterruption only matches when the message consists solely of the synthetic interruption placeholder (i.e., it has exactly one part, and that part is the placeholder text).

To optimize performance and avoid creating intermediate arrays, combine the .map(scrubPart) and .map(sanitizeInterruptionPart) calls into a single .map pass. This reduces iteration overhead on the chat history turns.

Similarly, combine the .map calls here into a single pass to avoid intermediate array allocation and improve efficiency during history scrubbing.

If an assertion fails, process.exit(1) is called immediately, which bypasses the cleanup of the temporary directory tmpHome and leaves orphaned directories in the OS temp folder. To prevent resource leaks on test failures, refactor assert to throw an Error instead of exiting the process directly. Then, wrap the test execution in main() with a try...finally block to guarantee that server.close() and fs.rmSync(tmpHome, ...) are always executed.

function assert(condition, message) {
  if (!condition) {
    throw new Error(message);
  }
}

The import of BENIGN_INTERRUPTION_REPLACEMENT is no longer needed if we revert the fallback result to the original explicit termination message.

Using BENIGN_INTERRUPTION_REPLACEMENT (which evaluates to 'Continuing.') as the fallback for an aborted or cancelled subagent run is a correctness bug. When a subagent is executed as a tool by a parent agent, the parent agent receives this result as the tool output. If the subagent was aborted, returning 'Continuing.' will mislead the parent agent into thinking the subagent completed successfully and returned 'Continuing.' as its output, rather than realizing it was aborted/cancelled. The original string 'Agent execution was terminated before completion.' does not contain the poisoned placeholder and therefore does not cause context poisoning. We should revert this fallback to the original explicit termination message. Additionally, when using an optional string with a fallback value, we should trim the optional string and use the fallback if the result is empty to avoid uninformative messages from whitespace-only strings.

The import of BENIGN_INTERRUPTION_REPLACEMENT is no longer needed in the test file once the assertions are updated to use the correct fallback string. In tests, we prefer using hardcoded literal values instead of importing constants to ensure tests are self-contained and less brittle.

Update the tests to assert the correct fallback string 'Agent execution was terminated before completion.' (with the trimmed optional string check) and verify that the raw interruption placeholder is not leaked, rather than asserting the incorrect 'Continuing.' fallback. Also, prefer using hardcoded literal values instead of importing constants to keep tests self-contained.

  describe('session context poisoning prevention (Issue #29264)', () => {
    it('should not include raw interruption placeholder in default abort result messages', async () => {
      const fs = await import('node:fs');
      const source = fs.readFileSync('src/agents/local-executor.ts', 'utf-8');
      expect(source).not.toContain('INTERRUPTED_RESPONSE_PLACEHOLDER');
      expect(source).not.toContain('INTERRUPTED_RESPONSE_TEXT');
      expect(source).toContain(
        "finalResult?.trim() || 'Agent execution was terminated before completion.'",
      );
    });
  });

The check for isSanitizedInterruption should explicitly ensure that the single part is not a thought part. If the model generated a thought that happens to contain the benign replacement text, it could be incorrectly classified as a sanitized interruption turn.

  const isSanitizedInterruption =
    lastComprehensiveMessage &&
    lastComprehensiveMessage.role === 'model' &&
    lastComprehensiveMessage.parts?.length === 1 &&
    lastComprehensiveMessage.parts[0] &&
    !lastComprehensiveMessage.parts[0].thought &&
    typeof lastComprehensiveMessage.parts[0].text === 'string' &&
    lastComprehensiveMessage.parts[0].text === BENIGN_INTERRUPTION_REPLACEMENT;

The check text === INTERRUPTED_RESPONSE_TEXT is redundant because text.includes(INTERRUPTED_RESPONSE_TEXT) already returns true when the strings are exactly equal. Simplifying this reduces redundant comparisons.

export function isInterruptionPlaceholder(text: string): boolean {
  return text.includes(INTERRUPTED_RESPONSE_TEXT);
}

Using a hardcoded relative path like 'src/agents/local-executor.ts' makes the test execution fragile and dependent on the current working directory (CWD). If the tests are run from the monorepo root instead of the packages/core directory, this test will fail with an ENOENT error.

Using new URL('local-executor.ts', import.meta.url) resolves the path relative to the test file itself, making it robust and independent of where the test runner is invoked.

Using path.resolve with a hardcoded relative path like 'packages/core/dist/...' makes the dynamic imports fragile and dependent on the current working directory (CWD). If the script is run from the packages/core directory instead of the monorepo root, it will fail to resolve the modules.

Using new URL(..., import.meta.url).href resolves the paths relative to the script itself, making it robust and independent of where the script is invoked.

Using a hardcoded relative path like 'packages/core/src/core/geminiChat.ts' makes the file read fragile and dependent on the current working directory (CWD). If the script is run from the packages/core directory instead of the monorepo root, it will fail with an ENOENT error.

Using new URL('src/core/geminiChat.ts', import.meta.url) resolves the path relative to the script itself, making it robust and independent of where the script is invoked.

Using a hardcoded relative path like 'packages/core/src/utils/nextSpeakerChecker.ts' makes the file read fragile and dependent on the current working directory (CWD). If the script is run from the packages/core directory instead of the monorepo root, it will fail with an ENOENT error.

Using new URL('src/utils/nextSpeakerChecker.ts', import.meta.url) resolves the path relative to the script itself, making it robust and independent of where the script is invoked.