Skip to content

fix(tui_gateway): persist full message history when branching a session - #42273

Open
friendshipisover wants to merge 1 commit into
NousResearch:mainfrom
friendshipisover:fix/branch-persist-tool-calls
Open

fix(tui_gateway): persist full message history when branching a session#42273
friendshipisover wants to merge 1 commit into
NousResearch:mainfrom
friendshipisover:fix/branch-persist-tool-calls

Conversation

@friendshipisover

Copy link
Copy Markdown
Contributor

What does this PR do?

When you branch (fork) a TUI conversation, the gateway copies the
current history into a brand-new session row in the SQLite store. The
problem is how it copied: it walked the history and called
append_message with only role and content. Everything else that
ties an agent turn together — tool_calls, tool_call_id, tool_name
and the reasoning fields — was silently dropped on the floor.

Here's why that bites you only later. Right after branching, the new
session is fine, because _init_session is handed the full in-memory
history list, so the live branch still has its tool linkage. The DB
copy is the persisted one, though, and the DB is the source of truth
on resume. So the moment you close the app (or it restarts) and /resume
the branch, the transcript is rebuilt from those incomplete rows by
get_messages_as_conversation. Now the assistant's tool call has no
matching tool result, and the tool result has no tool_call_id pointing
back at it. The next prompt ships that broken sequence to the provider,
and Anthropic/OpenAI reject it with a 400 — or quietly drop context. In
practice that means any branched agent session containing a tool call
(i.e. almost all of them) is unusable after a restart.

The fix is to reuse replace_messages, the same atomic, field-complete
path that /undo, /retry and /compress already use. It serializes
the whole message (tool linkage plus reasoning) and commits it in one
transaction. Because the branch session is freshly created one line
above, its internal DELETE is just a no-op, so we get the correct
behavior with less code.

Related Issue

N/A

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

  • tui_gateway/server.py: in the session.branch handler, replaced the
    per-message append_message loop (which forwarded only role and
    content) with a single db.replace_messages(new_key, history) call
    so the branched session persists tool_calls, tool_call_id,
    tool_name and reasoning fields. Added a comment explaining why.
  • tests/test_tui_gateway_server.py: added
    test_session_branch_persists_tool_calls_for_resume, which branches a
    session containing a tool-call round-trip and asserts the persisted
    transcript still round-trips the tool linkage through
    get_messages_as_conversation.

How to Test

  1. Run the new regression test:
    scripts/run_tests.sh tests/test_tui_gateway_server.py -- -k test_session_branch_persists_tool_calls_for_resume
  2. It passes with the fix. To see the bug, revert the server.py change
    and rerun — the test fails because the persisted assistant message has
    no tool_calls and the tool result has no tool_call_id/tool_name.
  3. Manual: in the TUI, run a prompt that triggers a tool call, branch the
    session, restart, then /resume the branch and send a follow-up — it
    no longer errors out on a malformed message sequence.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: macOS 15 (Darwin 25.5.0)

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) — or N/A
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

## What does this PR do?

When you branch (fork) a TUI conversation, the gateway copies the
current history into a brand-new session row in the SQLite store. The
problem is *how* it copied: it walked the history and called
`append_message` with only `role` and `content`. Everything else that
ties an agent turn together — `tool_calls`, `tool_call_id`, `tool_name`
and the reasoning fields — was silently dropped on the floor.

Here's why that bites you only later. Right after branching, the new
session is fine, because `_init_session` is handed the full in-memory
history list, so the live branch still has its tool linkage. The DB
copy is the *persisted* one, though, and the DB is the source of truth
on resume. So the moment you close the app (or it restarts) and `/resume`
the branch, the transcript is rebuilt from those incomplete rows by
`get_messages_as_conversation`. Now the assistant's tool call has no
matching tool result, and the tool result has no `tool_call_id` pointing
back at it. The next prompt ships that broken sequence to the provider,
and Anthropic/OpenAI reject it with a 400 — or quietly drop context. In
practice that means any branched agent session containing a tool call
(i.e. almost all of them) is unusable after a restart.

The fix is to reuse `replace_messages`, the same atomic, field-complete
path that `/undo`, `/retry` and `/compress` already use. It serializes
the whole message (tool linkage plus reasoning) and commits it in one
transaction. Because the branch session is freshly created one line
above, its internal `DELETE` is just a no-op, so we get the correct
behavior with less code.

## Related Issue

N/A

## Type of Change

- [x] 🐛 Bug fix (non-breaking change that fixes an issue)

## Changes Made

- `tui_gateway/server.py`: in the `session.branch` handler, replaced the
  per-message `append_message` loop (which forwarded only `role` and
  `content`) with a single `db.replace_messages(new_key, history)` call
  so the branched session persists `tool_calls`, `tool_call_id`,
  `tool_name` and reasoning fields. Added a comment explaining why.
- `tests/test_tui_gateway_server.py`: added
  `test_session_branch_persists_tool_calls_for_resume`, which branches a
  session containing a tool-call round-trip and asserts the persisted
  transcript still round-trips the tool linkage through
  `get_messages_as_conversation`.

## How to Test

1. Run the new regression test:
   `scripts/run_tests.sh tests/test_tui_gateway_server.py -- -k test_session_branch_persists_tool_calls_for_resume`
2. It passes with the fix. To see the bug, revert the `server.py` change
   and rerun — the test fails because the persisted assistant message has
   no `tool_calls` and the tool result has no `tool_call_id`/`tool_name`.
3. Manual: in the TUI, run a prompt that triggers a tool call, branch the
   session, restart, then `/resume` the branch and send a follow-up — it
   no longer errors out on a malformed message sequence.

## Checklist

### Code

- [x] I've read the Contributing Guide
- [x] My commit messages follow Conventional Commits (`fix(scope):`, `feat(scope):`, etc.)
- [x] I searched for existing PRs to make sure this isn't a duplicate
- [x] My PR contains **only** changes related to this fix/feature (no unrelated commits)
- [x] I've run `pytest tests/ -q` and all tests pass
- [x] I've added tests for my changes (required for bug fixes, strongly encouraged for features)
- [x] I've tested on my platform: macOS 15 (Darwin 25.5.0)

### Documentation & Housekeeping

- [x] I've updated relevant documentation (README, `docs/`, docstrings) — or N/A
- [x] I've updated `cli-config.yaml.example` if I added/changed config keys — or N/A
- [x] I've updated `CONTRIBUTING.md` or `AGENTS.md` if I changed architecture or workflows — or N/A
- [x] I've considered cross-platform impact (Windows, macOS) — or N/A
- [x] I've updated tool descriptions/schemas if I changed tool behavior — or N/A
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/tui Terminal UI (ui-tui/ + tui_gateway/) labels Jun 8, 2026
@liuhao1024

Copy link
Copy Markdown
Contributor

✅ Verified — session branch preserves tool call linkage

Reviewed the diff for tui_gateway/server.py session.branch handler.

  • Root cause confirmed: the old append_message loop forwarded only role and content, dropping tool_calls, tool_call_id, and tool_name fields
  • Fix: replaced with db.replace_messages(new_key, history) which atomically serializes every message field — the same path /undo, /retry, and /compress already use
  • Safety: replace_messages does an internal DELETE + INSERT; since the session is freshly created above, the DELETE is a no-op
  • Test: test_session_branch_persists_tool_calls_for_resume verifies the full tool-call round-trip survives DB persistence and reload

The fix is correct and the regression test covers the exact failure scenario (branch → close → resume → provider rejects missing tool result). No issues found.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for identifying a real transcript-durability issue. Current main still has the legacy session.branch role/content-only persistence loop at tui_gateway/server.py:8089-8094, so the proposed replace_messages approach fits that path.

Problems

  • Current main also has _persist_branch_seed at tui_gateway/server.py:1769-1795, introduced in 4e023f5bc990ac430d38a220c74162bbe92293f9. Its loop at tui_gateway/server.py:1791-1792 repeats the same role/content-only write for deferred branch sessions.
  • The added test exercises session.branch, but not the deferred seed path invoked from prompt.submit at tui_gateway/server.py:8487-8491.

Suggested changes

  • Salvage the full-message persistence change onto both current branch-copy paths and add coverage for _persist_branch_seed with a tool-call round trip.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users area/sessions Session lifecycle, resume, persistence, history labels Jul 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/sessions Session lifecycle, resume, persistence, history comp/tui Terminal UI (ui-tui/ + tui_gateway/) P2 Medium — degraded but workaround exists sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants