# v2.7.0 get_messages filter-after-mark — confirmed P1, fixed

## Status

**CONFIRMED REAL.** Fixed in v2.7.0 release prep on `release/v2.7.0` (commit TBD — will be the second commit on the branch after Phase 2 trace cleanup).

## What the external review flagged

> "From inspection, src/tools/messaging.ts appears to apply some caller-level filtering after src/db.ts getMessages() has already selected and marked rows as read, unless peek is true. That means a message could potentially be consumed/read by a lower-level fetch even if it gets filtered out before final return."
> — External review, 2026-05-11

## The actual bug path (pre-v2.7.0)

1. Caller invokes `get_messages({ agent_name: "alice", status: "pending", since: "15m" })`.
2. `handleGetMessages` (src/tools/messaging.ts:130) resolves `sinceIso` from the `since` shorthand.
3. Calls `getMessages(agent_name, status, limit, peek)` in src/db.ts:2837. **The `since` value is NOT passed** — `getMessages`'s signature took only 4 args pre-fix.
4. `getMessages` runs SQL: `SELECT * FROM messages WHERE to_agent = ? AND (read_by_session IS NULL OR read_by_session != ?) ORDER BY priority, created_at DESC LIMIT ?`. **No `since` clause.**
5. Same call: if `!peek` and `rows.length > 0`, the function runs `UPDATE messages SET status='read', read_by_session=? WHERE id IN (...)` inside a transaction. **Every row from step 4 is marked-read for this session.**
6. Returns the unfiltered set to `handleGetMessages`.
7. `handleGetMessages` runs `filterBySince(raw, sinceIso)` in JS at line 159, dropping rows older than the bound from the response.
8. Caller sees only the post-filter set.

Net effect: a message addressed to "alice", created 25 min ago, never previously read:
- `get_messages(..., since='15m')` returns it filtered out (caller never sees it).
- BUT the message was marked-read for this session in step 5.
- A subsequent `get_messages(..., status='pending', since='all')` from the same session WOULD NOT see it (`read_by_session = current_session` excludes it).
- Message is silently lost to this session until a new session starts.

This is the silent-data-loss class. The session that asked for "since=15m" loses everything outside that window with no warning.

## Why it slipped past audits

- The bug requires `since` < `'all'` AND a message older than the bound AND a follow-up call within the same session. None of the existing tests in `tests/messaging.test.ts` or `tests/handover.test.ts` exercise that exact sequence — they tend to test `since='all'` (default in v2.1.5 and earlier) or single-call snapshots.
- Code-review-wise, the bug is invisible if you read only `handleGetMessages` (looks clean) or only `getMessages` (looks clean). It only surfaces when you read both side-by-side and notice the filter site is downstream of the mutation site.
- The reviewer caught it on pure code reading without running it — that's the inspection-of-data-flow class of bug.

## Fix

Three coordinated changes in v2.7.0:

1. **src/db.ts:2837** — `getMessages` signature gains a 5th parameter `sinceIso: string | null = null`. Each branch of the SELECT stitches `AND created_at >= ?` BEFORE the LIMIT clause when `sinceIso != null`. The mark-as-read transaction (~line 2901) now only sees rows that survive the SQL filter.

2. **src/tools/messaging.ts:158** — handler passes `sinceIso` into the DB call. The downstream `filterBySince` JS function is removed entirely (no longer needed; SQL handles it).

3. **src/transport/consistency-probe.ts:67** — probe takes `sinceIso` too. Its SUPERSET SQL query (which intentionally ignores session-partition state) now also applies the same `since` filter, so a since-narrower-than-all call doesn't emit false-positive divergence warnings post-fix.

## Walk-analogous-surfaces results

Verified by reading source (verify by reading source, don't assume):

- **`getMessagesSummary` (src/db.ts:3021)** — already takes `sinceIso` as a parameter. Already applies it as SQL `AND created_at >= ?` clause. Pure read (no mark-as-read mutation). **Already correct, no change needed.**
- **`peekMailboxVersion` (src/db.ts:2782)** — separate code path returning aggregate counts. No `since` filter exists in its signature; the function returns counts across all of an agent's inbox. **No mutation, no bug.**
- **`broadcastMessage` (src/db.ts:3088)** — write path, doesn't read inbox state. **N/A.**

The bug was specific to `getMessages` + `filterBySince` interaction. Other inbox-reading paths are unaffected.

## Regression test

`tests/v2-7-0-get-messages-filter-after-mark.test.ts` (3 cases):

1. **The exact scenario the review described.** Seed a 25-min-old message + a 5-min-old message. Call with `since='15m'`. Assert (a) only the 5-min message returns, (b) a follow-up `since='all'` call STILL sees the 25-min message as pending. Pre-fix this assertion fails — the 25-min message is missing.
2. **Idempotency within the same window.** Second call with the same `since='15m'` correctly sees zero pending (the in-window message was correctly drained).
3. **`peek=true` no-mutation control.** Same shape as case 1 with `peek=true` — both messages remain pending after the call.

All 3 pass against the v2.7.0 fix. The first one fails against pre-v2.7.0 code, confirming the test exercises the right path.

## Severity assessment

**P1.** Silent data loss is a hard correctness contract. Operators using narrow `since` bounds (the common pattern in long-running orchestrators that filter recent mail) would have lost messages without ever seeing an error. No telemetry to detect it (no audit log entry differentiates "read because operator asked for it" from "read because filter-after-mark"). The Phase 5 consistency probe might have surfaced divergence under heavy sampling, but it's off by default.

Severity is mitigated only by: (a) the default `since='24h'` in v2.1.6+ makes the bug invisible most of the time (24h is wide enough to include most workflow-relevant messages), and (b) the bug only fires when the same session does both the narrow call AND a follow-up — single-session workflows that exit after the first call are unaffected.

## Out-of-scope adjacent improvements

Surfaced for future review-recommended hardening but NOT in v2.7.0 scope:

- The Phase 5 consistency probe could be expanded to alert on filter-then-mark divergence specifically (currently it catches "MCP returned fewer rows than SQL sees pending"). Now that the contract is "MCP returns exactly what SQL with the same filters would return," the probe is a natural fit for stricter equality checking. v2.8 hygiene round.
- Tool-registry-as-source-of-truth (review recommendation): the tool count is currently inferred from server.ts; documentation drifts from reality. A v2.8 hardening round could generate doc strings + MCP definitions + smoke-test expectations from the registry itself.