Skip to content

Commit a1a30cf

Browse files
authored
fix: resolve issues found by post-merge review (PR #40) (#40)
* fix(events): split JSONL on newline only, not str.splitlines() str.splitlines() also breaks on \r, \v, \f, NEL (U+0085) and Unicode line separators U+2028/U+2029 -- all legal *unescaped* inside JSON strings. A single event whose text (e.g. ASSISTANT_MESSAGE/TOOL_RESULT) carried one was one physical line with no \n, but splitlines() shredded it, so replay() raised a spurious 'corruption' error (whole session failed) and stream() crashed. Split on \n only and rstrip \r for CRLF tolerance. Test: a record containing NEL round-trips through replay without shredding. * fix(events): fsync parent dir on new session file (durability) With fsync=True, content was fsync'd but a newly-created session file's directory entry wasn't durably linked, so a crash could lose a just-created (content-fsync'd) file. fsync the parent dir once on file creation; guarded for filesystems that don't support directory fsync. * fix(config): drop dead HIVE_MAX_TURNS / HIVE_SESSION_TIMEOUT env mappings These mapped to model.max_turns / model.session_timeout, which ModelConfig does not define -- pydantic silently ignored them, so setting either env var was a no-op. Removed the dead rows (the remaining mappings all target real fields). * fix(daemon): fresh identity in checkpoint + drop duplicate crisis line - update_narrative reloads/saves its own identity copy, so the in-memory identity passed to checkpoint.save was stale (missing the just-appended entry/chapter). Reload before snapshotting. - Skip the derived mood line when in crisis: the suffering fragment already emits the crisis directive, so 'overwhelmed' duplicated it in the prompt. * fix(store): mark only the nudges just read as delivered (by id) get_pending_nudges did SELECT (delivered=0) then UPDATE delivered=1 for the whole agent, so a nudge inserted between the two statements (e.g. by another process) was marked delivered without being returned -- silently lost. Now it updates only the nudge_ids actually read. Test: a nudge added after a read is still delivered exactly once on the next read. * docs(changelog): cover mood, chaptered narrative, narrative-in-prompt + F1 in 0.5.4 The 0.5.4 entry listed only the durability work, but #34/#35/#38/#39 all merged into this unreleased version. Add Added entries (mood model, chaptered narrative, narrative-in-prompt, F1 coverage) and a Fixed section for the review follow-ups. Mirrored in docs/changelog.md. * test(store): exercise the actual nudge SELECT-vs-UPDATE race (Greptile) The prior test only validated sequential delivery -- the old WHERE delivered=0 sweep passed it too. New test commits a competing nudge from a separate connection in the window between the SELECT and the UPDATE (via a sync execute wrapper that preserves aiosqlite's await/async-with duality). It fails against the old sweep (n2 marked delivered and lost) and passes with the by-id UPDATE.
1 parent f1db830 commit a1a30cf

8 files changed

Lines changed: 188 additions & 17 deletions

File tree

CHANGELOG.md

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,17 +2,43 @@
22

33
## [0.5.4] — 2026-06-01
44

5-
Durability & dependency-hardening release. All changes are additive and
6-
backward compatible; existing databases upgrade automatically on first open.
5+
Durability, simulation, and hardening release. All changes are additive and
6+
backward compatible; existing databases and identities upgrade automatically.
77

