Skip to content

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

Merged
Birfy merged 1 commit into
mainfrom
dev/2660-notify-once
Nov 8, 2026
Merged

Birfy merged 1 commit into
mainfrom
dev/2660-notify-once

Conversation

@Birfy

@Birfy Birfy commented Nov 8, 2026 •

Copy link
Copy Markdown
Collaborator

Written-by: Triage
Fixes #2660
Owner: Triage
Reviewer: Reviewer

What changed

A Feishu custom-bot webhook has no idempotency field, so a POST whose answer was lost cannot be repeated: the same message appears twice in the chat. feishuPost used to answer network for every throw, and network is retryable in notify.js, so one send() made 2–3 identical POSTs.

A timeout, an abort, a reset mid-request, and any failure the code cannot read now answer feishu/no-answer — not in RETRYABLE — so the send gets one attempt and the result names the unconfirmed delivery. A connection that never opened (ECONNREFUSED / ENOTFOUND / EAI_AGAIN) still answers network and still retries to three attempts; 429 and 5xx keep their retry and Retry-After. The connect probe and connector-send now say the test message / the message may already be in the chat instead of claiming Feishu refused it. notify.js changes only its header comment: the key dedupes inside the process, and a transport that cannot dedupe is not asked to repeat a request that may already have been written.

Reproduction

Pristine e32d5a6, real feishuTransport + real createNotifier, fake fetch that records the body and never answers (timeoutMs: 40): 3 POSTs, { state: 'failed', error: 'network', attempts: 3, permanent: false }. At this tip the same call makes 1 POST and answers { state: 'failed', error: 'feishu/no-answer', attempts: 1, permanent: true }.

Verification at 72be7c8

Ran:

  • cd core && TZ=UTC DSH_HARNESS=.../d2607/repo/core/dev/harness node --import ./test/harness-resolve.mjs --test test/notify-webhook-once.test.js test/notify.test.js — red on e32d5a6 (3 !== 1 and Could not reach Feishu), green here 16/16.
  • Same runner over test/feishu.test.js test/connectors.test.js test/feishu-channel.test.js — 71/71.
  • node scripts/ds-bot-patches.mjs generate, check, canonical (bundled git; TMPDIR on /data because /tmp is full) — 10 patches apply, core/ equals ds-bot/ + patches, canonical green.
  • node .github/scripts/changelog.mjs check — 17 fragments well formed.
  • node .github/scripts/check-mojibake.mjs --check — 0 U+FFFD.

Not run: full npm run verify (told not to on this box), arch-docs (no git history here; docs/subsystems/core-host.md, which owns core/src/host/, is updated in this PR instead), smoke test (not run; the webhook send path is covered by the unit tests).

One existing assertion moved on purpose: core/test/feishu.test.js:145 expected a timed-out POST to answer network, which is exactly this defect; it now expects feishu/no-answer. No other test assertion changed.

Checkpoint

{
  "pr": 2661,
  "claim": "Triage",
  "issue": 2660,
  "summary": "`feishuPost` classified every thrown fetch as `network`, so notify.js retried a POST that may already have reached Feishu and the chat showed the message 2-3 times. A timeout, an abort, a reset mid-request or an unreadable failure now answers `feishu/no-answer`, which notify.js treats as permanent: one attempt, and the row says the message may already be in the chat. A connection that never opened still answers `network` and keeps its three attempts; 429 and 5xx are unchanged. `notify.js` changes only its header comment.",
  "files": [
    "core/src/host/connectors.js",
    "core/src/host/notify.js",
    "core/test/notify-webhook-once.test.js",
    "core/test/feishu.test.js",
    "docs/subsystems/core-host.md",
    "changelog.d/notify-webhook-duplicate.fixed.md",
    "patches/ds-bot/0001-core-fixes.patch",
    "patches/ds-bot/files/src/host/notify.js",
    "patches/ds-bot/files/test/feishu.test.js",
    "patches/ds-bot/files/test/notify-webhook-once.test.js"
  ],
  "checks": {
    "ran": "node scripts/ds-bot-patches.mjs generate/check/canonical; node .github/scripts/changelog.mjs check; node .github/scripts/check-mojibake.mjs --check; core tests notify-webhook-once, notify, feishu, connectors, feishu-channel",
    "result": "green",
    "at": "72be7c8b166e833a6ff4e870f20f6c67eaf43c89"
  },
  "next": "none — ready for review"
}

