Skip to content

PLAT-39412 connector: keep room state while the chat type is missing - #7

Open
Fizzadar wants to merge 1 commit into
mainfrom
nick/plat-39412-keep-untyped-room-state
Open

Fizzadar wants to merge 1 commit into
mainfrom
nick/plat-39412-keep-untyped-room-state

Conversation

@Fizzadar

Copy link
Copy Markdown
Contributor

Reddit can sync a new chat's members before its chat type, and the skipped batch's room state was dropped, so the room was built without the user's own join and bridgev2 kicked them. This keeps the merged state cached while the type is missing so the batch that sets it merges onto it.

🤖 Generated with Claude Code

Reddit can sync a new chat's members before its chat type, and the skipped batch's projection was dropped, so the room was later built without the user's own join and bridgev2 kicked them.

Cache the untyped projection so the batch that sets the type merges onto it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@indent

indent Bot commented Oct 21, 2026 •

Copy link
Copy Markdown

Issues

1 potential issue found:

  • The new untyped cache entry in roomState can shadow typed state that another login saved to a shared (non-split) portal. Later incremental syncs keep merging onto the untyped cache, hit errMissingChatType every time, and trigger a full snapshot fetch each time.
    Found by Indent Review Agent

↑ Select any checkbox above to have Indent auto-fix the issue

CI Checks

All CI checks passed on 3a111c2.

Review agents

Select any unchecked box below to run or rerun that agent.

Found issues (1)
  • Indent Review Agent · The fix looks right; one nit: the untyped cache can shadow typed state another login saved to a sha…
Full results

Indent Review Agent

  • Summary: The fix looks right; one nit: the untyped cache can shadow typed state another login saved to a sha…
  • Last ran on commit: 3a111c26
  • Latest result
    {
      "summary": "The fix looks right; one nit: the untyped cache can shadow typed state another login saved to a shared portal.",
      "findings": [
        {
          "fix": "Fall back to the saved state when the cache is untyped. Use `if (previous == nil || previous.Type == \"\") && existing != nil` with a typed `meta.State`, and either take `meta.State.Type` into the cached projection or merge the cached members onto `meta.State`.",
          "line": 138,
          "path": "pkg/connector/roomstate.go",
          "issue": "`previous` is loaded from the saved portal metadata only when `r.rooms[roomID] == nil`. Before this change an untyped merge was never cached, so every later sync fell back to the DB projection. Now, once a room's untyped state is cached (for example a non-split shared group whose members arrived before its type, where the full snapshot also lacked the type), later incremental syncs keep merging onto the untyped cache. That holds even after another login on the same bridge has created the shared portal (Receiver \"\") and saved typed `meta.State`. Those incremental syncs omit the chat type, so each one hits errMissingChatType again and triggers a full `Sync(\"\", 0)` snapshot, and the room stays unbridgeable for this login until Reddit's snapshot includes the type. The old code would have recovered right away from the saved state.",
          "title": "Cached untyped state now hides typed state saved to a shared portal by another login",
          "severity": "nit"
        }
      ]
    }

return nil, networkid.PortalKey{}, unbridgeable(err)
}
if next.Type == "" {
// Reddit can send a new chat's members before its type.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚪️ Nit · Cached untyped room state hides typed state saved by another login

previous is only loaded from the saved portal metadata when r.rooms[roomID] == nil. Now that the untyped state is cached here, later incremental syncs keep merging onto the untyped cache even if another login has since created the shared portal (Receiver "") and saved typed meta.State. Those syncs omit the chat type, so each one hits errMissingChatType again and triggers a full snapshot fetch. Before this change the DB projection was used right away. Consider falling back to the saved state when the cached previous.Type == "".

Found by Indent Review Agent

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant