fix(channel.member.list): always populate user_id/channel_id from schema #35

Merged

sameh-farouk merged 2 commits from fix/member-list-defensive-completeness into development

2026-04-27 18:09:21 +00:00

sameh-farouk commented

2026-04-27 17:52:40 +00:00

Member

Summary

Closes #34.

channel.member.list read user_id and channel_id only from the JSON cm.data blob. They're also schema columns on channel_members — the authoritative source — so any row whose data JSON didn't serialize those fields (buggy migration, manual SQL, test fixture) returned a response missing the identity fields, and the client rendered "User undefined" / "Unknown" via the m.user_id-keyed fallback paths.

What changed

crates/hero_collab_server/src/handlers/channel.rs::member_list now also SELECTs cm.user_id and cm.channel_id from the schema columns and unconditionally overwrites those keys in the response JSON. The PK + FK constraints guarantee schema columns can't legitimately disagree with the data blob; if they do, it's corruption and the schema is right.

// Before
SELECT cm.data, u.data FROM channel_members cm JOIN users u ON u.id = cm.user_id WHERE cm.channel_id = ?1
// After
SELECT cm.user_id, cm.channel_id, cm.data, u.data FROM channel_members cm JOIN users u ON u.id = cm.user_id WHERE cm.channel_id = ?1

Plus the response merge:

cm["user_id"] = json!(user_id);     // from schema column
cm["channel_id"] = json!(chan_id);  // from schema column

Test plan

Well-formed rows: response unchanged (identity fields were already in the data blob; schema overwrite is a no-op).
Deliberately malformed row (data='{}'): response now correctly populates user_id, channel_id, user_name from JOIN. Client renders correctly.
cargo build clean.
No client-side changes — both existing renderers (Members modal at chat-app.js:3121, channel detail panel at chat-app.js:3817) work correctly with the hardened response.

Scope discipline

I considered also consolidating the two client-side renderers' divergent fallbacks ('Unknown' vs 'User ' + m.user_id) into a shared helper that prefers the server-provided user_name, but reverted that change after self-review: with the server hardened to always include user_id, both renderers work correctly, and the divergent fallback strings only surface in code paths that the server fix already eliminates. Adding a helper for cosmetic consistency would have introduced abstraction beyond what the bug requires — exactly the kind of "while we're here, let's tidy" creep that the project guidelines call out.

🤖 Generated with Claude Code

## Summary Closes #34. `channel.member.list` read `user_id` and `channel_id` only from the JSON `cm.data` blob. They're also schema columns on `channel_members` — the authoritative source — so any row whose `data` JSON didn't serialize those fields (buggy migration, manual SQL, test fixture) returned a response missing the identity fields, and the client rendered "User undefined" / "Unknown" via the `m.user_id`-keyed fallback paths. ## What changed `crates/hero_collab_server/src/handlers/channel.rs::member_list` now also `SELECT`s `cm.user_id` and `cm.channel_id` from the schema columns and unconditionally overwrites those keys in the response JSON. The PK + FK constraints guarantee schema columns can't legitimately disagree with the data blob; if they do, it's corruption and the schema is right. ```rust // Before SELECT cm.data, u.data FROM channel_members cm JOIN users u ON u.id = cm.user_id WHERE cm.channel_id = ?1 // After SELECT cm.user_id, cm.channel_id, cm.data, u.data FROM channel_members cm JOIN users u ON u.id = cm.user_id WHERE cm.channel_id = ?1 ``` Plus the response merge: ```rust cm["user_id"] = json!(user_id); // from schema column cm["channel_id"] = json!(chan_id); // from schema column ``` ## Test plan - [x] Well-formed rows: response unchanged (identity fields were already in the data blob; schema overwrite is a no-op). - [x] Deliberately malformed row (`data='{}'`): response now correctly populates `user_id`, `channel_id`, `user_name` from JOIN. Client renders correctly. - [x] cargo build clean. - [x] No client-side changes — both existing renderers (Members modal at chat-app.js:3121, channel detail panel at chat-app.js:3817) work correctly with the hardened response. ## Scope discipline I considered also consolidating the two client-side renderers' divergent fallbacks (`'Unknown'` vs `'User ' + m.user_id`) into a shared helper that prefers the server-provided `user_name`, but reverted that change after self-review: with the server hardened to always include `user_id`, both renderers work correctly, and the divergent fallback strings only surface in code paths that the server fix already eliminates. Adding a helper for cosmetic consistency would have introduced abstraction beyond what the bug requires — exactly the kind of "while we're here, let's tidy" creep that the project guidelines call out. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

sameh-farouk added 1 commit

2026-04-27 17:52:40 +00:00

fix(channel.member.list): always populate user_id/channel_id from schema 53e5efed89

The handler read user_id and channel_id only via the JSON `cm.data`
blob. Since `channel_members` rows have user_id and channel_id as
schema columns (PK + FK to users.id), the data blob was redundant for
identity — but it was the SOLE source the client saw. If a row's
`data` was ever written without those fields properly serialised
(buggy migration, manual SQL, test fixture), the response carried
no user_id and the client's `member.user_id`-keyed lookup fell
through to "User undefined" / "Unknown" fallbacks.

Symptom from dogfooding: a `channel_members.data` row written without
`user_id` (an SQL fixture insert during testing) caused the channel
detail panel to render the affected member as "User undefined".

Fix: read `cm.user_id` and `cm.channel_id` from schema columns
alongside the data blob, then unconditionally overwrite the
corresponding JSON fields with the canonical values before returning.
Schema columns can't legitimately disagree (FK enforces equality);
disagreement would only ever indicate corruption, in which case the
schema column is the right answer.

Verified: a deliberately malformed `data='{}'` row now returns
`{user_id, channel_id, user_name}` populated from the JOIN, so the
client's existing `m.user_id` lookups work regardless of `data` shape.

Scope is intentionally limited to server-side defensive completeness.
The client-side renderers (Members modal at chat-app.js:3121, channel
detail panel at chat-app.js:3817) work correctly when `m.user_id` is
present in the response, so no client change is needed once the
server is hardened — they were both behaving exactly per their inputs
under the previous contract. Cleaning up their slightly-divergent
fallback strings ('Unknown' vs 'User <id>') is cosmetic-only and
deliberately not bundled here.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

sameh-farouk added 1 commit

2026-04-27 18:08:04 +00:00