Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
schema: spec-driven
created: 2026-06-25
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
## Context

The TUI (Terminal User Interface) in `src/tui/app.js` manages conversation state through a `SessionStateManager` instance (`sessionState`). When a user sends a message, `dispatchProvider()` adds the user message to `sessionState.conversation`, streams the assistant's response, and appends the response to the conversation.

The interrupt flow works as follows:
1. User triggers interrupt → `handleInterrupt()` is called (line 944)
2. `handleInterrupt()` aborts the `AbortController` and waits for `dispatchPromise` to resolve
3. The `dispatchProvider()` try/catch catches the `AbortError` and calls `sessionState.popExchange()` (line 909)
4. `popExchange()` removes the last message from `sessionState.conversation` — this is the user's message
5. The assistant's partial response (an AIMessage with tool_calls) remains in the conversation array

On resume, the graph replays from the checkpoint. The checkpoint has the AIMessage with tool_calls but no ToolMessages (the tool was interrupted before completion). When the conversation is sent to the LLM API, the message sequence is corrupted — the system message is no longer at position 0, or there are orphaned tool_calls without ToolMessages. The LLM API rejects the request with a 400 error.

The `SessionStateManager` (`src/session/stateManager.js`) manages a `conversation` array of `{ role, content, timestamp }` objects. `addExchange()` pushes messages, `popExchange()` removes the last one.

## Goals / Non-Goals

**Goals:**
- When a tool is interrupted, remove the assistant's AIMessage with tool_calls from session state in addition to the user's message
- Ensure the conversation state is consistent before the next dispatch to the LLM API
- Prevent the "system message must be at the beginning" 400 error on resume
- Make the cleanup idempotent — safe to call even when no assistant tool-call message exists

**Non-Goals:**
- Changes to LangGraph's core interrupt mechanism or checkpoint system
- Changes to ToolNode's deduplication logic
- Changes to the streaming UI behavior
- Handling interrupts during non-tool-call responses (normal text responses already work correctly)

## Decisions

### Decision 1: Clean up in both handleInterrupt() and the AbortError catch block
**Choice:** Add cleanup logic in two places — `handleInterrupt()` (the interrupt entry point) and the AbortError catch block in `dispatchProvider()` (where `popExchange()` is currently called).

**Rationale:** The two code paths handle different timing scenarios:
- `handleInterrupt()` is called when the user explicitly triggers an interrupt during streaming
- The AbortError catch block handles the case where the abort propagates through the dispatch chain

Having cleanup in both places ensures consistency regardless of how the abort is triggered. The helper function is idempotent, so calling it from both paths is safe.

### Decision 2: Use a helper function to remove assistant tool-call messages
**Choice:** Create a helper function `removeLastAssistantToolCallMessage(sessionState)` that checks if the last conversation entry is an assistant message with tool_calls and removes it if so.

**Rationale:**
- Encapsulates the cleanup logic in a single, testable function
- Makes the intent clear (removing assistant tool-call messages)
- Is idempotent — safe to call when no such message exists
- Can be reused in both `handleInterrupt()` and the AbortError catch block

### Decision 3: Check for tool_calls in the content field
**Choice:** The helper checks if the last message has `role === "assistant"` and if its `content` field contains tool call information (e.g., `tool_calls` array or a string representation of tool calls).

**Rationale:** The conversation array stores messages as `{ role, content, timestamp }`. The `content` field may contain either a string (for text responses) or an object with `tool_calls` (for tool-call responses). The helper needs to handle both cases.

### Decision 4: TUI-level fix, not LangGraph-level
**Choice:** Fix the issue at the TUI level by cleaning up session state, rather than modifying LangGraph's checkpoint or interrupt mechanisms.

**Rationale:**
- LangGraph's interrupt mechanism is designed for intentional human-in-the-loop pauses, not abrupt TUI aborts
- Modifying LangGraph would require changes to dependencies and introduce more risk
- The TUI is the source of the abrupt abort, so it should handle the cleanup
- This is a smaller, more contained change with lower risk

