Skip to content

Commit d8d37f9

Browse files
committed
fix: do not reuse message IDs
1 parent 16c5d22 commit d8d37f9

File tree

2 files changed

+24
-2
lines changed

2 files changed

+24
-2
lines changed

x/acpio/acp_conversation.go

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ type ACPConversation struct {
3030
cancel context.CancelFunc
3131
agentIO ChunkableAgentIO
3232
messages []st.ConversationMessage
33+
nextID int // monotonically increasing message ID
3334
prompting bool // true while agent is processing
3435
streamingResponse strings.Builder
3536
logger *slog.Logger
@@ -104,18 +105,20 @@ func (c *ACPConversation) Send(messageParts ...st.MessagePart) error {
104105
return st.ErrMessageValidationChanging
105106
}
106107
c.messages = append(c.messages, st.ConversationMessage{
107-
Id: len(c.messages),
108+
Id: c.nextID,
108109
Role: st.ConversationRoleUser,
109110
Message: message,
110111
Time: c.clock.Now(),
111112
})
113+
c.nextID++
112114
// Add placeholder for streaming agent response
113115
c.messages = append(c.messages, st.ConversationMessage{
114-
Id: len(c.messages),
116+
Id: c.nextID,
115117
Role: st.ConversationRoleAgent,
116118
Message: "",
117119
Time: c.clock.Now(),
118120
})
121+
c.nextID++
119122
c.streamingResponse.Reset()
120123
c.prompting = true
121124
status := c.statusLocked()

x/acpio/acp_conversation_test.go

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -511,6 +511,25 @@ func Test_ErrorRemovesPartialMessage(t *testing.T) {
511511
require.Len(t, messages, 1, "partial agent message should be removed on error")
512512
assert.Equal(t, screentracker.ConversationRoleUser, messages[0].Role)
513513
assert.Equal(t, "test", messages[0].Message)
514+
515+
// First exchange allocated IDs 0 (user) and 1 (agent, now removed).
516+
assert.Equal(t, 0, messages[0].Id)
517+
518+
// Send a second message — IDs must not reuse the removed agent message's ID (1).
519+
mock.mu.Lock()
520+
mock.writeErr = nil
521+
mock.writeBlock = nil
522+
mock.writeStarted = nil
523+
mock.mu.Unlock()
524+
525+
err := conv.Send(screentracker.MessagePartText{Content: "retry"})
526+
require.NoError(t, err)
527+
528+
messages = conv.Messages()
529+
require.Len(t, messages, 3, "first user + second user + second agent")
530+
assert.Equal(t, 0, messages[0].Id, "original user message keeps its ID")
531+
assert.Equal(t, 2, messages[1].Id, "new user message skips removed ID 1")
532+
assert.Equal(t, 3, messages[2].Id, "new agent message continues sequence")
514533
}
515534

516535
func Test_LateChunkAfterError_DoesNotCorruptUserMessage(t *testing.T) {

0 commit comments

Comments
 (0)