heartbeart while claude execution

[shivammittal274]

No description provided.

[greptile-apps]

Greptile Overview

Greptile Summary

Implements heartbeat mechanism to send periodic "processing" events during long-running Claude SDK operations, preventing client timeouts.

Key Changes

• Added nextWithHeartbeat() async generator wrapper that races iterator.next() against 20-second timeout
• Added 'processing' event type to EventFormatter for heartbeat signals
• Improved abort signal handling to prevent event listener accumulation
• Enhanced client disconnection handling in WebSocket server (return instead of throw)
• Expanded system prompt with detailed tool usage guidelines (unrelated improvement)

Critical Issues Found

• Iterator progression bug: iteratorPromise initialized once but never reassigned in loop at ClaudeSDKAgent.ts:113, causing same resolved promise to be reused repeatedly - this will break event streaming after first iteration
• Fragile result detection: Logic at line 228-236 relies on checking item.done to distinguish heartbeat events from iterator results, which is unreliable

Confidence Score: 0/5

• This PR is NOT safe to merge - contains a critical bug that breaks core functionality
• The iteratorPromise reassignment bug at line 113-164 will cause the heartbeat wrapper to stop advancing through SDK events after the first iteration. This fundamentally breaks the agent's ability to process and stream events, making the feature non-functional.
• packages/agent/src/agent/ClaudeSDKAgent.ts requires immediate attention - the nextWithHeartbeat function has a critical iterator progression bug

Important Files Changed

File Analysis

Filename	Score	Overview
packages/agent/src/agent/ClaudeSDKAgent.ts	1/5	Added heartbeat mechanism but critical bug: iteratorPromise never reassigned in loop, breaking event streaming after first iteration
packages/agent/src/agent/ClaudeSDKAgent.prompt.ts	5/5	Significantly expanded and improved system prompt with better tool guidance and examples (unrelated to heartbeat feature)
packages/agent/src/utils/EventFormatter.ts	5/5	Added 'processing' event type and createProcessingEvent() method for heartbeat functionality - straightforward addition
packages/agent/src/websocket/server.ts	5/5	Improved error handling for client disconnection during streaming - changed from throw to return to avoid double error messages

Sequence Diagram

sequenceDiagram
    participant Client as WebSocket Client
    participant Server as WebSocket Server
    participant Agent as ClaudeSDKAgent
    participant Iterator as SDK Iterator
    participant Heartbeat as nextWithHeartbeat

    Client->>Server: Send message
    Server->>Agent: execute(message)
    Agent->>Iterator: query().next()
    Agent->>Heartbeat: nextWithHeartbeat(iterator)
    
    loop While processing
        Heartbeat->>Heartbeat: Race iterator vs 20s timeout vs abort
        alt Heartbeat timeout (20s)
            Heartbeat->>Agent: Yield processing event
            Agent->>Server: FormattedEvent (processing)
            Server->>Client: ⏳ Processing...
        else Iterator returns event
            Iterator->>Heartbeat: SDK event
            Heartbeat->>Agent: Yield iterator result
            Agent->>Agent: Format event
            Agent->>Server: FormattedEvent
            Server->>Client: Event data
            Note over Heartbeat: BUG: Should reassign<br/>iteratorPromise here
        else Abort signal
            Heartbeat->>Heartbeat: Cleanup & exit
        end
    end
    
    Agent->>Server: Execution complete
    Server->>Client: Final events

Loading

_{4 files reviewed, 2 comments}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

logic: iteratorPromise never reassigned after first iteration - causes same resolved promise to be reused, breaking iterator progression

