fix(github): release the dedupe claim when the run fails or is skipped

[xenodeve]

EN

Parked deliberately, not blocked on effort. Filed so the reasoning survives.

Three entrypoints claim a dedupe key and then do the work, and each gets a different subset right:

• nestjs/src/github/github-webhook.service.ts:50 — seenBefore(deliveryId) is an INSERT…ON CONFLICT, so the claim is the check. DeliveryDedup (:9-12) exposes no forget, so a throw after the claim cannot release it: GitHub 403/5xx → 500 with the marker committed → the operator clicks Redeliver → same GUID → 200 {action:'duplicate'}, a success-shaped answer for work that never ran.
• nestjs/src/github/github-webhook.service.ts:66 and nestjs/src/github/vercel-webhook.controller.ts:195 — runExclusive's {ran:false} is discarded and the reply is an unconditional 202 with the key still claimed, so the loser of two concurrent deliveries is a permanently dropped event dressed as success. pipeline-sync.controller.ts:84-94 narrows the outcome correctly, so the codebase already disagrees with itself.
• nestjs/src/github/vercel-webhook.controller.ts:165 — the claim is taken at :160 but mapper.resolve (a pooler query) runs at :165, outside the try that starts at :193. A pooler blip during mapping loses that deployment permanently, for the exact reason the comment at :190-192 says must not happen.

Why an unattended agent should not land this. github-webhook.service.ts declares itself a security boundary in its own header — it verifies the HMAC and owns idempotency. The fix is not mechanical: releasing a claim on failure deliberately reopens a replay window so a redelivery can retry. Whether that is correct depends on whether every downstream effect is idempotent, and that is a judgment about the trust boundary with two defensible answers:

1. Release on any non-success — retries work, but a duplicate delivery arriving inside the failure window is processed twice.
2. Keep the claim and record the failure separately — no replay window, but a genuinely lost event needs an operator to notice.

A three-model review round split on the packaging too: two agents wanted the pipeline-honesty fixes merged into one PR, while the one that read the actual code argued they must stay separate because their retry and action-result contracts deserve independent tests. That disagreement is itself a reason for a human to choose.

What is already decided and should not be re-litigated: the seam is withClaim(key, fn) owning claim-release-on-any-non-success, plus an exhaustive outcome union the controllers must narrow so {ran:false} cannot be assigned to nothing. Only the replay-window question above is open.

What unblocks it

A decision on (1) vs (2), stated on this issue. Then it is ordinary TDD: a fake seenBefore that records claims plus a throwing refreshOwner asserting forget was called; runExclusive returning {ran:false} asserting the response is not a bare 202 and the claim is released; a mapper that rejects asserting the same. All three are red today — test/github-webhook.service.spec.ts:15 stubs seenBefore as a pure read, so claim-then-fail is not even representable.

TH

พักไว้อย่างเจตนา ไม่ใช่ติดเรื่องแรงงาน ยื่นไว้เพื่อให้เหตุผลอยู่รอด

สาม entrypoint จับ dedupe key แล้วทำงาน และแต่ละตัวทำถูกไม่เหมือนกัน:

• nestjs/src/github/github-webhook.service.ts:50 — seenBefore(deliveryId) เป็น INSERT…ON CONFLICT ฉะนั้น การจับ คือ การตรวจ · DeliveryDedup (:9-12) ไม่มี forget ฉะนั้นการ throw หลังจับปล่อยมันไม่ได้: GitHub 403/5xx → 500 พร้อม marker ที่ commit แล้ว → ผู้ดูแลกด Redeliver → GUID เดิม → 200 {action:'duplicate'} คำตอบรูปร่างสำเร็จสำหรับงานที่ไม่เคยรัน
• nestjs/src/github/github-webhook.service.ts:66 และ nestjs/src/github/vercel-webhook.controller.ts:195 — {ran:false} ของ runExclusive ถูกทิ้งและตอบ 202 แบบไม่มีเงื่อนไขโดยที่ key ยังถูกจับไว้ ฉะนั้นผู้แพ้ของการส่งสองรายการพร้อมกันคือ event ที่หายถาวรในเสื้อของความสำเร็จ · pipeline-sync.controller.ts:84-94 narrow outcome ถูกต้อง ฉะนั้นโค้ดเบสขัดกันเองอยู่แล้ว
• nestjs/src/github/vercel-webhook.controller.ts:165 — claim ถูกจับที่ :160 แต่ mapper.resolve (query ผ่าน pooler) รันที่ :165 นอก try ที่เริ่มที่ :193 · pooler สะดุดระหว่าง mapping ทำให้ deployment นั้นหายถาวร ด้วยเหตุเดียวกับที่คอมเมนต์ที่ :190-192 บอกว่าต้องไม่เกิด

