diff --git a/src/core/delivery-engine.ts b/src/core/delivery-engine.ts index 16172cc8..3f612ca5 100644 --- a/src/core/delivery-engine.ts +++ b/src/core/delivery-engine.ts @@ -6,6 +6,7 @@ import type { CapabilityToken } from "wire-mesh-core/generated/protocol"; import type { RevocationEntry } from "wire-mesh-core/generated/protocol"; import { bytesFromHex } from "wire-mesh-core/domain/device-id"; import { loadRoomTokens } from "./identity-store.js"; +import { dmRoomPath } from "./room-path.js"; import { MAX_QUEUED_DELIVERIES_PER_AGENT, mergeMessageHistories, @@ -429,7 +430,29 @@ export class DeliveryEngine { } /** - * Delivers an informational event (member_status, member_joined, name_changed, and the like -- never room_message/dm, which already ride handleRoomSend's own directed path) to every current member of a room. Queues locally for every member (matching every other queueDelivery caller's own "hold it for whoever reads it next" contract), then either fires local delivery directly for this store's own agent or sends a real, wire-authenticated room.notify to everyone else -- replacing the legacy mesh-wide broadcastPatch this used to ride via deliverLocallyAndBroadcast, per P3.8's own directed-delivery retirement (agent-comms#48). Silently skips a member this store holds no current room:member token for, the same best-effort-by-design choice markRead's own directed room.read already makes for an unreachable read receipt. + * Delivers a single informational event to one specific agent over a given room-path: queues it locally (matching every other queueDelivery caller's own "hold it for whoever reads it next" contract), then either fires local delivery directly for this store's own agent or sends a real, wire-authenticated room.notify -- replacing the legacy mesh-wide broadcastPatch every one of this method's callers used to ride via deliverLocallyAndBroadcast, per P3.8's own directed-delivery retirement (agent-comms#48). Silently does nothing beyond the local queue when this store holds no current room:member token for roomPath, the same best-effort-by-design choice markRead's own directed room.read already makes for an unreachable read receipt. + */ + private async deliverToMember( + memberId: string, + roomPath: string, + event: DeliveryEvent, + ): Promise { + this.queueDelivery(memberId, event); + if (memberId === this.deps.getPeerId()) { + this.fireLocalDelivery(memberId, event); + return; + } + const { slot } = this.deps.requireIdentity(); + const token = loadRoomTokens(slot)[roomPath]; + if (token === undefined) return; + await this.deps.sendRoomRequestToMember(memberId, roomPath, token, { + verb: "room.notify", + event, + }); + } + + /** + * Delivers an informational event (member_status, member_joined, name_changed, and the like -- never room_message/dm, which already ride handleRoomSend's own directed path) to every current member of a room, via deliverToMember for each. */ async deliverToRoom( roomId: string, @@ -438,21 +461,9 @@ export class DeliveryEngine { ): Promise { const room = this.deps.rooms.get(roomId); if (!room) return; - const peerId = this.deps.getPeerId(); for (const memberId of room.members) { if (memberId === excludeAgent) continue; - this.queueDelivery(memberId, event); - if (memberId === peerId) { - this.fireLocalDelivery(memberId, event); - continue; - } - const { slot } = this.deps.requireIdentity(); - const token = loadRoomTokens(slot)[roomId]; - if (token === undefined) continue; - await this.deps.sendRoomRequestToMember(memberId, roomId, token, { - verb: "room.notify", - event, - }); + await this.deliverToMember(memberId, roomId, event); } } @@ -488,8 +499,14 @@ export class DeliveryEngine { for (const roomId of agent.subscribedRooms) { await this.deliverToRoom(roomId, event, agentId); } - // Also deliver to the agent itself so it sees confirmation - await this.deliverLocallyAndBroadcast(agentId, event); + // Also deliver to the agent itself so it sees confirmation -- addressed via the implicit DM path when the renamed agent is a remote peer (e.g. renamed through the web console's own directory, which places no local-only restriction on which agent it targets), since a name change has no room context of its own to ride. + const peerId = this.deps.getPeerId(); + if (agentId === peerId) { + this.queueDelivery(agentId, event); + this.fireLocalDelivery(agentId, event); + return; + } + await this.deliverToMember(agentId, dmRoomPath(peerId, agentId), event); } private async emitDeliveryStatus( @@ -498,39 +515,25 @@ export class DeliveryEngine { status: DeliveryStatus, room?: string, ): Promise { - // Find the sender for this message - const senderId = this.findMessageSender(messageId, room); - if (senderId === undefined) return; - await this.deliverLocallyAndBroadcast(senderId, { + const location = this.findMessageLocation(messageId, room); + if (location === undefined) return; + const { roomPath, from: senderId } = location; + const event: DeliveryEvent = { type: "delivery_status", messageId, agent: agentId, status, room, - }); - } - - private findMessageSender( - messageId: string, - room?: string, - ): string | undefined { - if (room !== undefined) { - const msgs = this.deps.messages.get(room); - if (msgs) { - const msg = msgs.find((m) => m.id === messageId); - if (msg) return msg.from; - } - } else { - // DM — search all DM queues - for (const [, msgs] of this.deps.dms) { - const msg = msgs.find((m) => m.id === messageId); - if (msg) return msg.from; - } + }; + if (senderId === this.deps.getPeerId()) { + this.queueDelivery(senderId, event); + this.fireLocalDelivery(senderId, event); + return; } - return undefined; + await this.deliverToMember(senderId, roomPath, event); } - /** Like findMessageSender, but also returns the room-path a room.read needs to address: room itself for a room message, or the specific DM key (this.dms is keyed by dmRoomPath/"self:...", not the bare pair) the message was actually found under. */ + /** Returns the room-path a directed room.notify/room.read needs to address for a given message: room itself for a room message, or the specific DM key (this.dms is keyed by dmRoomPath/"self:...", not the bare pair) the message was actually found under. */ private findMessageLocation( messageId: string, room?: string, diff --git a/src/test/delivery-engine-delivery.test.ts b/src/test/delivery-engine-delivery.test.ts index 9ef9067a..c88df515 100644 --- a/src/test/delivery-engine-delivery.test.ts +++ b/src/test/delivery-engine-delivery.test.ts @@ -656,6 +656,8 @@ describe("DeliveryEngine — notifyRoomsOfNameChange", () => { expect(peerQueue.filter((e) => e.type === "name_changed")).toHaveLength(1); expect(selfQueue.filter((e) => e.type === "name_changed")).toHaveLength(1); }); + + // Remote-agent directed-notify cases for notifyRoomsOfNameChange's own trailing self/DM-path delivery live in delivery-engine-directed-notify.test.ts, alongside deliverToRoom's and emitDeliveryStatus's own directed cases, to stay under this file's own max-lines budget. }); // --------------------------------------------------------------------------- @@ -711,6 +713,8 @@ describe("DeliveryEngine — emitDeliveryStatus via deliverLocallyAndBroadcast", }), ); }); + + // Remote-sender directed-notify cases for emitDeliveryStatus live in delivery-engine-directed-notify.test.ts, alongside deliverToRoom's and notifyRoomsOfNameChange's own directed cases, to stay under this file's own max-lines budget. }); // --------------------------------------------------------------------------- diff --git a/src/test/delivery-engine-directed-notify.test.ts b/src/test/delivery-engine-directed-notify.test.ts index cdcfdc73..e39cd6dd 100644 --- a/src/test/delivery-engine-directed-notify.test.ts +++ b/src/test/delivery-engine-directed-notify.test.ts @@ -1,16 +1,24 @@ /** - * Direct, DI-based unit tests for DeliveryEngine.deliverToRoom's directed room.notify replacement (P3.8, agent-comms#48) -- moved into its own file to stay under the repo's max-lines cap once delivery-engine.test.ts and delivery-engine-delivery.test.ts were already at capacity. Shares the same fake-harness convention those two files established: a narrow, injectable DeliveryEngineDeps surface with every collaborator boundary (transport, sendRoomRequestToMember, onDelivery/onPatch callbacks) a vi.fn() this file controls per test. + * Direct, DI-based unit tests for DeliveryEngine's directed room.notify replacements (P3.8, agent-comms#48): deliverToRoom, notifyRoomsOfNameChange's own trailing self/DM-path delivery, and emitDeliveryStatus's own remote-sender delivery -- moved into its own file to stay under the repo's max-lines cap once delivery-engine.test.ts and delivery-engine-delivery.test.ts were already at capacity. Shares the same fake-harness convention those two files established: a narrow, injectable DeliveryEngineDeps surface with every collaborator boundary (transport, sendRoomRequestToMember, onDelivery/onPatch callbacks) a vi.fn() this file controls per test. */ import { beforeEach, describe, expect, it, vi } from "vitest"; import { loadRoomTokens } from "../core/identity-store.js"; +import { randomId } from "../core/random-id.js"; import { DeliveryEngine, type DeliveryEngineDeps, } from "../core/delivery-engine.js"; import type { MeshTransport } from "../core/transport.js"; import type { MeshStatePatch } from "../core/wire-protocol.js"; -import type { AgentIdentity, DeliveryEvent, Room } from "../core/types.js"; +import type { + AgentIdentity, + DeliveryEvent, + DmMessage, + Room, + RoomMessage, +} from "../core/types.js"; import type { CapabilityToken } from "wire-mesh-core/generated/protocol"; +import { bytesToHex } from "wire-mesh-core/domain/device-id"; vi.mock("../core/identity-store.js", () => ({ loadRoomTokens: vi.fn(), @@ -22,9 +30,15 @@ const FAKE_TOKEN = "fake-token" as unknown as CapabilityToken; /** A device-id is a 64-character lowercase hex SHA-256 digest; room-path.ts's assertDeviceIdHex rejects anything shorter. */ const DEVICE_ID_HEX_LENGTH = 64; const PEER_ID = "a".repeat(DEVICE_ID_HEX_LENGTH); +const OTHER_ID = "b".repeat(DEVICE_ID_HEX_LENGTH); const THIRD_ID = "c".repeat(DEVICE_ID_HEX_LENGTH); const NOW_MS = 1_700_000_000_000; +/** A fresh, distinct, real device-id-shaped hex id, matching delivery-engine-delivery.test.ts's own precedent for the same purpose. */ +function messageId(): string { + return bytesToHex(randomId()); +} + function room(overrides: Partial = {}): Room { return { id: "room-1", @@ -44,6 +58,47 @@ function room(overrides: Partial = {}): Room { }; } +function agent(overrides: Partial = {}): AgentIdentity { + return { + id: OTHER_ID, + version: 1, + name: "recipient", + harness: "pi", + cwd: "/tmp", + pid: 111, + startedAt: "2026-01-01T00:00:00.000Z", + visibility: "visible", + status: "active", + tags: [], + subscribedRooms: [], + ...overrides, + }; +} + +function roomMessage(overrides: Partial = {}): RoomMessage { + return { + id: messageId(), + from: PEER_ID, + room: "room-1", + content: "hi", + timestamp: "2026-01-01T00:00:00.000Z", + readBy: [], + ...overrides, + }; +} + +function dmMessage(overrides: Partial = {}): DmMessage { + return { + id: messageId(), + from: PEER_ID, + to: OTHER_ID, + content: "hi", + timestamp: "2026-01-01T00:00:00.000Z", + readBy: [], + ...overrides, + }; +} + function fakeTransport(): MeshTransport { return { dataPort: 4000, @@ -211,3 +266,140 @@ describe("DeliveryEngine — deliverToRoom directed room.notify", () => { expect(h.deps.deliveryQueues.get(THIRD_ID)).toBeUndefined(); }); }); + +describe("DeliveryEngine — notifyRoomsOfNameChange directed self/DM-path delivery", () => { + it("sends a directed room.notify to a remote renamed agent over the implicit DM path, never broadcasting mesh-wide", async () => { + const h = makeHarness(); + const dmPath = `${PEER_ID}+${OTHER_ID}`; + vi.mocked(loadRoomTokens).mockReturnValue({ [dmPath]: FAKE_TOKEN }); + h.deps.agents.set(OTHER_ID, agent({ subscribedRooms: [] })); + + await h.engine.notifyRoomsOfNameChange(OTHER_ID, "old-name", "new-name"); + + expect(h.sendRoomRequestToMember).toHaveBeenCalledWith( + OTHER_ID, + dmPath, + FAKE_TOKEN, + { + verb: "room.notify", + event: { + type: "name_changed", + agent: OTHER_ID, + oldName: "old-name", + newName: "new-name", + }, + }, + ); + expect(h.transport.broadcast).not.toHaveBeenCalled(); + }); + + it("fires local delivery directly when renaming this store's own agent, with no DM path involved", async () => { + const h = makeHarness(); + const onDelivery = vi.fn(); + h.setOnDelivery(onDelivery); + h.deps.agents.set(PEER_ID, agent({ id: PEER_ID, subscribedRooms: [] })); + + await h.engine.notifyRoomsOfNameChange(PEER_ID, "old-name", "new-name"); + + expect(onDelivery).toHaveBeenCalledWith( + PEER_ID, + expect.objectContaining({ type: "name_changed" }), + ); + expect(h.sendRoomRequestToMember).not.toHaveBeenCalled(); + expect(h.transport.broadcast).not.toHaveBeenCalled(); + }); + + it("silently skips a remote renamed agent this store holds no DM-path token for", async () => { + const h = makeHarness(); + vi.mocked(loadRoomTokens).mockReturnValue({}); + h.deps.agents.set(OTHER_ID, agent({ subscribedRooms: [] })); + + await expect( + h.engine.notifyRoomsOfNameChange(OTHER_ID, "old-name", "new-name"), + ).resolves.toBeUndefined(); + expect(h.sendRoomRequestToMember).not.toHaveBeenCalled(); + }); +}); + +describe("DeliveryEngine — emitDeliveryStatus directed remote-sender delivery", () => { + it("sends a directed room.notify to a remote room message's sender over the room's own path", async () => { + const h = makeHarness(); + const target = roomMessage({ id: messageId(), from: THIRD_ID }); + h.deps.messages.set("room-1", [target]); + vi.mocked(loadRoomTokens).mockReturnValue({ "room-1": FAKE_TOKEN }); + + await h.engine.deliver(OTHER_ID, { + type: "room_message", + message: target, + }); + + expect(h.sendRoomRequestToMember).toHaveBeenCalledWith( + THIRD_ID, + "room-1", + FAKE_TOKEN, + { + verb: "room.notify", + event: { + type: "delivery_status", + messageId: target.id, + agent: OTHER_ID, + status: "delivered", + room: "room-1", + }, + }, + ); + // deliver()'s own outer broadcastPatch (for the room_message delivery to OTHER_ID) is untouched by this change and still fires -- only the delivery_status sub-event this test targets must never itself ride a broadcast. + expect(h.transport.broadcast).not.toHaveBeenCalledWith( + expect.objectContaining({ + patch: expect.objectContaining({ + event: expect.objectContaining({ type: "delivery_status" }), + }), + }), + ); + }); + + it("sends a directed room.notify to a remote dm's sender over the dm's own path", async () => { + const h = makeHarness(); + const dmPath = "self:x"; + const target = dmMessage({ id: messageId(), from: THIRD_ID }); + h.deps.dms.set(dmPath, [target]); + vi.mocked(loadRoomTokens).mockReturnValue({ [dmPath]: FAKE_TOKEN }); + + await h.engine.deliver(OTHER_ID, { type: "dm", message: target }); + + expect(h.sendRoomRequestToMember).toHaveBeenCalledWith( + THIRD_ID, + dmPath, + FAKE_TOKEN, + { + verb: "room.notify", + event: { + type: "delivery_status", + messageId: target.id, + agent: OTHER_ID, + status: "delivered", + room: undefined, + }, + }, + ); + expect(h.transport.broadcast).not.toHaveBeenCalledWith( + expect.objectContaining({ + patch: expect.objectContaining({ + event: expect.objectContaining({ type: "delivery_status" }), + }), + }), + ); + }); + + it("silently skips a remote sender this store holds no room-path token for", async () => { + const h = makeHarness(); + const target = roomMessage({ id: messageId(), from: THIRD_ID }); + h.deps.messages.set("room-1", [target]); + vi.mocked(loadRoomTokens).mockReturnValue({}); + + await expect( + h.engine.deliver(OTHER_ID, { type: "room_message", message: target }), + ).resolves.toBeUndefined(); + expect(h.sendRoomRequestToMember).not.toHaveBeenCalled(); + }); +});