Skip to content

Legacy session parsing regression: inverted compatibility logic returns empty messages #1086

New issue
New issue
Open
Open
Legacy session parsing regression: inverted compatibility logic returns empty messages#1086
Labels
kind/bugSomething isn't working
@jeanlaurent

Description

@jeanlaurent
jeanlaurent
opened on Dec 13, 2025

Summary

  • Context: The session store's GetSession and GetSessions methods handle backward compatibility for sessions stored before August 2025, which used a different message format containing an AgentFilename field.
  • Bug: The backward compatibility logic is inverted - it attempts to unmarshal old format messages as the new []Item format instead of converting them from []Message.
  • Actual vs. expected: Old format sessions are retrieved with empty message content, roles, and agent names, instead of preserving the original message data.
  • Impact: Users with sessions created before August 21, 2025 cannot access their historical session data - all message content is lost when reading these sessions from the database.

Code with bug

pkg/session/store.go lines 186-196 (in GetSession):

var items []Item
var messages []Message
if err := json.Unmarshal([]byte(messagesJSON), &messages); err != nil {
	return nil, err
}
if len(messages) > 0 {  // <-- BUG 🔴 Logic is inverted
	if err := json.Unmarshal([]byte(messagesJSON), &items); err != nil {
		return nil, err
	}
} else {
	items = convertMessagesToItems(messages)
}

The same bug exists in GetSessions at lines 273-283.

