[Birfy]

Owner: Dev4
Reviewer: Reviewer2
Base: 75ca7ab
Tip SHA: c238c34
Written-by: Dev4
Fixes #1128

A friend's Bot that B removed from its home leaves no trace on A's side: the relationship stays active, so the row stayed, the click posted grant_bot_to_friend, and the panel printed Shared Chief with selftestb2. over a grant nothing can receive.

Reproduce. Two homes, a relationship the other side still lists, and list_remote_bots for that home without the Bot. social/share-friends returns the row with no removed mark and social/share-with-friend answers status: 'active' with one grant sent.

What changed. removedOnRoster (plugins/social/src/social.js) reads the roster the panel already paints from (remoteBots(), no extra round trip): a home the cache holds, a bots list with entries in it, and the Bot under neither its Bot id nor its handle is a removal. The row keeps its place, disabled, with the reason; the host refuses before sending with the code bot/removed-bot (bot/ is the one shape core/src/host/channel.js:24 passes through unchanged, so it is not flattened to bot/refused). A home the cache does not hold, a roster read that failed, an empty roster, or a handle hit under a different id decides nothing and shares as before.

What I ran (tip 06acbfc0, TMPDIR=/data/dsh/bot/workspace/tmp DSH_HARNESS=/opt/flybot TZ=UTC):

• cd plugins/social && node --import ./test/resolve.mjs --test test/social.test.js → 443 tests, 443 pass, 0 skipped
• the six #1128 cases on their own → 6/6
• node .github/scripts/changelog.mjs check → 40 fragments well formed; node scripts/ds-bot-patches.mjs check → ok 10 patches, ok core/ equals ds-bot/ + patches
• diff -rq against a clean c0a33696 unpack → only these 5 files

Red without the source change. Base src/social.js + client.js over the tip → the two behaviour cases red (row not marked, host shares anyway); the third is the negative control and stays green by design.

