From de68b4c04e8f2d1b2dc1a4a4f941eaa511a792f9 Mon Sep 17 00:00:00 2001 From: Fixer Date: Sat, 1 Aug 2026 03:26:07 +0200 Subject: [PATCH 1/2] pinky_memory: reflect() falls back to 'fact' on invalid type instead of crashing Dream runs occasionally call reflect() with a plausible-sounding but invalid type (observed: 'session_log'), which raised an unhandled ValueError from ReflectionType() and aborted the entire dream run rather than just failing that one memory write. Co-Authored-By: Claude Sonnet 5 --- src/pinky_memory/server.py | 16 ++++++++++++++-- tests/test_memory_server.py | 10 ++++++++++ 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/src/pinky_memory/server.py b/src/pinky_memory/server.py index e6632edb..4469cdde 100644 --- a/src/pinky_memory/server.py +++ b/src/pinky_memory/server.py @@ -32,6 +32,18 @@ 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, defaulting to `fact` + on an unrecognized value instead of raising. Callers (including dream + runs) occasionally pass a plausible-sounding but invalid type; failing + hard here previously aborted the entire dream run (#session_log bug).""" + try: + return ReflectionType(type_str) + except ValueError: + _log(f"reflect: invalid type={type_str!r}, defaulting to 'fact'") + return ReflectionType.fact + + # 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 +381,7 @@ def reflect( """ input_data = ReflectInput( content=content, - type=ReflectionType(type), + type=_parse_reflection_type(type), context=context, project=project, salience=salience, @@ -430,7 +442,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/tests/test_memory_server.py b/tests/test_memory_server.py index 2f72f700..3c90ac25 100644 --- a/tests/test_memory_server.py +++ b/tests/test_memory_server.py @@ -189,6 +189,16 @@ 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_reflect_supersedes(self, srv, store): # First memory r1 = json.loads(_tools(srv)["reflect"](content="old fact", type="fact")) From a4207fef9dc674c0cb7296dc0fdc7ff596855548 Mon Sep 17 00:00:00 2001 From: Fixer Date: Sun, 2 Aug 2026 03:28:53 +0200 Subject: [PATCH 2/2] fix(pinky_memory): coerce legacy invalid reflection type on row read MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit reflect()'s existing fallback (invalid type -> 'fact') only guarded the write path. Any row already stored with a since-retired/renamed type (e.g. 'session_log', 'episode', 'nav_audit' from before that fix landed) still crashed ReflectionType(row["type"]) on every hydration, aborting recall/introspect and dream post-processing (prune/link) whenever that row was touched — confirmed live against seo-pro's and satoshi's own memory.db. Moves the fallback into a shared coerce_reflection_type() in types.py, used by both the write path (server.py) and row hydration (store.py). --- src/pinky_memory/server.py | 15 +++++++-------- src/pinky_memory/store.py | 11 ++++++++++- src/pinky_memory/types.py | 12 ++++++++++++ tests/test_memory_server.py | 16 ++++++++++++++++ 4 files changed, 45 insertions(+), 9 deletions(-) diff --git a/src/pinky_memory/server.py b/src/pinky_memory/server.py index 4469cdde..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: @@ -33,15 +34,13 @@ def _log(msg: str) -> None: def _parse_reflection_type(type_str: str) -> ReflectionType: - """Coerce a caller-supplied type to ReflectionType, defaulting to `fact` - on an unrecognized value instead of raising. Callers (including dream - runs) occasionally pass a plausible-sounding but invalid type; failing - hard here previously aborted the entire dream run (#session_log bug).""" - try: - return ReflectionType(type_str) - except ValueError: + """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 ReflectionType.fact + return coerced # Strict agent-name slug for cross-agent memory targets (#614/#145). Mirrors 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 3c90ac25..93c253b7 100644 --- a/tests/test_memory_server.py +++ b/tests/test_memory_server.py @@ -199,6 +199,22 @@ def test_reflect_invalid_type_falls_back_to_fact(self, srv): 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"))