88
### Added
9+
- **Derived mood model** — a `CircumplexMood` maps an agent's `happiness` and
10+
`suffering` onto a valence/arousal circumplex and names the mood (content,
11+
motivated, steady, restless, discouraged, anxious, overwhelmed). Pure/derived
12+
(no persisted state), swappable via `MoodRegistry`, and surfaced in the
13+
goal-pursuit prompt.
14+
- **Chaptered narrative** — instead of FIFO-dropping its oldest lines, an
15+
agent's narrative seals into compact `Chapter` summaries (date span + entry
16+
count + goal theme) on overflow, and the preamble shows a "Story so far"
17+
section. `AgentIdentity.full_narrative()` exposes the complete history.
18+
- **Identity narrative in the runtime prompt** — the agent *pursuing* a goal
19+
now sees its persistent self (name + accumulated narrative), not just the
20+
goal-generation path.
921
- **Event-log fsync durability (C5)**`EventLog(fsync=True)` flushes and
1022
`os.fsync()`s every append so a power/OS crash cannot lose an acknowledged
1123
event. Gated by the new `event_log_fsync` config option (env
1224
`HIVE_EVENT_LOG_FSYNC`), default off to protect the hot heartbeat write path;
1325
the daemon honors it. Reads now tolerate a torn/partial last line.
1426
- **`HiveStore.delete_agent()`** — deletes an agent and, via the new cascade,
1527
all of its child rows in one call.
28+
- **Test coverage (F1)** — first tests for the CLI, the MCP server protocol,
29+
and the structured logging writer/reader.
30+
31+
### Fixed
32+
- **Event log no longer shreds records with Unicode line separators**
33+
`replay()`/`stream()` split on `\n` only (not `str.splitlines()`, which also
34+
breaks on `\r`/`\f`/NEL/`U+2028`/`U+2029` — all legal unescaped in JSON), so
35+
an event whose text carried one is no longer mis-parsed as corruption.
36+
- **Nudge delivery race**`get_pending_nudges` marks only the nudges it
37+
actually read as delivered, so one inserted concurrently isn't lost.
38+
- Dropped dead `HIVE_MAX_TURNS`/`HIVE_SESSION_TIMEOUT` env mappings (no backing
39+
fields); checkpoints now snapshot the freshly-saved identity; the crisis
40+
directive is no longer duplicated in the pursuit prompt; event-log fsync also
41+
fsyncs the parent directory on file creation.
1642

1743
### Changed
1844
- **FK cascades (C3)** — every child table's foreign key to `agents`

docs/changelog.md

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,20 @@
22

33
## [0.5.4] -- 2026-06-01
44

5-
Durability & dependency-hardening release. All changes are additive and backward compatible; existing databases upgrade automatically on first open.
5+
Durability, simulation, and hardening release. All changes are additive and backward compatible; existing databases and identities upgrade automatically.
66

77
### Added
8+
- **Derived mood model**: a `CircumplexMood` maps `happiness` + `suffering` onto a valence/arousal circumplex and names the mood (content/motivated/steady/restless/discouraged/anxious/overwhelmed). Pure/derived, swappable via `MoodRegistry`, surfaced in the goal-pursuit prompt.
9+
- **Chaptered narrative**: the narrative seals into compact `Chapter` summaries (date span + entry count + goal theme) on overflow instead of FIFO-dropping; the preamble shows a "Story so far" section; `AgentIdentity.full_narrative()` exposes the full history.
10+
- **Identity narrative in the runtime prompt**: the goal-*pursuing* agent now sees its persistent self (name + accumulated narrative), not just goal generation.
811
- **Event-log fsync durability (C5)**: `EventLog(fsync=True)` flushes and `os.fsync()`s every append so a power/OS crash can't lose an acknowledged event. Gated by the `event_log_fsync` config option (env `HIVE_EVENT_LOG_FSYNC`), default off to protect the hot heartbeat write path; the daemon honors it. Reads tolerate a torn/partial last line.
912
- **`HiveStore.delete_agent()`**: deletes an agent and, via cascade, all of its child rows in one call.
13+
- **Test coverage (F1)**: first tests for the CLI, the MCP server protocol, and the structured logging writer/reader.
14+
15+
### Fixed
16+
- **Event log no longer shreds records with Unicode line separators**: `replay()`/`stream()` split on `\n` only (not `str.splitlines()`, which also breaks on `\r`/`\f`/NEL/`U+2028`/`U+2029`, all legal unescaped in JSON).
17+
- **Nudge delivery race**: `get_pending_nudges` marks only the nudges it actually read as delivered, so one inserted concurrently isn't lost.
18+
- Dropped dead `HIVE_MAX_TURNS`/`HIVE_SESSION_TIMEOUT` env mappings; checkpoints snapshot the freshly-saved identity; the crisis directive isn't duplicated in the pursuit prompt; event-log fsync also fsyncs the parent directory on file creation.
1019

