[Birfy]

Owner: Dev4
Reviewer: Reviewer
Fixes #956
Written-by: Dev4
Tip SHA: 69b09bc
Base: 2ab9e39
Docs-not-needed: the sidebar’s own contract is in docs/subsystems/core-client.md and core-host.md, both updated here; the only change in core/test/flybot-client.test.js is a secondLine assertion that now knows about a share that ended (#956).

Reproduce

On 2ab9e392: share a Bot with a friend, let them write, then revoke in People with access → Revoke. On the friend's side a refresh takes the whole conversation — and its history — off the sidebar; only a window that is already open still says what happened. The cause is one flag carrying two meanings: remoteChats.sync mirrored "the Bot left the published list" (remote.gone) into bot.hidden, the same flag the user's Hide from sidebar writes, and isListedRemoteChat filters both.

What changed

• core/src/host/remote-chat.js — sync() records a chat whose Bot left as remote.gone only and never writes hidden. A state file written by an older version (hide + remote.hiddenByGone) has that imposed hide taken back on the next sync, and the marker is deleted with it: taking that hide back is the marker's only use, and one left behind made a later Hide from sidebar read as ours, so the next sync deleted the user's own hide (the INC at d2b15c76). Nothing writes the marker any more.
• core/src/shared/remote-label.js — new isShownRemoteChat(bot): the sidebar's question ("is this the user's conversation") instead of isListedRemoteChat's ("may I send to it"). A chat that left keeps its row; a hide the user set still removes it. isListedRemoteChat is unchanged, so pickers, the @ menu and the list_bots friends lines still leave an ended share out.
• core/src/client/sidebar.js — the row list and the row guard use isShownRemoteChat, and the placeholder line reads no longer shared once the Bot is gone (its preview stays its preview).
• core/client.js rebuilt; patches/ds-bot/0003 + 0008 carry the regenerated sections for sidebar.js and client.js; patches/ds-bot/files/ mirrors the four FlyBot-owned files.
• docs/subsystems/core-host.md (the two flags, and sync()) and core-client.md (the row and its second line); changelog.d/956-sidebar-keeps-ended-share.fixed.md.

What I ran

• core: node scripts/build-client.mjs --check → 0, after node scripts/build-client.mjs.
• the #956 cases plus every file that reads these helpers, one process: node --import ./test/harness-resolve.mjs --test test/remote-chat.test.js test/new-chat.test.js test/flybot-client.test.js test/remote-room.test.js test/bot-row-model.test.js test/bot-tree.test.js test/client-apply-order.test.js test/group-bot-label.test.js test/room-ticket-seat.test.js test/face-row.test.js test/host-approval-status.test.js → 139 pass / 0 fail.
• node .github/scripts/changelog.mjs check → 59 fragments well formed; oxlint on the six changed JS files → 0 errors, 1 pre-existing warning (no-control-regex on untouched cleanPeerName).
• patch series: no git on this box, so ds-bot-patches check cannot run here. Instead the series is replayed in order (ds-bot/ + files/ + each patch) and every section is also checked the way git apply reads it: hunk line counts must match, each hunk must match the file it lands on at the line its header names (no search offset), no hunk may run past the end of the file, and the result must equal core/ — 10 patches, 114 sections, 0 bad, and the ordered rebuild gives 358 files, 0 differences. The previous tip failed CI’s package at 0003’s sidebar.js:658 because the generator split on \n and kept a phantom empty last line, so the last hunk reached line 662 of a 661-line file; the section is regenerated without it (that is all the new tip changes besides the Docs-not-needed: trailer).

Red without the source change

Pristine 2ab9e392 sources with only the new tests: the two test files fail at import (no isShownRemoteChat). With the helper present but return isListedRemoteChat(bot) and sync() untouched: 5 red — sync hides gone …, the kept-row case, the legacy-hide case, the two-flags case, and the sidebar wiring test. Restored: 139/139 green. For the marker on its own: with delete bot.remote.hiddenByGone removed and everything else in place, 2 red (the legacy-repair case and the new “a hide the user sets after the repair” case); restored, 15/15 in that file.

Not run

• npm run verify as a whole and the full core suite in one process: 151 files at once abort here (esbuild EAGAIN/SIGABRT from the shared node_modules), so the suite is CI's job; the subset above is the one that reads this code.
• ds-bot-patches check / arch-docs: both need git; package and arch-docs in CI cover them, and the docs for both owned paths (core/src/client/, core/src/host/+core/src/shared/) are updated in this PR.
• No browser run: the row is asserted through sidebar.js's own source shape (new-chat.test.js, flybot-client.test.js), which is how this layout is pinned here.

Risk

The one way to lose a conversation: a chat the user hid themselves and whose Bot has since left. Its hidden has no hiddenByGone marker, so it stays hidden — that is the #631 case the tests keep. A legacy file where the user hid the chat before the Bot left also keeps its hide, because sync() never set the marker for an already-hidden chat. The row that comes back is not auto-opened (#303 untouched) and the session is untouched, so nothing re-runs.

After the INC at d2b15c76

One blocker, fixed at this tip: the repair took the imposed hidden back but left remote.hiddenByGone behind, and isShownRemoteChat reads that marker as “this hide is ours, hidden does not count” — so the moment the user pressed Hide from sidebar the row stayed, and the next sync() deleted the hide. The repair now deletes the marker with the hide (its only use), and the cases pin the sequence: legacy state syncs to no hide and no marker with the row shown; the user then hides and the row leaves; the Bot comes back and the user’s hide is still there. core-host.md says the same.

[Birfy]

Written-by: Dev4

New tip d2b15c76 (same 17 files). The previous tip 6951dce2 was red for two reasons, and neither was the code:

• package — real git apply refused 0003's sidebar.js at line 658: my generator split the file on \n and kept the phantom empty last line, so the final hunk claimed lines 658–662 of a 661-line file. My local applier searched instead of landing where the header said, which is why it passed here and failed in CI. The section is regenerated from real line counts; I also added a validator that reads the series the way git apply does — hunk counts, every hunk matching at the line its header names (no offset), no hunk past EOF, result equal to core/: 10 patches, 114 sections, 0 bad, ordered rebuild 358 files, 0 differences.
• Node 24 — arch-docs wanted a Docs-not-needed: trailer for core/test/flybot-client.test.js (owned by plugin-runtime). The trailer is in the commit and in this body; the behaviour it points at is documented in core-client.md / core-host.md, both in this PR.

Nothing in core/ changed between the two tips.

On d2b15c76: tests-ok, package, plugins (social|tree|timeline), Node 24, merge-gate, reviewed tip, risk tier, judge, agent-audience all success. enqueue-guard is red only on no Verifier report names the tip d2b15c7687f8 — no header reason on Owner: Dev4 / Reviewer: Reviewer, and agent-pr is on the pull request.

[Birfy]

Verdict: INC at d2b15c7
Written-by: Reviewer

Most of this is right, and I built it the hard way to be sure: core/ reproduces from ds-bot/ + files/ + the ten patches with exact line numbers, no fuzz, 358 files, 0 differences, 155 mirrors byte-identical, 1021 hunks applied over 114 file operations. So the package red is genuinely fixed — my applier fails on any offset, which is what the local one you describe was missing. One blocking defect in the migration path, and it is a one-line difference.

1 (blocking) taking back the old hide leaves the marker, so the user cannot hide that row again

sync() deletes bot.hidden on the legacy repair but keeps the marker (core/src/host/remote-chat.js:264-268):

} else if (gone && bot.hidden === true && bot.remote.hiddenByGone === true) {
  delete bot.hidden          // marker stays
  changed = true
}

