From bfdfeb3033b23c613e7963c4a9ae158229f7edf1 Mon Sep 17 00:00:00 2001 From: Philip Zeyliger Date: Mon, 12 Jan 2026 20:42:45 -0800 Subject: [PATCH] shelley: fix agent_working status being reset by metadata updates phil: Don't love the impl here, but not having cancel was really crimping my style. The whole agent_working status thing has been really error prone. Prompt: I'm not seeing Shelley show 'Agent is working...' any more. I suspect the change that dealt with reconnects may have broken it? Follow up: I think the issue is that messages from other conversations updating the conversation name or whatever are confusing the logic. The agent_working field in StreamResponse was being set to false (the zero value) when sending conversation metadata updates (like slug changes). This caused the UI to incorrectly clear the 'Agent working...' status immediately after the agent started working. Fix by changing AgentWorking from bool to *bool with omitempty, so metadata- only updates don't include the field at all. The UI already checks typeof agent_working === 'boolean' before updating state, so undefined values are correctly ignored. - Change StreamResponse.AgentWorking from bool to *bool with omitempty - Update agentWorking() to return *bool - Add boolPtr() helper function - Update tests to handle pointer comparisons - Regenerate TypeScript types Co-authored-by: Shelley --- cmd/go2ts.go | 2 +- server/agent_working_test.go | 25 +++++++++++----------- server/convo.go | 2 +- server/server.go | 40 +++++++++++++++++++++--------------- ui/src/generated-types.ts | 2 +- 5 files changed, 40 insertions(+), 31 deletions(-) diff --git a/cmd/go2ts.go b/cmd/go2ts.go index 593a81693a7aeb0569899272bc37c9f5fb52ef3f..a27e8776b487bd269f0b5393a91b1c0dcb1bed65 100644 --- a/cmd/go2ts.go +++ b/cmd/go2ts.go @@ -90,5 +90,5 @@ type apiMessageForTS struct { type streamResponseForTS struct { Messages []apiMessageForTS `json:"messages"` Conversation generated.Conversation `json:"conversation"` - AgentWorking bool `json:"agent_working"` + AgentWorking *bool `json:"agent_working,omitempty"` } diff --git a/server/agent_working_test.go b/server/agent_working_test.go index af825efc135fb895c982e96efd245a933d31b5c6..15a9bc70b212b834cc1bd80f934e9ebe5cc42ab0 100644 --- a/server/agent_working_test.go +++ b/server/agent_working_test.go @@ -1,15 +1,12 @@ package server import ( + "fmt" "testing" "shelley.exe.dev/db" ) -func boolPtr(b bool) *bool { - return &b -} - func TestAgentWorking(t *testing.T) { tests := []struct { name string @@ -24,14 +21,14 @@ func TestAgentWorking(t *testing.T) { { name: "agent with end_of_turn true", messages: []APIMessage{ - {Type: string(db.MessageTypeAgent), EndOfTurn: boolPtr(true)}, + {Type: string(db.MessageTypeAgent), EndOfTurn: truePtr}, }, want: false, }, { name: "agent with end_of_turn false", messages: []APIMessage{ - {Type: string(db.MessageTypeAgent), EndOfTurn: boolPtr(false)}, + {Type: string(db.MessageTypeAgent), EndOfTurn: falsePtr}, }, want: true, }, @@ -52,7 +49,7 @@ func TestAgentWorking(t *testing.T) { { name: "agent end_of_turn then tool message means working", messages: []APIMessage{ - {Type: string(db.MessageTypeAgent), EndOfTurn: boolPtr(true)}, + {Type: string(db.MessageTypeAgent), EndOfTurn: truePtr}, {Type: string(db.MessageTypeTool)}, }, want: true, @@ -60,7 +57,7 @@ func TestAgentWorking(t *testing.T) { { name: "gitinfo after agent end_of_turn should NOT indicate working", messages: []APIMessage{ - {Type: string(db.MessageTypeAgent), EndOfTurn: boolPtr(true)}, + {Type: string(db.MessageTypeAgent), EndOfTurn: truePtr}, {Type: string(db.MessageTypeGitInfo)}, }, want: false, @@ -68,7 +65,7 @@ func TestAgentWorking(t *testing.T) { { name: "multiple gitinfo after agent end_of_turn should NOT indicate working", messages: []APIMessage{ - {Type: string(db.MessageTypeAgent), EndOfTurn: boolPtr(true)}, + {Type: string(db.MessageTypeAgent), EndOfTurn: truePtr}, {Type: string(db.MessageTypeGitInfo)}, {Type: string(db.MessageTypeGitInfo)}, }, @@ -77,7 +74,7 @@ func TestAgentWorking(t *testing.T) { { name: "gitinfo after agent not end_of_turn should indicate working", messages: []APIMessage{ - {Type: string(db.MessageTypeAgent), EndOfTurn: boolPtr(false)}, + {Type: string(db.MessageTypeAgent), EndOfTurn: falsePtr}, {Type: string(db.MessageTypeGitInfo)}, }, want: true, @@ -95,8 +92,12 @@ func TestAgentWorking(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { got := agentWorking(tt.messages) - if got != tt.want { - t.Errorf("agentWorking() = %v, want %v", got, tt.want) + if got == nil || *got != tt.want { + gotVal := "nil" + if got != nil { + gotVal = fmt.Sprintf("%v", *got) + } + t.Errorf("agentWorking() = %v, want %v", gotVal, tt.want) } }) } diff --git a/server/convo.go b/server/convo.go index b0a1493fb8e8dd7378b530b383c2d801fdd00519..f4c528d6c5096f8e14caba6b06aa04538bc3169a 100644 --- a/server/convo.go +++ b/server/convo.go @@ -557,7 +557,7 @@ func (cm *ConversationManager) notifyGitStateChange(ctx context.Context, msg *ge streamData := StreamResponse{ Messages: apiMessages, Conversation: conversation, - AgentWorking: false, // Gitinfo is recorded at end of turn, agent is done + AgentWorking: falsePtr, // Gitinfo is recorded at end of turn, agent is done } cm.subpub.Publish(msg.SequenceID, streamData) } diff --git a/server/server.go b/server/server.go index b7e32c6e36ef95eaf8836154c085dd3c3f99736a..bcaa58fb96dafbbdd546848986841fc82d3eae4d 100644 --- a/server/server.go +++ b/server/server.go @@ -45,7 +45,7 @@ type APIMessage struct { type StreamResponse struct { Messages []APIMessage `json:"messages"` Conversation generated.Conversation `json:"conversation"` - AgentWorking bool `json:"agent_working"` + AgentWorking *bool `json:"agent_working,omitempty"` ContextWindowSize uint64 `json:"context_window_size,omitempty"` // ConversationListUpdate is set when another conversation in the list changed ConversationListUpdate *ConversationListUpdate `json:"conversation_list_update,omitempty"` @@ -149,9 +149,16 @@ func calculateContextWindowSize(messages []APIMessage) uint64 { return 0 } -func agentWorking(messages []APIMessage) bool { +var ( + truePtr = ptr(true) + falsePtr = ptr(false) +) + +func ptr[T any](v T) *T { return &v } + +func agentWorking(messages []APIMessage) *bool { if len(messages) == 0 { - return false + return falsePtr } // Find the last non-gitinfo message (gitinfo messages are passive notifications) @@ -160,20 +167,23 @@ func agentWorking(messages []APIMessage) bool { lastIdx-- } if lastIdx < 0 { - return false + return falsePtr } last := messages[lastIdx] // If the last message is an error, agent is not working if last.Type == string(db.MessageTypeError) { - return false + return falsePtr } if last.Type == string(db.MessageTypeAgent) { if last.EndOfTurn == nil { - return true + return truePtr } - return !*last.EndOfTurn + if *last.EndOfTurn { + return falsePtr + } + return truePtr } for i := lastIdx; i >= 0; i-- { @@ -181,18 +191,12 @@ func agentWorking(messages []APIMessage) bool { if msg.Type != string(db.MessageTypeAgent) { continue } - if msg.EndOfTurn == nil { - return true - } - if !*msg.EndOfTurn { - return true - } // Agent ended turn, but newer non-agent messages exist, so agent is working again. - return true + return truePtr } // No agent message found yet but conversation has activity, assume agent is working. - return true + return truePtr } // isEndOfTurn checks if a database message represents end of turn @@ -679,10 +683,14 @@ func (s *Server) notifySubscribersNewMessage(ctx context.Context, conversationID apiMessages := toAPIMessages([]generated.Message{*newMsg}) // Publish only the new message + agentWorking := falsePtr + if !isEndOfTurn(newMsg) { + agentWorking = truePtr + } streamData := StreamResponse{ Messages: apiMessages, Conversation: conversation, - AgentWorking: !isEndOfTurn(newMsg), + AgentWorking: agentWorking, // ContextWindowSize: 0 for messages without usage data (user/tool messages). // With omitempty, 0 is omitted from JSON, so the UI keeps its cached value. // Only agent messages have usage data, so context window updates when they arrive. diff --git a/ui/src/generated-types.ts b/ui/src/generated-types.ts index d438ab67eee810a7671cd7840ea98802c426ebfb..a75f08fc39cfbb1c055ce469cca97c63ba23516c 100644 --- a/ui/src/generated-types.ts +++ b/ui/src/generated-types.ts @@ -40,7 +40,7 @@ export interface ApiMessageForTS { export interface StreamResponseForTS { messages: ApiMessageForTS[] | null; conversation: Conversation; - agent_working: boolean; + agent_working?: boolean | null; } export type MessageType = "user" | "agent" | "tool" | "error" | "system" | "gitinfo";