1120
### Changed
1221
- **FK cascades (C3)**: every child table's foreign key to `agents` (`sessions`, `goals`, `nudges`, `schedules`, `sub_agents`, `tasks`, `alarms`) now declares `ON DELETE CASCADE`; a `user_version` 1->2 migration rebuilds existing tables to add it (data preserved). FK enforcement is opt-in per operation, so writing child rows for not-yet-persisted agents keeps working.

src/hive/config.py

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -188,8 +188,6 @@ def load(cls, hive_dir: Path | None = None) -> "HiveConfig":
188188
"HIVE_HEARTBEAT": ("daemon", "heartbeat", int),
189189
"HIVE_MAX_RETRIES": ("daemon", "max_retries", int),
190190
"HIVE_DEFAULT_MODEL": ("model", "default_model", str),
191-
"HIVE_MAX_TURNS": ("model", "max_turns", int),
192-
"HIVE_SESSION_TIMEOUT": ("model", "session_timeout", int),
193191
"HIVE_STARTING_BALANCE": ("economy", "starting_balance", float),
194192
"HIVE_PROFILES_DIR": ("profiles_dir", None, str),
195193
"HIVE_LOGS_DIR": ("logs_dir", None, str),

src/hive/daemon/loop.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -450,8 +450,10 @@ async def _run_agent_cycle_inner(self, agent: AgentState, suffering: SufferingSt
450450
# narrative/opinions -- which the persona/profile system prompt alone
451451
# doesn't carry. Same context channel the suffering fragment uses.
452452
# A derived mood (from happiness + suffering) colours the framing.
453+
# Skip it in a crisis: the suffering fragment already states the
454+
# crisis directive, so the "overwhelmed" mood line would duplicate it.
453455
mood_line = ""
454-
if persona is not None:
456+
if persona is not None and not suffering.in_crisis:
455457
mood = MoodRegistry.default().derive(
456458
persona.happiness, suffering.cumulative_load, suffering.in_crisis
457459
)
@@ -510,6 +512,9 @@ async def _run_agent_cycle_inner(self, agent: AgentState, suffering: SufferingSt
510512
metadata={"type": "goal_completed", "goal_id": active_goal["goal_id"]},
511513
)
512514
goals_snap = await self._store.list_agent_goals(agent.agent_id, limit=10)
515+
# Reload so the snapshot reflects the narrative entry/chapter that
516+
# update_narrative just wrote (it persists its own reloaded copy).
517+
identity = self._identity.load(agent.agent_id) or identity
513518
self._checkpoint.save(
514519
agent.agent_id,
515520
"goal_completed",

src/hive/memory/events.py

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -73,11 +73,23 @@ async def append(self, event: HiveEvent) -> None:
7373
await asyncio.to_thread(self._write_line, path, line)
7474

7575
def _write_line(self, path: Path, line: str) -> None:
76+
is_new = not path.exists()
7677
with open(path, "a") as f:
7778
f.write(line)
7879
if self._fsync:
7980
f.flush()
8081
os.fsync(f.fileno())
82+
if self._fsync and is_new:
83+
# fsync the parent dir so a freshly-created file's directory entry
84+
# is durably linked (content fsync alone doesn't guarantee this).
85+
try:
86+
dir_fd = os.open(path.parent, os.O_RDONLY)
87+
try:
88+
os.fsync(dir_fd)
89+
finally:
90+
os.close(dir_fd)
91+
except OSError:
92+
pass # some platforms/filesystems don't support directory fsync
8193

8294
async def replay(self, agent_id: str, session_id: str) -> list[HiveEvent]:
8395
path = self._session_path(agent_id, session_id)
@@ -91,9 +103,15 @@ async def replay(self, agent_id: str, session_id: str) -> list[HiveEvent]:
91103
# final line on a newline-terminated file) must parse -- a failure there
92104
# is real corruption and is surfaced, not silently dropped.
93105
ends_clean = text.endswith("\n")
94-
lines = text.splitlines()
106+
# Split ONLY on "\n" -- str.splitlines() also breaks on \r, \v, \f, NEL
107+
# and Unicode 
/
, which are legal (unescaped) inside JSON
108+
# strings, so a single record carrying one in its text would be shredded.
109+
lines = text.split("\n")
110+
if ends_clean and lines and lines[-1] == "":
111+
lines = lines[:-1] # drop the empty element after a trailing newline
95112
events = []
96-
for idx, line in enumerate(lines):
113+
for idx, raw in enumerate(lines):
114+
line = raw.rstrip("\r") # tolerate CRLF
97115
if not line.strip():
98116
continue
99117
try:
@@ -126,7 +144,10 @@ async def stream(self, agent_id: str) -> AsyncIterator[HiveEvent]:
126144
nl = chunk.rfind("\n")
127145
if nl != -1:
128146
complete = chunk[: nl + 1]
129-
for line in complete.splitlines():
147+
# Split only on "\n" (see replay): splitlines() would shred a
148+
# record containing a Unicode line separator in its text.
149+
for raw in complete.split("\n"):
150+
line = raw.rstrip("\r")
130151
if not line.strip():
131152
continue
132153
# These are complete (newline-terminated) lines, so a parse

src/hive/memory/store.py

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -593,16 +593,22 @@ async def get_pending_nudges(self, agent_id: str) -> list[str]:
593593
async with self._connect() as db:
594594
db.row_factory = aiosqlite.Row
595595
async with db.execute(
596-
"SELECT message FROM nudges WHERE agent_id = ? AND delivered = 0",
596+
"SELECT nudge_id, message FROM nudges WHERE agent_id = ? AND delivered = 0",
597597
(agent_id,),
598598
) as cursor:
599-
messages = [row["message"] async for row in cursor]
600-
await db.execute(
601-
"UPDATE nudges SET delivered = 1 WHERE agent_id = ? AND delivered = 0",
602-
(agent_id,),
603-
)
604-
await db.commit()
605-
return messages
599+
rows = [(row["nudge_id"], row["message"]) async for row in cursor]
600+
if rows:
601+
# Mark delivered only the rows just read, by id -- so a nudge
602+
# inserted (e.g. by another process) between the SELECT and the
603+
# UPDATE isn't marked delivered without ever being returned.
604+
ids = [r[0] for r in rows]
605+
placeholders = ",".join("?" * len(ids))
606+
await db.execute(
607+
f"UPDATE nudges SET delivered = 1 WHERE nudge_id IN ({placeholders})",
608+
ids,
609+
)
610+
await db.commit()
611+
return [r[1] for r in rows]
606612

607613
async def save_schedule(
608614
self,

tests/memory/test_store.py

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -227,6 +227,78 @@ async def writer(i: int) -> None:
227227
# No exception == no "database is locked". Spot-check a couple landed.
228228
assert await store.get_pending_nudges("agent-0")
229229

230+
@pytest.mark.asyncio
231+
async def test_get_pending_nudges_only_marks_returned(self, tmp_path: Path) -> None:
232+
"""A nudge added after a read isn't marked delivered, so it isn't lost."""
233+
store = HiveStore(tmp_path / "state.db")
234+
await store.initialize()
235+
await store.save_nudge("n1", "a1", "first")
236+
await store.save_nudge("n2", "a1", "second")
237+
238+
first = await store.get_pending_nudges("a1")
239+
assert sorted(first) == ["first", "second"]
240+
241+
# A nudge that arrives later must still be delivered exactly once.
242+
await store.save_nudge("n3", "a1", "third")
243+
assert await store.get_pending_nudges("a1") == ["third"]
244+
assert await store.get_pending_nudges("a1") == []
245+
246+
@pytest.mark.asyncio
247+
async def test_get_pending_nudges_survives_concurrent_insert(self, tmp_path: Path) -> None:
248+
"""A nudge committed by another connection *between* the SELECT and the
249+
UPDATE must not be marked delivered. This is the actual race the by-id
250+
UPDATE fixes -- the old `WHERE delivered = 0` sweep would have lost it."""
251+
from contextlib import asynccontextmanager
252+
253+
db_path = tmp_path / "state.db"
254+
store = HiveStore(db_path)
255+
await store.initialize()
256+
await store.save_nudge("n1", "a1", "first")
257+
258+
real_connect = store._connect
259+
state = {"injected": False}
260+
261+
@asynccontextmanager
262+
async def connect_with_injection(foreign_keys: bool = False): # type: ignore[no-untyped-def]
263+
async with real_connect(foreign_keys=foreign_keys) as db:
264+
real_execute = db.execute
265+
266+
# Sync wrapper preserves aiosqlite's `await`/`async with` duality.
267+
# Just before the UPDATE runs (rows already SELECTed), a *separate*
268+
# connection commits a new nudge -- the exact race window where the
269+
# old `WHERE delivered = 0` sweep would have marked it delivered.
270+
def execute(*args, **kwargs): # type: ignore[no-untyped-def]
271+
sql = args[0] if args else kwargs.get("sql", "")
272+
if (
273+
not state["injected"]
274+
and sql.lstrip().upper().startswith("UPDATE")
275+
and "nudges" in sql
276+
):
277+
state["injected"] = True
278+
279+
async def _inject_then_update(): # type: ignore[no-untyped-def]
280+
async with aiosqlite.connect(db_path) as other:
281+
await other.execute(
282+
"INSERT INTO nudges (nudge_id, agent_id, message, "
283+
"delivered, created_at) VALUES ('n2','a1','second',0,'t')"
284+
)
285+
await other.commit()
286+
return await real_execute(*args, **kwargs)
287+
288+
return _inject_then_update()
289+
return real_execute(*args, **kwargs)
290+
291+
db.execute = execute # type: ignore[method-assign]
292+
yield db
293+
294+
store._connect = connect_with_injection # type: ignore[assignment,method-assign]
295+
returned = await store.get_pending_nudges("a1")
296+
store._connect = real_connect # type: ignore[method-assign]
297+
298+
assert returned == ["first"]
299+
# n2 raced in mid-call; it must survive as still-pending, not be lost.
300+
assert await store.get_pending_nudges("a1") == ["second"]
301+
230302

231303
class TestCascades:
232304
@pytest.mark.asyncio

tests/test_events.py

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,40 @@ async def _run():
123123
asyncio.run(_run())
124124

125125

126+
def test_replay_handles_unicode_line_separators(tmp_dir):
127+
"""A record whose text contains U+2028/U+2029/NEL must not be shredded.
128+
129+
These are legal (unescaped) inside JSON strings and have no '\\n', so the
130+
record is one physical line -- but str.splitlines() would break it apart.
131+
"""
132+
log = EventLog(tmp_dir)
133+
134+
async def _run():
135+
tricky = "before
middle
after\x85end"
136+
await log.append(
137+
HiveEvent(
138+
event_type=EventType.ASSISTANT_MESSAGE,
139+
agent_id="agent-1",
140+
session_id="sess-1",
141+
data={"text": tricky},
142+
)
143+
)
144+
await log.append(
145+
HiveEvent(
146+
event_type=EventType.TOOL_USED,
147+
agent_id="agent-1",
148+
session_id="sess-1",
149+
data={"tool": "x"},
150+
)
151+
)
152+
events = await log.replay("agent-1", "sess-1")
153+
assert len(events) == 2 # not shredded into extra/broken lines
154+
assert events[0].data["text"] == tricky
155+
assert events[1].data["tool"] == "x"
156+
157+
asyncio.run(_run())
158+
159+
126160
def test_replay_raises_on_mid_log_corruption(tmp_dir):
127161
"""A malformed line that is NOT the torn final line is real corruption -- surface it."""
128162
import pytest

0 commit comments

Comments
 (0)