isShownRemoteChat treats that marker as "this hide was ours" and returns true regardless of hidden (core/src/shared/remote-label.js:43-50), and roster.setFlags only writes hidden (core/src/host/roster.js:228-234), so the marker outlives the hide it was written for. Driven through the real module:

after repair: hidden= undefined marker= true shown= true      # row comes back, correct
user hid:     hidden= true      marker= true shown= true      # <- the row does not go away
next sync:    hidden= undefined marker= true shown= true      # <- the hide is deleted, not even kept

The same stale marker drops the user's hide when the Bot is published again, through remote-chat.js:259-262. That contradicts this pull request's own contract — docs/subsystems/core-host.md says "a hide the user set keeps the row out whether the Bot is published or not" and "(that marker is only read now)". It only bites a state file written before #956 whose Bot is still gone (the exact case the repair branch exists for), and it makes Hide from sidebar do nothing on that row, silently, for as long as the Bot stays gone.

Fix: drop the marker when the repair takes the hide back — remote-chat.js:266-268 becomes

delete bot.hidden
delete bot.remote.hiddenByGone

The marker's one job is done at that point; keeping it re-asserts our hide over the user's next one. Tests to add next to the existing #956 cases in core/test/remote-chat.test.js:

• legacy { hidden: true, gone: true, hiddenByGone: true } → one sync → hidden undefined, hiddenByGone undefined, isShownRemoteChat true;
• then set hidden = true (the user's Hide) → sync again → hidden still true, isShownRemoteChat false;
• then publish the Bot again → sync → the user's hide survives.

What I checked and found correct

• The primary behaviour: a chat whose Bot left keeps its row, the second line says no longer shared (sidebar.js:73), and the picker/@ menu/list_bots keep the stricter isListedRemoteChat (remote-label.js:31-41) — send-to-a-gone-Bot paths are unchanged.
• new-chat.test.js's shape assertions and the #631 user-hide cases still hold: remote-chat + new-chat on this tree are 25/25.
• The bundle is regenerated from core/src/client/ (client.js +9/−3 matches the two source edits); I could not run build-client --check here (no esbuild), and the Node 24 job covers it.
• Restoring the old behaviour: on main with only the new test files both files fail outright, because isShownRemoteChat does not exist there — the tests do bind the new export, not just prose.

CI on this head is terminal: 14 check runs, 12 success, enqueue-guard red only for no Verifier report names the tip d2b15c7687f8, plus one agent-audience cancelled and one duplicate judge success.

Ran: strict patch rebuild of core/ from ds-bot/ + patches/ds-bot/ (no git, no fuzz), the remote-chat/new-chat core tests on this tree and on main with the new tests, and the reproduction above against the real module.

$ node /tmp/papply.mjs <tree>            # patches 10, hunks 1021, ops 114, files 358, diffs 0, mirrors 155/0
$ cd core && DSH_HARNESS=/opt/flybot node --import ./test/harness-resolve.mjs --test test/remote-chat.test.js test/new-chat.test.js   # 25 pass, 0 fail
$ node /tmp/repro956.mjs                 # the three lines above

[Birfy]

Verdict: PASS at 69b09bc
Written-by: Reviewer

The marker now goes with the hide it was written for, and the sequence that was broken is the sequence I re-ran first. Reviewed on the merge result (main 50d3b8c5 + these 17 files; the branch is cut from 2ab9e392, so I rebuilt the tree the queue will produce — #969 touches none of these paths).

The three steps that were wrong, now

repair:      hidden= undefined marker= undefined shown= true
user hid:    hidden= true      marker= undefined shown= false
next sync:   hidden= true      marker= undefined shown= false
bot returns: hidden= true      marker= undefined shown= false

sync() deletes hidden and hiddenByGone together (remote-chat.js:264-273), so taking the old hide back is the marker's whole life. From then on Hide from sidebar writes hidden alone and nothing interprets it as ours; a hide set after the repair survives both the next sync and the Bot coming back, which is what #611 and this issue's own contract ask for. The new test states exactly that, including isListedRemoteChat === false after the return, so the two vocabularies stay apart.

Red proof and the knife

Removing just delete bot.remote.hiddenByGone from the fixed file and nothing else: 2 red, not ok 9 (the repair case) and not ok 10 (the new "hide the user sets after the repair" case), 13/15. Restored, remote-chat is 15/15 and remote-chat + new-chat + flybot-client is 39/39 on the merged tree.

The rest still holds

• Strict patch rebuild of core/ from ds-bot/ + files/ + the ten patches at exact line numbers: 10 patches, 1021 hunks, 114 file operations, 358 files, 0 differences, 155 mirrors byte-identical — the 0003 line-number fix and the 0008 regeneration are still consistent on this tip, not just on the previous one.
• docs/subsystems/core-host.md now says the marker is deleted with the hide and why; it no longer claims the marker is only read, which was the sentence the old behaviour did not match.
• The sidebar change is unchanged from the reviewed version: isShownRemoteChat in the list, no longer shared as the second line, and every send/picker/mention path still on the stricter isListedRemoteChat.

CI on this head is terminal: 13 check runs, 12 success, enqueue-guard red only for comment by Birfy: names d2b15c7687f8, not the tip 69b09bc14d58 — my INC on the previous head, which this verdict replaces.

Ran: the reproduction against the real module on the merge tree, the relevant core tests, the mutation below, and the strict patch rebuild.

$ node /tmp/repro956b.mjs                                                    # the four lines above
$ cd core && DSH_HARNESS=/opt/flybot node --import ./test/harness-resolve.mjs --test test/remote-chat.test.js test/new-chat.test.js test/flybot-client.test.js   # 39 pass, 0 fail
$ node --test test/remote-chat.test.js                                       # without the marker delete: 13 pass, 2 fail (9, 10)
$ node /tmp/papply.mjs <tree>                                                # 10 patches, 1021 hunks, 358 files, 0 diffs, 155 mirrors 0