Skip to content

Conversation

@ammar-agent
Copy link
Collaborator

@ammar-agent ammar-agent commented Oct 31, 2025

Problem

TODOs were not being cleared when streams ended, causing stale todos to persist in the UI until the next user message. Additionally, on page reload, TODOs were incorrectly reconstructed from history even for completed streams.

The challenge: We need different behavior for two reload scenarios:

  • Reconnection during active stream: Should reconstruct TODOs (work in progress)
  • Reload after completed stream: Should NOT reconstruct TODOs (clean slate)

Solution

1. Clear TODOs on stream end

Modified cleanupStreamState() to clear currentTodos when streams complete (end/abort/error). TODOs are now truly stream-scoped.

2. Smart reconstruction on reload

Key insight: Check buffered events for stream-start to detect active streams.

  • loadHistoricalMessages() now accepts hasActiveStream parameter
  • WorkspaceStore checks buffered events before loading history
  • Active stream (hasActiveStream=true) → Reconstruct TODOs ✅
  • Completed stream (hasActiveStream=false) → Don't reconstruct TODOs ✅
  • agentStatus always reconstructed (persists across sessions) ✅

3. Improved separation of concerns

Centralized tool persistence logic in processToolResult:

  • Added context parameter: "streaming" | "historical"
  • loadHistoricalMessages no longer knows about specific tool behaviors
  • Each tool declares its own persistence policy in one place

Implementation

WorkspaceStore checks for active streams:

const pendingEvents = this.pendingStreamEvents.get(workspaceId) ?? [];
const hasActiveStream = pendingEvents.some(
  (event) => "type" in event && event.type === "stream-start"
);
aggregator.loadHistoricalMessages(historicalMsgs, hasActiveStream);

StreamingMessageAggregator handles context:

loadHistoricalMessages(messages: CmuxMessage[], hasActiveStream = false) {
  const context = hasActiveStream ? "streaming" : "historical";
  // Process tool results with context
  this.processToolResult(toolName, input, output, context);
}

processToolResult decides based on tool + context:

private processToolResult(toolName, input, output, context: "streaming" | "historical") {
  // TODOs: stream-scoped (only during streaming)
  if (toolName === "todo_write" && context === "streaming") {
    this.currentTodos = args.todos;
  }
  
  // agentStatus: persistent (always reconstruct)
  if (toolName === "status_set") {
    this.agentStatus = { emoji, message, url };
  }
}

Testing

Comprehensive test coverage for the full todo lifecycle:

✅ Clear todos on stream end
✅ Clear todos on stream abort
✅ Reconstruct todos when hasActiveStream=true (reconnection)
✅ Don't reconstruct todos when hasActiveStream=false (completed)
✅ Always reconstruct agentStatus (persists across sessions)
✅ Clear todos on new user message

All 146 message-related tests pass.

Behavior Matrix

Scenario TODOs agentStatus
During streaming ✅ Visible ✅ Visible
Stream ends ❌ Cleared ✅ Persists
Reload (active stream) ✅ Reconstructed ✅ Reconstructed
Reload (completed) ❌ Not reconstructed ✅ Reconstructed
New user message ❌ Cleared ❌ Cleared

Key Insight

The reconnection scenario is the critical edge case: when a user reloads during an active stream, historical messages contain completed tool calls from the current stream, and buffered events contain stream-start. In this case, we should reconstruct TODOs to show work in progress.

This is fundamentally different from reloading after stream completion, where TODOs should remain cleared.


Generated with cmux

@ammar-agent ammar-agent force-pushed the todo-tweak branch 3 times, most recently from a453354 to 885e8c3 Compare October 31, 2025 17:47
## Problem

1. **Todos not resetting on stream end**: Todos persisted after stream completion instead of being cleared
2. **Incorrect persistence on reload**: TODOs were reconstructed from history even for completed streams, causing stale todos to reappear
3. **Missing reconnection logic**: No distinction between reloading during an active stream vs after completion

## Solution

### Reset todos on stream end
- Modified `cleanupStreamState()` to clear `currentTodos` when stream completes (end/abort/error)
- Todos are now truly stream-scoped - cleared immediately when streams end

### Smart reconstruction on reload
- Added `hasActiveStream` parameter to `loadHistoricalMessages()`
- **WorkspaceStore** checks buffered events for `stream-start` to detect active streams
- **Active stream reconnection** (hasActiveStream=true): Reconstruct TODOs from history ✅
- **Completed stream reload** (hasActiveStream=false): Don't reconstruct TODOs ✅
- **agentStatus** always reconstructed (persists across sessions) ✅

### Simplified UI
- Removed filtering logic from `PinnedTodoList` - no longer needed since todos auto-clear
- Cleaner component with fewer moving parts

## Implementation

**StreamingMessageAggregator.loadHistoricalMessages():**
```typescript
loadHistoricalMessages(messages: CmuxMessage[], hasActiveStream: boolean = false): void {
  // ... add messages to map ...

  // Reconstruct based on tool type and stream state
  const shouldReconstructTodos = part.toolName === "todo_write" && hasActiveStream;
  const shouldReconstructStatus = part.toolName === "status_set";

  if (shouldReconstructTodos || shouldReconstructStatus) {
    this.processToolResult(part.toolName, part.input, part.output);
  }
}
```

**WorkspaceStore.handleChatMessage():**
```typescript
// Check if there's an active stream in buffered events
const pendingEvents = this.pendingStreamEvents.get(workspaceId) ?? [];
const hasActiveStream = pendingEvents.some(
  (event) => "type" in event && event.type === "stream-start"
);

aggregator.loadHistoricalMessages(historicalMsgs, hasActiveStream);
```

## Testing

✅ **Todo reset on stream end** - todos cleared on completion
✅ **Todo reset on stream abort** - todos cleared on interruption
✅ **Active stream reconnection** - todos reconstructed with hasActiveStream=true
✅ **Completed stream reload** - todos NOT reconstructed with hasActiveStream=false
✅ **agentStatus persistence** - always reconstructed regardless of stream state
✅ **User message clears todos** - existing behavior maintained

All 146 tests pass ✅

## Behavior After This PR

**During streaming:**
- Todos update in real-time as `todo_write` is called
- UI shows all todos (pending, in_progress, completed)

**On stream end:**
- Todos cleared immediately
- UI shows nothing (clean slate)

**On reload - active stream (reconnection):**
- Backend sends buffered `stream-start` event
- TODOs reconstructed from historical tool calls ✅
- Stream continues with todos visible

**On reload - completed stream:**
- No buffered stream events
- TODOs NOT reconstructed ✅
- UI remains clean

**On new user message:**
- Todos cleared for fresh start
- Existing behavior unchanged

---

_Generated with `cmux`_
Improved separation of concerns by moving tool-specific behavior into processToolResult:
- Added 'context' parameter to processToolResult (streaming vs historical)
- loadHistoricalMessages now just passes context, doesn't know about specific tools
- processToolResult decides what to do based on tool type and context
- Cleaner abstraction: tool behavior is centralized in one place
@ammar-agent ammar-agent changed the title 🤖 fix: reset todos on stream end and persist correctly on reload 🤖 fix: clear todos on stream end with smart reconnection handling Oct 31, 2025
@ammario ammario added this pull request to the merge queue Oct 31, 2025
Merged via the queue into main with commit cc64299 Oct 31, 2025
13 checks passed
@ammario ammario deleted the todo-tweak branch October 31, 2025 19:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants