Skip to content

social: a batch approve receipts a host refusal as sent, and a local hold keeps no reason on its card #3093

Closed

Description

@HugoXK

Implementation: FlyBot (ours) — PR follows within 24h. Do not pick up without a pickup-ok comment on this issue.

Written-by: FlyBot Lead
Claimed-by: FlyBot Lead
Claim-Id: 2a53d55ac9264fb68883a074205a67f8
Branch: dev/5-approve-all-failure-lines
Base: 3e901fa
At: 2026-11-15T00:58:51Z

Implementation: FlyBot (ours) — PR follows within 24h. Do not pick up without a pickup-ok comment on this issue.

Bot: FlyBot Dev-5
Tier: T2 (plugins/** is a T2 path; docs are T0)

Dedupe (live main 3e901fab + closed issues + code search): approveAll lives only in plugins/social/client.js; every batch-surface issue is closed — #3029/#3031 (receipt of what was sent), #3071/#3072 (a mixed batch erasing the reason line), #3075/#3076 (a remote failure losing its card to the poll), #3045/#3060 (one decision at a time) — and none covers a local hold that keeps no reason on its card, or a host refusal that is receipted as sent. rememberDockFailure has two call sites at main and neither serves item.source === 'local'. No open issue or PR on this surface.

What is wrong

Both halves are in the per-hold send inside approveAll, plugins/social/client.js, at live main 3e901fab.

  1. A local hold that fails keeps no reason on its card. The catch path at :2694 calls rememberDockFailure(...) only when item.source !== 'local'. So the dock line gets the reason, the card keeps none, and a batch of two local failures prints one line while both cards still read "Waiting for approval". The remote half lost the same thing in social: a send that failed in "Approve all" loses its failed card to the 5 s poll, and its Retry sends nothing #3075 and was fixed for remote only.
  2. A host refusal is receipted as sent. A local hold is sent with call('decide', { id, decision }). The host's decide() runs release(), which on a failed dispatch sets approval.status = 'failed' and approval.error and returns normally (core/src/host/hooks.js:501-541); decide() then saves and returns the clone. The plugin ignores that answer: it drops the card, calls forgetDockFailure, and the batch builds its receipt from the cards it dropped — so two failed local sends print Sent to Eve, Bob.

Reproduction, red first

At base 3e901fab with only the new test file added: node --test plugins/social/test/approve-all-failure.test.js → # tests 2 / # pass 0 / # fail 2, twice.

  • Case 1: two local holds fail → both cards still read "Waiting for approval", neither carries data-state="failed" or a reason; the only reason is the dock line.
  • Case 2: a local hold whose decide answers { status: 'failed', error } → the receipt reads Sent to Eve, Bob.

Fix

  • plugins/social/client.js:2651-2662 — one notSent(failure) helper: the same reason goes to the dock line, the card's own record and the Retry unseal for every kind of card; delete the item.source !== 'local' guard at :2694.
  • :2671-2675 — a local decide answer that carries error or answers status === 'failed' is a failure: the card stays, it is not counted in the receipt, and the reason stays on it. Deliberately not a blanket status !== 'approved': an explicit reject answers rejected (that is the real case a blanket check breaks — it would end the approval as a send failure), and a local retry answers { status: 'approved', ... } because core's decide() returns clone(approval) (core/src/host/hooks.js:645; social: a send that failed in "Approve all" loses its failed card to the 5 s poll, and its Retry sends nothing #3075 asserts the same at core/test/flybot.test.js:506). The error || half is the load-bearing one: the host's release() also answers a re-evaluation refusal as denied or stale with an error, and a status === 'failed'-only test would miss both.
  • :2482-2494 — wording only (P-819): the record is a Map, and the trim keeps at most 50 spent records, dropping the earliest-inserted first — not "50 most recent".
  • docs/subsystems/social.md — the dock-record paragraph gains the local-hold half and the corrected wording; plugins/social/** is an arch-docs owned path, so the page ships in this PR.
  • plugins/social/test/approve-all-failure.test.js — two cases; changelog.d/<this issue>.fixed.md.

No core file and no core/client.js rebuild (this is the plugin's own client file, not the bundle). No other plugin and no other file class.

Acceptance

Red first at 3e901fab with the source untouched and the test added: 2 tests, 0 pass / 2 fail, twice.

After the fix, on one push:

  1. node --test plugins/social/test/approve-all-failure.test.js → 2/2, twice.
  2. npm --prefix plugins/social test → 686/686 (baseline 684 + 2).
  3. TZ=UTC npm run verify → exit 0 (bench reading: 5686 checks, baseline 5684 + 2).
  4. node scripts/lint-plugins.mjs → 0 errors.
  5. Controls: a batch with one failure and one success still receipts only the people actually reached and prints no reason for the one that went; the remote path still keeps its social: a send that failed in "Approve all" loses its failed card to the 5 s poll, and its Retry sends nothing #3075 record; a card that failed locally still offers Retry, once.

Landing note

T2, so the reviewer's Verdict has to name the tip in the body before the queue takes it. plugins/social/client.js is the shared social lane and this PR takes it first: #3090 and #3061 carry no branch yet, so nothing is displaced.

Activity

  1. added 2 commits that reference this issue on Nov 15, 2026
    f8b30ad
    3b8dbad
  2. added a commit that references this issue on Nov 15, 2026
    cc59a97
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions