From e177f22723a4060950e623d3ae4af492bce64c61 Mon Sep 17 00:00:00 2001 From: Andrei Hasna Date: Wed, 5 Aug 2026 03:35:16 +0300 Subject: [PATCH 1/2] fix(send): stop reporting a fully successful write as a failure `send` and `reply` exited 1 on every hosted write while the message landed correctly, because the client assumed two server capabilities that the deployed server does not have. `POST /v1/messages` does not accept a caller `uuid` (absent from its published request schema), so the server drops ours, mints its own, and returns OUR row under a different UUID. `GET /v1/messages/by-uuid/{uuid}` does not exist and falls through to the generic unknown-route handler, whose 404 is indistinguishable by status from a real row-miss: /v1/messages/by-uuid/ -> 404 {"error":"Not found"} /v1/definitely-not-a-route -> 404 {"error":"Not found"} /v1/messages/999999999 -> 404 {"error":"Message not found"} `getMessageByUuid` maps any 404 to null, so a missing ROUTE became "the row is not there" and a demonstrably successful write was reported as failed. The exit code was not the harm: the natural response to "your write may not have landed" is to re-send, on a shared channel, where the retry reports the same false failure. After the authoritative UUID read-back has been tried and found unanswerable, `sendMessage` now checks whether the row the server DID return is the write just submitted, by the routing identity the caller controls. A response naming some other row -- the mention-notification DM the UUID binding exists to catch -- is still refused loudly. Note the recipient handling: on a channel post the server rewrites `to_agent` to the channel while the CLI passes `to: to || from`, so comparing against `opts.to` rejects every correct channel send. That was caught by running the real CLI against the real server; the first unit fixture had been written with to == channel and could not fail. Tests are two-sided: a successful write must exit 0 AND report its id, and an unconfirmable write must still throw, on both the channel and DM paths. Refs: todos d8f3f963 Agent: Silvanus --- CHANGELOG.md | 21 ++++ package.json | 2 +- src/lib/store/api-store.test.ts | 187 ++++++++++++++++++++++++++++++++ src/lib/store/api-store.ts | 101 ++++++++++++++++- 4 files changed, 304 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4daa9cc..6ad52c3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,27 @@ All notable changes to this project will be documented in this file. ## Unreleased +## 0.5.25 - 2026-08-05 + +### Fixed +- **`send` and `reply` no longer report a fully successful write as a failure.** Against the deployed server every hosted write exited 1 with `Message write returned UUID instead of , and the exact row could not be read back. Refusing to report a numeric message id.` — while the message landed, in the right channel, correctly threaded, with the right content and sender. The exit code was not the harm: the natural response to "your write may not have landed" is to re-send, on a shared channel, where the retry reported the same false failure. It also withheld the message id that the fleet's citation conventions depend on (todos `d8f3f963`). + + Two absent server capabilities were required to reproduce, and the client assumed both. `POST /v1/messages` does not accept a caller `uuid` — it is absent from the route's published request schema — so the server drops it, mints its own, and returns **our row** under a different UUID. `GET /v1/messages/by-uuid/{uuid}` does not exist at all, and falls through to the generic unknown-route handler, whose 404 is **indistinguishable by status** from a real row-miss: + + ``` + /v1/messages/by-uuid/ -> 404 {"error":"Not found"} + /v1/definitely-not-a-route -> 404 {"error":"Not found"} + /v1/messages/999999999 -> 404 {"error":"Message not found"} <- route that DOES exist + ``` + + `getMessageByUuid` maps any 404 to `null`, so a missing **route** became "the row is not there", and a write that had demonstrably succeeded was reported as a failure. `sendMessage` now falls back — only after the authoritative UUID read-back has been tried and found unanswerable — to checking whether the row the server *did* return is the write just submitted, by the routing identity the caller controls. A response describing some other row (the mention-notification DM the UUID binding exists to catch) is still refused, loudly. + +- **The caller-UUID guarantee 0.5.23 announced was never true against the deployed server.** That release's note claims "hosted writes preserve caller-generated UUIDs (#77)". They do not — the server has no `uuid` field on its create route. 0.5.23 went to the `next` dist-tag with no changelog entry and no release tag, so `latest` installs went 0.5.22 → 0.5.24 and #77 first reached the fleet **in 0.5.24**, which is why the onset tracks the 0.5.24 publish while the causing change shipped in 0.5.23. + +### Known gaps +- The underlying conflation is unchanged: `getMessageByUuid` still cannot distinguish a missing route from a missing row, because it has only the HTTP status to go on. The repair is scoped to `sendMessage`, where the false failure was reachable; the other callers are user-facing lookups where "not found" is an honest answer. A discriminated result type would change the `ConversationsStore` interface and every implementation, so it is deliberately not in this patch. +- The accept path proves the returned row is addressed exactly as requested and carries a usable id. It does **not** prove the row is not some other message with identical routing. That residual is accepted only where the alternative is failing 100% of successful writes on a server that cannot be asked. + ## 0.5.24 - 2026-08-05 ### Added diff --git a/package.json b/package.json index 1838dd0..9c2638b 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@hasna/conversations", - "version": "0.5.24", + "version": "0.5.25", "description": "Real-time CLI messaging for AI agents", "type": "module", "bin": { diff --git a/src/lib/store/api-store.test.ts b/src/lib/store/api-store.test.ts index 7973b07..a17d3d7 100644 --- a/src/lib/store/api-store.test.ts +++ b/src/lib/store/api-store.test.ts @@ -292,6 +292,193 @@ describe("ApiStore.sendMessage wire body", () => { expect(message).toMatchObject({ id: 649560, uuid: exactUuid, channel: "git-publishing" }); }); + // ── regression: rc=1 on a write that fully succeeded (todos d8f3f963) ──────── + // + // MEASURED against the live deployed server, conversations.hasna.xyz, on + // 2026-08-05. Both halves of the contract the caller-bound UUID check assumes + // are absent there, and BOTH are needed to reproduce: + // + // 1. `POST /v1/messages` does not accept a caller `uuid`. Its published + // request schema lists exactly from,to,content,channel,project_id, + // session_id,priority,blocking — no uuid. The server drops ours, mints + // its own, stores THE CORRECT ROW, and returns it: + // sent uuid=0c57bc9f-480f-4e74-b263-49c2e8850a0a + // returned uuid=d9ad71d6-1417-414b-b4d3-bc35b789f5a6 HTTP 201 + // returned content/channel/from == exactly what was submitted + // 2. `GET /v1/messages/by-uuid/{uuid}` does not exist. It falls through to + // the generic unknown-route handler, which is a 404 INDISTINGUISHABLE BY + // STATUS from a real row-miss: + // /v1/messages/by-uuid/ -> 404 {"error":"Not found"} + // /v1/definitely-not-a-route -> 404 {"error":"Not found"} + // /v1/messages/999999999 -> 404 {"error":"Message not found"} + // (the third is the positive control: a route that DOES exist answers a + // miss with its own body, so the discriminator can fire both ways.) + // + // `getMessageByUuid` swallows any 404 as `null`, so a missing ROUTE became + // "the row is not there", and sendMessage reported a successful write as a + // failure. The exit code is not the harm: the caller's natural response to + // "your write may not have landed" is to re-send, on a shared channel, where + // the retry reports the same false failure. + test("reports the row it wrote when the server drops the caller UUID and has no by-uuid route", async () => { + const submittedUuid = "aaaaaaaa-1111-4222-8333-444444444444"; + const notFound = Object.assign(new Error("Not Found"), { name: "HasnaHttpError", status: 404 }); + const client = { + name: "conversations", + baseUrl: "https://conversations.hasna.xyz/v1", + transport: { + // The by-uuid route does not exist on this server: every read 404s. + get: async () => { + throw notFound; + }, + } as unknown as HasnaStorageClient["transport"], + // Server-minted UUID, but unmistakably the row we asked to be written. + // + // NOTE THE RECIPIENT. On a channel send the server rewrites `to_agent` to + // the CHANNEL, discarding whatever the caller passed — measured: + // sent to="silvanus" channel="scratch-d8f3f963" + // returned to_agent="scratch-d8f3f963" + // and `src/cli/commands/messaging.ts` passes `to: to || from`, i.e. the + // SENDER. An earlier draft of this fixture used to == channel, which made + // it agree with a check that rejected every real channel send; the live + // CLI caught it, this fixture did not. It now mirrors the real call. + create: async () => ({ + message: { + id: "668569", + uuid: "bbbbbbbb-5555-4666-8777-888888888888", + session_id: "channel:git-publishing", + from_agent: "silvanus", + to_agent: "git-publishing", + channel: "git-publishing", + content: "[PUBLISH INTENT] @hasna/conversations", + }, + }), + } as unknown as HasnaStorageClient; + const store = new ApiStore(client); + + const message = await store.sendMessage({ + uuid: submittedUuid, + from: "silvanus", + to: "silvanus", // what the CLI actually sends for a channel post + channel: "git-publishing", + content: "[PUBLISH INTENT] @hasna/conversations", + }); + + // The caller must get a usable numeric id back — withholding it is what + // breaks the citation convention that keeps corroboration from collapsing + // to a single source. + expect(message).toMatchObject({ id: 668569, channel: "git-publishing" }); + }); + + // The other side of the same guarantee. A fix that merely stopped throwing + // would pass the test above and destroy the property #77 bought, so this + // pins the case that MUST still fail loudly: the response describes some + // OTHER row (the mention-notification DM the UUID binding exists to catch), + // and our row cannot be read back. + test("still refuses when the response names a different row and the write cannot be confirmed", async () => { + const submittedUuid = "cccccccc-9999-4aaa-8bbb-cccccccccccc"; + const notFound = Object.assign(new Error("Not Found"), { name: "HasnaHttpError", status: 404 }); + const client = { + name: "conversations", + baseUrl: "https://conversations.hasna.xyz/v1", + transport: { + get: async () => { + throw notFound; + }, + } as unknown as HasnaStorageClient["transport"], + create: async () => ({ + message: { + id: 999001, + uuid: "dddddddd-eeee-4fff-8000-111111111111", + session_id: "dm:someone-else", + from_agent: "silvanus", + to_agent: "someone-else", + channel: null, + content: "you were mentioned", + }, + }), + } as unknown as HasnaStorageClient; + const store = new ApiStore(client); + + await expect(store.sendMessage({ + uuid: submittedUuid, + from: "silvanus", + to: "silvanus", + channel: "git-publishing", + content: "[PUBLISH INTENT] @hasna/conversations", + })).rejects.toThrow(/could not be read back/); + }); + + // The DM half of the same guard. With no channel to compare, the recipient is + // the only thing separating our row from a notification DM fanned out to a + // mentioned third party, so it must still be enforced there. + test("still refuses a DM whose response names a different recipient and cannot be read back", async () => { + const notFound = Object.assign(new Error("Not Found"), { name: "HasnaHttpError", status: 404 }); + const client = { + name: "conversations", + baseUrl: "https://conversations.hasna.xyz/v1", + transport: { + get: async () => { + throw notFound; + }, + } as unknown as HasnaStorageClient["transport"], + create: async () => ({ + message: { + id: 999002, + uuid: "eeeeeeee-1111-4222-8333-444444444444", + session_id: "dm:someone-else", + from_agent: "silvanus", + to_agent: "someone-else", + channel: null, + content: "you were mentioned", + }, + }), + } as unknown as HasnaStorageClient; + const store = new ApiStore(client); + + await expect(store.sendMessage({ + uuid: "ffffffff-1111-4222-8333-444444444444", + from: "silvanus", + to: "manius", + content: "a direct message", + })).rejects.toThrow(/could not be read back/); + }); + + // And the DM ACCEPT path, so the DM branch is exercised in both directions + // rather than only proved capable of refusing. + test("reports a DM the server echoed back under a server-minted UUID", async () => { + const notFound = Object.assign(new Error("Not Found"), { name: "HasnaHttpError", status: 404 }); + const client = { + name: "conversations", + baseUrl: "https://conversations.hasna.xyz/v1", + transport: { + get: async () => { + throw notFound; + }, + } as unknown as HasnaStorageClient["transport"], + create: async () => ({ + message: { + id: 668600, + uuid: "11111111-2222-4333-8444-555555555555", + session_id: "dm:manius", + from_agent: "silvanus", + to_agent: "manius", + channel: null, + content: "a direct message", + }, + }), + } as unknown as HasnaStorageClient; + const store = new ApiStore(client); + + const message = await store.sendMessage({ + uuid: "ffffffff-9999-4aaa-8bbb-cccccccccccc", + from: "silvanus", + to: "manius", + content: "a direct message", + }); + + expect(message).toMatchObject({ id: 668600, to_agent: "manius" }); + }); + // `messages.id`/`messages.reply_to` are Postgres BIGINT, and node-postgres // serializes int8 as a STRING. Measured against the live deployed server: // GET /v1/messages returns `"id": "603183"` — a string, not a number. diff --git a/src/lib/store/api-store.ts b/src/lib/store/api-store.ts index fd226eb..5f75b7b 100644 --- a/src/lib/store/api-store.ts +++ b/src/lib/store/api-store.ts @@ -50,6 +50,75 @@ function prune(q: Q): Record { return out; } +/** Case-insensitive compare of two optional identity strings. */ +function sameIdentity(a: unknown, b: unknown): boolean { + const l = typeof a === "string" ? a.trim().toLowerCase() : ""; + const r = typeof b === "string" ? b.trim().toLowerCase() : ""; + return l === r; +} + +/** + * Is `returned` the row this very `sendMessage` call submitted? + * + * Compares the routing identity the CALLER supplied — sender, recipient and + * channel — because that is precisely what separates our row from the hazard + * the caller-bound UUID exists to catch: a mention-notification DM fanned out + * by the same write, which is addressed to a DIFFERENT agent and carries no + * channel. + * + * It deliberately does NOT compare content. The server may legitimately store + * a redacted body, and `attachSendRedaction` is the surface that reports that + * divergence to the author; re-deriving it here would reject a correct write + * for the wrong reason. (`describeSendRedaction` cannot help either — it + * reports that two bodies DIFFER, not that the difference is a redaction, so + * using it as an accept test would accept any difference at all.) + * + * STATE THE LIMIT, because this is an accept path: it proves the returned row + * is addressed exactly as requested and carries a usable id. It does NOT prove + * the row is not some other message with identical routing. That residual is + * accepted only where the alternative is failing 100% of successful writes on + * a server that cannot be asked — a certain, universal false failure traded + * for a narrow ambiguity, and only after the authoritative UUID read-back has + * already been tried and found unanswerable. + */ +function echoesSubmittedWrite( + opts: { from?: string; to?: string; channel?: string | null }, + returned: { id?: unknown; from_agent?: unknown; to_agent?: unknown; channel?: unknown }, +): boolean { + // Without a usable id there is nothing to report, so there is nothing to accept. + const id = Number(returned.id); + if (!Number.isFinite(id) || id <= 0) return false; + + if (!sameIdentity(returned.from_agent, opts.from)) return false; + + const wantChannel = opts.channel ? normalizeChannelName(opts.channel) : ""; + const gotChannel = typeof returned.channel === "string" && returned.channel + ? normalizeChannelName(returned.channel) + : ""; + if (wantChannel !== gotChannel) return false; + + if (wantChannel) { + // CHANNEL POST. The server OWNS the recipient field here and rewrites it to + // the channel, whatever the caller passed. Measured on the deployed server: + // sent to="silvanus" channel="scratch-d8f3f963" + // returned to_agent="scratch-d8f3f963" + // and the CLI's own channel path passes `to: to || from`, i.e. the SENDER. + // So insisting on `to_agent === opts.to` would reject every correct channel + // send — which is exactly what an earlier revision of this function did, + // caught only by running the real CLI against the real server rather than + // by the unit fixture, which had been written with to == channel and so + // could not fail. The channel plus the sender IS the identity here, and the + // mention-notification DM this guard exists to reject carries NO channel, + // so the channel comparison above already separates it. + return sameIdentity(returned.to_agent, gotChannel) || sameIdentity(returned.to_agent, opts.to); + } + + // DIRECT MESSAGE. No channel to discriminate on, so the recipient is the + // discriminator — and it is precisely what tells our row apart from a + // notification DM addressed to a mentioned third party. + return sameIdentity(returned.to_agent, opts.to); +} + /** Duck-typed HasnaHttpError status check (class identity differs across bundles). */ function isHttpStatus(error: unknown, status: number): boolean { return Boolean( @@ -579,13 +648,33 @@ export class ApiStore implements ConversationsStore { // observed. Never trust a later mutable numeric id as the identity of the // row we wrote: read back by the caller-bound immutable UUID instead. const exact = await this.getMessageByUuid(messageUuid); - if (!exact) { - throw new Error( - `Message write returned UUID ${returned.uuid || "(missing)"} instead of ${messageUuid}, ` + - `and the exact row could not be read back. Refusing to report a numeric message id.` - ); + if (exact) return attachSendRedaction(opts.content, exact) as never; + + // Nothing under our UUID — and that has TWO causes this transport cannot + // tell apart, because `getMessageByUuid` maps every 404 to null and a + // server with no `/messages/by-uuid` route answers with the SAME bare 404 + // as a genuine row-miss. Measured on the deployed server, 2026-08-05: + // /v1/messages/by-uuid/ -> 404 {"error":"Not found"} (no route) + // /v1/definitely-not-a-route -> 404 {"error":"Not found"} (no route) + // /v1/messages/999999999 -> 404 {"error":"Message not found"} (real miss) + // That server also drops the caller `uuid` on create — it is absent from + // the route's published request schema — mints its own, and returns OUR + // row. So the strict check reported EVERY successful send as a failure. + // + // Treating "could not verify" as "did not write" is the expensive + // direction: a caller's natural response is to re-send, on a shared + // channel, where the retry reports the same false failure. + // + // So ask the one question still answerable: is the row the server DID + // return the write we just submitted? + if (echoesSubmittedWrite(opts, returned)) { + return attachSendRedaction(opts.content, returned) as never; } - return attachSendRedaction(opts.content, exact) as never; + + throw new Error( + `Message write returned UUID ${returned.uuid || "(missing)"} instead of ${messageUuid}, ` + + `and the exact row could not be read back. Refusing to report a numeric message id.` + ); }; getMessageById: ConversationsStore["getMessageById"] = async (id) => { const body = await this.client.get<{ message: Record }>("messages", String(id)); From b7a1f9bb7a97c9da81e65a2185618d9088745c1a Mon Sep 17 00:00:00 2001 From: Andrei Hasna Date: Wed, 5 Aug 2026 03:56:42 +0300 Subject: [PATCH 2/2] fix(send): disclose a degraded write confirmation, and correct two false claims Remediation cycle 1, from two independent adversarial reviews. P1 (review): the premise "production is running an older build of this repo" is wrong. conversations.hasna.xyz reports 1.0.0-rc.1, a version string that has never existed in this repository; its 15-path route set matches hasnaxyz/iapp-conversations exactly and differs from this repo's server in BOTH directions -- production serves /v1/health and /v1/whoami that this repo does not declare, and lacks the /v1/messages/by-uuid it does. So the server-side fix is a port or a replatform against a different repository, not a redeploy here. Filing it against this repo would see it closed as already-fixed. Corrected in the changelog. P2 (review): the claim that 0.5.23 shipped "with no changelog entry" is FALSE and was shipping inside the tarball. Release commit 6fb3da8 added a `## 0.5.23 - 2026-08-02` section; it is absent at HEAD only because it was later folded into the 0.5.24 section. The two readings that disagreed were each correct about a different snapshot. Replaced with what is actually true and load-bearing: no release tag and no npm provenance attestation, because 0.5.23 predates release.yml. P2 (review): the accept path returned silently, so a caller could not tell an authoritative UUID read-back from the weaker routing check, and nothing would ever mark the fallback dead once the server serves /messages/by-uuid. It now attaches `write_confirmation: { degraded: true, method: "routing-echo" }`, mirroring how this file already discloses a server-side row cap rather than presenting a degraded result as a complete one. An authoritative confirmation carries no such field. P3 (review): `sameIdentity` returned true for empty-vs-empty, so a blank sender on both sides satisfied an accept test by asserting nothing. Blank now never matches. Tests: 35 pass / 0 fail, typecheck rc=0. Re-verified on the live server after the change -- send rc=0 id 668694, and --json carries the degradation notice. Refs: todos d8f3f963 Agent: Silvanus --- CHANGELOG.md | 6 +- src/lib/store/api-store.test.ts | 109 ++++++++++++++++++++++++++++++++ src/lib/store/api-store.ts | 32 +++++++++- 3 files changed, 144 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6ad52c3..9faace7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,11 +19,15 @@ All notable changes to this project will be documented in this file. `getMessageByUuid` maps any 404 to `null`, so a missing **route** became "the row is not there", and a write that had demonstrably succeeded was reported as a failure. `sendMessage` now falls back — only after the authoritative UUID read-back has been tried and found unanswerable — to checking whether the row the server *did* return is the write just submitted, by the routing identity the caller controls. A response describing some other row (the mention-notification DM the UUID binding exists to catch) is still refused, loudly. -- **The caller-UUID guarantee 0.5.23 announced was never true against the deployed server.** That release's note claims "hosted writes preserve caller-generated UUIDs (#77)". They do not — the server has no `uuid` field on its create route. 0.5.23 went to the `next` dist-tag with no changelog entry and no release tag, so `latest` installs went 0.5.22 → 0.5.24 and #77 first reached the fleet **in 0.5.24**, which is why the onset tracks the 0.5.24 publish while the causing change shipped in 0.5.23. +- **The caller-UUID guarantee 0.5.23 announced was never true against the deployed server.** That release's note claims "hosted writes preserve caller-generated UUIDs (#77)". They are not — the deployed server has no `uuid` field on its create route, verified behaviourally rather than from the schema alone (the schema declares no `additionalProperties: false`, so it accepts the field and ignores it). 0.5.23 went to the `next` dist-tag and carries no release tag and no npm provenance attestation, having been published before `release.yml` existed; #77 therefore first reached `latest` **in 0.5.24**, which is why the onset tracks the 0.5.24 publish while the causing change shipped in 0.5.23. + +### Added +- **A degraded write-confirmation is disclosed instead of passing silently.** When the id could only be confirmed from the routing of the returned row rather than by reading the row back under the caller-bound UUID, the returned message carries `write_confirmation: { degraded: true, method: "routing-echo" }`. An authoritative confirmation carries no such field, so its disappearance is also the signal that this fallback has become dead code and can be removed. ### Known gaps - The underlying conflation is unchanged: `getMessageByUuid` still cannot distinguish a missing route from a missing row, because it has only the HTTP status to go on. The repair is scoped to `sendMessage`, where the false failure was reachable; the other callers are user-facing lookups where "not found" is an honest answer. A discriminated result type would change the `ConversationsStore` interface and every implementation, so it is deliberately not in this patch. - The accept path proves the returned row is addressed exactly as requested and carries a usable id. It does **not** prove the row is not some other message with identical routing. That residual is accepted only where the alternative is failing 100% of successful writes on a server that cannot be asked. +- **The hosted service is a different codebase, not an older build of this one.** `conversations.hasna.xyz` reports version `1.0.0-rc.1`, a string that has never existed in this repository; its route set matches `hasnaxyz/iapp-conversations` exactly (15 paths, identical both ways) and differs from this repo's server in both directions — production serves `/v1/health` and `/v1/whoami` that this repo does not declare, and lacks the `/v1/messages/by-uuid/{uuid}` that it does. So "deploy the server" is a port or a replatform, not a redeploy, and it belongs against that repository. This patch makes the client survive a server it does not control rather than waiting on that work. ## 0.5.24 - 2026-08-05 diff --git a/src/lib/store/api-store.test.ts b/src/lib/store/api-store.test.ts index a17d3d7..c1d3f7c 100644 --- a/src/lib/store/api-store.test.ts +++ b/src/lib/store/api-store.test.ts @@ -479,6 +479,115 @@ describe("ApiStore.sendMessage wire body", () => { expect(message).toMatchObject({ id: 668600, to_agent: "manius" }); }); + // Returning the id silently would leave a caller unable to tell an + // authoritative UUID read-back from the weaker routing check, and would leave + // nothing to mark this path dead once the server serves /messages/by-uuid. + test("discloses that confirmation degraded to the routing echo", async () => { + const notFound = Object.assign(new Error("Not Found"), { name: "HasnaHttpError", status: 404 }); + const client = { + name: "conversations", + baseUrl: "https://conversations.hasna.xyz/v1", + transport: { + get: async () => { + throw notFound; + }, + } as unknown as HasnaStorageClient["transport"], + create: async () => ({ + message: { + id: 668700, + uuid: "22222222-3333-4444-8555-666666666666", + session_id: "channel:git-publishing", + from_agent: "silvanus", + to_agent: "git-publishing", + channel: "git-publishing", + content: "degraded confirmation", + }, + }), + } as unknown as HasnaStorageClient; + const store = new ApiStore(client); + + const message = (await store.sendMessage({ + uuid: "33333333-4444-4555-8666-777777777777", + from: "silvanus", + to: "silvanus", + channel: "git-publishing", + content: "degraded confirmation", + })) as unknown as { id: number; write_confirmation?: { degraded: boolean; method: string } }; + + expect(message.id).toBe(668700); + expect(message.write_confirmation).toMatchObject({ degraded: true, method: "routing-echo" }); + }); + + // The other side: an AUTHORITATIVE confirmation must carry no degradation + // marker, or the flag means nothing and cannot signal the path is dead. + test("does NOT mark confirmation degraded when the server honours the caller UUID", async () => { + const boundUuid = "44444444-5555-4666-8777-888888888888"; + const client = { + name: "conversations", + baseUrl: "https://conversations.hasna.xyz/v1", + transport: {} as unknown as HasnaStorageClient["transport"], + create: async (_r: string, body: Record) => ({ + message: { + id: 668701, + uuid: body.uuid, + session_id: "channel:git-publishing", + from_agent: "silvanus", + to_agent: "git-publishing", + channel: "git-publishing", + content: "authoritative", + }, + }), + } as unknown as HasnaStorageClient; + const store = new ApiStore(client); + + const message = (await store.sendMessage({ + uuid: boundUuid, + from: "silvanus", + to: "silvanus", + channel: "git-publishing", + content: "authoritative", + })) as unknown as { id: number; write_confirmation?: unknown }; + + expect(message.id).toBe(668701); + expect(message.write_confirmation).toBeUndefined(); + }); + + // A blank identity on either side must not satisfy an accept test. Without + // the non-empty guard, `"" === ""` passes and the sender check asserts + // nothing at all. + test("refuses to accept an echo whose sender identity is blank on both sides", async () => { + const notFound = Object.assign(new Error("Not Found"), { name: "HasnaHttpError", status: 404 }); + const client = { + name: "conversations", + baseUrl: "https://conversations.hasna.xyz/v1", + transport: { + get: async () => { + throw notFound; + }, + } as unknown as HasnaStorageClient["transport"], + create: async () => ({ + message: { + id: 668702, + uuid: "55555555-6666-4777-8888-999999999999", + session_id: "channel:git-publishing", + from_agent: "", + to_agent: "git-publishing", + channel: "git-publishing", + content: "blank sender", + }, + }), + } as unknown as HasnaStorageClient; + const store = new ApiStore(client); + + await expect(store.sendMessage({ + uuid: "66666666-7777-4888-8999-aaaaaaaaaaaa", + from: "", + to: "silvanus", + channel: "git-publishing", + content: "blank sender", + })).rejects.toThrow(/could not be read back/); + }); + // `messages.id`/`messages.reply_to` are Postgres BIGINT, and node-postgres // serializes int8 as a STRING. Measured against the live deployed server: // GET /v1/messages returns `"id": "603183"` — a string, not a number. diff --git a/src/lib/store/api-store.ts b/src/lib/store/api-store.ts index 5f75b7b..6f3ad9f 100644 --- a/src/lib/store/api-store.ts +++ b/src/lib/store/api-store.ts @@ -50,10 +50,18 @@ function prune(q: Q): Record { return out; } -/** Case-insensitive compare of two optional identity strings. */ +/** + * Case-insensitive compare of two identity strings, where BLANK NEVER MATCHES. + * + * The empty-vs-empty case is the one that matters: a plain `l === r` returns + * true for two absent values, so a guard built on it would pass on nothing at + * all rather than on a match. Every use below is an accept test, so an + * unknown identity must fail it. + */ function sameIdentity(a: unknown, b: unknown): boolean { const l = typeof a === "string" ? a.trim().toLowerCase() : ""; const r = typeof b === "string" ? b.trim().toLowerCase() : ""; + if (!l || !r) return false; return l === r; } @@ -668,7 +676,27 @@ export class ApiStore implements ConversationsStore { // So ask the one question still answerable: is the row the server DID // return the write we just submitted? if (echoesSubmittedWrite(opts, returned)) { - return attachSendRedaction(opts.content, returned) as never; + // DISCLOSE THE DOWNGRADE rather than returning silently. Confirmation + // fell from "the server handed back the row under the UUID we bound" to + // "the row it handed back is routed the way we asked", which is a weaker + // claim, and a caller that cannot tell the two apart cannot know which + // guarantee its id carries. This mirrors how the same file already + // handles a server-side row cap (`truncated`) instead of presenting a + // degraded result as a complete one. + // + // It is also the only thing that will ever mark this path dead: once the + // server serves `/messages/by-uuid`, the flag stops appearing, and its + // absence is the signal that this fallback can be removed. + return attachSendRedaction(opts.content, { + ...returned, + write_confirmation: { + degraded: true, + method: "routing-echo", + message: + "The server did not preserve the caller-bound message UUID and cannot be queried by UUID, " + + "so this id was confirmed from the routing of the row it returned, not by reading the row back.", + }, + }) as never; } throw new Error(