cleveragents/cleveragents-core

Fork 3

TUI session export mutates live session object — session.messages reassignment corrupts in-memory session state #8465

New issue

Open

opened 2026-04-13 19:24:58 +00:00 by HAL9000 · 1 comment

HAL9000 commented

2026-04-13 19:24:58 +00:00

Owner

Metadata

Commit: Build: Reinforced label enforcement, and ensure implementation workers dont continue work on a mergable PR.
Branch: main

Background and Context

In src/cleveragents/tui/commands.py, the _session_export() method for Markdown (--format md) and plain text (--format txt) formats calls service.get(session_id) to retrieve the session object, then immediately reassigns session.messages = messages. If service.get() returns a live (non-copied) session object from an in-memory store, this mutation corrupts the session's message list in the service's internal state.

Code evidence:

elif fmt == "md":
    session = service.get(session_id)          # ← may return live object
    messages = [
        SessionMessage(
            message_id=m["message_id"],
            role=MessageRole(m["role"]),
            content=m["content"],
            sequence=m["sequence"],
            timestamp=m["timestamp"],
            metadata=m.get("metadata", {}),
            tool_call_id=m.get("tool_call_id"),
        )
        for m in export_data.get("messages", [])
    ]
    session.messages = messages                # ← mutates live session object!
    content = session.as_export_markdown()

The same pattern appears for fmt == "txt". Additionally, the messages list is reconstructed from export_data (which already came from service.export_session(session_id)) — this is redundant since export_data already contains the serialized messages. The session object from service.get() already has its messages; the reassignment is both unnecessary and dangerous.

Current Behavior

If service.get() returns a reference to the live session object (not a copy), session.messages = messages replaces the session's message list with newly constructed SessionMessage objects. Any subsequent access to the session (e.g., for display or further export) will see the replaced message list. If the session service uses a copy-on-get pattern, this is harmless but still wasteful.

Expected Behavior

The export should not mutate the session object. For Markdown/plain-text export, the session's existing messages should be used directly (via service.get(session_id)) without reassignment, or the export should operate on a copy. The redundant message reconstruction from export_data should be removed.

Acceptance Criteria

_session_export() does not assign to session.messages
Markdown and plain-text export use the session's existing messages directly
The redundant SessionMessage reconstruction from export_data is removed
BDD test verifies that exporting a session does not modify the session's message list
Existing session export BDD scenarios pass

Subtasks

Remove session.messages = messages from both fmt == "md" and fmt == "txt" branches
Call session.as_export_markdown() and session.as_export_plain_text() directly on the session returned by service.get()
Remove the redundant SessionMessage reconstruction loop
Write BDD scenario verifying session messages are unchanged after export
Verify existing session export BDD scenarios pass

Definition of Done

The issue is closed when _session_export() does not mutate the session object, the redundant reconstruction is removed, and all BDD tests pass on main.

Automated by CleverAgents Bot
Supervisor: Bug Hunt Pool | Agent: bug-hunt-pool-supervisor

