Home

boring / v2

publicBoring-Software-Inc/v2
Overview Code History Branches Pull requestsIssuesInsights Campfire Docs
main
HomeOverview Code PRsIssues

#169 [Detail Bug] Workflows: Discord notifications never deliver on comment/schedule triggers and may post a bogus PR request-changes review

open

Opened by detail-app[bot] · 6 days ago

detail-app[bot]opened this issueAuthor· 6 days ago

Sign in to comment.

Linked pull requests

No linked pull requests yet.

Detail Bug Report

app.detail.dev/org_0f605249-55f3-46eb-880c-20ae3e45ae99/…/bug_ad78a65a-168b-40c4-b5ee-15a3e7c504e2

Introduced in #80 by @ripgrim on Oct 23, 2026

Summary

  • Context: Workflows can send outbound "delivery" notifications (generic webhook and Discord) as part of a run. The delivery body is not built when the action row is recorded — emitPrSurface later attaches a WebhookPayload to each recorded row, and a cron job (deliver-webhook) POSTs it. emitPrSurface has two paths: the PR path and the thread path (used for comment.created and schedule.matched triggers, shared via emitThreadActions).
  • Bug: On the thread path, delivery payloads are attached only to webhook rows — discord rows are left without a payload — and worse, discord rows are then fed through the forge adapter as if they were block actions.
  • Actual vs. expected: A Discord node in a comment- or schedule-triggered workflow should deliver a formatted Discord message exactly like it does on the PR path; instead the Discord message is never sent — the row is either consumed by a bogus REQUEST_CHANGES review on the PR, or (when that mis-execution fails) the delivery cron permanently destroys the row as "malformed".
  • Impact: Discord notifications configured on comment/schedule-triggered workflows silently never fire, and trigger an additional wrong side effect: a spurious "Blocked. See the comment." request-changes review can be posted onto a user's PR by a workflow that only contained a Discord node.

Code with Bug

emitThreadActions in apps/worker/src/jobs/pr-surface.ts attaches the delivery payload only to webhook rows:

for (const row of input.pendingActionRows) {
    if (row.kind !== "webhook") {          // <-- BUG 🔴 discord rows never get a delivery payload
        continue;
    }
    const delivery: WebhookPayload = { ... };
    await runServices.attachDeliveryPayload(db, row.id, delivery);
}

The thread path execution loop skips only webhook rows, so discord rows fall through into forge execution:

for (const row of input.pendingActionRows) {
    if (row.kind === "webhook") {          // <-- BUG 🔴 should also skip "discord" delivery rows
        continue;
    }
    if (VERDICT_ONLY_KINDS.has(row.kind)) { ... }
    if (thread.skips(row.kind)) { ... }
    try {
        const result = await adapter.execute(
            toForgeAction(row, repoFullName, thread.number, REVIEW_POINTER.block),
        );
        await runServices.markActionExecuted(db, row.id, result.externalId);
    } catch (error) { /* "row stays recorded for retry" */ }
}

toForgeAction has no discord case, so a discord row hits default and becomes a PR "block" action:

default: {
    const reason = (row.payload.reason as string | undefined) ?? reviewText;
    return activeReviewId
        ? { kind: "update-review" as const, repoFullName, number, reviewId: activeReviewId, reason }
        : { kind: "block" as const, repoFullName, number, reason };   // <-- BUG 🔴 discord row re-typed as a PR block
}

Downstream, apps/worker/src/jobs/deliver-webhook.ts abandons delivery rows that have no payload:

if (!url || !delivery) {
    // Malformed row (no destination or unbuilt payload) — abandon it
    // rather than retry forever.
    await runServices.markActionSuperseded(db, row.id);   // <-- BUG 🔴 payload-less discord rows are destroyed
    result.abandoned++;
    logger.error({ actionId: row.id, kind: row.kind },
        "delivery row missing url or payload — abandoned");
    continue;
}

Explanation

  • On comment.created and schedule.matched, emitPrSurface routes through emitThreadActions.
  • Unlike the PR path, emitThreadActions only attaches WebhookPayload to webhook rows; discord rows remain recorded without payload.delivery.
  • The same function also only skips webhook rows during execution, so discord rows are incorrectly executed via the forge adapter.
  • Since toForgeAction has no discord mapping, the default branch converts the row into a block action; for PR threads this can post a REQUEST_CHANGES review with the default text ("Blocked. See the comment."), and marks the discord row executed (so it will never be delivered).
  • If the mis-execution fails (e.g., scheduled run on an issue thread where the pulls API 404s), the row remains recorded and the deliver-webhook cron later marks it superseded as malformed due to missing payload.

Codebase Inconsistency

  • In the same file (apps/worker/src/jobs/pr-surface.ts), the PR path treats both webhook and discord as delivery kinds (attaches payloads for both and skips both during forge execution). The thread path handles only webhook, creating an asymmetry.

Recommended Fix

Mirror the PR path inside emitThreadActions: in the payload-attach loop, treat webhook and discord alike (if (row.kind !== "webhook" && row.kind !== "discord") continue;), and in the execution loop skip both kinds (if (row.kind === "webhook" || row.kind === "discord") continue;) so delivery rows never reach toForgeAction. Optionally, as defense in depth, make toForgeAction throw on unknown delivery kinds instead of silently re-typing them through the default: branch.

History

This bug was introduced in commit 67ed62f8 ("Revert "chore: remove the discord workflow step"", Oct 23). The revert restored Discord outbound action nodes across the codebase, but in apps/worker/src/jobs/pr-surface.ts it only re-added discord handling to the two loops on the PR path inside emitPrSurface. The thread path (emitThreadActions, generalized from emitScheduledActions) was never patched, so once Discord nodes existed again, discord rows on comment/schedule-triggered workflows fell through the webhook-only filters and were mis-executed as block forge actions or destroyed as malformed by the delivery cron.