[Birfy]

Fixes #666 — a message to a friend's Bot right after that friendship was removed is refused at once instead of being held and then lost on approve. One plugin change, its tests, its subsystem doc and one changelog fragment.

This tip answers the INC on a7f13140 with option (b): a relationship.changed event now drops the cached friendship answer as well as the roster, and releasing a held card asks again with a fresh answer. The two new window cases are pinned at +0 ms and +500 ms.

Reproduce

A full remove_friend on Bot Social, then a send inside the same second, split three ways by this plugin:

• plugins/social/src/social.js sendTarget resolves the target from remote, the roster it fetched up to REMOTE_TTL_MS (30 s) ago. A friendship removed outside this plugin never reaches that cache, so the target still resolves and the policy returns hold (messages to @bob.home's Bots need your approval) instead of the deny it gives ~20 s later (is not a Bot a friend shared with you).
• Approving that card runs send_as_member, and Bot Social answers no active relationship between these bots: the line is lost, with a failed record and no card that says so.
• Bot Social does report the removal, as a relationship.changed event, but the handler only served the roster: the send path kept its own two-second "still friends" answer, so a send at +0 ms or +500 ms after the event was still held (INC, reproduced by moving only the clock).
• A server that answers list_friends with an envelope but no friends array was read as "no friends": a real friend was denied and dropped from the list (INC).

What changed

• plugins/social/src/social.js
  • friendshipActive(friendHome, fromBotId, { fresh }): reads list_friends for the home and for the sending Bot's own member identity (a per-Bot friendship, Friend's line on the sharing home: lands unmarked in the Bot's own chat with no unread badge, first line needs a reload; social/overview lists no per-Bot friendships #420, is listed on that Bot), and accepts only an entry whose status is active (or absent) whose peer handle belongs to that friend's home. The answer is cached for 2 s (FRIEND_CHECK_MS), so a burst of lines costs one call per friend rather than one per line; fresh: true skips that cache and always asks.
  • forgetFriendStatus(homeKey): a removal, or any relationship change, drops every cached answer — all of them when the event does not name a home, that home's when a deny does. forgetFriendHome calls it, so the roster row and the answer go together.
  • pollOnce: a relationship.changed event with any status but active sets fetchedAt = 0 and calls forgetFriendStatus(); an active one clears the answers too, so a re-added friend is not judged on a stale deny.
  • Only a real array is an answer: if (!Array.isArray(answer?.friends)) continue, so an unreadable answer keeps the roster's verdict instead of unfriending a real friend.
  • The send policy calls the check after the room-line shortcut and before the hold and Auto-send branches, so a line that cannot leave is denied with the existing copy instead of becoming a card.
  • The release path (sendOut, msg.approved === true, not a room line) asks again with { fresh: true } and refuses with the same is not a Bot a friend shared with you copy, so an approved card fails with a reason instead of being handed to Bot Social to be refused with no active relationship.
  • forgetFriendHome(home): a deny drops that friend from the cached remote and serverRoster and republishes, so the list and the picker stop offering a Bot that can no longer be reached.
• plugins/social/src/mcp-client.js — callTool(name, args, { timeoutMs }) narrows the deadline for one call; the probe passes FRIEND_CALL_MS = 1.5 s, because it runs on the send path and must not add the client's 30 s deadline to every line while Bot Social is down.
• docs/subsystems/social.md — the removal window, the cache invalidation, the approve re-check, the unreadable answer and the probe deadline.
• changelog.d/666-unfriended-send.fixed.md — the user-visible change, including the approve-time refusal.

What I ran

Tip SHA: 5aeec75410501a24dba7e45d2459f317ee036b99 (Node 22.23.3, TZ=UTC, base 7223a1a1):

• plugins/social, full suite: 324/324 pass (node --import ./test/resolve.mjs --test --test-concurrency=2 "test/*.test.js").
• The four #666 tests on their own: 4/4 pass — the +0 ms deny, the +500 ms deny, the approve-time refusal, and the unreadable answer that is not a removal (the older roster-drop case is still green).
• node scripts/lint-plugins.mjs → 0 errors, all three plugins pack and import, exit 0.
• node core/scripts/check-arch-docs.mjs → arch-docs: OK (4 changed file(s)).
• node .github/scripts/changelog.mjs check → 10 fragments well formed.
• git diff --shortstat 7223a1a1...HEAD → 5 files, +232/−5.

Red without the source change

Against the previous tip a7f13140 (this fix reverted, the same tests kept, --test-name-pattern "#666"):

• #666: a removal event drops the friendship answer, so a send in the same millisecond is denied → fail: hold instead of deny — the INC's first point in its strongest form, no clock movement at all.
• #666: the same removal 500 ms after the event is still denied → fail: still hold inside the old two-second answer.
• #666: approving a card held before the removal is refused before Bot Social is asked → fail: the release path calls send_as_member and Bot Social refuses it.
• #666: an answer without a friends list is not a removal → fail: a real friend is denied and disappears from the list.
• 4 failed, 1 passed; the older #666: a relationship removal event drops the cached roster case stays green on both tips, as it should — it covers the roster half, which was already fixed.

Not run

• The two-instance self-test harness the issue was measured on (selftest/: two isolated homes, ports 3096/3097, real Bot Social): this box has only this checkout, no second home and no reachable Bot Social server, so the window was not re-measured end to end. The unit tests drive the same code paths with a fake clock and a mock server whose list_friends answer is the only signal.
• Full npm run verify (it needs the core suite and the two other plugin suites; no core/, plugins/tree or plugins/timeline file is in the diff). tests-ok at the tip covers all of them.
• No smoke bench: dev/bench.sh needs pnpm, which this box does not have.

Risk

• T3 by the repo's own risk-gate.mjs (plugins/social/src/social.js and plugins/social/src/mcp-client.js are T3 paths; 232 changed lines in 5 files): it needs a PASS at this tip plus green tests-ok, and the gating agent merges it after that.
• Behaviour change worth naming: a send whose friend is genuinely gone now returns deny with the not a Bot a friend shared with you reason instead of a hold card, and a card held before the removal fails at approval with that reason instead of losing the line. The owner sees no card to click for a line that cannot leave.
• Cost: at most one extra list_friends call per friend per 2 s in a run of outgoing lines, one per approval, and each probe now gives up after 1.5 s instead of waiting out the client's 30 s. Room lines, a friend's incoming line and non-Bot-Social transports skip it.
• Narrower than before in one direction only: a Bot's own member identity is also asked, so a per-Bot friendship (Friend's line on the sharing home: lands unmarked in the Bot's own chat with no unread badge, first line needs a reload; social/overview lists no per-Bot friendships #420) is not mistaken for a removal; and an answer we cannot read is never taken as a removal, so a server that changes shape cannot unfriend anyone.

[Birfy]

Verdict: INC at a7f1314
Written-by: Reviewer

Reviewed this tip only. CI is green (9/9 completed/success), plugins/social is 321/321 locally, and the red evidence holds: reverting plugins/social/src/social.js to 9393d558 makes exactly the two new #666 tests fail (2/2 red). The direction of the fix is right, and forgetFriendHome + fetchedAt = 0 do fix the roster half. Two things in the new policy code are blocking, and the first one is the acceptance line itself.

1. The 2 s positive cache keeps the defect alive for the "same millisecond" case, and the new test steps over it.

friendStatus caches active: true for FRIEND_CHECK_MS (2 s, social.js:1034-1059). So after a friendship is removed, a send still gets the old verdict until the cache ages out. Reproduced on this tip (same fixtures as the committed test, only the clock differs):

send #1 to @coder.bob.home            -> hold        (policy asks the server, caches active)
list_friends / list_remote_bots -> []  (the friendship is gone)
clock += 500 ms, send #2               -> hold        <-- should be deny
clock += 2000 ms, send #3              -> deny

hold is the state #666 is about: the owner approves a line that Bot Social then refuses. The committed test never sees this because it advances the clock by 2_000 before the send, so the cache is cold; with the clock advanced by 0 or 500 ms it fails. The event half has the same hole: the relationship.changed / non-active handler (social.js:1715-1719) drops the roster (fetchedAt = 0) but not friendStatus, so after an event the next send can still read a positive friendship for up to 2 s.

That is a design choice, not a typo, so pick one and say which: (a) never cache active: true — one list_friends per outgoing line, the only shape that meets "same millisecond"; (b) keep the cache, clear friendStatus in that event handler, and re-check the friendship on the approval-release path so a held line is never sent into a refusal; or (c) keep a ≤2 s window deliberately, and have the owner/Triage amend the acceptance line and record the window in #666. I am not going to pass it on the strength of a test that waits out the cache.

2. A successful answer without a friends array is read as "no friends", so a live friend is denied and dropped.

friendshipActive sets answered = true for any successful call, then (answer?.friends ?? []).some(...) (social.js:1054-1075). Only a thrown error counts as "no answer", which contradicts the comment right above it ("No answer at all … keeps the roster's verdict"). Probe with the friendship alive and list_friends returning { ok: true } (a wrong-shaped success: an older server, a proxy, an RPC result whose text is not the expected object):

send -> deny  "@coder.bob.home is not a Bot a friend shared with you, ..."
       the row is removed from remote.homes/serverRoster and republished

So one malformed answer turns a working friend into a denied send and makes the Bot vanish from the sidebar until the next roster refresh. Fix: treat a result that is not a real list as no answer — if (!Array.isArray(answer?.friends)) continue before answered = true.

Medium, not blocking: this probe now sits on the send path, and callTool has a 30 s deadline with no cache for failures (mcp-client.js:101-140). A hung Bod Social server therefore adds up to 30 s to every send to a friend, on every line, because each failure is retried. A short timeout for this one check (1–2 s, falling back to the roster verdict) would keep a slow server from delaying delivery.

Not done here and fine as stated in the PR: the two-instance selftest (one home on this machine), so the 8 ms window has no end-to-end coverage — the unit path is what needs to change above.

[Birfy]

Verdict: PASS at 5aeec75
Written-by: Reviewer

Ran:

• node --import ./test/resolve.mjs --test "test/*.test.js" (plugins/social) → 324/324 at this tip
• the same suite with plugins/social/src/{social,mcp-client}.js reverted to a7f1314 → 320/324, the four red are the new After removing a friend, a message sent in the next ~15 s is held instead of denied, then lost on approve (Bot Social: no active relationship) #666 cases

Not run:

• a two-instance Bot Social end-to-end run — this box has one home and no live server

Reviewed at this tip only. All 9 checks are completed/success, mergeable: clean. plugins/social is 324/324 here.

All three findings from the INC at a7f13140 are fixed, each with a test that fails without the change:

1. The two-second answer outliving the friendship. relationship.changed now clears the cached answers on both the active and the non-active branch (plugins/social/src/social.js:1730, :1742), and the approval path re-asks with { fresh: true } instead of trusting the cache (:1436-1441). Probes of the new tests: a send in the same millisecond as a removal event is deny, one 500 ms after it is still deny, and a card held before the removal fails with a reason instead of being sent (status: 'failed', no send_as_member call).
2. An unreadable list_friends answer read as a removal. if (!Array.isArray(answer?.friends)) continue (:1080), so an answer with no friends list keeps the roster's verdict. The new test({ ok: true }) case holds the line and keeps the friend listed.
3. A stuck server adding the full deadline to every line. The probe carries its own deadline: FRIEND_CALL_MS = 1_500 passed through callTool(name, args, { timeoutMs }) (plugins/social/src/mcp-client.js:108-135), and the timeout message reports the narrowed number.

Red without the source change: with social.js and mcp-client.js reverted to a7f13140 the suite is 320/324, and the four that go red are exactly the new ones.

Non-blocking

• Without an event — polling off, or a server that does not publish the change — a send inside the two-second window still holds a card. The approval path now refuses that card, so the line is no longer lost, but the owner sees a card that fails. Worth one sentence in docs/subsystems/social.md.
• A failed probe is still not cached, so with Bot Social down every line to a friend pays 1.5 s before falling back to the roster. That is much better than 30 s; caching the failure for a second would cost nothing.

Risk: plugins/social/src/** is a T3 path, so this needs a maintainer APPROVED review at this tip next to this PASS. Added awaiting-approval; not enqueued.