Skip to content

fix(client): keep the main view on a chat whose Bot left the roster (#261) - #880

Merged
Birfy merged 1 commit into
mainfrom
fix/261-landing-part
Oct 23, 2026
Merged

Birfy merged 1 commit into
mainfrom
fix/261-landing-part

Conversation

@Birfy

@Birfy Birfy commented Oct 23, 2026 •

Copy link
Copy Markdown
Collaborator

Fixes #261

What changed

The main view jumped to the Main Bot's chat on its own. useLandOnMainBot falls through to
actions.landOn(mainBotId, last, …) when the view empties or holds a blank chat whose Bot is no longer in
roster.byId; landingPart then looked up roster.byId[last.botId], got undefined, and answered with
the Main Bot's part. A friend revoking the share (or removing the Bot) is exactly the case that drops it
from the roster while the session is still open, so the view moved with no click behind it. The three
earlier attempts did not reproduce it because #280's landingPart already returns the held chat while
the Bot is in the roster, and sidebar.js's guard never swaps a chat with content out.

landingPart (core/src/client/text.js) now asks the session list, not the roster, whether that
conversation still exists:

  • the view emptied (stay) while last.sessionId is still in ctx.sessions.list → that same chat;
  • a blank chat of that same session (the revoke case, view === last.sessionId) → that same chat;
  • a New Session the user opened (a different id), or last.sessionId archived with its Bot → the Main
    Bot, as before;
  • a part archived for a fresh one still goes on in the fresh part, and no last still means the Main Bot.

landOn (core/src/client/index.js) passes the session snapshot and view; the landing effect
(core/src/client/sidebar.js) passes the id the view holds now. The guard at sidebar.js:667 that keeps a
chat with content from being swapped out is untouched. core/client.js is rebuilt (19 changed lines, the
three call sites only). Docs: docs/subsystems/core-client.md. Changelog:
changelog.d/261-landing-part-session-list.fixed.md. The patches/ds-bot/ mirrors are updated for the same
four files (0001-core-fixes, 0003-flybot-api-client, 0008-client-bundle, and the whole-file
patches/ds-bot/files/test/land-on.test.js).

Verification at the tip

Tip SHA: 51f803c
Ran: core/test/land-on.test.js 6/6; full core suite 1308/1309, the only failure being the known
environment red #177 "empty diff passes" (check-arch-docs runs git, and this tree is a codeload tar
with no .git); node scripts/build-client.mjs --check exit 0 after rebuilding core/client.js (diff
against the base bundle is 19 lines, all three call sites of this change).
Not run (and why): scripts/ds-bot-patches.mjs itself (needs git, absent here: the edited sections were
replayed against ds-bot/ and compared with core/, and the package job's ds-bot-patches check is the
authority); the browser walkthrough (no browser here — the deterministic land-on.test.js cases are the
reproduction); arch-docs locally (same missing git).

Evidence (withdrawing the change reds a test, and only the test that is about it)

Withdrawn piece Result
the session-list branch (back to the roster only) only #261: a roster that lost the Bot keeps the held chat whose session still exists red — 5/6, which is the reported bug
asking which session the view holds (stay || view === last.sessionId → always) only #261: a session that is gone, or a New Session the user opened, still goes to the Main Bot red — 5/6, so a New Session cannot start landing on the old chat

Risk

Low: one pure function plus two call sites, no wire change; the fallback for a session that is really gone
is unchanged, and a chat with content is still protected by the untouched guard. The only behaviour that
moves is the one the issue names: a Bot leaving the roster while its chat is open no longer moves the view.

@Birfy Birfy added the agent-pr Pull request opened by an agent label Oct 23, 2026
@Birfy Birfy self-assigned this Oct 23, 2026
@Birfy
Birfy force-pushed the fix/261-landing-part branch from ad2feb2 to 8075aba Compare October 23, 2026 13:25
@Birfy

Birfy commented Oct 23, 2026

Copy link
Copy Markdown
Collaborator Author

Written-by: Dev4

New tip: 8075abaf (same code, plus the patches/ds-bot/ mirrors).

The first tip failed package only: ds-bot-patches check compares core/ against ds-bot/ + patches/ds-bot/, and the four files I changed had no mirror update. There is no git on this machine, so scripts/ds-bot-patches.mjs generate cannot run; I updated the four sections by hand (one hunk each in 0001-core-fixes, 0003-flybot-api-client and 0008-client-bundle, plus the whole-file patches/ds-bot/files/test/land-on.test.js) and replayed them against ds-bot/ to check the result equals core/. tests and plugins were already green on the first tip; package is the job to watch now.

@Birfy
Birfy force-pushed the fix/261-landing-part branch from 8075aba to 51f803c Compare October 23, 2026 14:13
@Birfy

Birfy commented Oct 23, 2026

Copy link
Copy Markdown
Collaborator Author

Written-by: Dev4

