Public: anyone can read this; the crew can change it.
Settings › Webhooks shows a failed delivery's Redeliver button, as the terminal's `r` does #8406
Check · Review · Land · Live done — Landed and live in production.
mason/forge:mason/8076-settings-webhooks-shows-a-failed-deliver → main 3 commits fork 1 thread · 1 unresolved
4 files
changed +118
−4
docs/features/webhook-redeliver.md +3 −2
| @@ -1,9 +1,10 @@ | ||
| 1 | 1 | # Redeliver a failed webhook delivery |
| 2 | 2 | |
| 3 | A webhook delivery that ran out of attempts stays failed. A repository admin can send it again from the terminal. | |
| 3 | A webhook delivery that ran out of attempts stays failed. A repository admin can send it again from the terminal or from the repository's Settings. | |
| 4 | 4 | |
| 5 | 5 | - **Key.** Open a hook's Deliveries screen in the terminal and press `r` on a row. The screen is a repository admin's, and so is the key. |
| 6 | 6 | - **Which rows.** Only a `failed` row. On a pending or delivered row `r` does nothing. |
| 7 | 7 | - **What happens.** `redeliver` (`src/core/webhooks.ts`) puts the same row back to `pending` with its attempts reset to zero and its last status and error cleared; there is no new row and no new event. It reads, checks and updates the row in one transaction. The worker tries it on its next pass with the same event's payload; on a paused hook (`p`) it waits, pending, until the hook is resumed. |
| 8 | 8 | - **Sealed reports.** The delivery of a sealed report stays refused, as it is when the event first fires: `r` leaves the row failed. |
| 9 | - **Terminal only, for now.** There is no page button and no MCP action yet. The terminal was the issue's ask (#8075), and the read-only deliveries list on the web (`GET …/hooks/:id/deliveries`, MCP `hooks` with `action: deliveries`) has no control to put it on. | |
| 9 | - **Page.** Settings > Webhooks lists each hook's recent deliveries, and a `failed` row has a Redeliver button, a POST form to `/:owner/:name/settings/hooks/:id/deliveries/:delivery/redeliver` (`webhookRedeliver`, `src/web/pages/hooks.ts`). Only a repository admin sees it, the POST is same-origin only, and anyone else gets a 404. It redirects back to the page, where the row reads `pending`; a refused redelivery redirects with a notice and changes nothing. | |
| 10 | - **Not yet.** There is no MCP action; the read-only deliveries list (`GET …/hooks/:id/deliveries`, MCP `hooks` with `action: deliveries`) has no control. |
src/http/server.ts +2 −1
| @@ -411,7 +411,7 @@ import { | ||
| 411 | 411 | } from "../web/pages/settings.ts"; |
| 412 | 412 | import { dangerPage, repoDelete } from "../web/pages/danger.ts"; |
| 413 | 413 | import { detachSubmit } from "../web/pages/detach.ts"; |
| 414 | import { hooksPage, webhookCreate, webhookDelete, webhookPing } from "../web/pages/hooks.ts"; | |
| 414 | import { hooksPage, webhookCreate, webhookDelete, webhookPing, webhookRedeliver } from "../web/pages/hooks.ts"; | |
| 415 | 415 | import { |
| 416 | 416 | crewAdd, |
| 417 | 417 | crewRemove, |
| @@ -1545,6 +1545,7 @@ export function buildRouter(): Router<Handler> { | ||
| 1545 | 1545 | router.add("GET", "/:owner/:name/settings/hooks", hooksPage); |
| 1546 | 1546 | router.add("POST", "/:owner/:name/settings/hooks", webhookCreate); |
| 1547 | 1547 | router.add("POST", "/:owner/:name/settings/hooks/:id/delete", webhookDelete); |
| 1548 | router.add("POST", "/:owner/:name/settings/hooks/:id/deliveries/:delivery/redeliver", webhookRedeliver); | |
| 1548 | 1549 | router.add("POST", "/:owner/:name/settings/hooks/:id/ping", webhookPing); |
| 1549 | 1550 | router.add("GET", "/:owner/:name/settings/danger", dangerPage); |
| 1550 | 1551 | router.add("POST", "/:owner/:name/settings/delete", repoDelete); |
src/web/pages/hooks.ts +31 −1
| @@ -7,7 +7,7 @@ | ||
| 7 | 7 | |
| 8 | 8 | import { createWebhook, deleteWebhook } from "../../actions/repos.ts"; |
| 9 | 9 | import { notePhase, timePhase } from "../../core/route_timing.ts"; |
| 10 | import { getWebhook, listWebhooks, pingWebhook, recentDeliveries } from "../../core/webhooks.ts"; | |
| 10 | import { getWebhook, listWebhooks, pingWebhook, recentDeliveries, redeliver } from "../../core/webhooks.ts"; | |
| 11 | 11 | import type { Ctx } from "../../http/context.ts"; |
| 12 | 12 | import { repoBase, repoNotFound } from "../chrome.ts"; |
| 13 | 13 | import { field } from "../form.ts"; |
| @@ -56,6 +56,7 @@ export function hooksPage(ctx: Ctx): Response { | ||
| 56 | 56 | <th>State</th> |
| 57 | 57 | <th>Attempts</th> |
| 58 | 58 | <th>Last status</th> |
| 59 | <th><span class="sr-only">Actions</span></th> | |
| 59 | 60 | </tr> |
| 60 | 61 | </thead> |
| 61 | 62 | <tbody> |
| @@ -75,6 +76,18 @@ export function hooksPage(ctx: Ctx): Response { | ||
| 75 | 76 | </td> |
| 76 | 77 | <td>${d.attempts}</td> |
| 77 | 78 | <td>${d.lastError ?? d.lastStatus ?? "—"}</td> |
| 79 | <td> | |
| 80 | ${when( | |
| 81 | d.state === "failed", | |
| 82 | () => | |
| 83 | html`<form | |
| 84 | method="post" | |
| 85 | action="${base}/settings/hooks/${hook.id}/deliveries/${d.id}/redeliver" | |
| 86 | > | |
| 87 | ${btn("Redeliver", { size: "sm" })} | |
| 88 | </form>`, | |
| 89 | )} | |
| 90 | </td> | |
| 78 | 91 | </tr>`, |
| 79 | 92 | )} |
| 80 | 93 | </tbody> |
| @@ -192,6 +205,23 @@ export async function webhookDelete(ctx: Ctx): Promise<Response> { | ||
| 192 | 205 | ); |
| 193 | 206 | } |
| 194 | 207 | |
| 208 | export async function webhookRedeliver(ctx: Ctx): Promise<Response> { | |
| 209 | const gate = await adminForm(ctx); | |
| 210 | if (gate instanceof Response) return gate; | |
| 211 | const { repo } = gate; | |
| 212 | const hook = getWebhook(ctx.app.db, repo.id, ctx.params.id ?? ""); | |
| 213 | const done = hook | |
| 214 | ? redeliver(ctx.app.db, ctx.params.delivery ?? "", hook.id) | |
| 215 | : undefined; | |
| 216 | return flashTo( | |
| 217 | ctx, | |
| 218 | `${repoBase(repo)}/settings/hooks`, | |
| 219 | done?.ok | |
| 220 | ? { message: "Delivery queued again." } | |
| 221 | : { tone: "danger", message: "That delivery cannot be sent again." }, | |
| 222 | ); | |
| 223 | } | |
| 224 | ||
| 195 | 225 | export async function webhookPing(ctx: Ctx): Promise<Response> { |
| 196 | 226 | const gate = await adminForm(ctx); |
| 197 | 227 | if (gate instanceof Response) return gate; |
test/web-hook-redeliver.test.ts added +82
| @@ -0,0 +1,82 @@ | ||
| 1 | /** Settings › Webhooks: Redeliver on a failed delivery, admin only, same-origin only (#8076). */ | |
| 2 | import { afterAll, beforeAll, expect, test } from "bun:test"; | |
| 3 | import { mkdtemp, rm } from "node:fs/promises"; | |
| 4 | import { tmpdir } from "node:os"; | |
| 5 | import { join } from "node:path"; | |
| 6 | import { createApp, type App } from "../src/app.ts"; | |
| 7 | import { createRepo } from "../src/core/repos.ts"; | |
| 8 | import { createSession } from "../src/core/sessions.ts"; | |
| 9 | import { createUser } from "../src/core/users.ts"; | |
| 10 | import { createWebhook, recentDeliveries } from "../src/core/webhooks.ts"; | |
| 11 | import { serve, type Servers } from "../src/http/server.ts"; | |
| 12 | ||
| 13 | let dataDir = ""; | |
| 14 | let app: App, servers: Servers; | |
| 15 | let owner = "", reader = ""; | |
| 16 | let hookId = "", deliveryId = ""; | |
| 17 | ||
| 18 | const state = () => recentDeliveries(app.db, hookId, 10)[0]?.state; | |
| 19 | const action = () => `/own/h/settings/hooks/${hookId}/deliveries/${deliveryId}/redeliver`; | |
| 20 | ||
| 21 | beforeAll(async () => { | |
| 22 | dataDir = await mkdtemp(join(tmpdir(), "forge-hook-redeliver-")); | |
| 23 | app = createApp({ dataDir, port: 0 }); | |
| 24 | servers = await serve(app); | |
| 25 | const mk = async (username: string) => { | |
| 26 | const u = await createUser(app.db, { | |
| 27 | origin: "seed", trusted: true, username, email: `${username}@f.dev`, password: "correct-horse-battery", | |
| 28 | }); | |
| 29 | if (!u.ok) throw new Error(u.error); | |
| 30 | return u.value; | |
| 31 | }; | |
| 32 | const o = await mk("own"); | |
| 33 | const r = await mk("reader"); | |
| 34 | owner = `forge_session=${createSession(app.db, o.id).token}`; | |
| 35 | reader = `forge_session=${createSession(app.db, r.id).token}`; | |
| 36 | const repo = await createRepo(app.db, app.reposRoot, { ownerId: o.id, name: "h", visibility: "public" }); | |
| 37 | if (!repo.ok) throw new Error("repo"); | |
| 38 | const hook = createWebhook(app.db, { repoId: repo.value.id, url: "https://example.com/h" }); | |
| 39 | if (!hook.ok) throw new Error("hook"); | |
| 40 | hookId = hook.value.webhook.id; | |
| 41 | app.bus.append({ kind: "push", repoId: repo.value.id }); | |
| 42 | app.db.query("UPDATE webhook_delivery SET state = 'failed', attempts = 3, last_status = 500").run(); | |
| 43 | deliveryId = recentDeliveries(app.db, hookId, 1)[0]!.id; | |
| 44 | }); | |
| 45 | ||
| 46 | afterAll(async () => { | |
| 47 | servers.stop(); | |
| 48 | app.close(); | |
| 49 | await rm(dataDir, { recursive: true, force: true }); | |
| 50 | }); | |
| 51 | ||
| 52 | function post(cookie: string, origin: string | null = servers.url) { | |
| 53 | const headers: Record<string, string> = { "content-type": "application/x-www-form-urlencoded", cookie }; | |
| 54 | if (origin !== null) headers.origin = origin; | |
| 55 | return fetch(servers.url + action(), { method: "POST", headers, body: "", redirect: "manual" }); | |
| 56 | } | |
| 57 | ||
| 58 | test("a failed delivery shows Redeliver to the admin only", async () => { | |
| 59 | const page = await (await fetch(`${servers.url}/own/h/settings/hooks`, { headers: { cookie: owner } })).text(); | |
| 60 | expect(page).toContain(action()); | |
| 61 | expect(page).toContain("Redeliver"); | |
| 62 | const other = await fetch(`${servers.url}/own/h/settings/hooks`, { headers: { cookie: reader }, redirect: "manual" }); | |
| 63 | expect(await other.text()).not.toContain(action()); | |
| 64 | }); | |
| 65 | ||
| 66 | test("a reader's or cross-origin POST changes nothing", async () => { | |
| 67 | expect((await post(reader)).status).toBe(404); | |
| 68 | expect((await post(owner, "https://evil.example")).status).toBe(403); | |
| 69 | expect(state()).toBe("failed"); | |
| 70 | }); | |
| 71 | ||
| 72 | test("the admin's POST makes it pending and redirects back; a pending row has no button", async () => { | |
| 73 | const res = await post(owner); | |
| 74 | expect(res.status).toBe(303); | |
| 75 | expect(new URL(res.headers.get("location")!, servers.url).pathname).toBe("/own/h/settings/hooks"); | |
| 76 | expect(state()).toBe("pending"); | |
| 77 | const page = await (await fetch(`${servers.url}/own/h/settings/hooks`, { headers: { cookie: owner } })).text(); | |
| 78 | expect(page).not.toContain(action()); | |
| 79 | // a second POST is refused with the notice and changes nothing | |
| 80 | expect((await post(owner)).status).toBe(303); | |
| 81 | expect(state()).toBe("pending"); | |
| 82 | }); |
new above
offline
The docs/features/webhook-redeliver.md how-to says there is no page button. Update it in this pull: describe the Redeliver button on failed rows in Settings > Webhooks, and remove the "Terminal only, for now" bullet.