diff --git a/CHANGELOG.md b/CHANGELOG.md index a55ce08..62d9ea7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] ### Fixed +- **Briefing now surfaces manually-fired intentions.** `generate_briefing` previously only reported intentions that the collator transitioned from `pending` to `fired` on the current call (time-based auto-fire). Intentions set to `state="fired"` by an agent — the documented compaction-handoff convention — were counted correctly by `get_intentions(state="fired")` but invisible to `get_briefing`, so handoffs written at session end did not surface on the next session's startup briefing. In practice this meant multi-day-old handoff notes across every project accumulated in `fired` state and never reached the receiving agent. `generate_briefing` now queries all `state="fired"` intentions for the owner (including those the current call just transitioned), sorts by urgency desc then most-recent update, caps the listed entries at 10 to bound briefing size, and reports the full count in `evaluation.intentions_fired`. The receiving agent transitions the intention off `fired` (→ `active`/`completed`/`cancelled`/`snoozed`) once acknowledged; until then, the briefing keeps surfacing it. `fired_intentions` briefing entries now carry `urgency` and `updated` fields. `tests/test_collator.py` updates: `test_fired_intention_not_surfaced_on_second_briefing` → `test_fired_intention_persists_until_actioned` (inverted assertion + covers the fired → active transition that stops re-surfacing); new `test_briefing_surfaces_manually_fired_intention` exercises the handoff path directly; new `test_briefing_caps_fired_intention_list` locks the 10-entry cap and high-urgency-first ordering. - **`alembic/env.py` no longer disables application loggers.** Python's `logging.config.fileConfig()` defaults to `disable_existing_loggers=True`, which silences every logger not listed in `alembic.ini` — including `mcp_awareness.postgres_store`. Production CLI alembic runs are one-shot processes so the silencing is invisible, but any long-lived Python process that calls alembic programmatically (tests invoking `alembic.command.upgrade`, server-side migration hooks, admin scripts) inherits a broken log surface for the rest of its lifetime. `alembic/env.py` now passes `disable_existing_loggers=False` to `fileConfig()`, preserving host-app logging. Surfaced by the R3 migration-safety test (first programmatic alembic consumer in the test suite): running `test_rls_migration_safety.py` before `tests/test_store.py::test_do_cleanup_logs_errors` silently dropped the error log record the store test asserts on, causing order-dependent failures that looked like a return of the [#374](https://github.com/cmeans/mcp-awareness/issues/374) flake. - **Flake in `tests/test_store.py::test_do_cleanup_logs_errors`.** Test was using the `caplog.at_level(...)` context manager plus `caplog.text` substring check. That pattern failed intermittently on CI (observed on Python 3.11 and 3.14) even though the code under test (`_do_cleanup`) is fully synchronous — the flake was in pytest's log-capture path, not in the production code. Rewritten to use `caplog.set_level(...)` at fixture scope and inspect `caplog.records` directly (logger name + level + message substring), which is sturdier across pytest/Python versions and produces a richer failure message when it does fire. Verified 10/10 consecutive local runs post-fix. Closes [#374](https://github.com/cmeans/mcp-awareness/issues/374). diff --git a/src/mcp_awareness/collator.py b/src/mcp_awareness/collator.py index e64f9d5..705f414 100644 --- a/src/mcp_awareness/collator.py +++ b/src/mcp_awareness/collator.py @@ -29,6 +29,9 @@ from .schema import Entry, severity_rank, to_iso from .store import Store +_URGENCY_RANK = {"low": 0, "normal": 1, "high": 2} +_MAX_FIRED_IN_BRIEFING = 10 + def _suppression_tags_match(s_tags: list[str], alert: Entry) -> bool: """Check if suppression tags match the alert via tags or content keywords.""" @@ -218,7 +221,8 @@ def compose_summary(briefing: dict[str, Any]) -> str: fired = briefing.get("fired_intentions", []) if fired: - parts.append(f"{len(fired)} intention{'s' if len(fired) != 1 else ''} ready") + total = briefing.get("evaluation", {}).get("intentions_fired", len(fired)) + parts.append(f"{total} intention{'s' if total != 1 else ''} ready") return ". ".join(parts) + "." if parts else f"All clear across {total} sources." @@ -348,31 +352,41 @@ def generate_briefing(store: Store, owner_id: str) -> dict[str, Any]: briefing["active_suppressions"] = store.count_active_suppressions(owner_id) - # Evaluate time-based intentions — fire pending intentions whose deliver_at has passed - fired_intentions = store.get_fired_intentions(owner_id) - if fired_intentions: - # Transition each matched intention from "pending" to "fired" so they - # don't fire again on subsequent briefing reads - for intention in fired_intentions: - store.update_intention_state( - owner_id, intention.id, "fired", reason="Delivered via briefing" - ) + # Auto-fire path: pending intentions whose deliver_at has passed transition to "fired" + auto_fired = store.get_fired_intentions(owner_id) + for intention in auto_fired: + store.update_intention_state( + owner_id, intention.id, "fired", reason="Delivered via briefing" + ) + + # Surface all intentions in "fired" state — includes auto-fired this call AND + # handoffs manually transitioned to "fired" by other agents/sessions. These + # persist in the briefing until the receiving agent moves them to "active" + # / "completed" / "cancelled" / "snoozed". + all_fired = store.get_intentions(owner_id, state="fired") + all_fired.sort( + key=lambda i: ( + -_URGENCY_RANK.get(i.data.get("urgency", "normal"), 1), + -(i.updated or i.created).timestamp(), + ) + ) + if all_fired: briefing["fired_intentions"] = [ { "id": i.id, "goal": i.data.get("goal", i.data.get("description", "")), "source": i.source, "tags": i.tags, + "urgency": i.data.get("urgency", "normal"), + "updated": to_iso(i.updated or i.created), } - for i in fired_intentions + for i in all_fired[:_MAX_FIRED_IN_BRIEFING] ] briefing["attention_needed"] = True - # Count pending intentions, excluding those already fired (avoid double-counting) - all_pending = store.get_intentions(owner_id, state="pending") - fired_ids = {i.id for i in fired_intentions} - pending_not_fired = [i for i in all_pending if i.id not in fired_ids] - briefing["pending_intentions"] = len(pending_not_fired) + # Count pending intentions (auto-fire transitions removed them from pending) + pending = store.get_intentions(owner_id, state="pending") + briefing["pending_intentions"] = len(pending) briefing["evaluation"] = { "alerts_checked": eval_alerts_checked, @@ -380,8 +394,8 @@ def generate_briefing(store: Store, owner_id: str) -> dict[str, Any]: "pattern_matched": eval_pattern_matched, "stale_sources": eval_stale_sources, "surfaced": briefing["active_alerts"], - "intentions_pending": len(pending_not_fired), - "intentions_fired": len(fired_intentions), + "intentions_pending": len(pending), + "intentions_fired": len(all_fired), } briefing["summary"] = compose_summary(briefing) diff --git a/tests/test_collator.py b/tests/test_collator.py index a9f8a19..0f7c071 100644 --- a/tests/test_collator.py +++ b/tests/test_collator.py @@ -876,15 +876,21 @@ def test_fired_intention_transitions_to_fired_state(self, store): fired_after = store.get_intentions(TEST_OWNER, state="fired") assert any(i.id == intention_id for i in fired_after) - def test_fired_intention_not_surfaced_on_second_briefing(self, store): - """Once an intention fires and transitions, it doesn't appear in subsequent briefings.""" + def test_fired_intention_persists_until_actioned(self, store): + """Fired intentions keep surfacing in briefings until moved off 'fired' state. + + Handoff convention: agents transition 'pending' -> 'fired' manually to surface + work in the next session's briefing. The briefing must show these until the + receiving agent transitions them to 'active' / 'completed' / 'cancelled' / 'snoozed'. + """ from datetime import timedelta past = now_utc() - timedelta(hours=1) + intention_id = make_id() store.add( TEST_OWNER, Entry( - id=make_id(), + id=intention_id, type=EntryType.INTENTION, source="personal", tags=["errands"], @@ -901,10 +907,115 @@ def test_fired_intention_not_surfaced_on_second_briefing(self, store): briefing1 = generate_briefing(store, TEST_OWNER) assert len(briefing1.get("fired_intentions", [])) == 1 - # Second briefing should not fire it again + # Second briefing still surfaces it — it hasn't been actioned yet briefing2 = generate_briefing(store, TEST_OWNER) - assert briefing2.get("fired_intentions") is None - assert briefing2["evaluation"]["intentions_fired"] == 0 + assert len(briefing2.get("fired_intentions", [])) == 1 + assert briefing2["fired_intentions"][0]["id"] == intention_id + assert briefing2["evaluation"]["intentions_fired"] == 1 + + # Agent transitions fired -> active. Next briefing stops surfacing it. + store.update_intention_state(TEST_OWNER, intention_id, "active") + briefing3 = generate_briefing(store, TEST_OWNER) + assert briefing3.get("fired_intentions") is None + assert briefing3["evaluation"]["intentions_fired"] == 0 + + def test_briefing_surfaces_manually_fired_intention(self, store): + """Handoff pattern: an agent transitions pending -> fired directly + (no deliver_at). The next briefing must surface it.""" + intention_id = make_id() + store.add( + TEST_OWNER, + Entry( + id=intention_id, + type=EntryType.INTENTION, + source="mcp-awareness-project", + tags=["handoff", "wip"], + created=now_utc(), + expires=None, + data={ + "goal": "Resume PR #387 after reboot", + "state": "pending", + "deliver_at": None, + "urgency": "high", + }, + ), + ) + # Agent fires it manually (no deliver_at, no auto-fire path) + store.update_intention_state(TEST_OWNER, intention_id, "fired", reason="Compaction handoff") + + briefing = generate_briefing(store, TEST_OWNER) + assert briefing["attention_needed"] is True + fired = briefing.get("fired_intentions", []) + assert len(fired) == 1 + assert fired[0]["id"] == intention_id + assert fired[0]["goal"] == "Resume PR #387 after reboot" + assert briefing["evaluation"]["intentions_fired"] == 1 + + def test_briefing_caps_fired_intention_list(self, store): + """When many intentions are fired, briefing lists at most 10 but counts all.""" + ids = [] + for i in range(15): + eid = make_id() + ids.append(eid) + store.add( + TEST_OWNER, + Entry( + id=eid, + type=EntryType.INTENTION, + source="personal", + tags=["handoff"], + created=now_utc(), + expires=None, + data={ + "goal": f"Task {i}", + "state": "pending", + "deliver_at": None, + "urgency": "high" if i < 3 else "normal", + }, + ), + ) + store.update_intention_state(TEST_OWNER, eid, "fired") + + briefing = generate_briefing(store, TEST_OWNER) + fired = briefing.get("fired_intentions", []) + assert len(fired) == 10 + assert briefing["evaluation"]["intentions_fired"] == 15 + # Strict slot check: high-urgency entries occupy the first three slots. + # A count-only assertion would also pass if the sort key flipped + # direction (high-urgency entries at slots 7-9 instead of 0-2). + assert [f["urgency"] for f in fired[:3]] == ["high", "high", "high"] + assert all(f["urgency"] == "normal" for f in fired[3:]) + + def test_briefing_summary_reports_total_fired_count_not_capped(self, store): + """compose_summary must report the full fired count, not the capped + displayed list length. Regression guard: prior implementation used + len(fired_intentions) which is capped at 10, silently under-reporting + backlogs of 11+ fired handoffs.""" + for i in range(15): + eid = make_id() + store.add( + TEST_OWNER, + Entry( + id=eid, + type=EntryType.INTENTION, + source="personal", + tags=["handoff"], + created=now_utc(), + expires=None, + data={ + "goal": f"Task {i}", + "state": "pending", + "deliver_at": None, + }, + ), + ) + store.update_intention_state(TEST_OWNER, eid, "fired") + + briefing = generate_briefing(store, TEST_OWNER) + assert briefing["evaluation"]["intentions_fired"] == 15 + assert len(briefing["fired_intentions"]) == 10 + assert "15 intentions ready" in briefing["summary"] + assert "10 intentions ready" not in briefing["summary"] def test_briefing_no_intentions_when_none_pending(self, store): """Briefing doesn't include intentions when none exist."""