New tip: 51f803cb. The previous mirror edit had one blank line at the end of 0001-core-fixes.patch, which git apply reads as a corrupt patch (the package job failed at line 7817). Rebuilt the three patch sections from the pristine mirrors with the trailing blank line dropped; every hunk now replays onto ds-bot/ and reproduces core/ byte for byte for all four files, including client.js.

A Bot a friend revokes (or removes) drops out of `roster.byId` while its session stays open. The main
view's landing effect then fell through to `landOn(mainBotId, last, …)` and `landingPart` resolved
`roster.byId[last.botId]` to `undefined`, so `landingPart` returned the Main Bot's part: the view jumped
to Chief's chat with no click behind it.

`landingPart` now asks the session list whether that conversation still exists, because the roster says
which Bots are reachable, not which sessions are open. `landOn` passes `ctx.sessions.list`, and the
sidebar hands over the id the view holds now, so a blank chat of that same session stays put while a New
Session the user opened (a different id) still goes to the Main Bot. A session archived together with its
Bot keeps the old answer: the Main Bot. The protected guard that keeps a chat with content from being
swapped out is untouched.
@Birfy
Birfy force-pushed the fix/261-landing-part branch from 51f803c to e3823c4 Compare October 23, 2026 14:16
@Birfy

Birfy commented Oct 23, 2026

Copy link
Copy Markdown
Collaborator Author

Written-by: Dev4

New tip: e3823c42. The corrupt-patch cause was the file ending without a trailing newline (the renderer dropped it), not the hunk counts: rebuilt from the pristine mirrors, all three patch files now end with a newline, and package is success on this tip. tests-ok/Node 24 were still finishing when I looked; everything else is green.

@Birfy

Birfy commented Oct 23, 2026

Copy link
Copy Markdown
Collaborator Author

Verdict: PASS at e3823c4
Written-by: Reviewer2

Ran:

  • GET /repos/ThinkFlowLab/FlyBot/pulls/880 — head e3823c42…, base 690093bc…, mergeable: true, mergeable_state: clean; GET /commits/e3823c42…/check-runs — 11/11 completed and success (tests-ok, package, plugins (social|tree|timeline), Node 24, merge-gate, judge, enqueue-guard).
  • tarball of the tip; cmp of core/src/client/{text,index,sidebar}.js, core/client.js, core/test/land-on.test.js, docs/subsystems/core-client.md, changelog.d/261-… against 8075abaf → identical, so this tip only changed patches/ds-bot/.
  • tail -c 1 patches/ds-bot/{0001-core-fixes,0003-flybot-api-client,0008-client-bundle}.patch → all end with \n (at 8075abaf they ended ), ), ;, which git apply read as a corrupt patch at :7817); cmp core/test/land-on.test.js patches/ds-bot/files/test/land-on.test.js → identical; CI package at this tip: success.
  • cd core && DSH_HARNESS=… node --import ./test/harness-resolve.mjs --test test/land-on.test.js → 6/6.
  • revert A: drop the held branch in core/src/client/text.js (the two lines ending if (held) return last.sessionId) → 5/6, only #261: a roster that lost the Bot keeps the held chat whose session still exists fails.
  • revert B: view === last.sessionId → view !== '' → 5/6, only #261: a session that is gone, or a New Session the user opened, still goes to the Main Bot fails.
  • full core suite, serial: 1309/1310; the one failure is the known environment red empty diff passes (check-arch-docs runs git, absent in a codeload tree); CI's Node 24 job runs the same suite and is green.
  • diff against 690093bc: sidebar.js differs only at :673 (the landOn call plus its two comment lines); the guard at :667 is byte-identical. index.js changes only the two landOn lines; text.js adds the view/sessions options and the 2-line held branch. node core/scripts/build-client.mjs --check exit 0.

What I checked: the defect is real (the roster drops a Bot whose session is still open, so the view lands on the Main Bot with no click), this is the change that reproduces it, and the rule is now "the session list, not the roster, says whether that chat still exists". Both sides are pinned: the held chat stays; a New Session the user opened and a session archived with its Bot still go to the Main Bot. Each half reddens exactly one test when reverted, so neither is carried by the other. sidebar.js:667 is untouched, and the packaging mirror is correct at the byte level now (the trailing newlines are restored, the whole-file mirror is identical to core/).

Non-blocking notes:

  1. held needs sessions?.byId?.[last.sessionId] !== undefined; a caller that lands without the session snapshot gets the old Main Bot behaviour back. landOn passes it unconditionally now, so this is a note, not a defect.
  2. The reproduction is the deterministic land-on.test.js case, not a browser walkthrough; nobody has watched a live revoke yet. The first one after release is worth a look.

@github-actions github-actions Bot added the pass label Oct 23, 2026
@Birfy
Birfy added this pull request to the merge queue Oct 23, 2026
Merged via the queue into main with commit b6ab4b2 Oct 23, 2026
11 checks passed
@Birfy
Birfy deleted the fix/261-landing-part branch October 23, 2026 14:46
@github-actions github-actions Bot removed the agent-pr Pull request opened by an agent label Oct 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A friend chat sometimes jumps back to Chief's chat on its own (not reproduced)

1 participant