Skip to content

Fix session duplicate and branch field propagation - #3101

Closed
AlexeyDsov wants to merge 1 commit into
nesquena:masterfrom
AlexeyDsov:alexeydsov-2026-05-28-fixes-copying-sessions
Closed

AlexeyDsov wants to merge 1 commit into
nesquena:masterfrom
AlexeyDsov:alexeydsov-2026-05-28-fixes-copying-sessions

Conversation

@AlexeyDsov

Copy link
Copy Markdown
Contributor

Short description

Session duplicate and branch operations were missing several critical fields, causing data loss scenarios:

  • truncation_watermark not copied on duplicate → after editing a message in a duplicated session, merge with state.db would drop messages
  • context_messages not copied → agent context in the duplicate would diverge from what the user sees
  • gateway_routing not copied → routing customizations lost on duplicate/branch
  • context_engine_state not copied → context engine state lost on duplicate/branch
  • cache tokensenabled_toolsetsllm_title_generatedcomposer_draft — all silently dropped on duplicate

Duplicate now copies all session configuration fields (with proper deep-copy for mutable types). Branch inherits model configuration, routing, and context engine state from the source session.

Added tests covering both duplicate and branch field propagation to prevent regression.

AI Assistance

This fix was developed with assistance from Qwen3.6-27B. The AI helped analyze the reconciliation logic, identify root cause, and draft the test scenarios.

@nesquena-hermes

Copy link
Copy Markdown
Collaborator

Reading the diff at api/routes.py:5130-5160 (duplicate) and 5658-5675 (branch) against origin/master. Cross-checked against api/models.py:504-595 (Session.__init__).

The bug is real and the fix surface is correct

Session.__init__ at api/models.py accepts all of these fields:

context_messages, context_engine, context_engine_state,
gateway_routing, gateway_routing_history, llm_title_generated,
composer_draft, cache_read_tokens, cache_write_tokens,
truncation_watermark

The pre-PR duplicate handler at api/routes.py:5209 only passed a subset:

copied_session = Session(
    session_id=uuid.uuid4().hex[:12],
    title=(session.title or "Untitled") + " (copy)",
    workspace=session.workspace,
    model=session.model,
    model_provider=session.model_provider,
    messages=copy.deepcopy(session.messages),
    tool_calls=copy.deepcopy(session.tool_calls),
    ...
    personality=session.personality,
    enabled_toolsets=getattr(session, "enabled_toolsets", None),
    context_length=getattr(session, "context_length", None),
    threshold_tokens=getattr(session, "threshold_tokens", None),
    created_at=time.time(),
    updated_at=time.time(),
)

Everything else fell back to the Session.__init__ defaults: context_messages → [], gateway_routing → None, context_engine_state → {}, truncation_watermark → None, composer_draft → {}. For a truncation_watermark-bearing session, that's exactly the data-loss path #2914 described: after edit/undo on the duplicate the agent re-reads from state.db and silently drops messages because the duplicate didn't carry the watermark forward.

The fix shape is right

The deepcopy choices are correct:

  • messages, tool_calls, context_messages — mutable lists of mutable dicts, must deepcopy or both sessions share underlying refs. The PR does this.
  • gateway_routing, gateway_routing_history, context_engine_state, composer_draft — dicts/lists potentially mutated by routing logic, also deepcopied. Good.
  • context_engine is passed by reference (it's a config selector string per the field shape in models.py:567 area), not deepcopied. That matches the field semantics.
  • cache_read_tokens / cache_write_tokens carried as scalars — fine.
  • llm_title_generated preserved so the duplicate doesn't trigger a re-title round, which would race the user's manual title edit.

Branch path at api/routes.py:5658 follows the same pattern with the additional parent_session_id=source.session_id + session_source="fork" — that's the existing branch contract, untouched.

Test coverage

Both tests/test_session_duplicate_edit.py (208 LOC) and tests/test_session_duplicate_fields.py (467 LOC) directly assert each preserved field. test_duplicate_preserves_truncation_watermark is the regression pin for #2914 specifically.

One minor concern: the tests construct a Session() manually and call Session() again to simulate the duplicate, rather than driving the full HTTP handler. That's pragmatic given the handler's surface area, but if someone adds a new field to Session.__init__ in the future and forgets to extend the duplicate path, the test won't catch it because the test itself enumerates the fields explicitly. A complementary "test that every public field on Session is preserved on duplicate" introspection test would be more durable — but that's gold-plating, not blocking.

Verdict

Fix is correct, surgical, well-tested, addresses a real data-correctness bug. AI assistance disclosure is appreciated.

LGTM. One nit: composer_draft on duplicate is debatable UX-wise — copying the in-progress composer text from the original into the duplicate could surprise users who duplicated specifically to start a fresh draft. But the field default is {} and most callers will see no observable change, so this is closer to "preserve all state" correctness than UX regression.

@nesquena-hermes

Copy link
Copy Markdown
Collaborator

Shipped in v0.51.163 (Release EI, stage-batch45) via release PR #3149 — thank you, @AlexeyDsov! 🎉

Your duplicate/branch field-propagation fix is now on master, authorship preserved. Opus advisor reviewed (LOW-RISK-SHIP): confirmed the exclusion set is correct, all mutable fields are deepcopied, and truncation_watermark is value-passthrough (distinct from the clamp issue in your other PR #3102).

Cherry-picked onto current master (stale base), so GitHub didn't auto-close — closing manually. Full suite: 6871 passed, 0 failures. Closing as shipped.

AJV20 pushed a commit to AJV20/hermes-webui that referenced this pull request May 30, 2026
SysAdminDoc pushed a commit to SysAdminDoc/hermes-webui that referenced this pull request Jun 26, 2026
bernyforce pushed a commit to bernyforce/hermes-webui that referenced this pull request Jul 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants