diff --git a/src/pinky_memory/server.py b/src/pinky_memory/server.py index e6632edb..83caaf5f 100644 --- a/src/pinky_memory/server.py +++ b/src/pinky_memory/server.py @@ -20,6 +20,7 @@ ReflectInput, Reflection, ReflectionType, + coerce_reflection_type, ) if TYPE_CHECKING: @@ -32,6 +33,16 @@ def _log(msg: str) -> None: print(msg, file=sys.stderr, flush=True) +def _parse_reflection_type(type_str: str) -> ReflectionType: + """Coerce a caller-supplied type to ReflectionType, logging when it falls + back to `fact` (callers, including dream runs, occasionally pass a + plausible-sounding but invalid type — see #session_log bug).""" + coerced = coerce_reflection_type(type_str) + if coerced.value != type_str: + _log(f"reflect: invalid type={type_str!r}, defaulting to 'fact'") + return coerced + + # Strict agent-name slug for cross-agent memory targets (#614/#145). Mirrors # the trust-boundary slug discipline tracked in #105 — defends the # store-factory path resolution against traversal / injection even though the @@ -369,7 +380,7 @@ def reflect( """ input_data = ReflectInput( content=content, - type=ReflectionType(type), + type=_parse_reflection_type(type), context=context, project=project, salience=salience, @@ -430,7 +441,7 @@ def reflect_for( s = _resolve_target_store(target_agent) input_data = ReflectInput( content=content, - type=ReflectionType(type), + type=_parse_reflection_type(type), context=context, project=project, salience=salience, diff --git a/src/pinky_memory/store.py b/src/pinky_memory/store.py index 7f329aec..5ed5a8ab 100644 --- a/src/pinky_memory/store.py +++ b/src/pinky_memory/store.py @@ -23,6 +23,7 @@ Reflection, ReflectionLink, ReflectionType, + coerce_reflection_type, resolve_preset, ) @@ -1527,9 +1528,17 @@ def _row_to_reflection(row: sqlite3.Row) -> Reflection: source_message_ids = json.loads(raw_msg_ids) if raw_msg_ids else [] next_review_date = row["next_review_date"] if "next_review_date" in keys else None review_interval_days = row["review_interval_days"] if "review_interval_days" in keys else 7 + row_type = row["type"] + coerced_type = coerce_reflection_type(row_type) + if coerced_type.value != row_type: + logger.warning( + "row_to_reflection: invalid stored type=%r for id=%r, defaulting to 'fact'", + row_type, + row["id"], + ) return Reflection( id=row["id"], - type=ReflectionType(row["type"]), + type=coerced_type, content=row["content"], context=row["context"], project=row["project"], diff --git a/src/pinky_memory/types.py b/src/pinky_memory/types.py index 07371d4b..9eccb9f6 100644 --- a/src/pinky_memory/types.py +++ b/src/pinky_memory/types.py @@ -15,6 +15,18 @@ class ReflectionType(str, Enum): fact = "fact" +def coerce_reflection_type(type_str: str) -> ReflectionType: + """Coerce a caller- or DB-supplied type to ReflectionType, defaulting to + `fact` on an unrecognized value instead of raising. Used both when writing + a new reflection (a caller passed a plausible-sounding but invalid type) + and when hydrating a row written before this type was retired/renamed — + a stale value must not crash every future read of that row.""" + try: + return ReflectionType(type_str) + except ValueError: + return ReflectionType.fact + + class Reflection(BaseModel): model_config = ConfigDict(extra="ignore") diff --git a/tests/test_memory_server.py b/tests/test_memory_server.py index 2f72f700..93c253b7 100644 --- a/tests/test_memory_server.py +++ b/tests/test_memory_server.py @@ -189,6 +189,32 @@ def test_reflect_with_all_fields(self, srv): assert result["salience"] == 5 assert result["type"] == "project_state" + def test_reflect_invalid_type_falls_back_to_fact(self, srv): + """Regression: an unrecognized type (e.g. dream-hallucinated 'session_log') + must not raise — it previously crashed the whole dream run.""" + result = json.loads(_tools(srv)["reflect"]( + content="something the dream agent decided to call session_log", + type="session_log", + )) + assert result["stored"] is True + assert result["type"] == "fact" + + def test_recall_survives_legacy_invalid_stored_type(self, srv, store): + """Regression: a row written before a type was retired/renamed (e.g. + 'session_log', 'episode', 'nav_audit' from pre-fallback dream runs) + must not crash hydration on read — it previously aborted every + subsequent recall/dream-linking pass that touched the row.""" + import sqlite3 + + stored = json.loads(_tools(srv)["reflect"](content="pre-fallback dream note", type="fact")) + conn = sqlite3.connect(store._db_path) + conn.execute("UPDATE reflections SET type = 'session_log' WHERE id = ?", (stored["id"],)) + conn.commit() + conn.close() + + result = _parse_recall(_tools(srv)["recall"](query="pre-fallback dream note")) + assert any(r["id"] == stored["id"] and r["type"] == "fact" for r in result["reflections"]) + def test_reflect_supersedes(self, srv, store): # First memory r1 = json.loads(_tools(srv)["reflect"](content="old fact", type="fact"))