diff --git a/index.js b/index.js index cbdee7e..804a148 100644 --- a/index.js +++ b/index.js @@ -1,4 +1,4 @@ -import { randomUUID } from "node:crypto"; +import { createHash } from "node:crypto"; import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; import { StdioServerTransport } from "@modelcontextprotocol/sdk/server/stdio.js"; import { z } from "zod"; @@ -32,6 +32,56 @@ async function call(path, { body, params } = {}) { return { content: [{ type: "text", text }], isError: text.includes('"FAILED"') }; } +/** + * Client-side send guard. + * + * `transactionId` was minted with randomUUID() on every call, so two calls + * describing the SAME shipment looked like two different shipments to the API. + * An agent that retries after a timeout — the response was lost, the SMS was + * already sent — therefore sends the message twice. + * + * Making transactionId deterministic is not on its own enough: the README is + * explicit that the API does not assign or interpret that field + * ("transactionId generuje serwer (UUID) - API go nie nadaje"), so there is no + * basis to assume it deduplicates on it. The guard has to live here. + * + * Scope, stated plainly: this is in-memory, so it protects a retry within one + * running server. It does NOT survive a restart, and it is per-process. That + * covers the common case (an agent retrying a call seconds later) and not the + * rarer one (the process dies mid-send). A durable store would close that too; + * this is the smallest change that fixes the failure people actually hit. + */ +const DEDUP_TTL_MS = Number(process.env.SMSON_DEDUP_TTL_MS ?? 15 * 60 * 1000); +const recentSends = new Map(); // transactionId -> { at, result } + +/** Derive a stable id from what makes one shipment different from another. */ +function deriveTransactionId(payload) { + return ( + "tx_" + + createHash("sha256").update(JSON.stringify(payload)).digest("hex").slice(0, 32) + ); +} + +function rememberedResult(id) { + const hit = recentSends.get(id); + if (!hit) return null; + if (Date.now() - hit.at > DEDUP_TTL_MS) { + recentSends.delete(id); + return null; + } + return hit.result; +} + +function remember(id, result) { + // Bound the map so a long-running server cannot grow without limit. + if (recentSends.size > 1000) { + for (const [k, v] of recentSends) { + if (Date.now() - v.at > DEDUP_TTL_MS) recentSends.delete(k); + } + } + recentSends.set(id, { at: Date.now(), result }); +} + const server = new McpServer({ name: "smson", version: "1.0.0" }); server.registerTool( @@ -53,30 +103,60 @@ server.registerTool( ttl: z.string().optional().describe("Message validity, e.g. 1_HOUR"), normalize: z.boolean().optional().describe("Strip Polish diacritics from content"), timestamp: z.string().optional().describe("Scheduled send time, e.g. 2025-08-02T13:45:00"), + transactionId: z + .string() + .optional() + .describe( + "Idempotency key for this send. Repeat the same value to make a retry safe: if this server already sent that shipment within SMSON_DEDUP_TTL_MS (default 15 min) it returns the original result instead of sending again. Omit it and one is derived from the shipment itself, so retrying an identical send is safe by default." + ), }, }, - ({ messages, sender, ttl, normalize, timestamp }) => - call("/createShipment", { - body: { - transactionId: randomUUID(), - ...(timestamp && { timestamp }), - system: SYSTEM, - createShipment: { - sender: { textId: sender ?? SENDER }, - ...(ttl && { ttl }), - normalize: String(normalize ?? false), - messages: { - multiContent: messages.map((m, i) => ({ - externalId: m.externalId ?? String(i + 1), - MSISDN: m.msisdn, - content: m.content, - })), - }, + async ({ messages, sender, ttl, normalize, timestamp, transactionId }) => { + const shipment = { + ...(timestamp && { timestamp }), + system: SYSTEM, + createShipment: { + sender: { textId: sender ?? SENDER }, + ...(ttl && { ttl }), + normalize: String(normalize ?? false), + messages: { + multiContent: messages.map((m, i) => ({ + externalId: m.externalId ?? String(i + 1), + MSISDN: m.msisdn, + content: m.content, + })), }, }, - }) + }; + + // Caller-supplied key wins; otherwise derive one from the shipment so that + // an identical resend collapses and a genuinely different send does not. + const txId = transactionId ?? deriveTransactionId(shipment); + + const already = rememberedResult(txId); + if (already) { + return { + ...already, + content: [ + { + type: "text", + text: + `Already sent (transactionId ${txId}). Returning the original result ` + + `rather than sending again.\n\n` + + already.content.map((c) => c.text).join("\n"), + }, + ], + }; + } + + const result = await call("/createShipment", { body: { transactionId: txId, ...shipment } }); + // Only remember a send that actually succeeded — a failure must stay retryable. + if (!result.isError) remember(txId, result); + return result; + } ); + server.registerTool( "get_sms_status", { diff --git a/test-dedup.mjs b/test-dedup.mjs new file mode 100644 index 0000000..40e3764 --- /dev/null +++ b/test-dedup.mjs @@ -0,0 +1,100 @@ +/** + * Proves the send guard: a repeated shipment does not reach the API twice, + * a different shipment does, and a failure stays retryable. + * + * Runs the real server over stdio against a local stand-in for smsOn, so it + * exercises the actual tool handler rather than a copy of its logic. + */ +import http from "node:http"; +import { spawn } from "node:child_process"; + +let sends = 0; +let failNext = false; + +const api = http.createServer((req, res) => { + let body = ""; + req.on("data", (c) => (body += c)); + req.on("end", () => { + if (req.url.startsWith("/createShipment")) { + sends++; + const payload = JSON.parse(body); + res.writeHead(200, { "content-type": "application/json" }); + res.end(JSON.stringify( + failNext + ? { status: "FAILED", transactionId: payload.transactionId } + : { shipmentUid: "s" + sends, transactionId: payload.transactionId }, + )); + return; + } + res.writeHead(404); res.end("{}"); + }); +}); + +await new Promise((r) => api.listen(0, "127.0.0.1", r)); +const port = api.address().port; + +const child = spawn(process.execPath, ["index.js"], { + env: { ...process.env, + SMSON_TOKEN: "t", SMSON_ENDPOINT: `http://127.0.0.1:${port}`, + SMSON_SYSTEM: "sys", SMSON_SENDER: "TEST" }, + stdio: ["pipe", "pipe", "inherit"], +}); + +let buf = ""; +const pending = new Map(); +child.stdout.on("data", (d) => { + buf += d; + let i; + while ((i = buf.indexOf("\n")) >= 0) { + const line = buf.slice(0, i).trim(); buf = buf.slice(i + 1); + if (!line) continue; + try { const m = JSON.parse(line); if (pending.has(m.id)) { pending.get(m.id)(m); pending.delete(m.id); } } catch {} + } +}); + +let id = 0; +const rpc = (method, params) => new Promise((resolve) => { + const msgId = ++id; + pending.set(msgId, resolve); + child.stdin.write(JSON.stringify({ jsonrpc: "2.0", id: msgId, method, params }) + "\n"); +}); + +await rpc("initialize", { protocolVersion: "2026-07-28", capabilities: {}, clientInfo: { name: "t", version: "0" } }); +child.stdin.write(JSON.stringify({ jsonrpc: "2.0", method: "notifications/initialized" }) + "\n"); + +const send = (args) => rpc("tools/call", { name: "send_sms", arguments: args }); +const MSG = { messages: [{ msisdn: "48600600600", content: "hello" }] }; + +let failures = 0; +const check = (name, ok, detail = "") => { + console.log(`${ok ? "PASS" : "FAIL"} ${name}${detail ? " — " + detail : ""}`); + if (!ok) failures++; +}; + +await send(MSG); +check("first send reaches the API", sends === 1, `sends=${sends}`); + +await send(MSG); +check("an identical retry does NOT send again", sends === 1, `sends=${sends}`); + +const r = await send(MSG); +check("the retry returns the original result", + JSON.stringify(r).includes("Already sent")); + +await send({ messages: [{ msisdn: "48600600600", content: "different text" }] }); +check("a genuinely different message DOES send", sends === 2, `sends=${sends}`); + +await send({ ...MSG, transactionId: "caller-supplied-key-1" }); +check("a caller-supplied key is honoured", sends === 3, `sends=${sends}`); +await send({ ...MSG, transactionId: "caller-supplied-key-1" }); +check("...and repeating it does not resend", sends === 3, `sends=${sends}`); + +failNext = true; +await send({ messages: [{ msisdn: "48600600601", content: "will fail" }] }); +const after = sends; +await send({ messages: [{ msisdn: "48600600601", content: "will fail" }] }); +check("a FAILED send stays retryable", sends === after + 1, `sends=${sends}`); + +child.kill(); api.close(); +console.log(failures ? `\n${failures} check(s) failed` : "\nsend guard: all checks passed"); +process.exit(failures ? 1 : 0);