The "never guess" half now has teeth (Reviewer2's INC at 5052388a): one test per non-evidence input — roster read throws, home listed with bots: [], Bot listed under another id — and three knives, each reding exactly one of them:

if (!entry) return false → return true        → not ok 4 (roster read fails)
if (!bots.length) return false → return true  → not ok 5 (empty roster)
delete the handle-hit line                     → not ok 6 (other id)

Not run: npm run verify (no esbuild/binary on this host), oxlint, the tree/timeline/core suites (untouched), and a real Bot Social server; patch/git do not exist here, so the rebase onto c0a33696 was replayed hunk by hunk and re-tested, not reused.

Risk. The refusal depends on the cached roster: a warm cache that predates a removal still shares once, and a stale cache that predates an addition cannot mark anything removed (it needs a bots list without the id). Both fail toward sharing, which is the pre-fix behaviour rather than a new refusal.

[Birfy]

Verdict: INC at 5052388
Written-by: Reviewer2

Ran:

• cd plugins/social && node --import ./test/resolve.mjs --test "test/*.test.js" → # tests 493 / pass 493 / fail 0, 0 skipped
• red: put the base src/social.js and client.js (44f3e97) back over the tip and re-ran the #1128 cases → not ok - a Bot the other home no longer lists is not shared, and the row says why and not ok - a row for a Bot the other home removed is shown with the reason and cannot be shared; restored → 493/493. (The third case, "a Bot the other home still lists goes through as before", is green on base by design — it is the negative control against over-refusing, not evidence of the fix.)
• knife: if (!entry) return false → return true (a home missing from the cached roster counts as removed) → 493/493 still green
• knife: if (!bots.length) return false → return true (an empty roster counts as removed) → 493/493 still green
• knife: delete the line if (handle && bots.some(bot => bot.handle && norm(bot.handle) === norm(handle))) return false (a handle hit no longer means "still there") → 493/493 still green

Blocking — the "never guess a removal" half has no test:
docs/subsystems/social.md and the changelog both promise that a home missing from the cached roster, a roster read that failed, an empty bots list, and a Bot whose handle is still listed under another id all decide nothing — the row shares as before. The three knives above each break one of those promises and no test notices; a one-line regression there silently marks every friend row as removed, disables Share, and the host refuses with bot_removed — a worse user-visible bug than the one this fixes. The two red cases do not cover it: list_remote_bots succeeds in both, the home is present, and the id hit is the only branch exercised.

Work to do (same file, plugins/social/test/social.test.js; push a new tip and @ me):

1. server.tools.list_remote_bots = () => { throw new Error('roster down') } → share-friends returns the row without removed, and share-with-friend returns ok with one grant. Knife: if (!entry) return false → true must red only this case.
2. roster has @bob.home with bots: [] → same two assertions. Knife: if (!bots.length) return false → true must red only this case.
3. roster lists @writer.bob.home under a different bot_id (x9, not the relationship's pb-writer) → row not removed, share goes through. Knife: delete the handle-hit line must red only this case.
4. While you are there: the refusal case should assert error.code === 'bot_removed' (today only the message text is checked; the client branches on removed, the API contract is the code).

What is fine and needs no rework: the two red cases above (row marked with the reason, host refuses before any grant with granted.length === 0), the client half (disabled button, reason line, a click that still arrives sets an error note and posts nothing), the changelog and the social.md section, and the diff itself — +173/−2, nothing unrelated deleted. Every check on the tip is completed; only enqueue-guard is red, and it is red because this verdict is INC.

[Birfy]

Written-by: Dev4

New tip 06acbfc0, base the current main c0a33696 (rebased, since #1145 landed on plugins/social/src/social.js and its tests; the five changed files were replayed and the suite re-run on the new base, not reused).

All four INC items are in:

1. Roster read fails — server.tools.list_remote_bots throws → share-friends returns the row with no removed/reason, and share-with-friend answers active with exactly one grant.
2. Home listed with bots: [] — same two assertions.
3. Bot listed under another id (x9) with the same handle — row not marked, share goes through.
4. The refusal carries a code — error.code === 'bot/removed-bot'. One thing to flag: the channel only passes codes that start with bot/ unchanged (core/src/host/channel.js:24), so a plain bot_removed would have reached the client as bot/refused — the code itself had to move to the bot/ namespace for the assertion to mean anything. docs/subsystems/social.md and the changelog fragment say the same string now.

Knives, each reding exactly one case (--test-name-pattern="1128", 5 pass / 1 fail each time):

if (!entry) return false → return true        → not ok 4
if (!bots.length) return false → return true  → not ok 5
delete the handle-hit line                     → not ok 6

On the new tip: plugins/social test/social.test.js 443/443, 0 skipped; changelog check 40 fragments; ds-bot-patches check two ok lines; diff -rq against a clean c0a33696 → only the five files.

The 5052388a verdict is void with the new tip. @reviewer2 please judge 06acbfc0; I will not move the tip again.

[Birfy]

Written-by: Dev4

Tip moved once more, before you judged it: c238c340, base the live main 75ca7ab9 (#1146 landed on plugins/social/test/social.test.js and social.md). The four INC items are unchanged and the suite was re-run on the new base: 444/444, 0 skipped (test/social.test.js); knives still red exactly one case each.

@reviewer2 please judge c238c340; 06acbfc0 is spent with the base move. No further commits from me on this branch.

[Birfy]

Verdict: PASS at c238c34
Written-by: Reviewer2

Ran:

• tip tree of c238c34: cd plugins/social && TMPDIR=... node --import ./test/resolve.mjs --test --test-concurrency=1 "test/*.test.js" -> # tests 501 / pass 501 / fail 0 / skipped 0; test/social.test.js alone 444/444.
• red: put base 75ca7ab's src/social.js and client.js back, --test-name-pattern="1128" -> 4 pass / 2 fail (not ok 2 the row says why and the host refuses with no grant, not ok 3 the client row); restored -> 6/6.
• knives, one guard each, on the real tip: if (!entry) return false -> return true -> 5 pass / 1 fail, only not ok 4 (roster read fails); if (!bots.length) return false -> return true -> only not ok 5 (home listed with bots: []); delete the handle-hit line -> only not ok 6 (same handle under id x9). Each knife reds exactly the case it is meant to, and not ok 1 (a Bot still listed) stays green in all three.
• core/src/host/channel.js:24: errorCode returns a code unchanged only when it starts with bot/, otherwise bot/refused / bot/internal. So bot/removed-bot is the only shape that can carry this refusal to the panel as a code, and assert.equal(result.error?.code, 'bot/removed-bot') is a real contract assertion, not a string.
• local changes reverted after each run: md5 of src/social.js and client.js equal the blobs at c238c34 before the verdict.

Checked:

• The four INC items are all present and pinned: roster read throws, home with bots: [], same handle under another id, and the refusal's code. The "never guess" guards are read before any refusal, so nothing here marks a row on a missing home, a failed read or an empty roster.
• The row and the host use the same predicate (removedOnRoster) and the same cached read (remoteBots()), so an old row that still posts a click is refused with the reason instead of printing a share; no grant is sent (granted.length === 0).
• Client half: a removed row stays drawn with the reason, its button is disabled, and a click that arrives anyway sets an error note and posts nothing; the row beside it still works.
• Docs/changelog say the same string and the same guards as the code; diff is the five files, +223/-2, the only removed line is the handler signature (one more field).
• CI at this tip: every check completed success except enqueue-guard, which is red only because the conclusion it names is the old tip. The two agent-audience runs had both been cancelled by a duplicate trigger on the same sha, leaving no green run; I re-ran 40184452570 (attempt 2 -> success) so the tip has a green run, then re-read the check list.

Non-blocking notes:

• The removal judgement reads the cached roster (REMOTE_TTL_MS), so a Bot a friend added moments ago can be judged removed for the length of that cache; the outcome is a named refusal and a retry recovers. If it ever shows up, the fix is remoteBots({ fresh: true }) only on a row that would be marked.
• "No extra round trip" in the docs holds while the cache is warm; a cold cache pays the fetch the panel needed anyway.

I am enqueueing this tip.