diff --git a/api/routes.py b/api/routes.py index 36f58c21..c4d90406 100644 --- a/api/routes.py +++ b/api/routes.py @@ -1580,9 +1580,12 @@ def _get_or_materialize_session(sid: str): s = get_session(sid) s = _ensure_full_session_before_mutation(sid, s) # Read-only guard on the happy path too: an already-stored read-only / - # imported session (messaging or Claude Code) must not be mutated via - # rename/update/move. Session.save() does not enforce this itself. - if getattr(s, "read_only", False) or _is_messaging_session_record(s): + # imported session must not be mutated via rename/update/move + # (Session.save() does not enforce this). Scope this to the explicit + # read_only flag — a stored messaging session already owns its sidecar, + # so the messaging-fork concern only applies to the materialize fallback + # below (and the heuristic record-check would mis-trip on mock sessions). + if getattr(s, "read_only", False): raise PermissionError("read-only imported session") return s except KeyError: diff --git a/tests/test_issue1436_context_indicator_load_path.py b/tests/test_issue1436_context_indicator_load_path.py index 608f616e..a3287b71 100644 --- a/tests/test_issue1436_context_indicator_load_path.py +++ b/tests/test_issue1436_context_indicator_load_path.py @@ -52,6 +52,13 @@ class TestIssue1436BackendFallback: s = MagicMock() s.context_length = context_length s.threshold_tokens = 0 + # Real persisted sessions default read_only to False; set it explicitly so + # the rename/update/move read-only guard (#3994) doesn't see a truthy + # auto-attribute on the bare MagicMock. Same for _loaded_metadata_only, + # which _ensure_full_session_before_mutation() checks — a truthy mock + # would trigger a spurious Session.load() reload onto a different object. + s.read_only = False + s._loaded_metadata_only = False s.last_prompt_tokens = last_prompt_tokens s.input_tokens = input_tokens s.output_tokens = 0 diff --git a/tests/test_issue3994_materialize_session.py b/tests/test_issue3994_materialize_session.py index 678544de..6070186e 100644 --- a/tests/test_issue3994_materialize_session.py +++ b/tests/test_issue3994_materialize_session.py @@ -83,17 +83,19 @@ def test_materialize_rejects_stored_readonly_session(): routes._get_or_materialize_session("ro_stored") -def test_materialize_rejects_stored_messaging_session(): - """A stored messaging session (state.db-owned) must be refused even with no - read_only flag (Codex CORE #1).""" +def test_materialize_allows_stored_messaging_session_without_readonly(): + """A stored messaging session that ALREADY owns its sidecar is mutable on the + happy path (the messaging-fork concern only applies to the materialize + fallback that would CREATE a sidecar). Only an explicit read_only flag blocks + a stored session.""" import api.routes as routes msg = SimpleNamespace(session_id="msg_stored", profile="default", messages=[], - session_source="messaging", source_tag="telegram") + session_source="messaging", source_tag="telegram", read_only=False) with patch("api.routes.get_session", return_value=msg), \ patch("api.routes._ensure_full_session_before_mutation", return_value=msg): - with pytest.raises(PermissionError): - routes._get_or_materialize_session("msg_stored") + out = routes._get_or_materialize_session("msg_stored") + assert out is msg def test_materialize_rejects_messaging_cli_meta_without_readonly_flag():