test(#5572): cover messaging clear watermark semantics

[rodboev]

Thinking Path

• The clear-session fix already covers WebUI-owned state, but imported messaging sessions have a second ownership boundary: the external channel transcript.
• The right follow-up is to prove that clearing the WebUI sidecar does not erase or misrepresent that external transcript.
• The implementation starts as coverage, with production changes only if the tests expose a real mismatch.

What Changed

• tests/test_issue5572_messaging_clear_semantics.py: adds focused coverage for imported messaging clear semantics, the 0.0 clear watermark, state.db replay suppression, and external transcript preservation.

Why It Matters

Imported messaging sessions should keep their channel-owned transcript intact even when the WebUI-visible sidecar is cleared. The tests make that boundary explicit so future clear-session changes cannot blur ownership again.

Verification

python -m pytest tests/test_issue5572_messaging_clear_semantics.py -v --timeout=60
python scripts/ruff_lint.py --diff origin/master

Full-suite CI context, not a required local check unless requested: python -m pytest tests/ -v --timeout=60.

Upstream

Closes #5572.

Model Used

GPT 5.5 via Codex CLI

[greptile-apps]

Greptile Summary

This PR adds a regression test for #5572 that covers the clear-session semantics for imported messaging sessions (Telegram, Discord). The test verifies that clearing a messaging sidecar preserves channel identity fields (session_source, source_tag, raw_source, source_label), leaves the external state.db transcript untouched, and confirms the display route still surfaces the external messages while the 0.0 watermark correctly blocks append-only replay in merge_session_messages_append_only.

• A single parameterized test covers both Telegram and Discord messaging sources through the full clear route → disk reload → HTTP display → merge-function cycle.
• The test creates a minimal state.db with external messages and verifies (via get_cli_session_messages) that those rows survive the clear operation, then exercises handle_get against the patched isolated session environment to confirm the display payload matches the external transcript.

Confidence Score: 5/5

Test-only change; no production code paths are modified, so the risk of regressions is minimal.

The change is confined to a single new test file. The test correctly exercises the real route handlers in an isolated monkeypatched environment and performs genuine state.db reads. No production logic is touched, so existing functionality cannot regress from this PR. The one flagged item (string-format assertions instead of json.loads) is a consistency nit with no impact on pass/fail in the current environment.

No files require special attention; the only concern is the substring-based JSON assertions in the new test file, which are consistent with the current serialization format.

Important Files Changed

Filename	Overview
tests/test_issue5572_messaging_clear_semantics.py	New regression test for messaging session clear semantics; uses fragile string-in-JSON substring assertions (lines 199-207) instead of json.loads() like the parallel test_issue5532 does, and duplicates helper utilities already flagged in a previous review thread.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant T as Test
    participant PC as _post_clear
    participant R as routes.handle_post /api/session/clear
    participant SO as session_ops.truncate_session_at_keep
    participant S as Session (disk)
    participant DB as state.db (test)
    participant GH as routes.handle_get /api/session

    T->>PC: _post_clear(monkeypatch, sid)
    PC->>R: handle_post(handler, /api/session/clear)
    R->>SO: truncate_session_at_keep(s, 0)
    SO->>S: "messages=[], watermark=0.0, boundary=0.0"
    S-->>R: session saved
    R-->>PC: "captured {ok, session metadata}"
    PC-->>T: captured

    T->>DB: _make_state_db(tmp_path/state.db, sid, external_messages)
    T->>T: get_cli_session_messages(sid) reads state.db
    Note over T,DB: Asserts external rows survived clear

    T->>GH: "handle_get(handler, /api/session?session_id=sid)"
    GH->>GH: "_is_messaging_session_record(s) = True"
    GH->>DB: "get_cli_session_messages(sid) = cli_messages"
    GH->>GH: _merged_session_messages_for_display(s, cli_messages)
    Note over GH: sidecar=[], len([])>len(cli) is False, returns cli_messages
    GH-->>T: "payload session messages == external_messages"

    T->>T: "merge_session_messages_append_only([], state_db_msgs, watermark=0.0)"
    Note over T: watermark==0 sentinel returns []
    T->>T: "assert merged == []"

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant T as Test
    participant PC as _post_clear
    participant R as routes.handle_post /api/session/clear
    participant SO as session_ops.truncate_session_at_keep
    participant S as Session (disk)
    participant DB as state.db (test)
    participant GH as routes.handle_get /api/session

    T->>PC: _post_clear(monkeypatch, sid)
    PC->>R: handle_post(handler, /api/session/clear)
    R->>SO: truncate_session_at_keep(s, 0)
    SO->>S: "messages=[], watermark=0.0, boundary=0.0"
    S-->>R: session saved
    R-->>PC: "captured {ok, session metadata}"
    PC-->>T: captured

    T->>DB: _make_state_db(tmp_path/state.db, sid, external_messages)
    T->>T: get_cli_session_messages(sid) reads state.db
    Note over T,DB: Asserts external rows survived clear

    T->>GH: "handle_get(handler, /api/session?session_id=sid)"
    GH->>GH: "_is_messaging_session_record(s) = True"
    GH->>DB: "get_cli_session_messages(sid) = cli_messages"
    GH->>GH: _merged_session_messages_for_display(s, cli_messages)
    Note over GH: sidecar=[], len([])>len(cli) is False, returns cli_messages
    GH-->>T: "payload session messages == external_messages"

    T->>T: "merge_session_messages_append_only([], state_db_msgs, watermark=0.0)"
    Note over T: watermark==0 sentinel returns []
    T->>T: "assert merged == []"

Loading

_{Reviews (3): Last reviewed commit: "test(#5572): prove imported transcript s..." | Re-trigger Greptile}

[nesquena-hermes]

🔬 Gate certification — GREEN ✅ (test-only: meaningful messaging-clear regression coverage)

Certified head: sha:9a213c92 (clean rebase, branch gate-rebase/5589-messaging-clear-test) · PR: #5589 · rodboev, test(#5572): cover messaging clear watermark semantics
Verdict: Pure test-only regression coverage — 2 meaningful tests that exercise the real /api/session/clear flow on an imported messaging session and assert it preserves messaging metadata while zeroing the transcript (+ blocks state.db replay). Codex SAFE, tests pass, no collision with existing clear tests.

What I ran (rebased worktree /tmp/wt-rebase-5589)

Gate	Result
Rebase onto current master	✅ git apply clean; diff = one new tests/ file only (test-only, no shipped code)
Codex (meaningfulness/accuracy)	SAFE TO SHIP — 0 findings (real clear path + assertions, not vacuous/over-mocked, accurate regression lock)
The new file	✅ 2/2 pass (real Session load/clear, concrete assertions)
Related messaging/clear/session subset (#5572/#5532/#5570/messaging)	✅ 63 passed (no collision/duplication)

Findings

✅ Meaningful + accurate: the tests exercise /api/session/clear on an imported messaging session and assert real loaded state — preserves source_tag/raw_source/session_source=="messaging"/source_label/is_cli_session while zeroing messages==[]/context_messages==[]/tool_calls==[]/truncation_watermark==0.0/truncation_boundary==0.0, and locks that state.db replay is blocked. Concrete assertions on a real re-loaded Session (not a mock), sqlite/tmp-isolated, no flakiness. Codex confirmed the assertions match current clear semantics (accurate regression lock, not encoding a wrong expectation) and it doesn't duplicate/conflict with the existing #5532/#5570 tests. This locks in exactly the messaging-clear behavior that the #5504/#5556 findings care about — good defensive coverage.

Recommendation to the next agent

Ready to merge — use branch gate-rebase/5589-messaging-clear-test (sha:9a213c92), NOT the PR's stale head cc7896cf. Pure test-only regression coverage for messaging-clear watermark semantics (#5572), meaningful (real clear path + concrete assertions, not vacuous), Codex SAFE, 2/2 + 63 related tests pass, no shipped-code risk. This is the mergeable-autonomously test-only class. concept 4/5 (locks a behavior the clear-family bugs touched — valuable regression net). Author @rodboev (T1). crit=2.

---

Gate-certifier layer (warm-up → gate → release). I do not merge/tag/deploy. Rebased onto current master; verified test-only (one new tests/ file, no shipped code) + meaningful (real /api/session/clear on imported messaging session, concrete assertions on re-loaded Session state, not vacuous/over-mocked), Codex SAFE + 2/2 + 63 related messaging/clear tests pass (no collision). Cert valid for sha:9a213c92.

[nesquena-hermes]

Shipped in v0.51.886. Thanks @rodboev for the messaging clear-watermark regression coverage. 🙏