Drop unused agent_messages.read_at column via migration v49 (M1)
The `agent_messages.read_at` column has had no production writer since v12 moved read tracking into the per-viewer `agent_message_reads` receipts table. It survived anyway, carrying a NULL through every SELECT list and begging the next maintainer to accidentally read stale data from it. - v49 migration: `ALTER TABLE agent_messages DROP COLUMN read_at` (idempotent: skips if absent). - messages.ts: SELECTs no longer fetch the column; per-viewer `readAt` is computed from `agent_message_reads` as before, with `null` for call-sites that have no viewer context (getAgentMessage / searchMessages / listAgentInboxUnread). - runtime-bridge.ts / runtime-message-injector.ts: comments referencing the dropped column updated to name the read-receipts table instead.
This commit is contained in:
1 parent
dade5eb333
commit
d7e267031f
6 files changed
+90
-36
No files matched your search
@@ -562,16 +562,16 @@ export class RuntimeBridge {
|
||||
// as plain user turns in its context.
|
||||
//
|
||||
// Replay safety lives entirely on the `agent_message_projections`
|
||||
// ledger. We deliberately do NOT touch `agent_messages.read_at`
|
||||
// here even though that column exists: "we put the message into
|
||||
// the agent's transcript" and "the agent has acknowledged the
|
||||
// message" are two different facts, and welding them onto one
|
||||
// column made every projected message immediately invisible to
|
||||
// the agent's `?unreadOnly=true` API call. The agent then read
|
||||
// its IC prompt ("respond to ALL unread messages"), called the
|
||||
// API, got an empty list, and posted a status comment instead of
|
||||
// replying. The explicit POST /messages/{id}/read is now the
|
||||
// sole signal that flips `read_at`.
|
||||
// ledger. "We put the message into the agent's transcript" and
|
||||
// "the agent has acknowledged the message" are two different
|
||||
// facts, tracked in two different tables: projections are append-
|
||||
// only on dispatch, while reads are inserted into
|
||||
// `agent_message_reads` only when the agent POSTs
|
||||
// /messages/{id}/read. Conflating the two (as an earlier build
|
||||
// that wrote to `agent_messages.read_at` at projection time did)
|
||||
// made every projected message immediately invisible to
|
||||
// `?unreadOnly=true`, so agents posted status comments instead of
|
||||
// replies. The explicit read endpoint is the sole signal.
|
||||
const store = this.getOrCreateStore(payload.taskSessionId);
|
||||
const inboxRecords = this.deps.db.listAgentInboxUnread(payload.companyId, payload.agentId);
|
||||
if (inboxRecords.length > 0) {
|
||||
|
||||
@@ -134,10 +134,10 @@ export class RuntimeMessageInjector {
|
||||
* the recipient is woken so the next `prepareRun` picks the message
|
||||
* up through the regular inbox projection.
|
||||
*
|
||||
* `read_at` is intentionally NOT touched here: "the worker queued
|
||||
* this message onto the agent" is not "the agent replied". The
|
||||
* agent flips `read_at` itself by POSTing /messages/{id}/read once
|
||||
* it has actually processed the row.
|
||||
* The read receipt is intentionally NOT written here: "the worker
|
||||
* queued this message onto the agent" is not "the agent replied".
|
||||
* The agent inserts into `agent_message_reads` itself by POSTing
|
||||
* /messages/{id}/read once it has actually processed the row.
|
||||
*/
|
||||
handleSteerAck(correlationId: string, accepted: boolean): void {
|
||||
this.resolvePending(correlationId, accepted, "worker_ack");
|
||||
|
||||
@@ -107,10 +107,15 @@ export function buildMessageMethods(
|
||||
getAgentMessage(messageId: string, companyId: string): AgentMessageRecord | null {
|
||||
const row = getDb()
|
||||
.prepare(
|
||||
"select id, company_id as companyId, from_agent_id as fromAgentId, to_agent_id as toAgentId, sender_kind as senderKind, channel, channel_target_id as channelTargetId, subject, body, priority, read_at as readAt, parent_message_id as parentMessageId, attachments_json as attachmentsJson, created_at as createdAt from agent_messages where id = ? and company_id = ?",
|
||||
"select id, company_id as companyId, from_agent_id as fromAgentId, to_agent_id as toAgentId, sender_kind as senderKind, channel, channel_target_id as channelTargetId, subject, body, priority, parent_message_id as parentMessageId, attachments_json as attachmentsJson, created_at as createdAt from agent_messages where id = ? and company_id = ?",
|
||||
)
|
||||
.get(messageId, companyId) as AgentMessageRecord | undefined;
|
||||
return row ?? null;
|
||||
.get(messageId, companyId) as Omit<AgentMessageRecord, "readAt"> | undefined;
|
||||
if (!row) return null;
|
||||
// `readAt` is a per-viewer computed field resolved from
|
||||
// agent_message_reads at the callers that need it (inbox
|
||||
// projection, renderer snapshot). This single-row lookup has no
|
||||
// viewer context, so the field is surfaced as `null`.
|
||||
return { ...row, readAt: null };
|
||||
},
|
||||
|
||||
listAgentMessages(
|
||||
@@ -125,8 +130,12 @@ export function buildMessageMethods(
|
||||
},
|
||||
): AgentMessageRecord[] {
|
||||
const limit = opts?.limit ?? 200;
|
||||
// `readAt` is resolved from `agent_message_reads` further down,
|
||||
// per viewer (BOARD_READER_ID for the operator-wide path, the
|
||||
// specific agent id for the per-agent path). The column lives
|
||||
// outside the row itself since v49.
|
||||
const selectCols =
|
||||
"select id, company_id as companyId, from_agent_id as fromAgentId, to_agent_id as toAgentId, sender_kind as senderKind, channel, channel_target_id as channelTargetId, subject, body, priority, read_at as readAt, parent_message_id as parentMessageId, attachments_json as attachmentsJson, created_at as createdAt from agent_messages";
|
||||
"select id, company_id as companyId, from_agent_id as fromAgentId, to_agent_id as toAgentId, sender_kind as senderKind, channel, channel_target_id as channelTargetId, subject, body, priority, parent_message_id as parentMessageId, attachments_json as attachmentsJson, created_at as createdAt from agent_messages";
|
||||
let sql = `${selectCols} where company_id = ?`;
|
||||
const params: SQLInputValue[] = [companyId];
|
||||
if (opts?.messageId) {
|
||||
@@ -148,7 +157,7 @@ export function buildMessageMethods(
|
||||
sql += " limit ?";
|
||||
params.push(limit);
|
||||
}
|
||||
const rows = readAll<AgentMessageRecord>(sql, ...params).reverse();
|
||||
const rows = readAll<Omit<AgentMessageRecord, "readAt">>(sql, ...params).reverse();
|
||||
const boardReadReceiptMap = new Map(
|
||||
readAll<{ messageId: string; readAt: string }>(
|
||||
`select reads.message_id as messageId, reads.read_at as readAt
|
||||
@@ -219,7 +228,7 @@ export function buildMessageMethods(
|
||||
sql += " limit ?";
|
||||
params.push(limit);
|
||||
}
|
||||
const rows = readAll<AgentMessageRecord>(sql, ...params).reverse();
|
||||
const rows = readAll<Omit<AgentMessageRecord, "readAt">>(sql, ...params).reverse();
|
||||
|
||||
const readReceiptMap = new Map(
|
||||
readAll<{ messageId: string; readAt: string }>(
|
||||
@@ -260,7 +269,7 @@ export function buildMessageMethods(
|
||||
const pattern = `%${escaped}%`;
|
||||
let sql = `SELECT id, company_id AS companyId, from_agent_id AS fromAgentId,
|
||||
to_agent_id AS toAgentId, sender_kind AS senderKind, channel, channel_target_id AS channelTargetId,
|
||||
subject, body, priority, read_at AS readAt, parent_message_id AS parentMessageId,
|
||||
subject, body, priority, parent_message_id AS parentMessageId,
|
||||
attachments_json AS attachmentsJson, created_at AS createdAt
|
||||
FROM agent_messages WHERE company_id = ? AND (subject LIKE ? ESCAPE '\\' OR body LIKE ? ESCAPE '\\')`;
|
||||
const params: SQLInputValue[] = [companyId, pattern, pattern];
|
||||
@@ -274,7 +283,11 @@ export function buildMessageMethods(
|
||||
}
|
||||
sql += " ORDER BY created_at DESC LIMIT ?";
|
||||
params.push(opts?.limit ?? 50);
|
||||
return readAll<AgentMessageRecord>(sql, ...params);
|
||||
// Operator-facing search has no viewer context — `readAt` is
|
||||
// surfaced as `null`. Callers that need per-viewer read state
|
||||
// pull it from `agent_message_reads` separately (inbox projection,
|
||||
// snapshot).
|
||||
return readAll<Omit<AgentMessageRecord, "readAt">>(sql, ...params).map((row) => ({ ...row, readAt: null }));
|
||||
},
|
||||
|
||||
markMessageReadForReader(
|
||||
@@ -301,12 +314,11 @@ export function buildMessageMethods(
|
||||
* department when agent belongs, project when assigned/lead).
|
||||
*
|
||||
* "Unread" is defined uniformly across every channel: no row in
|
||||
* `agent_message_reads` for (message_id, agent_id). The legacy
|
||||
* `agent_messages.read_at` column is never written anywhere in the
|
||||
* codebase and must not be used as a read signal — it only appears in
|
||||
* SELECT lists for wire-shape compatibility and is overwritten from the
|
||||
* read-receipts table by every caller that surfaces `readAt` to agents
|
||||
* or the renderer.
|
||||
* `agent_message_reads` for (message_id, agent_id). The row's
|
||||
* `readAt` field is always `null` here because callers want the
|
||||
* unread set — per-viewer read state lives exclusively in the
|
||||
* `agent_message_reads` join table since v49 dropped the redundant
|
||||
* `agent_messages.read_at` column.
|
||||
*
|
||||
* Messages where `from_agent_id = agentId` are excluded (agents don't
|
||||
* inject their own sent messages back into their transcript).
|
||||
@@ -362,9 +374,9 @@ export function buildMessageMethods(
|
||||
}
|
||||
|
||||
const selectCols =
|
||||
"select id, company_id as companyId, from_agent_id as fromAgentId, to_agent_id as toAgentId, sender_kind as senderKind, channel, channel_target_id as channelTargetId, subject, body, priority, read_at as readAt, parent_message_id as parentMessageId, attachments_json as attachmentsJson, created_at as createdAt from agent_messages";
|
||||
"select id, company_id as companyId, from_agent_id as fromAgentId, to_agent_id as toAgentId, sender_kind as senderKind, channel, channel_target_id as channelTargetId, subject, body, priority, parent_message_id as parentMessageId, attachments_json as attachmentsJson, created_at as createdAt from agent_messages";
|
||||
const sql = `${selectCols} where company_id = ? and from_agent_id != ? and (${visibilityClauses.join(" or ")}) order by created_at asc`;
|
||||
const rows = readAll<AgentMessageRecord>(sql, companyId, agentId, ...visibilityParams);
|
||||
const rows = readAll<Omit<AgentMessageRecord, "readAt">>(sql, companyId, agentId, ...visibilityParams).map((row) => ({ ...row, readAt: null }));
|
||||
|
||||
const readIds = new Set(
|
||||
readAll<{ messageId: string }>(
|
||||
|
||||
@@ -0,0 +1,40 @@
|
||||
import log from "electron-log/main.js";
|
||||
import type { Migration, MigrationDeps } from "./types";
|
||||
import { columnExists, tableExists } from "./types";
|
||||
|
||||
/**
|
||||
* Drop the unused `agent_messages.read_at` column.
|
||||
*
|
||||
* The column was added in v11 when reads were tracked per-message on a
|
||||
* single shared receipt. v12 moved read tracking to the
|
||||
* `agent_message_reads` join table, which is now the sole source of
|
||||
* truth: every consumer that surfaces `readAt` (renderer snapshot,
|
||||
* per-agent inbox, board receipts) resolves it from that table.
|
||||
* `agent_messages.read_at` has not been written by any production path
|
||||
* for multiple releases; leaving it around just forced every SELECT to
|
||||
* carry a NULL column through wire-shape layers and invited the next
|
||||
* maintainer to accidentally read stale data from it.
|
||||
*/
|
||||
export function createMigrationsV49V49(deps: MigrationDeps): Migration[] {
|
||||
const { db } = deps;
|
||||
return [
|
||||
{
|
||||
version: 49,
|
||||
name: "drop_agent_messages_read_at",
|
||||
fn: () => {
|
||||
if (!tableExists(db, "agent_messages")) {
|
||||
log.info("[migration v49] agent_messages table missing; skipping");
|
||||
return;
|
||||
}
|
||||
if (!columnExists(db, "agent_messages", "read_at")) {
|
||||
log.info("[migration v49] agent_messages.read_at already absent; skipping");
|
||||
return;
|
||||
}
|
||||
db.exec("ALTER TABLE agent_messages DROP COLUMN read_at");
|
||||
log.info(
|
||||
"[migration v49] dropped unused agent_messages.read_at column — reads are tracked per-reader in agent_message_reads",
|
||||
);
|
||||
},
|
||||
},
|
||||
];
|
||||
}
|
||||
@@ -14,6 +14,7 @@ import { createMigrationsV45V45 } from "./migrations/v45-v45";
|
||||
import { createMigrationsV46V46 } from "./migrations/v46-v46";
|
||||
import { createMigrationsV47V47 } from "./migrations/v47-v47";
|
||||
import { createMigrationsV48V48 } from "./migrations/v48-v48";
|
||||
import { createMigrationsV49V49 } from "./migrations/v49-v49";
|
||||
import {
|
||||
columnExists,
|
||||
nowIso,
|
||||
@@ -208,6 +209,7 @@ export function runMigrations(
|
||||
...createMigrationsV46V46(deps),
|
||||
...createMigrationsV47V47(deps),
|
||||
...createMigrationsV48V48(deps),
|
||||
...createMigrationsV49V49(deps),
|
||||
];
|
||||
|
||||
// Pre-migration snapshot. If any migration is pending we take a full
|
||||
|
||||
@@ -41,8 +41,9 @@ export function buildSnapshotMethods(
|
||||
// view has exactly one human viewer (the Board operator), so the
|
||||
// renderer snapshot fills it from that reader's agent_message_reads
|
||||
// rows. Agent-side queries (listAgentMessages) resolve it against
|
||||
// the current agent's receipts. The raw agent_messages.read_at
|
||||
// column is never read on any path and stays NULL by design.
|
||||
// the current agent's receipts. The old `agent_messages.read_at`
|
||||
// column was dropped in migration v49 — the join table is the
|
||||
// sole source of truth.
|
||||
const boardReadReceiptMap = new Map(
|
||||
readAll<{ messageId: string; readAt: string }>(
|
||||
`select reads.message_id as messageId, reads.read_at as readAt
|
||||
@@ -183,10 +184,9 @@ export function buildSnapshotMethods(
|
||||
createdAt: r.created_at, updatedAt: r.updated_at,
|
||||
})),
|
||||
agentMessages: readAll<SqliteRow>(
|
||||
// The raw `read_at` column is intentionally omitted from the
|
||||
// SELECT list. It is never written — readAt semantics live in
|
||||
// the agent_message_reads receipts table, resolved against the
|
||||
// current viewer above.
|
||||
// readAt semantics live in the agent_message_reads receipts
|
||||
// table, resolved against the current viewer above. The old
|
||||
// `agent_messages.read_at` column was dropped in v49.
|
||||
`select id, company_id, from_agent_id, to_agent_id, sender_kind, channel, channel_target_id, subject, body, priority, parent_message_id, attachments_json, created_at from agent_messages${companyWhere} order by created_at desc limit 1000`,
|
||||
...companyArgs,
|
||||
).map((r) => ({
|
||||
|
||||
Reference in new issue
Block a user