Skip to content

AdvancedSQLiteSession.create_branch_from_turn silently merges into an existing branch #4150

Description

@hsusul

Please read this first

  • Have you read the docs? Yes
  • Have you searched for related issues? Yes — no existing issue or PR covers branch-id collisions in AdvancedSQLiteSession.

Describe the bug

AdvancedSQLiteSession.create_branch_from_turn() never checks whether the target branch id already exists. _copy_messages_to_new_branch() unconditionally inserts message_structure rows tagged with that branch id, so creating a branch whose id is already in use appends the copied history to the existing branch instead of creating a new one. The call still returns the branch id and switches to it, so the caller has no signal that anything went wrong.

This is reachable two ways:

  1. The SDK's own default branch name can collide. When branch_name is omitted the name is f"branch_from_turn_{turn_number}_{int(time.time())}". That has one-second resolution, so branching twice from the same turn within the same second — the documented usage, await session.create_branch_from_turn(2) — produces the same id and silently duplicates the copied history. No user error is involved.
  2. An explicitly supplied existing name is accepted, including "main", which duplicates the base branch's own history into itself.

Debug information

  • Agents SDK version: main @ c546ca1
  • Python version: 3.13.5
  • OS: macOS (Darwin 24.6.0)
  • No API key, model call, or network access is needed to reproduce.

Repro steps

import asyncio
import time
from unittest.mock import patch

from agents.extensions.memory import AdvancedSQLiteSession

ITEMS = [
    {"role": "user", "content": "turn one"},
    {"role": "assistant", "content": "reply one"},
    {"role": "user", "content": "turn two"},
    {"role": "assistant", "content": "reply two"},
    {"role": "user", "content": "turn three"},
    {"role": "assistant", "content": "reply three"},
]


async def main() -> None:
    # 1. Auto-generated names collide within the same second.
    session = AdvancedSQLiteSession(session_id="collide", create_tables=True)
    await session.add_items(ITEMS)
    with patch.object(time, "time", lambda: 1_700_000_000.0):
        first = await session.create_branch_from_turn(3)
        await session.switch_to_branch("main")
        second = await session.create_branch_from_turn(3)
    print(first, second)                     # identical ids
    print(len(await session.get_items()))    # 8, expected 4
    print([b["branch_id"] for b in await session.list_branches()])  # only one new branch
    session.close()

    # 2. An existing name is accepted and corrupts that branch.
    session = AdvancedSQLiteSession(session_id="into-main", create_tables=True)
    await session.add_items(ITEMS)
    before = await session.get_items(branch_id="main")
    await session.create_branch_from_turn(2, "main")
    after = await session.get_items(branch_id="main")
    print(len(before), len(after))            # 6 8
    session.close()


asyncio.run(main())

Actual behavior

  1. Both calls return branch_from_turn_3_1700000000. The branch now holds the first two turns twice (8 items instead of 4), and list_branches() reports a single branch where the caller created two.
  2. create_branch_from_turn(2, "main") copies turn 1 into main, growing main from 6 to 8 items.

Expected behavior

Creating a branch should never write into an existing branch.

  • A branch id supplied by the caller that is already in use should raise ValueError — the method already documents ValueError for invalid branch requests, and delete_branch()/switch_to_branch() already use ValueError for branch-state errors.
  • An auto-generated branch id should be unique, so the documented no-argument form cannot fail or collide.

Root-cause hypothesis

(hypothesis) _copy_messages_to_new_branch() (src/agents/extensions/memory/advanced_sqlite_session.py) selects the rows to copy and inserts them under new_branch_id with no existence check on that id, and create_branch_from_turn() derives its default id from a one-second timestamp without checking for an existing branch. Because the check would have to be atomic with the insert, it belongs inside the same locked connection that performs the copy.

Proposed scope

Resolve the target branch id under the connection lock that performs the copy: reject an explicit id that already exists, and disambiguate a generated id. No public API or schema change, and no change to the copy semantics themselves.

I'd like to work on this.

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions