shelley: fix agentWorking to ignore gitinfo messages

Philip Zeyliger created 4 months ago

Prompt: in a new worktree, reset to origin/main: i was expecting this gitinfo message to be marked end of turn

gitinfo messages are passive notifications about git state changes.
They should not affect whether the agent is considered to be working.

Previously, a gitinfo message after an agent's end-of-turn message
would cause agentWorking() to return true, because it looked like
there was a new non-agent message requiring agent attention.

Now we skip over trailing gitinfo messages when determining the
'last' message for the agentWorking check.

Change summary

server/agent_working_test.go | 103 ++++++++++++++++++++++++++++++++++++++
server/server.go             |  12 +++
2 files changed, 113 insertions(+), 2 deletions(-)

Detailed changes

server/agent_working_test.go

@@ -0,0 +1,103 @@
+package server
+
+import (
+	"testing"
+
+	"shelley.exe.dev/db"
+)
+
+func boolPtr(b bool) *bool {
+	return &b
+}
+
+func TestAgentWorking(t *testing.T) {
+	tests := []struct {
+		name     string
+		messages []APIMessage
+		want     bool
+	}{
+		{
+			name:     "empty messages",
+			messages: []APIMessage{},
+			want:     false,
+		},
+		{
+			name: "agent with end_of_turn true",
+			messages: []APIMessage{
+				{Type: string(db.MessageTypeAgent), EndOfTurn: boolPtr(true)},
+			},
+			want: false,
+		},
+		{
+			name: "agent with end_of_turn false",
+			messages: []APIMessage{
+				{Type: string(db.MessageTypeAgent), EndOfTurn: boolPtr(false)},
+			},
+			want: true,
+		},
+		{
+			name: "agent with end_of_turn nil",
+			messages: []APIMessage{
+				{Type: string(db.MessageTypeAgent), EndOfTurn: nil},
+			},
+			want: true,
+		},
+		{
+			name: "error message",
+			messages: []APIMessage{
+				{Type: string(db.MessageTypeError)},
+			},
+			want: false,
+		},
+		{
+			name: "agent end_of_turn then tool message means working",
+			messages: []APIMessage{
+				{Type: string(db.MessageTypeAgent), EndOfTurn: boolPtr(true)},
+				{Type: string(db.MessageTypeTool)},
+			},
+			want: true,
+		},
+		{
+			name: "gitinfo after agent end_of_turn should NOT indicate working",
+			messages: []APIMessage{
+				{Type: string(db.MessageTypeAgent), EndOfTurn: boolPtr(true)},
+				{Type: string(db.MessageTypeGitInfo)},
+			},
+			want: false,
+		},
+		{
+			name: "multiple gitinfo after agent end_of_turn should NOT indicate working",
+			messages: []APIMessage{
+				{Type: string(db.MessageTypeAgent), EndOfTurn: boolPtr(true)},
+				{Type: string(db.MessageTypeGitInfo)},
+				{Type: string(db.MessageTypeGitInfo)},
+			},
+			want: false,
+		},
+		{
+			name: "gitinfo after agent not end_of_turn should indicate working",
+			messages: []APIMessage{
+				{Type: string(db.MessageTypeAgent), EndOfTurn: boolPtr(false)},
+				{Type: string(db.MessageTypeGitInfo)},
+			},
+			want: true,
+		},
+		{
+			name: "only gitinfo messages",
+			messages: []APIMessage{
+				{Type: string(db.MessageTypeGitInfo)},
+				{Type: string(db.MessageTypeGitInfo)},
+			},
+			want: false,
+		},
+	}
+
+	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)
+			}
+		})
+	}
+}

server/server.go

@@ -147,7 +147,15 @@ func agentWorking(messages []APIMessage) bool {
 		return false
 	}
 
-	last := messages[len(messages)-1]
+	// Find the last non-gitinfo message (gitinfo messages are passive notifications)
+	lastIdx := len(messages) - 1
+	for lastIdx >= 0 && messages[lastIdx].Type == string(db.MessageTypeGitInfo) {
+		lastIdx--
+	}
+	if lastIdx < 0 {
+		return false
+	}
+	last := messages[lastIdx]
 
 	// If the last message is an error, agent is not working
 	if last.Type == string(db.MessageTypeError) {
@@ -161,7 +169,7 @@ func agentWorking(messages []APIMessage) bool {
 		return !*last.EndOfTurn
 	}
 
-	for i := len(messages) - 1; i >= 0; i-- {
+	for i := lastIdx; i >= 0; i-- {
 		msg := messages[i]
 		if msg.Type != string(db.MessageTypeAgent) {
 			continue