## Risks / Trade-offs

### Risk: Conversation array structure changes
**Impact:** If the message structure in `sessionState.conversation` changes (e.g., messages are stored differently, or additional message types are added), the cleanup logic may need adjustment.
**Mitigation:** The helper function is defensive — it checks for specific conditions before removing messages. If the structure changes, the helper will simply be a no-op, which is safe.

### Risk: Cleanup interferes with normal flow
**Impact:** If the cleanup logic incorrectly removes messages during normal (non-interrupted) flow, it could corrupt the conversation.
**Mitigation:** The cleanup only runs in the AbortError catch block and in `handleInterrupt()`, both of which are only triggered during interruptions. The helper is also idempotent.

### Risk: Multiple tool calls in flight
**Impact:** If multiple tool calls are in flight when an interrupt occurs, the cleanup might only remove the last one, leaving others in an inconsistent state.
**Mitigation:** In practice, the TUI streams one response at a time, so only one assistant message with tool_calls should be in flight at any given time. If this changes in the future, the helper can be extended to remove all assistant messages with tool_calls.

### Risk: Timing edge case
**Impact:** If the interrupt happens between the assistant message being added to sessionState and the user message being added, the cleanup might remove the wrong message.
**Mitigation:** The cleanup removes the last message first (which should be the assistant's tool-call message), then the user message. The order is deterministic based on the message addition sequence.
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
## Why

Interrupting a tool execution in the TUI causes an unrecoverable 400 error from the LLM API with the message "system message must be at the beginning." The root cause is that when `handleInterrupt()` aborts a running tool, it only removes the user's message from session state via `popExchange()`, but leaves the assistant's partial AIMessage (containing tool_calls that were never completed) in the conversation history. On resume, this corrupted message sequence — with an orphaned assistant message containing tool_calls but no corresponding ToolMessages — causes the LLM API to reject subsequent requests.

## What Changes

- Enhance `handleInterrupt()` in `src/tui/app.js` to clean up the assistant's tool-call message from session state when a tool is interrupted
- Add a helper function to safely remove the last assistant message with tool_calls from the conversation array
- Ensure the conversation state is consistent before the next dispatch to the LLM API
- Preserve existing behavior for non-tool-call interruptions (normal text responses)

## Capabilities

### New Capabilities
- `interrupt-cleanup`: Robust cleanup of assistant tool-call messages from session state when a tool execution is interrupted, preventing corrupted conversation history from being sent to the LLM API

### Modified Capabilities
<!-- None — this is a bug fix, not a requirement change -->

## Impact

- `src/tui/app.js` — `handleInterrupt()` function and the AbortError catch block in dispatchProvider
- `src/session/stateManager.js` — potential helper method for removing assistant messages with tool_calls
- No API changes, no dependency changes
- Affects only the TUI interrupt flow during tool execution
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
## ADDED Requirements

### Requirement: Interrupt cleanup removes assistant tool-call messages from session state
When a tool execution is interrupted, the TUI SHALL remove the assistant's AIMessage containing tool_calls from the session state conversation array, in addition to removing the user's message. This ensures the conversation history sent to the LLM API is consistent and does not contain orphaned tool calls.

#### Scenario: Interrupt during tool execution removes assistant tool-call message
- **WHEN** a tool is executing and the user triggers an interrupt
- **THEN** the `handleInterrupt()` function removes the last assistant message with tool_calls from `sessionState.conversation`
- **AND** the user's message is also removed via `popExchange()`
- **AND** the conversation array no longer contains the orphaned assistant tool-call message

#### Scenario: Interrupt during normal text response preserves conversation
- **WHEN** the assistant is responding with text (no tool calls) and the user triggers an interrupt
- **THEN** the cleanup logic does not remove any assistant message (no tool_calls to clean up)
- **AND** only the user's message is removed via `popExchange()`
- **AND** the conversation remains consistent

#### Scenario: Interrupt with no assistant message is safe
- **WHEN** an interrupt is triggered but no assistant message exists in the conversation
- **THEN** the cleanup helper function performs no removal (no-op)
- **AND** no error is thrown
- **AND** the conversation state is unchanged

### Requirement: Cleanup helper is idempotent and defensive
The helper function that removes assistant tool-call messages SHALL be safe to call multiple times and SHALL not throw errors when no matching message exists.

#### Scenario: Calling cleanup helper multiple times is safe
- **WHEN** the cleanup helper is called when no assistant tool-call message exists
- **THEN** the function returns without modifying the conversation array
- **AND** no error is thrown

#### Scenario: Cleanup helper handles empty conversation
- **WHEN** the conversation array is empty
- **THEN** the cleanup helper returns without modifying state
- **AND** no error is thrown

### Requirement: Cleanup occurs before next LLM dispatch
After interrupt cleanup completes, the session state SHALL be consistent before the next dispatch to the LLM API, ensuring the system message remains at position 0 in the messages array.

#### Scenario: Conversation is clean before resume dispatch
- **WHEN** an interrupt completes and the user sends a new message
- **THEN** the conversation sent to the LLM API does not contain orphaned assistant messages with tool_calls
- **AND** the LLM API accepts the request without a 400 error
- **AND** the conversation continues normally
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
## 1. Add cleanup helper to SessionStateManager

- [x] 1.1 Add `removeLastAssistantToolCallMessage()` method to `src/session/stateManager.js` that checks if the last conversation entry is an assistant message with tool_calls and removes it if so
- [x] 1.2 Ensure the helper is idempotent — safe to call when no matching message exists (no-op)
- [x] 1.3 Ensure the helper handles empty conversation array without errors

## 2. Update handleInterrupt() to use cleanup helper

- [x] 2.1 In `src/tui/app.js`, modify `handleInterrupt()` (line 944) to call `sessionState.removeLastAssistantToolCallMessage()` after aborting the controller and before waiting for dispatch to complete
- [x] 2.2 Ensure the cleanup happens before the dispatchPromise resolves, so the conversation is clean when the AbortError catch block runs

## 3. Update AbortError catch block to use cleanup helper

- [x] 3.1 In `src/tui/app.js`, modify the AbortError catch block (line 904-913) to call `sessionState.removeLastAssistantToolCallMessage()` before calling `sessionState.popExchange()`
- [x] 3.2 Ensure the order is correct: remove assistant tool-call message first, then pop user message

## 4. Add unit tests

- [x] 4.1 Add tests for `removeLastAssistantToolCallMessage()` in `src/session/stateManager.js` covering: assistant message with tool_calls present, assistant message without tool_calls, no assistant message, empty conversation
- [x] 4.2 Add tests for `handleInterrupt()` cleanup behavior in `src/tui/app.js` covering: interrupt during tool execution, interrupt during text response, interrupt with no assistant message

## 5. Verify and lint

- [x] 5.1 Run `npm run lint` to verify no linting errors
- [x] 5.2 Run `npm run test` to verify all tests pass
- [x] 5.3 Run `npm start` to verify application starts without crashing
48 changes: 48 additions & 0 deletions openspec/specs/interrupt-cleanup/spec.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
# interrupt-cleanup Specification

## Purpose
TBD - created by archiving change fix-interrupt-tool-call-400-error. Update Purpose after archive.
## Requirements
### Requirement: Interrupt cleanup removes assistant tool-call messages from session state
When a tool execution is interrupted, the TUI SHALL remove the assistant's AIMessage containing tool_calls from the session state conversation array, in addition to removing the user's message. This ensures the conversation history sent to the LLM API is consistent and does not contain orphaned tool calls.

#### Scenario: Interrupt during tool execution removes assistant tool-call message
- **WHEN** a tool is executing and the user triggers an interrupt
- **THEN** the `handleInterrupt()` function removes the last assistant message with tool_calls from `sessionState.conversation`
- **AND** the user's message is also removed via `popExchange()`
- **AND** the conversation array no longer contains the orphaned assistant tool-call message

#### Scenario: Interrupt during normal text response preserves conversation
- **WHEN** the assistant is responding with text (no tool calls) and the user triggers an interrupt
- **THEN** the cleanup logic does not remove any assistant message (no tool_calls to clean up)
- **AND** only the user's message is removed via `popExchange()`
- **AND** the conversation remains consistent

#### Scenario: Interrupt with no assistant message is safe
- **WHEN** an interrupt is triggered but no assistant message exists in the conversation
- **THEN** the cleanup helper function performs no removal (no-op)
- **AND** no error is thrown
- **AND** the conversation state is unchanged

### Requirement: Cleanup helper is idempotent and defensive
The helper function that removes assistant tool-call messages SHALL be safe to call multiple times and SHALL not throw errors when no matching message exists.

#### Scenario: Calling cleanup helper multiple times is safe
- **WHEN** the cleanup helper is called when no assistant tool-call message exists
- **THEN** the function returns without modifying the conversation array
- **AND** no error is thrown

#### Scenario: Cleanup helper handles empty conversation
- **WHEN** the conversation array is empty
- **THEN** the cleanup helper returns without modifying state
- **AND** no error is thrown

### Requirement: Cleanup occurs before next LLM dispatch
After interrupt cleanup completes, the session state SHALL be consistent before the next dispatch to the LLM API, ensuring the system message remains at position 0 in the messages array.

#### Scenario: Conversation is clean before resume dispatch
- **WHEN** an interrupt completes and the user sends a new message
- **THEN** the conversation sent to the LLM API does not contain orphaned assistant messages with tool_calls
- **AND** the LLM API accepts the request without a 400 error
- **AND** the conversation continues normally

32 changes: 32 additions & 0 deletions src/session/stateManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,38 @@ export class SessionStateManager {
return removed;
}

/**
* Remove the last assistant message with tool_calls from the conversation.
* Used during interrupt cleanup to remove orphaned tool-call messages.
* This method is idempotent — safe to call when no matching message exists.
* @returns {{ role: string, content: string, timestamp: string } | undefined} The removed message, or undefined if no matching message was found.
*/
removeLastAssistantToolCallMessage() {
const lastMessage = this.#state.conversation[this.#state.conversation.length - 1];
if (
lastMessage &&
lastMessage.role === "assistant" &&
this.#hasToolCalls(lastMessage)
) {
const removed = this.#state.conversation.pop();
this.#state.updatedAt = new Date().toISOString();
return removed;
}
return undefined;
}

/**
* Check if a message contains tool calls.
* @param {{ role: string, content: string | object }} message - The message to check.
* @returns {boolean} True if the message has tool_calls.
*/
#hasToolCalls(message) {
if (typeof message.content === "object" && message.content !== null) {
return Array.isArray(message.content.tool_calls) && message.content.tool_calls.length > 0;
}
return false;
}

/**
* Get the active skills list.
* @returns {string[]}
Expand Down
14 changes: 13 additions & 1 deletion src/tui/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -904,8 +904,11 @@ export default function App({
} catch (err) {
// Abort is a normal interruption, not an error
if (err.name === "AbortError") {
// Clean up: remove the user message that was added before the aborted stream
// Clean up: remove the assistant's tool-call message (if any) and the user message.
// The assistant's tool-call message is removed first to prevent orphaned tool_calls
// from corrupting the conversation history sent to the LLM API on resume.
if (sessionState) {
sessionState.removeLastAssistantToolCallMessage();
sessionState.popExchange();
}
// Clear the partial streaming assistant message from UI
Expand Down Expand Up @@ -947,6 +950,15 @@ export default function App({
abortControllerRef.current = null;
}
isStreamingRef.current = false;

// Clean up the assistant's tool-call message from session state.
// This removes any orphaned AIMessage with tool_calls that was never
// completed, preventing corrupted conversation history from being sent
// to the LLM API on resume.
if (sessionState) {
sessionState.removeLastAssistantToolCallMessage();
}

setMessages((prev) => {
const cloned = [...prev];
const last = cloned[cloned.length - 1];
Expand Down
Loading