Merge pull request 'Standardize POST /notify/:id response shape — always include stored field' (#13) from mailship-nl0-post-notify-response-shape-ok-skipped-la-387 into main

Reviewed-on: #13
Reviewed-by: matt <matt@lorentz.is>

README.md

@@ -85,6 +85,10 @@ Response: { ok: true, stored: boolean }
 Returns 404 if subscription not found or inactive.
 ```
 
+When the event is not found at the relay (e.g. it was deleted or never arrived),
+the endpoint returns `{ ok: true, stored: false }` — the event is silently skipped
+rather than erroring. The `stored` field is always present in a 200 response.
+
 ### GET /confirm?token=...
 Confirm email address via link from confirmation email.
 

src/server.ts

@@ -285,8 +285,8 @@ addRoute('post', '/notify/:id', async (req: Request, res: Response) => {
       storedEvent = fetched
 
       if (!storedEvent) {
-        // Event not found at relay — don't 404, just skip
+        // Event not found at relay — don't 404, reflect that nothing was stored
-        return res.json({ ok: true, skipped: true })
+        return res.json({ ok: true, stored: false })
       }
     }
 

test/notify-response-shape.test.ts

@@ -0,0 +1,78 @@
+// POST /notify/:id response shape test
+//
+// Verifies that the endpoint always includes a `stored` boolean
+// in its response, matching the documented contract in README.md:
+//   Response: { ok: true, stored: boolean }
+//
+// Bug: when the event is not found at the relay, the handler returns
+//   { ok: true, skipped: true }
+// missing the documented `stored` field.
+
+import { describe, it, expect, beforeAll, afterAll } from 'vitest'
+import * as db from '../src/database.js'
+import { server } from '../src/server.js'
+import { createServer, type Server } from 'http'
+
+// Partially mock @welshman/net so that `load()` returns an empty array,
+// simulating the case where the relay does not have the requested event,
+// while preserving all other exports that other modules depend on.
+vi.mock('@welshman/net', async (importOriginal) => {
+  const actual = await importOriginal()
+  return {
+    ...(actual as Record<string, unknown>),
+    load: vi.fn().mockResolvedValue([]),
+  }
+})
+
+describe('notify_response_shape', () => {
+  let httpServer: Server
+  let baseUrl: string
+  let subId: string
+
+  beforeAll(async () => {
+    await db.migrate()
+
+    // Create and confirm a subscription we can use for the notify call
+    const pubkey = 'shape-test-pk-' + Date.now()
+    const email = 'shape-test-' + Date.now() + '@example.com'
+    const sub = await db.insertSubscription(pubkey, email, 'daily')
+    const confirmed = await db.confirmSubscription(sub.key)
+    subId = confirmed.id
+
+    // Start the express server on a random available port
+    await new Promise<void>((resolve) => {
+      httpServer = createServer(server)
+      httpServer.listen(0, () => {
+        const addr = httpServer.address()
+        if (addr && typeof addr === 'object') {
+          baseUrl = `http://localhost:${addr.port}`
+        }
+        resolve()
+      })
+    })
+  })
+
+  afterAll(async () => {
+    httpServer?.close()
+  })
+
+  it('returns stored=false instead of skipped=true when event not found', async () => {
+    const res = await fetch(`${baseUrl}/notify/${subId}`, {
+      method: 'POST',
+      headers: { 'Content-Type': 'application/json' },
+      body: JSON.stringify({
+        id: 'nonexistent-' + Date.now(),
+        relay: 'wss://relay.damus.io',
+      }),
+    })
+
+    const body = await res.json()
+
+    // The documented contract says: { ok: true, stored: boolean }
+    // The current buggy code returns: { ok: true, skipped: true }
+    expect(body).not.toHaveProperty('skipped')
+    expect(body).toHaveProperty('stored')
+    expect(body.stored).toBe(false)
+    expect(body.ok).toBe(true)
+  })
+})