Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
154 changes: 154 additions & 0 deletions assistant/src/runtime/__tests__/mirror-invite-to-gateway.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,154 @@
/**
* mirrorInviteToGateway — daemon-side best-effort behavior.
*
* Track B PR-B-1: a mirror failure must NEVER throw to the createIngressInvite
* caller (the daemon-owned authoritative write is the source of truth). This
* test pins that contract by mocking the gateway IPC client to reject and
* asserting the helper resolves normally.
*
* Also asserts the wire payload contains every field daemon-side
* IngressInvite carries — so future schema additions on either side don't
* silently drift.
*/

import { beforeEach, describe, expect, mock, test } from "bun:test";

type IpcCallArgs = {
method: string;
params?: Record<string, unknown>;
};

const ipcCalls: IpcCallArgs[] = [];
let ipcCallImpl: (
method: string,
params?: Record<string, unknown>,
) => Promise<unknown> = async () => ({});

mock.module("../../ipc/gateway-client.js", () => ({
ipcCall: async (method: string, params?: Record<string, unknown>) => {
ipcCalls.push({ method, params });
return ipcCallImpl(method, params);
},
}));

// invite-service pulls a bunch of LLM/channel-adapter modules eagerly. Stub
// the ones that touch real I/O so the import doesn't side-effect.
mock.module("../channel-invite-transport.js", () => ({
getInviteAdapterRegistry: () => ({}),
resolveAdapterHandle: () => undefined,
}));
mock.module("../invite-instruction-generator.js", () => ({
generateInviteInstruction: async () => "",
}));
mock.module("../invite-redemption-service.js", () => ({
redeemInvite: async () => ({}),
redeemVoiceInviteCode: async () => ({}),
redeemInviteByCode: async () => ({}),
}));
mock.module("../calls/call-domain.js", () => ({
startInviteCall: async () => ({}),
}));

const { mirrorInviteToGateway } = await import("../invite-service.js");

const baseInvite = () => ({
id: "inv-daemon-1",
sourceChannel: "telegram",
tokenHash: "tok-h",
sourceConversationId: null,
note: null,
maxUses: 1,
useCount: 0,
expiresAt: Date.now() + 60_000,
status: "active" as const,
redeemedByExternalUserId: null,
redeemedByExternalChatId: null,
redeemedAt: null,
expectedExternalUserId: null,
voiceCodeHash: null,
voiceCodeDigits: null,
inviteCodeHash: null,
friendName: null,
guardianName: null,
contactId: "co-1",
createdAt: Date.now(),
updatedAt: Date.now(),
});

beforeEach(() => {
ipcCalls.length = 0;
ipcCallImpl = async () => ({});
});

describe("mirrorInviteToGateway", () => {
test("fires mirror_invite_create with the full payload", async () => {
const invite = baseInvite();
await mirrorInviteToGateway(invite);

expect(ipcCalls).toHaveLength(1);
expect(ipcCalls[0]!.method).toBe("mirror_invite_create");
const params = ipcCalls[0]!.params!;

// Spot-check every field that flows over the wire.
for (const key of [
"id",
"sourceChannel",
"tokenHash",
"sourceConversationId",
"note",
"maxUses",
"useCount",
"expiresAt",
"status",
"redeemedByExternalUserId",
"redeemedByExternalChatId",
"redeemedAt",
"expectedExternalUserId",
"voiceCodeHash",
"voiceCodeDigits",
"inviteCodeHash",
"friendName",
"guardianName",
"contactId",
"createdAt",
"updatedAt",
] as const) {
expect(params).toHaveProperty(key);
}

expect(params.id).toBe(invite.id);
expect(params.contactId).toBe(invite.contactId);
expect(params.tokenHash).toBe(invite.tokenHash);
});

test("swallows IPC errors (best-effort dual-write)", async () => {
ipcCallImpl = async () => {
throw new Error("gateway down");
};

// The promise must resolve, not reject.
await expect(mirrorInviteToGateway(baseInvite())).resolves.toBeUndefined();
expect(ipcCalls).toHaveLength(1);
});

test("forwards voice-invite fields when present", async () => {
const invite = {
...baseInvite(),
sourceChannel: "phone",
expectedExternalUserId: "+15551234567",
voiceCodeHash: "voice-h",
voiceCodeDigits: 6,
friendName: "Alice",
guardianName: "Bob",
};
await mirrorInviteToGateway(invite);

const params = ipcCalls[0]!.params!;
expect(params.sourceChannel).toBe("phone");
expect(params.expectedExternalUserId).toBe("+15551234567");
expect(params.voiceCodeHash).toBe("voice-h");
expect(params.voiceCodeDigits).toBe(6);
expect(params.friendName).toBe("Alice");
expect(params.guardianName).toBe("Bob");
});
});
54 changes: 54 additions & 0 deletions assistant/src/runtime/invite-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