The current logic:

  1. Unmarshal as []Message (succeeds for old format)
  2. If successful and len(messages) > 0, try to unmarshal as []Item (creates empty items because JSON structure doesn't match)
  3. Else (if messages is empty), convert messages to items

The correct logic should be:

  1. Try to unmarshal as new format []Item first
  2. If that fails, try to unmarshal as old format []Message and convert

Evidence

Example

Consider an old format session stored in the database with this JSON:

[
  {
    "agentFilename": "demo.yaml",
    "agentName": "",
    "message": {
      "role": "user",
      "content": "Hello from old format"
    }
  }
]

With the current buggy code:

  1. json.Unmarshal(messagesJSON, &messages) succeeds (Go's JSON unmarshaler ignores unknown field agentFilename)
  2. len(messages) > 0 is true (1 message)
  3. json.Unmarshal(messagesJSON, &items) is attempted - this creates an Item but the nested structure doesn't match
  4. The JSON has {"agentFilename": ..., "agentName": ..., "message": {...}} but Item expects {"message": {"agentName": ..., "message": {...}}, "sub_session": ...}
  5. The unmarshal "succeeds" but all fields are empty because the structure doesn't match
  6. Result: Item with empty Message.Content, empty Message.Role, empty Message.AgentName

Inconsistency within the codebase

Reference code

pkg/session/store.go at commit 3268ca8 (August 21, 2025 - when Item format was introduced):

// Parse the data
var items []Item
if err := json.Unmarshal([]byte(messagesJSON), &items); err != nil {
	// Try to unmarshal as legacy messages format for backward compatibility
	var messages []Message
	if err2 := json.Unmarshal([]byte(messagesJSON), &messages); err2 != nil {
		return nil, err // Return original error if both fail
	}
	items = convertMessagesToItems(messages)
}

Current code

pkg/session/store.go at current HEAD:

var items []Item
var messages []Message
if err := json.Unmarshal([]byte(messagesJSON), &messages); err != nil {
	return nil, err
}
if len(messages) > 0 {
	if err := json.Unmarshal([]byte(messagesJSON), &items); err != nil {
		return nil, err
	}
} else {
	items = convertMessagesToItems(messages)
}

Contradiction

The original implementation (commit 3268ca8) correctly:

  1. First tries to unmarshal as NEW format ([]Item)
  2. On failure, falls back to OLD format ([]Message) and converts

The current implementation incorrectly:

  1. First unmarshals as OLD format ([]Message) - always succeeds due to JSON flexibility
  2. Then tries to unmarshal as NEW format when len(messages) > 0 - creates empty Items
  3. Only converts messages when len(messages) == 0 - which never happens for old data

The logic was broken in commit 3f5d48f when the AgentFilename field was removed from the Message struct. The intermediate fix in commit 1705a24 checked messages[0].AgentFilename == "" to distinguish formats, but when AgentFilename was removed, the check was changed to just len(messages) > 0, which inverted the logic.

Full context

The session store is the persistence layer for agent conversation sessions. It supports two implementations:

  • InMemorySessionStore: For ephemeral sessions
  • SQLiteSessionStore: For persistent sessions in a SQLite database

The SQLiteSessionStore.GetSession and SQLiteSessionStore.GetSessions methods read sessions from the database and must handle two different JSON formats:

  1. Old format (pre-August 2025): An array of Message objects with structure:

    [{"agentFilename": "...", "agentName": "...", "message": {...}}]

  2. New format (post-August 2025): An array of Item objects that can contain either messages or sub-sessions:

    [{"message": {"agentName": "...", "message": {...}}, "sub_session": null}]

The backward compatibility code is critical because:

  • Sessions can persist for months
  • Users may have valuable conversation history from before August 2025
  • The migration is automatic (no schema change) - it happens during read

The bug is in the detection logic that determines which format is being read. When the code incorrectly unmarshals old format as new format, the nested JSON structure doesn't align, resulting in empty fields. This data loss is silent - no error is returned, just empty messages.

The code is called by:

  • The server API (pkg/server/session_manager.go) when retrieving sessions for the UI
  • The runtime (pkg/runtime/runtime.go) when resuming sessions
  • Any code that lists or retrieves historical sessions

Failing test

Test script

package session

import (
	"context"
	"database/sql"
	"encoding/json"
	"fmt"
	"path/filepath"
	"testing"
	"time"

	"github.com/stretchr/testify/assert"
	"github.com/stretchr/testify/require"

	"github.com/docker/cagent/pkg/chat"
)

// OldMessage represents the old message format with AgentFilename field
// This format was used before commit 3f5d48f2
type OldMessage struct {
	AgentFilename string       `json:"agentFilename"`
	AgentName     string       `json:"agentName"`
	Message       chat.Message `json:"message"`
}

// TestBackwardCompatibilityWithOldMessageFormat tests that sessions stored with the old
// message format (with AgentFilename field) can still be read correctly.
// This is a regression test for a bug introduced in commit 3f5d48f2 where the backward
// compatibility logic was inverted.
func TestBackwardCompatibilityWithOldMessageFormat(t *testing.T) {
	// Create a temporary database
	tempDir := t.TempDir()
	dbPath := filepath.Join(tempDir, "test_old_format.db")

	// Open database directly to insert old format data
	db, err := sql.Open("sqlite", dbPath)
	require.NoError(t, err)

	// Create the sessions table (old schema - just the basics, migrations will add the rest)
	_, err = db.ExecContext(context.Background(), `
		CREATE TABLE IF NOT EXISTS sessions (
			id TEXT PRIMARY KEY,
			messages TEXT,
			created_at TEXT
		)
	`)
	require.NoError(t, err)

	// Create old format messages (with AgentFilename field that no longer exists)
	oldMessages := []OldMessage{
		{
			AgentFilename: "demo.yaml",
			AgentName:     "",
			Message: chat.Message{
				Role:      chat.MessageRoleUser,
				Content:   "Hello from old format",
				CreatedAt: time.Now().Format(time.RFC3339),
			},
		},
		{
			AgentFilename: "demo.yaml",
			AgentName:     "assistant-agent",
			Message: chat.Message{
				Role:      chat.MessageRoleAssistant,
				Content:   "Response in old format",
				CreatedAt: time.Now().Format(time.RFC3339),
			},
		},
	}

	// Marshal old messages
	messagesJSON, err := json.Marshal(oldMessages)
	require.NoError(t, err)

	t.Logf("Old format JSON: %s", string(messagesJSON))

	// Insert session with old message format
	_, err = db.ExecContext(context.Background(),
		"INSERT INTO sessions (id, messages, created_at) VALUES (?, ?, ?)",
		"old-session-id", string(messagesJSON), time.Now().Format(time.RFC3339))
	require.NoError(t, err)

	db.Close()

	// Now try to read it using the session store
	store, err := NewSQLiteSessionStore(dbPath)
	require.NoError(t, err)
	defer store.(*SQLiteSessionStore).Close()

	// Try to retrieve the session
	sess, err := store.GetSession(context.Background(), "old-session-id")
	require.NoError(t, err, "Should be able to retrieve session with old format messages")

	// Verify the messages were correctly converted
	assert.Len(t, sess.Messages, 2, "Should have 2 messages")

	// First message should be a message item
	require.True(t, sess.Messages[0].IsMessage(), "First item should be a message")
	assert.Equal(t, "Hello from old format", sess.Messages[0].Message.Message.Content,
		"First message content should be preserved from old format")
	assert.Equal(t, chat.MessageRoleUser, sess.Messages[0].Message.Message.Role,
		"First message role should be preserved")

	// Second message should also be a message item
	require.True(t, sess.Messages[1].IsMessage(), "Second item should be a message")
	assert.Equal(t, "Response in old format", sess.Messages[1].Message.Message.Content,
		"Second message content should be preserved from old format")
	assert.Equal(t, "assistant-agent", sess.Messages[1].Message.AgentName,
		"Second message agent name should be preserved")
	assert.Equal(t, chat.MessageRoleAssistant, sess.Messages[1].Message.Message.Role,
		"Second message role should be preserved")
}

// TestBackwardCompatibilityWithOldMessageFormatInGetSessions tests the same backward
// compatibility for the GetSessions method
func TestBackwardCompatibilityWithOldMessageFormatInGetSessions(t *testing.T) {
	// Create a temporary database
	tempDir := t.TempDir()
	dbPath := filepath.Join(tempDir, "test_old_format_list.db")

	// Open database directly to insert old format data
	db, err := sql.Open("sqlite", dbPath)
	require.NoError(t, err)

	// Create the sessions table
	_, err = db.ExecContext(context.Background(), `
		CREATE TABLE IF NOT EXISTS sessions (
			id TEXT PRIMARY KEY,
			messages TEXT,
			created_at TEXT
		)
	`)
	require.NoError(t, err)

	// Create old format messages
	oldMessages := []OldMessage{
		{
			AgentFilename: "agent.yaml",
			AgentName:     "",
			Message: chat.Message{
				Role:      chat.MessageRoleUser,
				Content:   "Old format user message",
				CreatedAt: time.Now().Format(time.RFC3339),
			},
		},
	}

	messagesJSON, err := json.Marshal(oldMessages)
	require.NoError(t, err)

	// Insert multiple sessions with old format
	for i := 1; i <= 3; i++ {
		_, err = db.ExecContext(context.Background(),
			"INSERT INTO sessions (id, messages, created_at) VALUES (?, ?, ?)",
			fmt.Sprintf("old-session-%d", i), string(messagesJSON), time.Now().Format(time.RFC3339))
		require.NoError(t, err)
	}

	db.Close()

	// Now try to read them using the session store
	store, err := NewSQLiteSessionStore(dbPath)
	require.NoError(t, err)
	defer store.(*SQLiteSessionStore).Close()

	// Try to retrieve all sessions
	sessions, err := store.GetSessions(context.Background())
	require.NoError(t, err, "Should be able to retrieve sessions with old format messages")

	assert.Len(t, sessions, 3, "Should have 3 sessions")

	// Verify each session has correctly converted messages
	for i, sess := range sessions {
		assert.Len(t, sess.Messages, 1, "Session %d should have 1 message", i)
		require.True(t, sess.Messages[0].IsMessage(), "Session %d first item should be a message", i)
		assert.Equal(t, "Old format user message", sess.Messages[0].Message.Message.Content,
			"Session %d message content should be preserved from old format", i)
	}
}

Test output

=== RUN   TestBackwardCompatibilityWithOldMessageFormat
    store_backward_compat_test.go:75: Old format JSON: [{"agentFilename":"demo.yaml","agentName":"","message":{"role":"user","content":"Hello from old format","created_at":"2025-12-12T19:17:47Z"}},{"agentFilename":"demo.yaml","agentName":"assistant-agent","message":{"role":"assistant","content":"Response in old format","created_at":"2025-12-12T19:17:47Z"}}]
    store_backward_compat_test.go:99:
        	Error Trace:	/home/user/cagent/pkg/session/store_backward_compat_test.go:99
        	Error:      	Not equal:
        	            	expected: "Hello from old format"
        	            	actual  : ""

        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
        	            	@@ -1 +1 @@
        	            	-Hello from old format
        	            	+
        	Test:       	TestBackwardCompatibilityWithOldMessageFormat
        	Messages:   	First message content should be preserved from old format
    store_backward_compat_test.go:101:
        	Error Trace:	/home/user/cagent/pkg/session/store_backward_compat_test.go:101
        	Error:      	Not equal:
        	            	expected: "user"
        	            	actual  : ""

        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
        	            	@@ -1,2 +1,2 @@
        	            	-(chat.MessageRole) (len=4) "user"
        	            	+(chat.MessageRole) ""

        	Test:       	TestBackwardCompatibilityWithOldMessageFormat
        	Messages:   	First message role should be preserved
    store_backward_compat_test.go:106:
        	Error Trace:	/home/user/cagent/pkg/session/store_backward_compat_test.go:106
        	Error:      	Not equal:
        	            	expected: "Response in old format"
        	            	actual  : ""

        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
        	            	@@ -1 +1 @@
        	            	-Response in old format
        	            	+
        	Test:       	TestBackwardCompatibilityWithOldMessageFormat
        	Messages:   	Second message content should be preserved from old format
    store_backward_compat_test.go:108:
        	Error Trace:	/home/user/cagent/pkg/session/store_backward_compat_test.go:108
        	Error:      	Not equal:
        	            	expected: "assistant-agent"
        	            	actual  : ""

        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
        	            	@@ -1 +1 @@
        	            	-assistant-agent
        	            	+
        	Test:       	TestBackwardCompatibilityWithOldMessageFormat
        	Messages:   	Second message agent name should be preserved
    store_backward_compat_test.go:110:
        	Error Trace:	/home/user/cagent/pkg/session/store_backward_compat_test.go:110
        	Error:      	Not equal:
        	            	expected: "assistant"
        	            	actual  : ""

        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
        	            	@@ -1,2 +1,2 @@
        	            	-(chat.MessageRole) (len=9) "assistant"
        	            	+(chat.MessageRole) ""

        	Test:       	TestBackwardCompatibilityWithOldMessageFormat
        	Messages:   	Second message role should be preserved
--- FAIL: TestBackwardCompatibilityWithOldMessageFormat (0.01s)
=== RUN   TestBackwardCompatibilityWithOldMessageFormatInGetSessions
    store_backward_compat_test.go:176:
        	Error Trace:	/home/user/cagent/pkg/session/store_backward_compat_test.go:176
        	Error:      	Not equal:
        	            	expected: "Old format user message"
        	            	actual  : ""

        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
        	            	@@ -1 +1 @@
        	            	-Old format user message
        	            	+
        	Test:       	TestBackwardCompatibilityWithOldMessageFormatInGetSessions
        	Messages:   	Session 0 message content should be preserved from old format
    store_backward_compat_test.go:176:
        	Error Trace:	/home/user/cagent/pkg/session/store_backward_compat_test.go:176
        	Error:      	Not equal:
        	            	expected: "Old format user message"
        	            	actual  : ""

        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
        	            	@@ -1 +1 @@
        	            	-Old format user message
        	            	+
        	Test:       	TestBackwardCompatibilityWithOldMessageFormatInGetSessions
        	Messages:   	Session 1 message content should be preserved from old format
    store_backward_compat_test.go:176:
        	Error Trace:	/home/user/cagent/pkg/session/store_backward_compat_test.go:176
        	Error:      	Not equal:
        	            	expected: "Old format user message"
        	            	actual  : ""

        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
        	            	@@ -1 +1 @@
        	            	-Old format user message
        	            	+
        	Test:       	TestBackwardCompatibilityWithOldMessageFormatInGetSessions
        	Messages:   	Session 2 message content should be preserved from old format
--- FAIL: TestBackwardCompatibilityWithOldMessageFormatInGetSessions (0.01s)
FAIL
FAIL	github.com/docker/cagent/pkg/session	0.028s
FAIL

Why has this bug gone undetected?

  1. Recent introduction: The bug was introduced on November 27, 2025 (commit 3f5d48f), only about 15 days ago.

  2. Limited affected data: The old message format was only used before August 21, 2025 (commit 3268ca8). Most active users likely have sessions created after that date in the new format, which works correctly.

  3. Casual users less affected: Users who started using the application after August 2025 have no old format data. Only users with sessions from before August 21, 2025 AND who try to access those sessions after November 27, 2025 would encounter this bug.

  4. Silent failure: The bug doesn't cause an error - it silently returns empty message content. A user might see their old sessions in a list but with no visible content, which could be mistaken for a display issue or dismissed as expected data loss.

  5. No test coverage: There were no tests specifically checking backward compatibility with the old message format. The existing tests in store_test.go all create sessions using the new Item format, so they pass successfully.

  6. Comment is misleading: The code comment mentions checking for AgentFilename to determine format, but that field no longer exists in the Message struct. This suggests the code was changed mechanically without full understanding of the backward compatibility logic.

Recommended fix

Restore the original backward compatibility logic from commit 3268ca8:

// In GetSession (lines 186-196) and GetSessions (lines 273-283)
var items []Item
if err := json.Unmarshal([]byte(messagesJSON), &items); err != nil {  // <-- FIX 🟢 Try new format first
	// Try to unmarshal as legacy messages format for backward compatibility
	var messages []Message
	if err2 := json.Unmarshal([]byte(messagesJSON), &messages); err2 != nil {
		return nil, err // Return original error if both fail
	}
	items = convertMessagesToItems(messages)  // <-- FIX 🟢 Convert old format
}

This approach:

  1. First attempts to unmarshal as the new []Item format
  2. If that fails (because it's old format), falls back to []Message and converts
  3. If both fail, returns an error
  4. Works correctly for both old and new formats without needing to check for removed fields

Metadata

Metadata

Assignees

No one assigned

    Labels

    kind/bugSomething isn't working

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions