fix(notify): send a Feishu webhook message at most once

changelog.d/notify-webhook-duplicate.fixed.md

@@ -0,0 +1 @@
+A Feishu webhook message whose answer was lost is no longer posted a second time: a timed-out or aborted send gets one attempt and says the message may already be in the chat, instead of claiming Feishu refused it ([#2660](https://github.com/ThinkFlowLab/FlyBot/issues/2660)).

core/src/host/connectors.js

@@ -275,6 +275,25 @@ function withDeadline(promise, ms) {
   return Promise.race([promise, deadline]).finally(() => clearTimeout(timer))
 }
 
+// A request that never reached Feishu wrote nothing, so notify.js may safely send it again. A
+// request that timed out, was aborted, or failed in a way we cannot read may already be in the
+// chat, and a custom-bot webhook has no idempotency field: repeating it would post it twice.
+// `feishu/no-answer` is deliberately absent from notify.js's `RETRYABLE`, so it gets one attempt.
+const UNSENT_CODES = new Set(['ECONNREFUSED', 'ENOTFOUND', 'EAI_AGAIN'])
+const NO_ANSWER = 'feishu/no-answer'
+
+function connectionNeverOpened(error) {
+  const seen = new Set()
+  const inspect = cause => {
+    if (!cause || typeof cause !== 'object' || seen.has(cause)) return false
+    seen.add(cause)
+    if (UNSENT_CODES.has(String(cause.code))) return true
+    if (inspect(cause.cause)) return true
+    return Array.isArray(cause.errors) && cause.errors.some(inspect)
+  }
+  return inspect(error)
+}
+
 // One text message through a custom-bot webhook, answered in notify.js's vocabulary: `{ ok, ts }`
 // or `{ ok: false, error }`, where `rate_limited` and `service_unavailable` may pass on their own
 // and every other error is a shape Feishu will refuse again. The URL is never part of the answer.
@@ -288,8 +307,9 @@ export async function feishuPost(webhook, text, { fetch: request = fetch, timeou
       redirect: 'error',
       signal: AbortSignal.timeout(Math.max(1, timeoutMs)),
     }), timeoutMs)
-  } catch {
-    return { ok: false, error: 'network' }
+  } catch (error) {
+    // Only a connection that never opened is retried; anything else is an unconfirmed delivery.
+    return connectionNeverOpened(error) ? { ok: false, error: 'network' } : { ok: false, error: NO_ANSWER }
   }
   const body = await response.json().catch(() => ({}))
   if (response.status === 429) {
@@ -311,6 +331,7 @@ export async function feishuPost(webhook, text, { fetch: request = fetch, timeou
 export async function feishuCheck(webhook, { api = FEISHU_DEFAULTS.api, fetch: request = fetch, timeoutMs = CHECK_TIMEOUT_MS } = {}) {
   const answer = await feishuPost(webhookAddress(webhook, api), FEISHU_TEST_TEXT, { fetch: request, timeoutMs })
   if (answer.ok) return {}
+  if (answer.error === NO_ANSWER) throw new Error('Feishu did not answer in time — the test message may already be in the chat; check before connecting again')
   if (answer.error === 'network' || answer.error === 'service_unavailable') throw new Error('Could not reach Feishu')
   if (answer.error === 'rate_limited') throw new Error('Feishu is rate-limiting this webhook; try again in a minute')
   throw new Error(`Feishu did not accept the webhook (${answer.error})`)
@@ -1018,7 +1039,9 @@ export function install(rt) {
       // A fresh key per call: two messages with the same text are two messages, not a duplicate.
       const result = await notifier.send({ key: randomUUID(), text })
       const sent = result.state === 'sent' || result.state === 'duplicate'
-      const failure = `Feishu refused the message (${result.error ?? result.reason ?? 'no answer'})`
+      const failure = result.error === NO_ANSWER
+        ? 'Feishu did not answer in time — the message may already be in the chat; check before sending again'
+        : `Feishu refused the message (${result.error ?? result.reason ?? 'no answer'})`
       await keep(c, { ...saved(c), ...(sent ? { lastOkAt: Date.now() } : { lastError: failure }) })
       if (!sent) throw new Error(failure)
       return connectorsView()

core/src/host/notify.js

@@ -1,10 +1,11 @@
 // Out-of-app delivery, first half (#355): the adapter that will carry a reminder to Slack.
 // A reminder has to reach the owner when the desktop app is closed, so delivery cannot be a
 // client toast. The transport is injected; what this module owns is the policy around it: one
-// idempotency key per reminder, so a retried send cannot arrive twice; exponential backoff that
-// honours `Retry-After`; three attempts at most; no retry at all for a failure that will not
-// change (the channel is gone, the app was removed). A channel switched off makes zero transport
-// calls, including one that is switched off while it waits to retry.
+// idempotency key per reminder, deduplicated inside this process only, and a send that may already
+// have left the process is never repeated, because the outbound transport cannot dedupe it;
+// exponential backoff that honours `Retry-After`; three attempts at most; no retry at all for a
+// failure that will not change (the channel is gone, the app was removed). A channel switched off
+// makes zero transport calls, including one that is switched off while it waits to retry.
 //
 // The real Slack transport waits for the owner's credential, so it is not here: the module speaks
 // Slack's `chat.postMessage` shape and ships a fake transport that records every call and scripts

core/test/feishu.test.js

@@ -142,7 +142,8 @@ test('#1263: a webhook Feishu refuses stays on the row as an error that names no
 test('#1263: a Feishu that never answers fails on the deadline instead of hanging', async () => {
   const started = Date.now()
   const answerOrNothing = await feishuPost(webhookOf('x'), 'hello', { fetch: () => new Promise(() => {}), timeoutMs: 25 })
-  assert.deepEqual(answerOrNothing, { ok: false, error: 'network' })
+  // #2660: the POST may already be in the chat, so it is an unconfirmed delivery, not a retryable link.
+  assert.deepEqual(answerOrNothing, { ok: false, error: 'feishu/no-answer' })
   assert.ok(Date.now() - started < 2_000, 'the deadline, not the fetch, ends the wait')
 })
 

core/test/notify-webhook-once.test.js

@@ -0,0 +1,84 @@
+import assert from 'node:assert/strict'
+import { test } from 'node:test'
+import { feishuCheck, feishuTransport } from '../src/host/connectors.js'
+import { createNotifier } from '../src/host/notify.js'
+
+// #2660: a Feishu custom-bot webhook has no idempotency field, so a POST whose answer was lost
+// cannot be sent again: the receiver would show the same message twice. Notify.js retries
+// `network`, which is right for a connection that never opened and wrong for a request that may
+// already have been written — the transport has to tell those two apart.
+
+const WEBHOOK = 'https://open.feishu.cn/open-apis/bot/v2/hook/unit-test'
+const ok = data => ({ status: 200, ok: true, headers: { get: () => null }, json: async () => ({ code: 0, data }) })
+const status = (code, headers = {}) => ({
+  status: code,
+  ok: code >= 200 && code < 300,
+  headers: { get: name => headers[String(name).toLowerCase()] ?? null },
+  json: async () => ({}),
+})
+const notifier = (fetch, extra = {}) => createNotifier({
+  channel: 'feishu',
+  transport: feishuTransport(WEBHOOK, { fetch, timeoutMs: 40 }),
+  log: () => {},
+  ...extra,
+})
+
+test('#2660: a POST whose answer was lost is not sent a second time', async () => {
+  const posts = []
+  const fetch = async (url, init) => {
+    posts.push(JSON.parse(init.body))
+    return new Promise(() => {}) // accepted, but the answer never comes back
+  }
+
+  const result = await notifier(fetch).send({ key: 'k1', text: 'standup in 5 minutes' })
+
+  assert.equal(posts.length, 1, 'one send makes exactly one POST when the answer is lost')
+  assert.deepEqual(posts[0], { msg_type: 'text', content: { text: 'standup in 5 minutes' } })
+  assert.deepEqual(result, { state: 'failed', error: 'feishu/no-answer', attempts: 1, permanent: true })
+  assert.equal(result.error, 'feishu/no-answer', 'the error names the unconfirmed delivery')
+})
+
+test('#2660: a connection that never opened still retries three times', async () => {
+  let calls = 0
+  const fetch = async () => {
+    calls += 1
+    const cause = Object.assign(new Error('connect ECONNREFUSED'), { code: 'ECONNREFUSED' })
+    throw new TypeError('fetch failed', { cause })
+  }
+
+  const result = await notifier(fetch, { delay: async () => {} }).send({ key: 'k2', text: 'hello' })
+
+  assert.equal(calls, 3, 'a refused connection wrote nothing, so the three attempts stay')
+  assert.deepEqual(result, { state: 'failed', error: 'network', attempts: 3, permanent: false })
+})
+
+test('#2660: a 429 still retries and waits what Retry-After asks', async () => {
+  let calls = 0
+  const waits = []
+  const fetch = async () => {
+    calls += 1
+    return calls === 1 ? status(429, { 'retry-after': '2' }) : ok({ message_id: 'om_1' })
+  }
+
+  const result = await notifier(fetch, { delay: async ms => { waits.push(ms) } }).send({ key: 'k3', text: 'hello' })
+
+  assert.equal(result.state, 'sent')
+  assert.equal(result.attempts, 2, 'a rate limit is transient: the same call is tried again')
+  assert.ok(waits[0] >= 2_000, 'Retry-After is a floor, not a suggestion')
+})
+
+test('#2660: the connect sentence says the test message may already be posted', async () => {
+  let calls = 0
+  const fetch = async () => { calls += 1; return new Promise(() => {}) }
+
+  await assert.rejects(
+    feishuCheck(WEBHOOK, { fetch, timeoutMs: 40 }),
+    error => {
+      assert.equal(calls, 1, 'the probe is one POST, never a retry')
+      assert.match(error.message, /did not answer in time/i)
+      assert.match(error.message, /may already be in the chat/i)
+      assert.ok(!error.message.includes('open.feishu.cn'), 'the sentence never carries the webhook URL')
+      return true
+    },
+  )
+})