import { startInviteCall } from "../calls/call-domain.js";
import { isChannelId } from "../channels/types.js";
import { ipcCall } from "../ipc/gateway-client.js";
import {
createInvite,
findById,
Expand All @@ -26,6 +27,7 @@ import {
DEFAULT_USER_REFERENCE,
resolveGuardianName,
} from "../prompts/user-reference.js";
import { getLogger } from "../util/logger.js";
import { isValidE164 } from "../util/phone.js";
import { generateVoiceCode, hashVoiceCode } from "../util/voice-code.js";
import {
Expand All @@ -39,6 +41,51 @@ import {
type VoiceRedemptionOutcome,
} from "./invite-redemption-service.js";

const log = getLogger("invite-service");

/**
* Mirror an authoritative daemon-side invite row to the gateway's
* `ingress_invites` table via IPC. Best-effort: logs a warn on failure
* and never throws — the daemon's own write is the source of truth during
* Track B PR-B-1.
*
* See: memory/concepts/workstreams/track-b-invite-redemption.md
*/
export async function mirrorInviteToGateway(
invite: IngressInvite,
): Promise<void> {
try {
await ipcCall("mirror_invite_create", {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Detect undefined IPC mirror result before treating as success

This call assumes mirror failures will throw and be caught, but the IPC helper returns undefined for transport and handler errors instead of rejecting. As a result, normal mirror failures bypass the catch block and never emit the invite-scoped warning (inviteId/contactId), making dual-write drift harder to diagnose; you need an explicit result === undefined check after the call.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No. We do not call any gateway write actions from the assistant daemon

id: invite.id,
sourceChannel: invite.sourceChannel,
tokenHash: invite.tokenHash,
sourceConversationId: invite.sourceConversationId,
note: invite.note,
maxUses: invite.maxUses,
useCount: invite.useCount,
expiresAt: invite.expiresAt,
status: invite.status,
redeemedByExternalUserId: invite.redeemedByExternalUserId,
redeemedByExternalChatId: invite.redeemedByExternalChatId,
redeemedAt: invite.redeemedAt,
expectedExternalUserId: invite.expectedExternalUserId,
voiceCodeHash: invite.voiceCodeHash,
voiceCodeDigits: invite.voiceCodeDigits,
inviteCodeHash: invite.inviteCodeHash,
friendName: invite.friendName,
guardianName: invite.guardianName,
contactId: invite.contactId,
createdAt: invite.createdAt,
updatedAt: invite.updatedAt,
});
} catch (err) {
log.warn(
{ err, inviteId: invite.id, contactId: invite.contactId },
"createIngressInvite: gateway mirror dual-write failed (best-effort)",
);
}
Comment on lines +81 to +86

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 mirrorInviteToGateway catch block is dead code — ipcCall never rejects

The catch block in mirrorInviteToGateway will never execute for standard IPC failures because the underlying one-shot ipcCall (from packages/gateway-client/src/ipc-client.ts:72-172) is explicitly designed to never reject — it resolves with undefined on every error path (socket error, timeout, gateway error response). The Promise constructor at packages/gateway-client/src/ipc-client.ts:81 only takes a resolve callback with no reject. Every other caller of ipcCall in the codebase correctly checks result === undefined (e.g., assistant/src/runtime/routes/conversation-routes.ts:1276, assistant/src/permissions/gateway-threshold-reader.ts:214) instead of using try/catch. As a result, the daemon-side log.warn with inviteId and contactId context at line 82-85 will never fire, making mirror failures unobservable from the daemon's perspective (the gateway-client's own generic warning logs still fire, but without invite-specific context).

Prompt for agents
The one-shot ipcCall from @vellumai/gateway-client never throws/rejects — it always resolves (with undefined on failure). The try/catch in mirrorInviteToGateway at assistant/src/runtime/invite-service.ts:57-86 is dead code for all standard IPC failure modes.

To fix: replace the try/catch pattern with a return-value check, consistent with how other callers use ipcCall (see assistant/src/permissions/gateway-threshold-reader.ts:210-215 and assistant/src/runtime/routes/conversation-routes.ts:1272-1277 for the established pattern).

The fix should look like:

  const result = await ipcCall("mirror_invite_create", { ...fields... });
  if (result === undefined) {
    log.warn(
      { inviteId: invite.id, contactId: invite.contactId },
      "createIngressInvite: gateway mirror dual-write failed (best-effort)",
    );
  }

Also update the test at assistant/src/runtime/__tests__/mirror-invite-to-gateway.test.ts:124-132 — the 'swallows IPC errors' test currently mocks ipcCall to throw, which doesn't match the real behavior. Instead, mock it to return undefined and verify the function still resolves normally.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

}

// ---------------------------------------------------------------------------
// Response shapes — used by both HTTP routes and message handlers
// ---------------------------------------------------------------------------
Expand Down Expand Up @@ -232,6 +279,13 @@ export async function createIngressInvite(params: {
: { inviteCodeHash }),
});

// Dual-write to the gateway's mirror table. Best-effort during Track B
// PR-B-1 — gateway DB is the future source of truth, assistant DB is the
// present-day source of truth, and the daemon owns invite creation today
// (LLM-generated guardian-instruction + channel-adapter resolution stay
// daemon-side for now). PR-B-2 will flip redemption gateway-native.
await mirrorInviteToGateway(invite);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Avoid awaiting best-effort mirror in invite creation path

createIngressInvite now waits for the gateway mirror call before returning, so invite creation latency is coupled to gateway IPC health even though the mirror is documented as best-effort. The one-shot IPC client can wait on connect/call timeouts before returning undefined (see assistant/src/ipc/gateway-client.ts and packages/gateway-client/src/ipc-client.ts), which means a slow or wedged gateway can add multi-second delay to a user-facing invite create that should succeed from the daemon write alone.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚩 mirrorInviteToGateway blocks invite creation on IPC round-trip

At assistant/src/runtime/invite-service.ts:287, await mirrorInviteToGateway(invite) is called synchronously in the createIngressInvite flow, meaning invite creation blocks until the gateway mirror write completes (or the IPC call times out — default 5 seconds from packages/gateway-client/src/ipc-client.ts:52). Since the mirror is documented as best-effort and the daemon's own write is the source of truth, this could be fire-and-forget (void mirrorInviteToGateway(invite)) to avoid adding latency to the invite creation hot path. The current await means a slow/down gateway adds up to 5 seconds of latency to every invite creation. This is a design choice but worth flagging given the PR's "best-effort" framing.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.


// Build invite instruction for non-voice invites via LLM generation
let guardianInstruction: string | undefined;
let channelHandle: string | undefined;
Expand Down
Loading
Loading