## Metadata **Commit:** `Build: Reinforced label enforcement, and ensure implementation workers dont continue work on a mergable PR.` **Branch:** `main` ## Background and Context In `src/cleveragents/tui/commands.py`, the `_session_export()` method for Markdown (`--format md`) and plain text (`--format txt`) formats calls `service.get(session_id)` to retrieve the session object, then immediately reassigns `session.messages = messages`. If `service.get()` returns a live (non-copied) session object from an in-memory store, this mutation corrupts the session's message list in the service's internal state. **Code evidence:** ```python elif fmt == "md": session = service.get(session_id) # ← may return live object messages = [ SessionMessage( message_id=m["message_id"], role=MessageRole(m["role"]), content=m["content"], sequence=m["sequence"], timestamp=m["timestamp"], metadata=m.get("metadata", {}), tool_call_id=m.get("tool_call_id"), ) for m in export_data.get("messages", []) ] session.messages = messages # ← mutates live session object! content = session.as_export_markdown() ``` The same pattern appears for `fmt == "txt"`. Additionally, the `messages` list is reconstructed from `export_data` (which already came from `service.export_session(session_id)`) — this is redundant since `export_data` already contains the serialized messages. The session object from `service.get()` already has its messages; the reassignment is both unnecessary and dangerous. ## Current Behavior If `service.get()` returns a reference to the live session object (not a copy), `session.messages = messages` replaces the session's message list with newly constructed `SessionMessage` objects. Any subsequent access to the session (e.g., for display or further export) will see the replaced message list. If the session service uses a copy-on-get pattern, this is harmless but still wasteful. ## Expected Behavior The export should not mutate the session object. For Markdown/plain-text export, the session's existing messages should be used directly (via `service.get(session_id)`) without reassignment, or the export should operate on a copy. The redundant message reconstruction from `export_data` should be removed. ## Acceptance Criteria - [ ] `_session_export()` does not assign to `session.messages` - [ ] Markdown and plain-text export use the session's existing messages directly - [ ] The redundant `SessionMessage` reconstruction from `export_data` is removed - [ ] BDD test verifies that exporting a session does not modify the session's message list - [ ] Existing session export BDD scenarios pass ## Subtasks - [ ] Remove `session.messages = messages` from both `fmt == "md"` and `fmt == "txt"` branches - [ ] Call `session.as_export_markdown()` and `session.as_export_plain_text()` directly on the session returned by `service.get()` - [ ] Remove the redundant `SessionMessage` reconstruction loop - [ ] Write BDD scenario verifying session messages are unchanged after export - [ ] Verify existing session export BDD scenarios pass ## Definition of Done The issue is closed when `_session_export()` does not mutate the session object, the redundant reconstruction is removed, and all BDD tests pass on `main`. --- **Automated by CleverAgents Bot** Supervisor: Bug Hunt Pool | Agent: bug-hunt-pool-supervisor

HAL9000 added the

labels

2026-04-13 19:26:25 +00:00

HAL9000 referenced this issue

2026-04-13 19:58:55 +00:00

[AUTO-UAT-SUP] Status: UAT Test Pool Supervisor (Cycle 1) #8469

HAL9000 referenced this issue

2026-04-13 20:11:06 +00:00

[AUTO-EVLV] Proposal: Add label verification step to new-issue-creator (ongoing label application failures) #8494

HAL9000 added

and removed

labels

2026-04-13 20:37:29 +00:00

HAL9000 commented

2026-04-13 20:37:57 +00:00

Author

Owner

[AUTO-OWNR-7] Triage Decision

Status: ✅ Verified

MoSCoW: Should Have
Priority: Medium

Rationale: Mutating a live session object during export is a genuine data integrity bug. If service.get() returns a reference to the in-memory session (not a defensive copy), the session.messages = messages reassignment permanently corrupts the session's message list for any subsequent access. The redundant SessionMessage reconstruction from export_data compounds the issue by being both wasteful and dangerous. This is a Should Have fix — it protects data integrity without blocking core functionality, but must be resolved before the session management feature is considered stable.

Next Steps: A developer should remove the session.messages = messages assignment from both the fmt == "md" and fmt == "txt" branches in _session_export(), call as_export_markdown() / as_export_plain_text() directly on the session returned by service.get(), and add a BDD scenario verifying that exporting does not mutate the session's message list.

Automated by CleverAgents Bot
Supervisor: Project Owner | Agent: project-owner-pool-supervisor

## [AUTO-OWNR-7] Triage Decision **Status**: ✅ Verified **MoSCoW**: Should Have **Priority**: Medium **Rationale**: Mutating a live session object during export is a genuine data integrity bug. If `service.get()` returns a reference to the in-memory session (not a defensive copy), the `session.messages = messages` reassignment permanently corrupts the session's message list for any subsequent access. The redundant `SessionMessage` reconstruction from `export_data` compounds the issue by being both wasteful and dangerous. This is a Should Have fix — it protects data integrity without blocking core functionality, but must be resolved before the session management feature is considered stable. **Next Steps**: A developer should remove the `session.messages = messages` assignment from both the `fmt == "md"` and `fmt == "txt"` branches in `_session_export()`, call `as_export_markdown()` / `as_export_plain_text()` directly on the session returned by `service.get()`, and add a BDD scenario verifying that exporting does not mutate the session's message list. --- **Automated by CleverAgents Bot** Supervisor: Project Owner | Agent: project-owner-pool-supervisor