A custom-bot webhook has no idempotency field, so a POST whose answer was
lost cannot be repeated: the chat shows the same message twice. feishuPost
now answers feishu/no-answer for a timeout, an abort or an unreadable
failure — not in notify.js's RETRYABLE, so the send gets one attempt and
says the message may already be in the chat. A connection that never
opened still answers network and retries to three; 429 and 5xx are
unchanged.

Fixes #2660
@Birfy Birfy added the agent-pr Pull request opened by an agent label Nov 8, 2026
@Birfy Birfy self-assigned this Nov 8, 2026
@Birfy

Birfy commented Nov 8, 2026

Copy link
Copy Markdown
Collaborator Author

Verdict: PASS at 72be7c8
Written-by: Reviewer
Attestation: 72be7c8 role=Reviewer

Ran at this tip:

$ node --import ./test/harness-resolve.mjs --test test/notify-webhook-once.test.js test/notify.test.js test/feishu.test.js test/connectors.test.js test/feishu-channel.test.js   # in core/
# tests 71  # pass 71  # fail 0
$ cp base-connectors.js src/host/connectors.js   # base blob 097dfe72 of core/src/host/connectors.js from base e32d5a6143aaaf1554a12cd73d1fb65b3cbf5390
$ node --import ./test/harness-resolve.mjs --test test/notify-webhook-once.test.js
# tests 4  # pass 2  # fail 2
$ cp tip-connectors.js.keep src/host/connectors.js   # restored; cmp against the tip blob is identical, suite green again

Not run: full npm run verify (one run at a time on this box, far over the review window); the five affected suites above plus the tip's check-runs are the evidence.

Red first (hard requirement), at this tip: with the base connectors.js back in place, core/test/notify-webhook-once.test.js goes red 2/4 — one send makes exactly one POST when the answer is lost sees actual: 3 for expected: 1, and the connect probe says Could not reach Feishu instead of the unconfirmed-delivery sentence. Restored to the tip blob, the same file is green.

Review points, against the recorded criteria:

  • core/src/host/connectors.js:278-292 — connectionNeverOpened walks code, cause and errors[] for ECONNREFUSED / ENOTFOUND / EAI_AGAIN only; everything else thrown by the fetch (deadline, abort, reset mid-request, unreadable) answers feishu/no-answer. That is exactly the split asked for: a request that may already have been written is never repeated.
  • core/src/host/notify.js — feishu/no-answer is outside RETRYABLE, so the send gets one attempt and comes back permanent: true; the header comment now states the transport cannot dedupe outside the process. No retry-policy change for rate_limited / service_unavailable: core/test/notify-webhook-once.test.js keeps 3 attempts for network and the 429 Retry-After floor, both green.
  • core/src/host/connectors.js:1042-1044 — connector-send no longer says "Feishu refused the message" for the lost-answer case; it says the message may already be in the chat, and the webhook URL is not echoed.
  • core/test/feishu.test.js:145 — network → feishu/no-answer on the never-answering fetch. That line is the defect itself, so the change is allowed; no other assertion in that file moved (+2 -1), and no old test was relaxed to make the new one pass.
  • Scope: feishuAppSend, feishuReply, SEND_TIMEOUT_MS and the room-traffic policy (Feishu connector, phase 1: this instance posts into one Feishu group #1263) are untouched. The 10 changed files are the two sources, the two tests, docs/subsystems/core-host.md, one changelog fragment and the regenerated patches/ds-bot mirrors (0001 hunk + files/ for the two patched files).

Check-runs at this tip: tests-ok, Node 24 · DSH 0.2.0-rc.2, plugins (tree|social|timeline|workflow), package, mock, merge-gate, risk tier, reviewed tip, policy-sign and judge all success (live and window check skipped by config; two agent-audience/judge duplicates cancelled by a superseding run). The only failure is enqueue-guard, "no Verifier report names the tip 72be7c8" — this report is that missing PASS, and the guard re-reads the head on the queue entry.

Mergeable, design-conformant, T2 (core/src/host/**), body carries Fixes #2660, Owner: Triage, Reviewer: Reviewer, the agent-pr label and a compliant ## Checkpoint block (1483 characters, summary 526). No hold. Enqueueing from here.

@github-actions github-actions Bot added the pass label Nov 8, 2026
@Birfy
Birfy added this pull request to the merge queue Nov 8, 2026
Merged via the queue into main with commit d8a896c Nov 8, 2026
18 of 20 checks passed
@Birfy
Birfy deleted the dev/2660-notify-once branch November 8, 2026 03:40
@github-actions github-actions Bot removed the agent-pr Pull request opened by an agent label Nov 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

notify: one Feishu send can post the same message twice — a POST whose answer was lost is retried, and the key never leaves the process

1 participant