เหตุที่ agent แบบไม่มีคนดูไม่ควรลงมือ github-webhook.service.ts ประกาศตัวเองว่าเป็นขอบเขตความปลอดภัยในหัวไฟล์ของมันเอง — มันตรวจ HMAC และเป็นเจ้าของ idempotency · การแก้ไม่ใช่งานเชิงกล: การปล่อย claim เมื่อล้มเหลวเป็นการเปิดหน้าต่าง replay อย่างเจตนา เพื่อให้การส่งซ้ำลองใหม่ได้ · ว่านั่นถูกหรือไม่ขึ้นกับว่าผลข้างเคียงปลายน้ำทุกอย่าง idempotent จริงไหม และนั่นเป็นการตัดสินเกี่ยวกับขอบเขตความเชื่อถือที่มีคำตอบที่ป้องกันได้สองแบบ:

1. ปล่อยเมื่อไม่สำเร็จทุกกรณี — การลองใหม่ทำงาน แต่การส่งซ้ำที่มาถึงในหน้าต่างความล้มเหลวจะถูกประมวลผลสองครั้ง
2. เก็บ claim ไว้และบันทึกความล้มเหลวแยก — ไม่มีหน้าต่าง replay แต่ event ที่หายจริงต้องมีผู้ดูแลสังเกต

การรีวิวสามโมเดลเห็นต่างกันเรื่องการห่อ PR ด้วย: สอง agent ต้องการรวมการแก้เรื่องความซื่อสัตย์ของ pipeline เป็น PR เดียว ขณะที่ตัวที่อ่านโค้ดจริงเถียงว่าต้องแยกเพราะ contract ของ retry กับผลลัพธ์ของ action ควรมีเทสต์แยกกัน · ความเห็นต่างนั้นเองเป็นเหตุให้มนุษย์ควรเลือก

สิ่งที่ตัดสินแล้วและไม่ควรถกใหม่: รอยต่อคือ withClaim(key, fn) ที่เป็นเจ้าของการปล่อย claim เมื่อไม่สำเร็จทุกกรณี บวก union ของ outcome ที่ครบถ้วนซึ่ง controller ต้อง narrow เพื่อให้ {ran:false} ถูกกำหนดให้ไม่มีอะไรไม่ได้ · เปิดอยู่เฉพาะคำถามเรื่องหน้าต่าง replay ข้างต้น

สิ่งที่ปลดล็อก

การตัดสินระหว่าง (1) กับ (2) ระบุไว้ในอิชชูนี้ · จากนั้นเป็น TDD ธรรมดา: seenBefore ปลอมที่บันทึกการจับ บวก refreshOwner ที่ throw เพื่อ assert ว่า forget ถูกเรียก · runExclusive ที่คืน {ran:false} เพื่อ assert ว่าคำตอบไม่ใช่ 202 เปล่าและ claim ถูกปล่อย · mapper ที่ reject เพื่อ assert อย่างเดียวกัน · ทั้งสามแดงวันนี้ — test/github-webhook.service.spec.ts:15 stub seenBefore เป็นการอ่านล้วน ฉะนั้น claim-แล้ว-ล้ม แสดงออกมาไม่ได้เลย