Suggested change

	let iteratorPromise = iterator.next()
	// Create abort promise ONCE outside loop to avoid accumulating event listeners
	const abortPromise = new Promise<never>((_, reject) => {
	if (this.abortController) {
	abortHandler = () => {
	reject(new Error('Agent execution aborted by client'))
	}
	this.abortController.signal.addEventListener('abort', abortHandler, { once: true })
	}
	})
	try {
	while (true) {
	// Check if execution was aborted
	if (this.abortController?.signal.aborted) {
	logger.info('⚠️ Agent execution aborted during heartbeat wait')
	return
	}
	const timeoutPromise = new Promise(resolve => {
	heartbeatTimer = setTimeout(() => resolve({ type: 'heartbeat' }), heartbeatInterval)
	})
	type RaceResult = { type: 'result'; result: any } | { type: 'heartbeat' }
	let race: RaceResult
	try {
	race = await Promise.race([
	iteratorPromise.then(result => ({ type: 'result' as const, result })),
	timeoutPromise.then(() => ({ type: 'heartbeat' as const })),
	abortPromise
	])
	} catch (abortError) {
	// Abort was triggered during wait
	logger.info('⚠️ Agent execution aborted (caught during iterator wait)')
	// Cleanup iterator
	if (iterator.return) {
	await iterator.return(undefined).catch(() => {})
	}
	return
	}
	if (heartbeatTimer) {
	clearTimeout(heartbeatTimer)
	heartbeatTimer = null
	}
	if (race.type === 'heartbeat') {
	yield EventFormatter.createProcessingEvent()
	} else {
	// Yield the iterator result (not return!) so the consumer receives it
	yield race.result
	let iteratorPromise = iterator.next()
	// Create abort promise ONCE outside loop to avoid accumulating event listeners
	const abortPromise = new Promise<never>((_, reject) => {
	if (this.abortController) {
	abortHandler = () => {
	reject(new Error('Agent execution aborted by client'))
	}
	this.abortController.signal.addEventListener('abort', abortHandler, { once: true })
	}
	})
	try {
	while (true) {
	// Check if execution was aborted
	if (this.abortController?.signal.aborted) {
	logger.info('⚠️ Agent execution aborted during heartbeat wait')
	return
	}
	const timeoutPromise = new Promise(resolve => {
	heartbeatTimer = setTimeout(() => resolve({ type: 'heartbeat' }), heartbeatInterval)
	})
	type RaceResult = { type: 'result'; result: any } | { type: 'heartbeat' }
	let race: RaceResult
	try {
	race = await Promise.race([
	iteratorPromise.then(result => ({ type: 'result' as const, result })),
	timeoutPromise.then(() => ({ type: 'heartbeat' as const })),
	abortPromise
	])
	} catch (abortError) {
	// Abort was triggered during wait
	logger.info('⚠️ Agent execution aborted (caught during iterator wait)')
	// Cleanup iterator
	if (iterator.return) {
	await iterator.return(undefined).catch(() => {})
	}
	return
	}
	if (heartbeatTimer) {
	clearTimeout(heartbeatTimer)
	heartbeatTimer = null
	}
	if (race.type === 'heartbeat') {
	yield EventFormatter.createProcessingEvent()
	// Continue waiting for the same iterator promise
	} else {
	// Yield the iterator result (not return!) so the consumer receives it
	yield race.result
	// CRITICAL: Get next promise for next iteration
	iteratorPromise = iterator.next()
	}
	}

Prompt To Fix With AI

This is a comment left during a code review.
Path: packages/agent/src/agent/ClaudeSDKAgent.ts
Line: 113:164

Comment:
**logic:** `iteratorPromise` never reassigned after first iteration - causes same resolved promise to be reused, breaking iterator progression

```suggestion
    let iteratorPromise = iterator.next()

    // Create abort promise ONCE outside loop to avoid accumulating event listeners
    const abortPromise = new Promise<never>((_, reject) => {
      if (this.abortController) {
        abortHandler = () => {
          reject(new Error('Agent execution aborted by client'))
        }
        this.abortController.signal.addEventListener('abort', abortHandler, { once: true })
      }
    })

    try {
      while (true) {
        // Check if execution was aborted
        if (this.abortController?.signal.aborted) {
          logger.info('⚠️  Agent execution aborted during heartbeat wait')
          return
        }

        const timeoutPromise = new Promise(resolve => {
          heartbeatTimer = setTimeout(() => resolve({ type: 'heartbeat' }), heartbeatInterval)
        })

        type RaceResult = { type: 'result'; result: any } | { type: 'heartbeat' }
        let race: RaceResult
        try {
          race = await Promise.race([
            iteratorPromise.then(result => ({ type: 'result' as const, result })),
            timeoutPromise.then(() => ({ type: 'heartbeat' as const })),
            abortPromise
          ])
        } catch (abortError) {
          // Abort was triggered during wait
          logger.info('⚠️  Agent execution aborted (caught during iterator wait)')
          // Cleanup iterator
          if (iterator.return) {
            await iterator.return(undefined).catch(() => {})
          }
          return
        }

        if (heartbeatTimer) {
          clearTimeout(heartbeatTimer)
          heartbeatTimer = null
        }

        if (race.type === 'heartbeat') {
          yield EventFormatter.createProcessingEvent()
          // Continue waiting for the same iterator promise
        } else {
          // Yield the iterator result (not return!) so the consumer receives it
          yield race.result
          // CRITICAL: Get next promise for next iteration
          iteratorPromise = iterator.next()
        }
      }
```

How can I resolve this? If you propose a fix, please make it concise.

[greptile-apps]

logic: logic issue with detecting iterator result - item.done check is unreliable since heartbeat events are FormattedEvent objects without a done property

The current code assumes heartbeat events won't have done property, but this is fragile. If a FormattedEvent happens to have metadata or properties, this could fail. Better to explicitly return an object with a discriminator field from nextWithHeartbeat.

// In nextWithHeartbeat, change the yields to:
if (race.type === 'heartbeat') {
  yield { isHeartbeat: true, event: EventFormatter.createProcessingEvent() }
} else {
  yield { isHeartbeat: false, result: race.result }
}

// Then here:
for await (const item of this.nextWithHeartbeat(iterator)) {
  if (!item.isHeartbeat) {
    result = item.result
  } else {
    yield item.event
  }
}

Prompt To Fix With AI

This is a comment left during a code review.
Path: packages/agent/src/agent/ClaudeSDKAgent.ts
Line: 228:236

Comment:
**logic:** logic issue with detecting iterator result - `item.done` check is unreliable since heartbeat events are FormattedEvent objects without a `done` property

The current code assumes heartbeat events won't have `done` property, but this is fragile. If a FormattedEvent happens to have metadata or properties, this could fail. Better to explicitly return an object with a discriminator field from `nextWithHeartbeat`.

```
// In nextWithHeartbeat, change the yields to:
if (race.type === 'heartbeat') {
  yield { isHeartbeat: true, event: EventFormatter.createProcessingEvent() }
} else {
  yield { isHeartbeat: false, result: race.result }
}

// Then here:
for await (const item of this.nextWithHeartbeat(iterator)) {
  if (!item.isHeartbeat) {
    result = item.result
  } else {
    yield item.event
  }
}
```

How can I resolve this? If you propose a fix, please make it concise.

[felarof99]

YOLO, LGTM

Just fix the warning that greptile is complaining about.