fix: DM conversations always visible in all Spaces (#484)

[lml2468]

Root Cause Analysis (GH#484 — Symptom 2)

decideConvKeepInSpace in modules/message/space_filter.go decides whether a DM conversation appears in a Space's sidebar by checking hasSpaceMsg — which scans conv.Recents (the last N messages) for payload.space_id matches.

Since DMs are a single physical channel shared across all Spaces (conversation-level SpaceID is intentionally left empty), the Recents window is also shared. When Space A is more recently active, its messages fill the Recents window and evict Space B's tagged messages. personConvHasSpaceMessages(spaceB) then returns false → conversation disappears from Space B's sidebar.

Fix

Regular (non-bot) DM conversations are now always visible in every Space, regardless of Recents window contents. The fix is a one-line change in decideConvKeepInSpace:

// Before:
return hasSpaceMsg != nil && hasSpaceMsg(filterSpaceID)
// After:
return true

Message-level filtering (filterPersonMessagesBySpace) still controls which messages are shown within each Space, so cross-Space message pollution (symptom 1) is not worsened by this change.

Scope

This PR fixes symptom 2 (mutually exclusive visibility / disappearing DMs).

Symptom 1 (cross-Space history pollution via filterPersonMessagesBySpace rule 2 — keeping untagged messages in all Spaces) requires a product decision: should DMs be per-Space or global? The current behavior (untagged messages visible everywhere) is consistent with the "DMs are global" interpretation and is left as-is pending that decision. See #484 blocking product decision.

Test

• Renamed TestDecideConvKeepInSpace_DM_NonDefaultSpace_NoSpaceMsg_Excluded → TestDecideConvKeepInSpace_DM_NonDefaultSpace_AlwaysVisible (asserts True).
• Added TestDecideConvKeepInSpace_DM_NonDefaultSpace_RecentsEvicted_StillVisible — regression test for the exact bug scenario where Recents has no Space B messages.

All 25 space_filter tests pass.

Fixes #484 (symptom 2)

[OctoBoooot]

Review: fix: DM conversations always visible in all Spaces (#484) (#490)

Verdict: Approve (with one product-semantics confirmation called out — PR is a draft, so this is feedback-while-in-progress).

CI green. Risk: normal, but trust-boundary-adjacent (controls which conversations appear in which Space's sidebar). 2 files +32/-3.

Verified — the fix is correctly scoped to non-bot DMs; bot isolation preserved

The change is a single return true in the spaceID == "" && ChannelTypePerson branch of decideConvKeepInSpace (space_filter.go:281-290). Byte-verified the guard ordering above it preserves all the isolation-relevant cases:

• skipBotFilter → true (unchanged)
• SystemBots[channelID] → true (unchanged)
• bot AND botInSpace[channelID] → true (unchanged)
• non-bot DM (!botSet[channelID]) → now always true (was: hasSpaceMsg(filterSpaceID) Recents-scan)
• a bot NOT in the space still falls through to return false at function end — bot space-isolation is NOT widened.

So the visibility widening is confined to human-to-human DMs. A bot that isn't a member of the filter Space still does not appear — the change doesn't touch the bot path that the prior PRs (#458/#469) worked to keep isolated. [bidirectional-edge-construction] check: the return true can't leak a bot conversation into a wrong space because the bot branches are evaluated and returned before this line.

Root-cause + fix logic is sound

The Recents-window-scan visibility check was genuinely unreliable for the single-physical-channel DM architecture: both Spaces share one Recents window, so the more-active Space evicts the other's tagged messages → personConvHasSpaceMessages(spaceB) returns false → conversation vanishes from Space B. Making non-bot DM visibility unconditional fixes the disappearing-DM symptom (symptom 2). Message-level filtering (filterPersonMessagesBySpace) still controls which messages render per-Space, so this only changes conversation-list visibility, not message content. The PR correctly scopes itself to symptom 2 and defers symptom 1 (cross-Space message pollution) to a product decision.

Needs confirmation — the privacy/product semantics of "DM visible in all Spaces"

This is the one thing worth an explicit human nod before un-drafting (it's why I'm flagging rather than plain-approving): the change makes every regular DM conversation appear in every Space's sidebar for that user. The PR frames this as the "DMs are global" interpretation, which is internally consistent with the existing untagged-message-visible-everywhere behavior. But it IS a visibility-surface change:

• A user who has DMs with contact X, and belongs to Spaces A and B, will now see the X conversation in both A and B's sidebars (previously: only where Recents happened to have a matching tag).
• This is a per-user view (the DM is the user's own conversation), so it's not a cross-user leak — the user already has access to their own DM. The question is purely UX/product ("should my DMs show in every Space tab, or be Space-scoped?"), not authz.

The PR body explicitly says symptom-1 / per-Space-vs-global is a pending product decision (#484). Since this change leans on the "global" interpretation, the same product decision governs whether this is the desired end state. Recommend an explicit product/stakeholder ack on "DMs visible in all Spaces" before merge — it's a deliberate product choice, correctly implemented, not a code defect.

Tests pin both directions

• TestDecideConvKeepInSpace_DM_NonDefaultSpace_AlwaysVisible (renamed from ..._NoSpaceMsg_Excluded) flips the assertion False→True with a comment explaining the GH#484 rationale — the rename + flip is the honest way to update a test whose expected behavior intentionally changed (not silently editing the assertion).
• New TestDecideConvKeepInSpace_DM_NonDefaultSpace_RecentsEvicted_StillVisible pins the exact regression: hasSpaceMsg returns false (Recents evicted) → still visible. This is the bug's precise failing input, captured as a test.
• TestDecideConvKeepInSpace_DM_BotInSpace and the bot cases remain — confirming the bot path is unchanged.

Praise

• The fix flips the test assertion via rename + comment, not a silent edit — ..._NoSpaceMsg_Excluded → ..._AlwaysVisible with a comment explaining why the expected behavior changed. When a fix intentionally inverts a previously-correct assertion, renaming the test to describe the new contract (rather than editing the bool in place) makes the behavior change reviewable in the diff. Right discipline.
• Regression test captures the exact failing input — RecentsEvicted_StillVisible with hasSpaceMsg → false is the precise condition that made DMs vanish. A future "optimization" that re-introduces a Recents-based visibility gate fails this test loudly.
• Scope honesty — the PR fixes symptom 2 and explicitly defers symptom 1 (cross-Space pollution) to a product decision rather than silently changing both. The comment + body both name what's fixed and what's deliberately left. Clean scoping on a two-symptom issue.
• Bot isolation preserved — the return true is placed after all the bot-branch guards, so the human-DM widening doesn't touch the bot space-isolation that #458/#469 established. The change is minimal and surgical.

Minor (non-blocking)

• The hasSpaceMsg func(targetSpaceID string) bool param + personConvHasSpaceMessages helper are now effectively dead on the non-bot DM path (the only caller of that branch's check). The caller (space_filter.go:198) still constructs and passes it. Leaving it wired (rather than removing) is reasonable while symptom-1's product decision is pending — if per-(contact, Space) conversations land later, the Recents check may come back. Worth a // TODO(#484): hasSpaceMsg unused on the always-visible path; remove if global-DM is the final decision so the next reader knows it's intentionally-retained-pending-decision, not an oversight.

Out of scope (informational)

• Symptom 1 (untagged DM messages visible in all Spaces via filterPersonMessagesBySpace rule 2) is correctly deferred to the #484 product decision. The "DMs are global" interpretation this PR adopts should be confirmed as the intended end state, since both symptoms hinge on the same per-Space-vs-global question.

[Jerry-Xin]

REQUEST_CHANGES — The GH#484 direction (make non-bot DM conversations visible in every Space, since DM is a single shared physical channel) is reasonable and the v2 unit tests added here directly cover the disappearing-DM scenario. However the change is not mergeable yet: it breaks existing v1 tests and introduces a cross-Space message-preview exposure in the conversation-list sync path.

🔴 Blocking

• 🔴 Existing v1 tests now fail (CI break). decideConvKeepInSpace is shared by both v1 (filterConversationsCore/FilterConversationsBySpace) and v2 sidebar. This PR only updated modules/message/space_filter_v2_test.go, but the v1 tests in modules/message/space_filter_test.go (untouched) still assert the old "hidden when no Recents match" contract and now fail:

  • TestFilterConversationsBySpace_DefaultSpaceBareConvs (space_filter_test.go:62) — expects 0 in non-default Space, now gets 2.
  • TestFilterConversationsBySpace_NonDefaultSpaceDMVisible (space_filter_test.go:88,:95) — asserts user2 excluded, now included.
  • TestFilterConversationsBySpace_NewSpaceCleanSlate (space_filter_test.go:119) — expects clean-slate 0, now gets 3.
    Verified locally at head e63ae5d0: go test ./modules/message -run 'TestFilterConversationsBySpace' → FAIL. Update these tests (and the now-incorrect "clean slate" comment) to the new DM contract.

• 🔴 Cross-Space DM Recents exposure in the conversation-list response. modules/message/space_filter.go:290 now returns true for any non-bot DM in any Space, but the conversation-list sync path filters DMs only via FilterConversationsBySpace (api_conversation.go:893), which keeps/drops the whole conversation and does not scrub each conv's Recents by space_id. The code comment claims filterPersonMessagesBySpace still scopes messages per Space — but that function is only called on the channel-history sync path (api.go:1279), never on the conversation-list Recents. Result: a Space B conversation sync can now return a DM whose Recents contain only Space A messages (the exact "evicted" case the PR targets), exposing Space A's latest DM preview/content in Space B's list. Previously such a DM was dropped, so the preview never leaked. Please filter/clear DM Recents to the requested Space in the conversation-list path when making DMs globally visible, and add a regression test for the "Recents contain only the other Space's messages" response shape.

💬 Non-blocking

• 🔵 The hasSpaceMsg helper doc/contract at space_filter.go:215 is now stale — after this change it no longer governs regular DM visibility (only bot-related paths and v2 still reference it). Update the comment to avoid confusion.

✅ Highlights

• Behavior change is correctly gated to the ChannelTypePerson + non-bot branch; group/community-topic and bot-DM paths (incl. v2 fail-closed group scoping) are untouched, so there is no group/space-channel widening and no cross-user exposure — the conversation list is already loginUID-scoped.
• New v2 tests (..._AlwaysVisible, ..._RecentsEvicted_StillVisible) clearly document and cover the GH#484 disappearing-DM regression.

Reviewed at head e63ae5d099a376b708a958d9688b259b535d54af.

[mochashanyao]

[Octo-Q · automated review]

Verdict: Approve — no blocking findings; notes below (data-flow traced).

---

octo-server PR#490 Review Report

Reviewer: Octo-Q (automated review)
PR: #490
Head SHA: e63ae5d099a376b708a958d9688b259b535d54af
Title: fix: DM conversations always visible in all Spaces (#484)
Changed files: 2 (+32 / -3)

---

1. Verification Summary

Item	Status	Evidence
Code correctness	✅	Single return-statement change in decideConvKeepInSpace non-bot DM branch; control flow intact for Group/Thread/Bot paths
Test coverage	✅	25/25 space_filter tests pass; renamed test + new regression test cover the fix
Bot DM isolation preserved	✅	botSet[channelID] && !botInSpace[channelID] → false path (line 276) unchanged; bot-not-in-space still excluded
Message-level filtering unchanged	✅	filterPersonMessagesBySpace (lines 451–480) untouched; rules 1–4 intact
v1/v2 parity	✅	Both filterConversationsCore and FilterRawConversationsBySpace call the same decideConvKeepInSpace; change applies symmetrically

---

2. Findings

F1 — P2 — Message-level filtering does NOT fully prevent cross-Space message visibility

Diff-scope: pre-existing limitation, amplified by this PR's conversation-visibility change

The PR description claims: "Message-level filtering (filterPersonMessagesBySpace) still controls which messages are shown within each Space, so cross-Space message pollution (symptom 1) is not worsened by this change."

This is partially correct but overstated. filterPersonMessagesBySpace rule 2 (space_filter.go:470–472) keeps untagged non-systembot messages in every Space:

case msid == "" && !isSysBot:
    filtered = append(filtered, m) // legacy compat — kept in ALL Spaces

Untagged messages include: legacy pre-Space messages, messages from clients omitting X-Space-ID, and forwarded/card messages. These remain visible in every Space after message-level filtering.

Impact analysis (R1 check):

• Not "unavailable": DM conversations were already in the user's conversation list (default Space); the PR changes which Space sidebars show them.
• Not "incorrect data": the same untagged messages were accessible before (via default Space sidebar); no new data exposed.
• Not "worse than before" in terms of data exposure: tagged messages (spaceA-tagged) are still properly dropped when viewing from spaceB (rule 4). Only untagged messages cross — same as before.

Severity: P2. The information disclosure is pre-existing; the PR changes the access path (sidebar convenience) but not the total data surface.

F2 — P2 — PR description mischaracterizes "Fixes #484" scope

The PR says "Fixes #484 (symptom 2)" but GitHub's Fixes keyword will auto-close the issue on merge. Issue #484 covers two symptoms; symptom 1 (cross-Space history pollution via untagged messages) is explicitly deferred pending a product decision. The PR body acknowledges this in the Scope section, but the Fixes keyword contradicts it — merging would close #484 as resolved while symptom 1 remains open.

Recommendation: Change Fixes #484 to Fixes #484 symptom 2 or Partial fix for #484 (remove the GitHub auto-close keyword), or use Contributes to #484.

F3 — P2 (nit) — Dead hasSpaceMsg callback parameter

After this change, the hasSpaceMsg callback in decideConvKeepInSpace is never invoked for non-bot DMs (the !botSet[channelID] branch returns true before reaching the callback). Both callers (filterConversationsCore line 198, FilterRawConversationsBySpace line 686) still construct and pass closures that are now dead code for this path. The callback is still used for the bot-in-default-space path (line 271), so the parameter can't be removed, but the closures in the non-bot DM callers are now wasted computation. Minor — no correctness impact.

---

3. Recommendations

1. Fix Fixes keyword (F2): Replace Fixes #484 (symptom 2) with Partial fix for #484: symptom 2 (mutually-exclusive visibility) to avoid auto-closing the umbrella issue.

2. Clarify PR description (F1): The claim "cross-Space message pollution (symptom 1) is not worsened" is technically accurate at the message-filter level but misleading at the user-experience level. Consider rewording to: "Message-level filtering is unchanged; symptom 1 (untagged messages visible in all Spaces) is a pre-existing architectural limitation that requires a product decision to resolve."

3. Document the product decision: The PR makes an implicit product choice ("DMs are global at conversation level") that was listed as one of three options in #484. Consider adding a note in the PR or #484 that this decision has been made, with a tracking item for the full per-(contact, Space) architecture if/when product decides to go that direction.

---

4. Additional Observations

• Test TestDecideConvKeepInSpace_DM_NonDefaultSpace_HasSpaceMsg_Visible is now redundant with AlwaysVisible — both assert true for non-bot DM in non-default Space. The HasSpaceMsg variant tests the now-unreachable callback path. Not a bug, but could be simplified.
• Backward compatibility: Clients that previously did NOT see a DM in a non-default Space sidebar will now see it. No client-side breaking change; just new visibility. Three-platform rendering should be verified (web/iOS/Android) to confirm no client assumes conversation-in-sidebar implies all messages are Space-tagged.

---

5. Data Flow Trace

Consumer	Data	Upstream Source	Runtime Flow Verified
decideConvKeepInSpace (modified branch)	!botSet[channelID]	resolveBotFilter → spacepkg.GetBotUIDs DB query	✅ Non-bot DMs reach this branch; bots take earlier paths
decideConvKeepInSpace return value	true (was hasSpaceMsg(filterSpaceID))	Hardcoded	✅ Always returns true for non-bot DMs
filterConversationsCore	keep from decideConvKeepInSpace	Same function	✅ Non-bot DM conversations always kept
FilterRawConversationsBySpace	keep from decideConvKeepInSpace	Same function	✅ v2 sidebar parity confirmed
filterPersonMessagesBySpace (api.go:1279)	msgs, channelID, spaceID	/v1/message/channel/sync handler, middleware-validated X-Space-ID	✅ Unchanged; rules 1–4 intact. Rule 2 (untagged non-sysbot kept) is the pre-existing gap.
personConvHasSpaceMessages / rawConvHasSpaceMessages	callback passed to decideConvKeepInSpace	Callers construct closures	⚠️ Callback now dead code for non-bot DM path (F3)

---

6. Blind Spot Checklist (R5)

• C1 — Dual-path parity: N/A. The change affects only the non-bot DM branch. Bot paths (add/remove from space) are symmetric and unchanged.
• C2 — Control-flow ordering / nested reuse: Clear. decideConvKeepInSpace is called from v1 and v2 filter paths; both reach the same modified branch. No nesting or double-invocation risk.
• C3 — Authorization boundary ≠ capability boundary: N/A. Space sidebar filtering is UX-level, not authorization. DM messages are accessible to the conversation participants regardless of Space. No authorization boundary crossed.
• C4 — Authorization lifecycle / container-member cascade: N/A. No auth changes in this PR.
• C5 — Build ≠ runtime: N/A. Go backend change; no build artifacts / extensions / CLI packaging involved.
• C6 — Governance/policy doc self-consistency: Clear with note. The PR description's Fixes #484 claim conflicts with #484's explicit "Do not simply relax filtering" guidance. See F2 recommendation.

---

7. Cross-Round Blocker Recheck (R6)

N/A — automated review review.

---

Summary

#	Severity	Category	Description
F1	P2	Process/claim	Message-level filtering doesn't fully prevent cross-Space visibility for untagged messages
F2	P2	Process	Fixes #484 will auto-close issue while symptom 1 remains unresolved
F3	P2 (nit)	Cleanup	Dead hasSpaceMsg callback for non-bot DM path

The code change is clean, focused, and well-tested. It correctly fixes symptom 2 (disappearing DMs between Spaces) without introducing code-level regressions. The primary concerns are process-level: the PR makes an implicit product decision, contradicts #484's explicit guidance, and its description slightly overstates the security properties of message-level filtering. None of these rise to P0/P1 by the R1 rubric (no new unavailability, no new incorrect data, no regression in total data exposure).

[Octo-Q] verdict: APPROVE

The fix is technically sound and addresses a real user-facing bug. The P2 findings are process/documentation concerns that should be addressed (especially F2 — the Fixes keyword issue) but do not block merge from a code-correctness standpoint.

[OctoBoooot]

Concurring with @Jerry-Xin's 🔴-2 — and correcting a claim in my own review (#490 (review)). Jerry-Xin is right; I missed it.

My review asserted: "Message-level filtering (filterPersonMessagesBySpace) still controls which messages render per-Space, so this only changes conversation-list visibility, not message content." That conflated two different code paths and is wrong for the sidebar preview.

Byte-verified: filterConversationsBySpace (space_filter.go:189-202) appends the whole conv when decideConvKeepInSpace returns true — conv.Recents is NOT filtered. And fillMissingPersonConversations's comment at :392 states explicitly "对 Recents 内 payload.space_id 字段不做任何修改" (Recents space_id is not modified). The conversation-sync sidebar preview comes from conv.Recents[0], which is the unfiltered shared Recents window.

So the concrete leak:

• User in Spaces A and B, DM with contact X. Last message in the shared DM channel was tagged space_id=A.
• Pre-PR: the DM didn't appear in Space B's sidebar at all (the bug being fixed), so no preview.
• Post-PR: the DM is now always visible in Space B's sidebar — and its preview snippet is Recents[0], the Space-A-tagged message. Space B's sidebar shows a Space-A message preview.

filterPersonMessagesBySpace (the thing I cited) governs the message-LIST endpoint when you open the conversation, NOT the conversation-SYNC sidebar preview. Two different layers — I credited the wrong one as the guard. The sidebar preview is genuinely unfiltered after this change.

Severity: this is a real cross-Space preview exposure (Jerry-Xin's 🔴). It's still per-user (the user owns the DM), so not a cross-user authz leak — but it surfaces a Space-A message in Space-B's UI, which is exactly the Space-isolation expectation this codebase invests in. It also undercuts the PR's own symptom-1-deferral framing: the PR says "message-level pollution is deferred", but this change actively introduces preview-level pollution that didn't exist before (the DM wasn't visible in Space B at all before).

Fix direction (matching Jerry-Xin): when keeping a DM visible in a non-tagging Space, filter conv.Recents to that Space's messages for the preview (or blank/placeholder the preview if no Space-tagged message exists), so the sidebar snippet doesn't show another Space's content. That keeps the disappearing-DM fix (symptom 2) while not introducing preview pollution.

Jerry-Xin's 🔴-1 (a v1 test / CI) — CI currently shows only the code-review bot check failing, not a Go test; worth the author confirming whether a v1 test path also needs the assertion flip (I verified round3_findings_test.go doesn't assert the old DM-excluded behavior, so the v1 break may be elsewhere or already green — author should check go test ./modules/message/...).

Applying [praise-has-weight] retroactively: I cited a downstream filter as the guard without byte-verifying it actually covers the sidebar-preview path. "Message filtering still applies" was a path-confusion — the conversation-sync preview and the message-list endpoint are separate layers, and only the latter has the filter I named. My code-side APPROVE is retracted on this; the fix needs the Recents preview filtered before it's shippable.

[yujiawei]

Code Review — PR #490 (octo-server)

Verdict: CHANGES_REQUESTED

Thanks for the clear root-cause write-up — the diagnosis of symptom 2 (visibility bound to the shared Recents window) is correct. However, the chosen fix has a blocking test break and, more importantly, contradicts its own scope claim: making every DM unconditionally visible in every Space re-opens cross-Space message exposure on the conversation-sync path, which is exactly what issue #484 warned against ("Do NOT simply relax filtering to show DMs in all Spaces — that collapses straight into symptom 1").

---

1. Scope / spec compliance

The PR description states:

Message-level filtering (filterPersonMessagesBySpace) still controls which messages are shown within each Space, so cross-Space message pollution (symptom 1) is not worsened by this change.

This is not accurate for the conversation-sync path. filterPersonMessagesBySpace only runs on /v1/message/channel/sync (modules/message/api.go:1277-1281). It is not applied to the recents array that /v1/conversation/sync serializes (newSyncUserConversationResp, modules/message/api_conversation.go:1469-1555). Before this change, a DM with no Space-B-tagged messages was excluded from Space B, so its Space-A recents never reached a Space-B client. After this change every such DM is returned, with its raw (often Space-A) recents content included. So the change does materially widen cross-Space exposure on this endpoint — symptom 1, on a different route.

This needs either (a) a product sign-off that "DMs are global" (the issue lists this as a blocking product decision, not yet made), or (b) scoping the visibility relaxation so it does not also ship unfiltered cross-Space message content.

2. Code quality

P0 — Build/CI break: stale tests in space_filter_test.go

The PR updated space_filter_v2_test.go but left three tests in space_filter_test.go asserting the old "DM excluded from non-default Space" rule. They now fail, so go test ./modules/message/... is red and the merge gate cannot pass:

• TestFilterConversationsBySpace_DefaultSpaceBareConvs (space_filter_test.go:60-62) — expects 0 DMs in a non-default Space, now gets 2.
• TestFilterConversationsBySpace_NonDefaultSpaceDMVisible (space_filter_test.go:84-96) — expects user2 (only Space-A recents) excluded, now kept.
• TestFilterConversationsBySpace_NewSpaceCleanSlate (space_filter_test.go:116-119) — expects 0 DMs in a brand-new Space, now gets 3.

Confirmed locally:

--- FAIL: TestFilterConversationsBySpace_DefaultSpaceBareConvs
--- FAIL: TestFilterConversationsBySpace_NonDefaultSpaceDMVisible
--- FAIL: TestFilterConversationsBySpace_NewSpaceCleanSlate
FAIL	github.com/Mininglamp-OSS/octo-server/modules/message

These are pure unit tests (no DB/build tags), so they run in normal CI. Update them to the new contract.

P1 — /v1/conversation/sync leaks raw DM recents across Spaces

As described in §1: recents is built from the raw IM messages and is not space-filtered. fillPersonSpaceUnread adds space_last_message / space_unread (modules/message/space_unread.go:38-58) but does not replace or strip recents (api_conversation.go:1555). Result: a Space-B /v1/conversation/sync response can contain a DM whose recents payloads are Space-A message content. Even if the client renders space_last_message for the preview, the raw message bodies have already crossed the Space boundary on the wire. Either filter recents by Space here (reusing the filterPersonMessagesBySpace rule), or don't surface the conversation when it has no in-Space content.

P1 — Aggregate unread badge is not Space-scoped

SyncUserConversationResp.Unread (api_conversation.go:1453, :1547) and the sidebar SidebarItem.Unread (api_sidebar.go:1163 buildRecentItems) are serialized from the raw aggregate conv.Unread. With all DMs now visible in every Space, a Space-B sidebar shows DMs whose unread counts are driven entirely by Space-A messages. space_unread is computed separately, but any client reading the raw unread field gets cross-Space-polluted badges. This is a visible functional regression of the "per-Space" promise.

P2 — Dead code & now-incorrect comments

After the change, decideConvKeepInSpace no longer calls its hasSpaceMsg parameter for the non-bot DM branch, yet both call sites still build the closures (personConvHasSpaceMessages at space_filter.go:198, rawConvHasSpaceMessages at :703) and the parameter doc at space_filter.go:215 still claims it "decides non-default Space Person DM visibility" — which is no longer true. If the always-visible decision stands, remove the now-unused parameter + helpers (or document why they're retained) so the next reader isn't misled.

P2 — Payload bloat & context leak in scoped Spaces

Returning every historical DM in every Space inflates the conversation-list payload for heavy DM users and surfaces "ghost" DM entries that have no visible messages in the current Space. It also exposes the full contact/DM list (names, avatars) into strictly-scoped Spaces, weakening the isolation users expect (e.g. when screen-sharing a work Space). Worth weighing in the product decision referenced in #484.

3. Suggested direction

The issue explicitly flags this as needing a product decision ("one conversation vs. per-Space conversations"). If "DMs are global" is confirmed, the cleaner stop-gap the issue itself suggests is to exclude Person channels from Space filtering entirely AND make the conversation-sync/sidebar message-content + unread consistently global-or-scoped — not to flip visibility to always-true while leaving recents/unread carrying per-Space-inconsistent data. At minimum, before merge:

1. Fix the three failing tests.
2. Either space-filter recents on /v1/conversation/sync or gate visibility so no unfiltered cross-Space content ships (resolve the §1 contradiction).
3. Decide whether unread should be the space-scoped value on these endpoints.
4. Remove/relabel the dead hasSpaceMsg path.

4. Coverage notes

• Verified against head SHA e63ae5d099a376b708a958d9688b259b535d54af; build break reproduced locally on this branch.
• Not verified at runtime: actual client rendering (whether mobile/web use recents vs space_last_message for the preview) and the real payload-size impact for high-volume users — these need a product/QA call, not a static read.
• The WebSocket realtime-routing path and v2 sidebar metadata (contact existence/unread/timestamp) are affected at the metadata level but do not carry message bodies; the content leak is specific to /v1/conversation/sync recents.