Joseph Doherty
3a81e540a1
Same defect class as 0f8bf94: routes that ``append_event`` then
``project(conn)`` 500 once any prior event makes the full-log replay
hit a raw-INSERT handler (chat_created, container_created,
scene_opened, memory_written, meanwhile_scene_started, etc.).
Fixes the two remaining live-path callers:
- chat/web/bots.py (bot_create) — bot_authored
- chat/web/settings.py (settings_post) — you_authored
Both swap ``append_event`` + ``project`` → ``append_and_apply`` so only
the new event is applied through its registered handler. Unused
imports of ``append_event`` and ``project`` removed from each file.
The rewind path (chat/services/rewind.py) intentionally calls
``project()`` after wiping every projected table — that's the
canonical "rebuild from log against an empty DB" entry point and is
left unchanged.
Inventory of every projector handler that uses raw INSERT
(chat_created, container_created, scene_opened, memory_written,
meanwhile_scene_started, meanwhile_digest_created, edge_update) is
documented with the trade-offs of why we don't blindly switch them to
INSERT OR REPLACE — for autoincrement-id rows there is no key to match
on, and for chat_created a lossy overwrite would silently clobber
chat_state mutations from later events. The handler layer stays
correctly non-idempotent under event-sourcing semantics; the rule is
enforced at the call site.
Adds a regression test (tests/test_chat_created_non_idempotent.py)
that pins the contract: appending two chat_created events for the same
id and then ``project()``ing a second time MUST raise
``IntegrityError`` on chats.id. Any future "make it idempotent" change
must update the test, forcing a deliberate review.
Suite: 471 passed in 11.82s (was 470 + this regression test).
Report: docs/audits/2026-04-27-project-callers.md
70 lines
2.9 KiB
Python
70 lines
2.9 KiB
Python
"""Pin the contract: ``_apply_chat_created`` is NOT replay-safe.
|
|
|
|
See ``docs/audits/2026-04-27-project-callers.md`` for the full audit.
|
|
The handler at ``chat/state/world.py:_apply_chat_created`` uses raw
|
|
``INSERT INTO chats ...`` and ``INSERT INTO chat_state ...`` with no
|
|
``OR REPLACE``/``OR IGNORE``. Running ``project()`` twice over the same
|
|
``chat_created`` event MUST raise ``sqlite3.IntegrityError`` on the
|
|
second pass — this is the bug that produced the 500 fixed in commit
|
|
``0f8bf94`` (and the latent equivalents fixed in this commit).
|
|
|
|
Pinning the contract here means any future "make it idempotent" change
|
|
to the handler MUST update this test, which forces a deliberate review
|
|
of the trade-offs: most notably, that ``chat_state`` columns mutated by
|
|
later events (``time_skip_elision`` bumps ``time``;
|
|
``scene_opened``/``scene_closed`` toggle ``active_scene_id``) would be
|
|
silently overwritten by an ``INSERT OR REPLACE`` on every replay. The
|
|
audit explains why we keep the handler raw-INSERT and enforce the rule
|
|
at the call site via ``append_and_apply`` instead.
|
|
"""
|
|
from __future__ import annotations
|
|
|
|
import sqlite3
|
|
|
|
import pytest
|
|
|
|
from chat.db.connection import open_db
|
|
from chat.db.migrate import apply_migrations
|
|
from chat.eventlog.log import append_event
|
|
from chat.eventlog.projector import project
|
|
import chat.state.world # noqa: F401 — import registers the handler
|
|
|
|
|
|
def _chat_payload():
|
|
return {
|
|
"id": "chat_bot_a",
|
|
"host_bot_id": "bot_a",
|
|
"guest_bot_id": None,
|
|
"initial_time": "2026-04-27T12:00:00+00:00",
|
|
"narrative_anchor": "Day 1 noon",
|
|
"weather": "clear",
|
|
}
|
|
|
|
|
|
def test_chat_created_handler_is_not_replay_safe(tmp_path):
|
|
"""A second projection over an extra ``chat_created`` for the same id raises.
|
|
|
|
This is the exact failure shape from incident ``0f8bf94``: a raw
|
|
INSERT against ``chats.id`` (PK) trips ``UNIQUE constraint failed``
|
|
on the second pass. If this test ever starts FAILING (i.e. the
|
|
second project() succeeds), someone has changed the handler to be
|
|
idempotent — read the audit before approving.
|
|
"""
|
|
db = tmp_path / "t.db"
|
|
apply_migrations(db)
|
|
with open_db(db) as conn:
|
|
# First chat_created + first project: must succeed.
|
|
append_event(conn, kind="chat_created", payload=_chat_payload())
|
|
project(conn)
|
|
|
|
# Append a SECOND chat_created with the same id. project() will
|
|
# walk both, re-INSERT the same chats row, and trip the UNIQUE
|
|
# constraint on chats.id.
|
|
append_event(conn, kind="chat_created", payload=_chat_payload())
|
|
with pytest.raises(sqlite3.IntegrityError) as exc_info:
|
|
project(conn)
|
|
# Match on the column to make sure we caught the *intended*
|
|
# constraint, not some unrelated FK/check failure that happens
|
|
# to also be an IntegrityError.
|
|
assert "chats.id" in str(exc_info.value)
|