Skip to content

A hide the user set is still lost when the friend's Bot leaves the published list and returns (hidden is a single flag) #631

Closed

Description

@Birfy

Summary

A friend chat the user hid with "Hide from sidebar" comes back on its own after a round trip where the friend's Bot leaves the published list and returns. The fix in progress (#611, fix/hidden-remote-chat-sticks) closes the reported path — a re-publish no longer clears the hide — but one case is still open: hidden is a single boolean, so remote-chats.sync cannot tell "the user hid this" apart from "we hid this because the Bot was gone".

Reproduce (3 steps, from the review probe)

// published + hidden by the user -> gone -> published again
await rt.remoteChats.sync('social', [member]);
assert.equal(state.bots.b.hidden, true);

await rt.remoteChats.sync('social', []);       // the friend's Bot leaves the list
assert.equal(state.bots.b.remote.gone, true);

await rt.remoteChats.sync('social', [member]); // it comes back
assert.equal(state.bots.b.hidden, true);       // FAILS: hidden was deleted

Where it happens

core/src/client/remote-chat.js:

  • :246 — sync reacts only to a change of gone (the Keep a friend chat hidden when the user hid it #611 change);
  • :250 — a chat is (re-)hidden when the friend's Bot goes gone;
  • :251 — delete bot.hidden when it comes back, which cannot distinguish the two reasons the flag was set.

Expected behaviour

A hide the user set survives the round trip. Suggested smallest fix (same shape as the review): remember who hid the chat — set a marker (e.g. remote.hiddenByGone = true) when the hide is imposed because the Bot went gone; delete bot.hidden on return only when that marker is present, and clear the marker on both paths.

Tests: (a) published + user hides → gone → published again ⇒ still hidden; (b) gone → returns published, never hidden by the user ⇒ visible.

Evidence

Scope

core/src/client/remote-chat.js (sync), test/remote-chat.test.js (two cases above), and the Fixes # line in the #611 body. Implemented on the existing fix/hidden-remote-chat-sticks branch — one issue, one PR; the tip changes, so #611 needs one fresh verdict (the old INC is superseded).

Activity

  1. added
    bugSomething isn't working
    agent-readyMaintainer-approved spec: an agent may take it. Only maintainers set this; never in a template
    area:socialFriends, invites, grants, Bot Social
    area:uiWeb/desktop UI and copy
    on Oct 21, 2026
  2. Birfy commented on Oct 21, 2026

    @Birfy
    CollaboratorAuthor

    Claimed: branch fix/631-hidden-by-gone, pull request #634.

    #611s branch (fix/hidden-remote-chat-sticks) lives in a fork this account can only read, so the work continues on #634, which carries the change of #611 unchanged and adds the hiddenByGone marker plus the two tests this issue asks for.

  3. added
    agent-workingAn agent has claimed this issue (the claim comment names the branch)
    and removed
    agent-readyMaintainer-approved spec: an agent may take it. Only maintainers set this; never in a template
    on Oct 21, 2026
  4. Birfy commented on Oct 21, 2026

    @Birfy
    CollaboratorAuthor

    Why this issue is carried by #634 and not by #611

    AGENTS.md ("One PR per issue") says an existing PR is the place to continue — rebase, update-branch, or push a commit onto its head — and that a contributor's fork is treated the same way while it allows maintainer edits. #611 allows them (maintainer_can_modify: true), so that was the intended path, and it is the one announced on the PR.

    It does not work through the API, and this is measured, not assumed:

    query { repository(owner:"HugoXK", name:"FlyBot") { viewerPermission } }
    -> "READ"
    

    Maintainer edits in GitHub are a web-editor affordance: that editor commits to the fork with the maintainer's own session, and no API surface we have can push a commit into another owner's fork. So the branch cannot be pushed to from this side — which is the exception the same section names.

    Consequence, recorded here as required:

    For the record on the tooling side: createCommitOnBranch against ThinkFlowLab/FlyBot works — that is how #626's rework landed as 5ed6851f — so the limit is specifically writing into another owner's fork, not the mutation itself.

    Written by: Chief (owner proxy).

  5. Birfy commented on Oct 21, 2026

    @Birfy
    CollaboratorAuthor

    Correction to my previous comment on this issue: the fork branch can be written to.

    I argued that maintainer_can_modify: true is only a web-editor affordance and that no API can push into another owner's fork, citing repository(owner:"HugoXK", name:"FlyBot") { viewerPermission } -> READ. Both halves of that were wrong:

    PUT /repos/ThinkFlowLab/FlyBot/pulls/611/update-branch   {expected_head_sha:4da46bff…}
    -> 202; branch fix/hidden-remote-chat-sticks 4da46bff -> 2a6e5afe
    
    mutation createCommitOnBranch(branch:{repositoryNameWithOwner:"HugoXK/FlyBot",
      branchName:"fix/hidden-remote-chat-sticks"}, expectedHeadOid:"2a6e5afe…")
    -> 03e26706
    

    The maintainer-edit permission on a PR is independent of the repository role, so viewerPermission says nothing about it. Lesson recorded: a role query is not evidence about a PR-scoped permission — try the operation before writing a rule about it.

    What changes as a result: #611 is the PR that merges (its branch now carries hiddenByGone and the probe as a test), and #634 is closed as a duplicate — not as a rejection — with this reason on it. #631 closes when #611 merges.

    Written by: Chief (owner proxy).

  6. removed
    agent-workingAn agent has claimed this issue (the claim comment names the branch)
    on Oct 21, 2026
  7. Birfy commented on Oct 22, 2026

    @Birfy
    CollaboratorAuthor

    Closing as fixed by #611 (merge commit 16dc44b2), which is on main.

    The case this issue left open — hidden as a single boolean cannot tell "the user hid this" from "we hid this because the Bot was gone" — is now two pieces of state in core/src/host/remote-chat.js: remote.hiddenByGone is set only when the sync itself hides a chat whose Bot left the published list (if (bot.hidden !== true) bot.remote.hiddenByGone = true), and on return only that flag is dropped (else if (bot.remote.hiddenByGone === true) { delete bot.hidden; delete bot.remote.hiddenByGone }). The code comment cites #631 by number.

    goneBefore also means the hide runs only on a real gone transition, so a re-publish of an unchanged list no longer clears a user hide.

    Reopen if a hidden chat still reappears after a gone → published round trip on current main.

    Written-by: Triage

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

    area:socialFriends, invites, grants, Bot Socialarea:uiWeb/desktop UI and copybugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions