lhumina_code/hero_collab
24
0
Fork 0
Code Issues 2 Pull requests Projects Releases Packages Wiki Activity Actions

Slack Feature Parity — Implementation Plan #9

New issue
Open
opened 2026-04-14 10:50:22 +00:00 by sameh-farouk · 10 comments
sameh-farouk commented 2026-04-14 10:50:22 +00:00
Member
Copy link

Context

Hero Collab is a markdown-centric collaboration platform built as a Hero OS service. The backend is ~95% complete (65 RPC methods, 18 DB tables, group-based permissions), but the user-facing experience has critical gaps that prevent production use as a Slack alternative.

Current State

What Works

  • Backend: 65 RPC methods across 13 domains (workspace, user, channel, message, thread, document, attachment, room, presence, read cursors, mentions, groups, permissions)
  • Chat UI: Message send/edit/delete, thread panel with replies, emoji picker with reactions, @mention autocomplete, channel creation, DM creation, context menu, keyboard shortcuts, markdown rendering with mermaid
  • Admin Dashboard (Dioxus): CRUD for all entities, metrics, API docs viewer
  • CLI: Full CRUD for all entities + lifecycle management
  • Infrastructure: Unix socket architecture, hero_proc integration, hero_router compatible

What's Broken (before this work)

  • Reaction toggle: toggleReaction() always called message.react first; server INSERT OR IGNORE silently succeeded on duplicates, so unreact never fired
  • Attachment button: Dead button with no onclick handler, zero upload code in JS
  • Authentication: No auth system. User picker is localStorage-based. caller_id is client-sent and optional (bypasses all permission checks if missing)
  • rpc_proxy drops identity headers: req.into_body() consumes the request, losing all X-Hero-User/Context/Claims headers before forwarding to rpc.sock

What's Missing (Slack Feature Map)

Feature Backend Chat UI Status
Auth (hero_proxy integration) Headers extracted but unused User picker MISSING
Unread counts/badges read.mark/get exist Not called from UI MISSING UI
Typing indicators No RPC needed (WS) Placeholder div exists MISSING
Presence (online/away) presence.update/list exist Not wired MISSING
Full-text search No search RPC Channel filter only MISSING
Pinned messages Not implemented Not in UI MISSING
Channel details panel channel.get/update exist Not in UI MISSING
User profile popover user.get exists Not in UI MISSING
Canvases/doc editor document.CRUD exists Empty template MISSING
Huddles (voice/video) room.CRUD (no tokens) No UI MISSING
Mention notifications mention.list/mark_read exist No notification UI MISSING
Channel browse/discovery channel.list exists No browse UI MISSING

Implementation Plan (7 Phases)

Phase 0: Critical Bug Fixes (1-2 days)

  • Database backup before changes
  • Fix reaction toggle + add message.toggle_react atomic RPC
  • Wire attachment button with compose-with-files flow (nullable message_id, workspace-scoped storage paths, pending uploads, attachment_ids in message.send, attachments in message.get/list)
  • Update OpenRPC spec (now 65 methods)

Phase 1: Authentication via hero_proxy (3-5 days)

  • Prerequisites: Install hero_proxy, verify X-Hero-User header end-to-end, add COLLAB_AUTH_MODE feature flag
  • Read X-Hero-User header, map to collab user via new external_id column, inject as caller_id
  • Forward identity headers in rpc_proxy (currently drops all headers)
  • WebSocket authentication
  • Add user.me RPC method
  • Validate user_id matches caller_id in all 14 handlers (9 self-operations + 3 target-other with permission checks)
  • User lookup from hero_proxy users.list RPC for invites
  • Claims-based authorization (optional, layered with existing workspace permissions)

Phase 2: Chat UI Completion (5-7 days)

  • Unread counts & read cursors (backend ready)
  • Typing indicators (via WebSocket relay)
  • Presence indicators with last_seen stale detection (5-min timeout)
  • Improved WebSocket events (all TECH_SPEC types: message.created/updated/deleted, message.reacted, typing, presence, read.updated)
  • Inline message editing
  • Channel details panel (slide-out, like thread panel)
  • User profile popover

Phase 3: Missing Core Features (5-7 days)

  • Full-text message search (SQLite FTS5 + Ctrl+K modal)
  • Mention-based notifications (parse @mentions on send, notification bell + badge)
  • Pinned messages (pin/unpin via context menu)
  • Channel browse/discovery modal

Phase 4: Canvases & Documents (5-7 days)

  • Document sidebar section
  • Functional markdown editor (split-pane, auto-save)
  • Share document in channel

Phase 5: Voice/Video Huddles (7-10 days, optional)

  • LiveKit token generation
  • Huddle UI (audio bar + headphone icon)

Phase 6: Hardening & Polish (3-4 days)

  • Missing DB indexes (workspace_members.user_id, channel_members.user_id, messages.user_id)
  • Input validation, rate limiting, activity log population
  • Error sanitization, CORS policy
  • Pending attachment cleanup (TTL for orphaned uploads)
  • Load testing, monitoring, integration tests, browser compat

TECH_SPEC Divergences

The original TECH_SPEC.md predates hero_proxy, hero_collab_app, and the CLI crate:

  • Auth: TECH_SPEC has no auth model. Plan uses hero_proxy headers (ecosystem standard).
  • Schema: TECH_SPEC shows column-per-field. Code uses JSON blob pattern. Plan follows code.
  • Crates: TECH_SPEC says 4 crates. Actual project has 6.
  • WebDAV: TECH_SPEC requires WebDAV. Code uses local filesystem. Deferred to post-MVP.
  • IDs: TECH_SPEC uses TEXT UUIDs. Code uses INTEGER AUTOINCREMENT. Plan follows code.

TECH_SPEC should be updated after Phase 2. Until then, plan/slack-feature-parity.md supersedes it.

Post-Implementation Updates

Each phase requires updating: openrpc.json (SDK auto-regenerates), CLI subcommands, Dioxus app types, and documentation. External repos: hero_proxy (configure roles/claims after Phase 1), hero_os (verify island after Phase 6).

## Context Hero Collab is a markdown-centric collaboration platform built as a Hero OS service. The backend is ~95% complete (65 RPC methods, 18 DB tables, group-based permissions), but the user-facing experience has critical gaps that prevent production use as a Slack alternative. ## Current State ### What Works - **Backend:** 65 RPC methods across 13 domains (workspace, user, channel, message, thread, document, attachment, room, presence, read cursors, mentions, groups, permissions) - **Chat UI:** Message send/edit/delete, thread panel with replies, emoji picker with reactions, @mention autocomplete, channel creation, DM creation, context menu, keyboard shortcuts, markdown rendering with mermaid - **Admin Dashboard (Dioxus):** CRUD for all entities, metrics, API docs viewer - **CLI:** Full CRUD for all entities + lifecycle management - **Infrastructure:** Unix socket architecture, hero_proc integration, hero_router compatible ### What's Broken (before this work) - **Reaction toggle:** `toggleReaction()` always called `message.react` first; server `INSERT OR IGNORE` silently succeeded on duplicates, so unreact never fired - **Attachment button:** Dead button with no onclick handler, zero upload code in JS - **Authentication:** No auth system. User picker is localStorage-based. `caller_id` is client-sent and optional (bypasses all permission checks if missing) - **`rpc_proxy` drops identity headers:** `req.into_body()` consumes the request, losing all `X-Hero-User/Context/Claims` headers before forwarding to rpc.sock ### What's Missing (Slack Feature Map) | Feature | Backend | Chat UI | Status | |---|---|---|---| | Auth (hero_proxy integration) | Headers extracted but unused | User picker | MISSING | | Unread counts/badges | read.mark/get exist | Not called from UI | MISSING UI | | Typing indicators | No RPC needed (WS) | Placeholder div exists | MISSING | | Presence (online/away) | presence.update/list exist | Not wired | MISSING | | Full-text search | No search RPC | Channel filter only | MISSING | | Pinned messages | Not implemented | Not in UI | MISSING | | Channel details panel | channel.get/update exist | Not in UI | MISSING | | User profile popover | user.get exists | Not in UI | MISSING | | Canvases/doc editor | document.CRUD exists | Empty template | MISSING | | Huddles (voice/video) | room.CRUD (no tokens) | No UI | MISSING | | Mention notifications | mention.list/mark_read exist | No notification UI | MISSING | | Channel browse/discovery | channel.list exists | No browse UI | MISSING | ## Implementation Plan (7 Phases) ### Phase 0: Critical Bug Fixes (1-2 days) - Database backup before changes - Fix reaction toggle + add `message.toggle_react` atomic RPC - Wire attachment button with compose-with-files flow (nullable message_id, workspace-scoped storage paths, pending uploads, attachment_ids in message.send, attachments in message.get/list) - Update OpenRPC spec (now 65 methods) ### Phase 1: Authentication via hero_proxy (3-5 days) - **Prerequisites:** Install hero_proxy, verify X-Hero-User header end-to-end, add `COLLAB_AUTH_MODE` feature flag - Read `X-Hero-User` header, map to collab user via new `external_id` column, inject as `caller_id` - Forward identity headers in rpc_proxy (currently drops all headers) - WebSocket authentication - Add `user.me` RPC method - Validate user_id matches caller_id in all 14 handlers (9 self-operations + 3 target-other with permission checks) - User lookup from hero_proxy `users.list` RPC for invites - Claims-based authorization (optional, layered with existing workspace permissions) ### Phase 2: Chat UI Completion (5-7 days) - Unread counts & read cursors (backend ready) - Typing indicators (via WebSocket relay) - Presence indicators with last_seen stale detection (5-min timeout) - Improved WebSocket events (all TECH_SPEC types: message.created/updated/deleted, message.reacted, typing, presence, read.updated) - Inline message editing - Channel details panel (slide-out, like thread panel) - User profile popover ### Phase 3: Missing Core Features (5-7 days) - Full-text message search (SQLite FTS5 + Ctrl+K modal) - Mention-based notifications (parse @mentions on send, notification bell + badge) - Pinned messages (pin/unpin via context menu) - Channel browse/discovery modal ### Phase 4: Canvases & Documents (5-7 days) - Document sidebar section - Functional markdown editor (split-pane, auto-save) - Share document in channel ### Phase 5: Voice/Video Huddles (7-10 days, optional) - LiveKit token generation - Huddle UI (audio bar + headphone icon) ### Phase 6: Hardening & Polish (3-4 days) - Missing DB indexes (workspace_members.user_id, channel_members.user_id, messages.user_id) - Input validation, rate limiting, activity log population - Error sanitization, CORS policy - Pending attachment cleanup (TTL for orphaned uploads) - Load testing, monitoring, integration tests, browser compat ## TECH_SPEC Divergences The original `TECH_SPEC.md` predates hero_proxy, hero_collab_app, and the CLI crate: - **Auth:** TECH_SPEC has no auth model. Plan uses hero_proxy headers (ecosystem standard). - **Schema:** TECH_SPEC shows column-per-field. Code uses JSON blob pattern. Plan follows code. - **Crates:** TECH_SPEC says 4 crates. Actual project has 6. - **WebDAV:** TECH_SPEC requires WebDAV. Code uses local filesystem. Deferred to post-MVP. - **IDs:** TECH_SPEC uses TEXT UUIDs. Code uses INTEGER AUTOINCREMENT. Plan follows code. TECH_SPEC should be updated after Phase 2. Until then, `plan/slack-feature-parity.md` supersedes it. ## Post-Implementation Updates Each phase requires updating: `openrpc.json` (SDK auto-regenerates), CLI subcommands, Dioxus app types, and documentation. External repos: hero_proxy (configure roles/claims after Phase 1), hero_os (verify island after Phase 6).
sameh-farouk commented 2026-04-14 10:52:27 +00:00
Author
Member
Copy link

Phase 0 Progress — Complete ✅ (not pushed yet)

Changes are on local development branch, pending push:

  • b541818 fix: add atomic reaction toggle and fix reaction bug
  • bc9a843 feat: compose-with-files attachment flow + DB migrations
  • 1bebdee chore: update OpenRPC spec for Phase 0 changes

What was done:

Reaction toggle fix:

  • Added message.toggle_react RPC — atomic server-side method that checks existence and does INSERT or DELETE, returns {action: "added"|"removed"}
  • Updated toggleReaction() in both chat-app.js (Dioxus island) and chat.html (standalone) to use the new method
  • Verified: react → added, same emoji again → removed, again → added

Attachment compose-with-files flow:

  • attachments.message_id made nullable via safe table recreation migration (PRAGMA check pattern)
  • Storage path decoupled from message_id: attachments/{workspace_id}/{attachment_id}/{filename}
  • attachment.upload accepts optional message_id + workspace_id, validates 25MB file size limit
  • message.send accepts optional attachment_ids array, associates pending attachments (syncs both DB column and data blob)
  • message.get/message.list now return attachments[] array (same JOIN pattern as reactions)
  • Frontend: paperclip button wired, file picker → base64 → upload as pending → chips in composer → send with attachment_ids
  • Path traversal protection: filename sanitized in storage path
  • Added external_id column to users table (for Phase 1 hero_proxy mapping)

OpenRPC spec updated:

  • Added message.toggle_react method
  • Updated attachment.upload (message_id optional, +workspace_id)
  • Updated message.send (+attachment_ids)
  • Total: 65 methods

Blocker for Phase 1+

WebSocket real-time sync is broken when accessed via hero_router. The proxy_to_socket function in hero_router:

  1. Buffers the entire response body (resp.into_body().collect()) — kills streaming
  2. Strips the Upgrade response header (line 556 in routes.rs) — kills WebSocket handshake

This affects ALL Hero services with WebSocket, not just hero_collab. hero_whiteboard has the identical pattern (/ws/{board_id} on ui.sock) and would have the same issue.

The WebSocket architecture (UI socket serves WS relay, browser connects via hero_router) is the standard Hero pattern — hero_collab's TECH_SPEC explicitly says it was "learned from hero_whiteboard's proven dual-channel pattern." The issue is in hero_router's proxy layer.

See hero_router issue for proposed fix.

## Phase 0 Progress — Complete ✅ (not pushed yet) Changes are on local `development` branch, pending push: - `b541818` fix: add atomic reaction toggle and fix reaction bug - `bc9a843` feat: compose-with-files attachment flow + DB migrations - `1bebdee` chore: update OpenRPC spec for Phase 0 changes ### What was done: **Reaction toggle fix:** - Added `message.toggle_react` RPC — atomic server-side method that checks existence and does INSERT or DELETE, returns `{action: "added"|"removed"}` - Updated `toggleReaction()` in both `chat-app.js` (Dioxus island) and `chat.html` (standalone) to use the new method - Verified: react → added, same emoji again → removed, again → added **Attachment compose-with-files flow:** - `attachments.message_id` made nullable via safe table recreation migration (PRAGMA check pattern) - Storage path decoupled from message_id: `attachments/{workspace_id}/{attachment_id}/{filename}` - `attachment.upload` accepts optional `message_id` + `workspace_id`, validates 25MB file size limit - `message.send` accepts optional `attachment_ids` array, associates pending attachments (syncs both DB column and data blob) - `message.get`/`message.list` now return `attachments[]` array (same JOIN pattern as reactions) - Frontend: paperclip button wired, file picker → base64 → upload as pending → chips in composer → send with attachment_ids - Path traversal protection: filename sanitized in storage path - Added `external_id` column to users table (for Phase 1 hero_proxy mapping) **OpenRPC spec updated:** - Added `message.toggle_react` method - Updated `attachment.upload` (message_id optional, +workspace_id) - Updated `message.send` (+attachment_ids) - Total: 65 methods ### Blocker for Phase 1+ **WebSocket real-time sync is broken when accessed via hero_router.** The `proxy_to_socket` function in hero_router: 1. Buffers the entire response body (`resp.into_body().collect()`) — kills streaming 2. Strips the `Upgrade` response header (line 556 in routes.rs) — kills WebSocket handshake This affects ALL Hero services with WebSocket, not just hero_collab. hero_whiteboard has the identical pattern (`/ws/{board_id}` on ui.sock) and would have the same issue. The WebSocket architecture (UI socket serves WS relay, browser connects via hero_router) is the standard Hero pattern — hero_collab's TECH_SPEC explicitly says it was "learned from hero_whiteboard's proven dual-channel pattern." The issue is in hero_router's proxy layer. **See hero_router issue for proposed fix.**
sameh-farouk commented 2026-04-14 11:43:39 +00:00
Author
Member
Copy link

WebSocket Blocker Resolved ✅

The WebSocket issue was not a hero_router bug. hero_router already has WebSocket passthrough support (commit d1632cd — "feat: add WebSocket tunnel support to hero_router", April 13). The locally installed binary was simply built before that commit.

After rebuilding hero_router from latest development branch, WebSocket works correctly:

< HTTP/1.1 101 Switching Protocols
< sec-websocket-accept: T3TQpOmnRqmZUfV9OrgZq2FJw54=

The hero_router issue (#35) has been closed. WebSocket real-time sync is no longer a blocker for Phase 1 and Phase 2.

Note: hero_collab's chat.html had a missing base-path prefix in the WebSocket URL (was /ws/{id}, fixed to {basePath}/ws/{id}). This has been fixed locally (not pushed yet).

## WebSocket Blocker Resolved ✅ The WebSocket issue was **not a hero_router bug**. hero_router already has WebSocket passthrough support (commit `d1632cd` — "feat: add WebSocket tunnel support to hero_router", April 13). The locally installed binary was simply built before that commit. After rebuilding hero_router from latest `development` branch, WebSocket works correctly: ``` < HTTP/1.1 101 Switching Protocols < sec-websocket-accept: T3TQpOmnRqmZUfV9OrgZq2FJw54= ``` The hero_router issue (#35) has been closed. WebSocket real-time sync is no longer a blocker for Phase 1 and Phase 2. **Note:** hero_collab's `chat.html` had a missing base-path prefix in the WebSocket URL (was `/ws/{id}`, fixed to `{basePath}/ws/{id}`). This has been fixed locally (not pushed yet).
sameh-farouk commented 2026-04-14 11:56:02 +00:00
Author
Member
Copy link

Phase 0 pushed to development ✅

4 commits pushed:

  • b541818 fix: add atomic reaction toggle and fix reaction bug
  • bc9a843 feat: compose-with-files attachment flow + DB migrations
  • 1bebdee chore: update OpenRPC spec for Phase 0 changes
  • b77429d fix: WebSocket base-path prefix and dual-codebase sync

Working: Reaction toggle, attachment upload/download, OpenRPC spec (65 methods).

Known issue: WebSocket connects (101 Switching Protocols via curl) but the browser sees the connection close immediately ("Finished" status, empty response). The WS tunnel in hero_router (proxy_ws_tunnel) may be dropping the connection after handshake. RPC-based features work fine — real-time sync via WebSocket needs further debugging. This does not block Phase 1 (auth) or Phase 2 UI work.

Proceeding to Phase 1 (authentication via hero_proxy).

## Phase 0 pushed to development ✅ 4 commits pushed: - `b541818` fix: add atomic reaction toggle and fix reaction bug - `bc9a843` feat: compose-with-files attachment flow + DB migrations - `1bebdee` chore: update OpenRPC spec for Phase 0 changes - `b77429d` fix: WebSocket base-path prefix and dual-codebase sync **Working:** Reaction toggle, attachment upload/download, OpenRPC spec (65 methods). **Known issue:** WebSocket connects (101 Switching Protocols via curl) but the browser sees the connection close immediately ("Finished" status, empty response). The WS tunnel in hero_router (`proxy_ws_tunnel`) may be dropping the connection after handshake. RPC-based features work fine — real-time sync via WebSocket needs further debugging. This does not block Phase 1 (auth) or Phase 2 UI work. Proceeding to Phase 1 (authentication via hero_proxy).
sameh-farouk commented 2026-04-14 13:59:30 +00:00
Author
Member
Copy link

Phase 1 (Auth) — Core Complete, pushed to development

Commits:

  • 9077493 feat: Phase 1 auth — hero_proxy header integration, user.me, rpc_proxy forwarding
  • 395cb70 feat: Phase 1E+1F — user_id validation, channel member permissions, auth-aware UI

What was done:

Auth header reading (1A): http_rpc() reads X-Hero-User, X-Hero-Context, X-Hero-Claims. handle_rpc() looks up user by external_id/email/alias, injects as caller_id. Verified e2e: hero_proxy → hero_router → hero_collab_server.

Feature flag (1-PRE-2): COLLAB_AUTH_MODE=proxy|dev env var. Default dev.

RPC proxy header forwarding (1B): rpc_proxy() extracts identity headers BEFORE consuming request body, forwards them to rpc.sock. Previously all headers were silently dropped.

user.me RPC (1D): Returns authenticated user or auto-creates from X-Hero-User. Returns {authenticated: false} in dev mode.

user_id validation (1E): 12 self-operation handlers use resolve_self_user_id() — blocks impersonation when authenticated, backward compatible in dev mode. Channel member add/remove got new permission checks (was zero before).

Chat UI auth (1F): init() calls user.me first. If authenticated, skips user picker. Falls back to dev mode picker otherwise.

Remaining Phase 1 items (deferred):

  • 1C: WebSocket auth (WS connects but drops — separate debugging needed)
  • 1G: User lookup from hero_proxy for invites (nice-to-have, not blocking)
  • 1H: Claims-based authorization (optional enhancement)

Next: Phase 2 (Chat UI Completion)

## Phase 1 (Auth) — Core Complete, pushed to development Commits: - `9077493` feat: Phase 1 auth — hero_proxy header integration, user.me, rpc_proxy forwarding - `395cb70` feat: Phase 1E+1F — user_id validation, channel member permissions, auth-aware UI ### What was done: **Auth header reading (1A):** `http_rpc()` reads `X-Hero-User`, `X-Hero-Context`, `X-Hero-Claims`. `handle_rpc()` looks up user by `external_id`/`email`/`alias`, injects as `caller_id`. Verified e2e: hero_proxy → hero_router → hero_collab_server. **Feature flag (1-PRE-2):** `COLLAB_AUTH_MODE=proxy|dev` env var. Default `dev`. **RPC proxy header forwarding (1B):** `rpc_proxy()` extracts identity headers BEFORE consuming request body, forwards them to rpc.sock. Previously all headers were silently dropped. **user.me RPC (1D):** Returns authenticated user or auto-creates from `X-Hero-User`. Returns `{authenticated: false}` in dev mode. **user_id validation (1E):** 12 self-operation handlers use `resolve_self_user_id()` — blocks impersonation when authenticated, backward compatible in dev mode. Channel member add/remove got new permission checks (was zero before). **Chat UI auth (1F):** `init()` calls `user.me` first. If authenticated, skips user picker. Falls back to dev mode picker otherwise. ### Remaining Phase 1 items (deferred): - 1C: WebSocket auth (WS connects but drops — separate debugging needed) - 1G: User lookup from hero_proxy for invites (nice-to-have, not blocking) - 1H: Claims-based authorization (optional enhancement) ### Next: Phase 2 (Chat UI Completion)
sameh-farouk commented 2026-04-15 01:31:42 +00:00
Author
Member
Copy link

Post-Phase 1 Fixes — Pushed to development

Commits since last update:

  • eb93d62 fix: switch hero_collab_ui to axum::serve() for WebSocket support
  • a513aed fix: consolidate dual JS codebase, image previews, message ordering, soft-delete filtering
  • 3c9645f fix: include attachments in message.send response for immediate preview
  • 5dc9417 fix: export missing global functions (selectChannel, startDm, pickUser, etc.)

WebSocket Fixed

Root cause: hero_collab_ui used manual hyper::server::conn::http1::Builder which didn't properly propagate WebSocket upgrades through hero_router's tunnel. Switched to axum::serve() (same as hero_proc_ui which has working WebSocket). Connection now stays open — "Connected" status in sidebar.

Dual Codebase Consolidated

  • Removed 1,655 lines of duplicated inline JavaScript from chat.html
  • chat.html is now a pure HTML template that loads chat-app.js via <script src>
  • Single source of truth — no more double-fixing bugs in both files
  • All onclick-referenced functions exported on window.* for HTML handlers
  • Auth detection (user.me call) added to chat-app.js (was only in the deleted inline copy)

Bug Fixes

  • Image attachment preview: Photos now render inline with async base64 loading. Previously showed "(attachment)" text.
  • Message ordering: message.list results reversed from DESC to chronological order. Messages now appear in same order before and after refresh.
  • Soft-delete filtering: Deleted messages filtered from message.list results (was showing deleted messages after refresh).
  • Attachment in send response: message.send now includes attachments[] in response so image previews appear immediately, not just after refresh.
  • Empty content for attachment-only messages: Server accepts empty content string.

Current state

All Phase 0 + Phase 1 core features working:

  • Reaction toggle, file attachments with image preview, WebSocket real-time sync
  • hero_proxy auth integration (X-Hero-User header reading, caller_id injection, user.me auto-create)
  • user_id validation (impersonation blocked), channel member permission checks
  • Single JS codebase, correct message ordering, proper soft-delete

Ready for Phase 2 (Chat UI Completion): unread counts, typing indicators, presence, inline editing.

## Post-Phase 1 Fixes — Pushed to development Commits since last update: - `eb93d62` fix: switch hero_collab_ui to axum::serve() for WebSocket support - `a513aed` fix: consolidate dual JS codebase, image previews, message ordering, soft-delete filtering - `3c9645f` fix: include attachments in message.send response for immediate preview - `5dc9417` fix: export missing global functions (selectChannel, startDm, pickUser, etc.) ### WebSocket Fixed Root cause: hero_collab_ui used manual `hyper::server::conn::http1::Builder` which didn't properly propagate WebSocket upgrades through hero_router's tunnel. Switched to `axum::serve()` (same as hero_proc_ui which has working WebSocket). Connection now stays open — "Connected" status in sidebar. ### Dual Codebase Consolidated - Removed **1,655 lines** of duplicated inline JavaScript from `chat.html` - `chat.html` is now a pure HTML template that loads `chat-app.js` via `<script src>` - Single source of truth — no more double-fixing bugs in both files - All onclick-referenced functions exported on `window.*` for HTML handlers - Auth detection (`user.me` call) added to `chat-app.js` (was only in the deleted inline copy) ### Bug Fixes - **Image attachment preview:** Photos now render inline with async base64 loading. Previously showed "(attachment)" text. - **Message ordering:** `message.list` results reversed from DESC to chronological order. Messages now appear in same order before and after refresh. - **Soft-delete filtering:** Deleted messages filtered from `message.list` results (was showing deleted messages after refresh). - **Attachment in send response:** `message.send` now includes `attachments[]` in response so image previews appear immediately, not just after refresh. - **Empty content for attachment-only messages:** Server accepts empty content string. ### Current state All Phase 0 + Phase 1 core features working: - Reaction toggle, file attachments with image preview, WebSocket real-time sync - hero_proxy auth integration (X-Hero-User header reading, caller_id injection, user.me auto-create) - user_id validation (impersonation blocked), channel member permission checks - Single JS codebase, correct message ordering, proper soft-delete **Ready for Phase 2 (Chat UI Completion):** unread counts, typing indicators, presence, inline editing.
sameh-farouk commented 2026-04-15 09:01:31 +00:00
Author
Member
Copy link

Comprehensive Review Complete — Fixes Pushed

Ran a full code review of all Phase 0+1 work. Found 12 issues, fixed the critical ones:

Fixed (pushed):

  • cb781ef — user.me race condition: concurrent calls for new user could create duplicates. Fixed with existence check inside lock + INSERT OR IGNORE.
  • cb781ef — Send button stayed disabled when files attached but no text typed. Fixed button logic to check state.pendingAttachments.length.
  • 17059e4 — DM functions (pickDmUser, dmSearchKey, filterDmUsers) not exported to window after IIFE consolidation. Caused ReferenceError when clicking DM autocomplete entries.

Accepted (not blocking):

  • Dev mode default is intentionally insecure (logged at startup, production needs COLLAB_AUTH_MODE=proxy)
  • Soft-delete filter is post-fetch in Rust (perf, not correctness)
  • rpc_proxy always returns HTTP 200 (works for JSON-RPC)
  • toggle_react TOCTOU (safe under current Mutex)
  • Lock held across attachment loop (SQLite is single-writer anyway)

Review verified working:

  • Reaction toggle (add/remove/add cycle)
  • user.me (unauthenticated → null, authenticated → user, auto-create)
  • Impersonation blocked (user_id mismatch error)
  • WebSocket 101 handshake
  • Message ordering (chronological)
  • Soft-delete filtering

Starting Phase 2 (Chat UI Completion).

## Comprehensive Review Complete — Fixes Pushed Ran a full code review of all Phase 0+1 work. Found 12 issues, fixed the critical ones: **Fixed (pushed):** - `cb781ef` — `user.me` race condition: concurrent calls for new user could create duplicates. Fixed with existence check inside lock + INSERT OR IGNORE. - `cb781ef` — Send button stayed disabled when files attached but no text typed. Fixed button logic to check `state.pendingAttachments.length`. - `17059e4` — DM functions (`pickDmUser`, `dmSearchKey`, `filterDmUsers`) not exported to `window` after IIFE consolidation. Caused ReferenceError when clicking DM autocomplete entries. **Accepted (not blocking):** - Dev mode default is intentionally insecure (logged at startup, production needs `COLLAB_AUTH_MODE=proxy`) - Soft-delete filter is post-fetch in Rust (perf, not correctness) - rpc_proxy always returns HTTP 200 (works for JSON-RPC) - toggle_react TOCTOU (safe under current Mutex) - Lock held across attachment loop (SQLite is single-writer anyway) **Review verified working:** - Reaction toggle (add/remove/add cycle) - user.me (unauthenticated → null, authenticated → user, auto-create) - Impersonation blocked (user_id mismatch error) - WebSocket 101 handshake - Message ordering (chronological) - Soft-delete filtering **Starting Phase 2 (Chat UI Completion).**
sameh-farouk commented 2026-04-15 10:15:59 +00:00
Author
Member
Copy link

Phase 2 Complete + Security Fixes — Pushed

Commits since Phase 1 update:

Phase 2 features:

  • e76b5ca feat: Phase 2A-D — unread counts, typing indicators, presence, WebSocket events
  • 1c5f7f6 fix: Phase 2 review — WS broadcast, sendBeacon, typing cleanup, heartbeat leak

Review fixes:

  • edd697f fix: gate delete button to message owner, filter deleted messages in get
  • 17059e4 fix: export missing DM functions (pickDmUser, dmSearchKey, filterDmUsers)
  • cb781ef fix: review fixes — user.me race condition, send button with attachments

Security fixes:

  • 9897044 fix: path traversal protection in storage, channel membership check on attachments
  • 88ec970 feat: WebSocket auth — extract X-Hero-User on WS upgrade (Phase 1C)
  • b58cfe0 docs: add known deferred issues section to plan

Phase 2 features working:

  • Unread counts: Red badge on channels with unread messages, cleared on select
  • Typing indicators: "X is typing..." via WebSocket, debounced 3s, cleared on channel switch
  • Presence: last_seen timestamp, 5-min stale detection, 60s heartbeat, sendBeacon offline
  • WebSocket events: message.created/updated/deleted, message.reacted, typing.start/stop

Security hardening done:

  • Path traversal blocked in storage (canonicalization + base_path check)
  • Attachment get/delete checks channel membership
  • Delete button only shown to message author
  • message.get returns error for soft-deleted messages
  • WebSocket extracts X-Hero-User from upgrade request (Phase 1C — was deferred, now done)

Ecosystem research on WebSocket auth:

Checked hero_whiteboard and hero_proc — neither checks auth on WebSocket connections. The Hero pattern is: WebSocket relays are unauthenticated broadcast pipes, auth happens at hero_proxy edge. Our implementation (extracting X-Hero-User + logging) is ahead of the ecosystem standard.

Remaining deferred (Phase 6):

  • N+1 unread queries (perf — needs server-side batch RPC)
  • Attachment ownership validation (needs schema change)
  • hero_proxy user lookup for invites (UX enhancement)
  • Claims-based auth (optional enhancement)

Ready for Phase 3 (search, notifications, pins, channel browse).

## Phase 2 Complete + Security Fixes — Pushed Commits since Phase 1 update: **Phase 2 features:** - `e76b5ca` feat: Phase 2A-D — unread counts, typing indicators, presence, WebSocket events - `1c5f7f6` fix: Phase 2 review — WS broadcast, sendBeacon, typing cleanup, heartbeat leak **Review fixes:** - `edd697f` fix: gate delete button to message owner, filter deleted messages in get - `17059e4` fix: export missing DM functions (pickDmUser, dmSearchKey, filterDmUsers) - `cb781ef` fix: review fixes — user.me race condition, send button with attachments **Security fixes:** - `9897044` fix: path traversal protection in storage, channel membership check on attachments - `88ec970` feat: WebSocket auth — extract X-Hero-User on WS upgrade (Phase 1C) - `b58cfe0` docs: add known deferred issues section to plan ### Phase 2 features working: - **Unread counts:** Red badge on channels with unread messages, cleared on select - **Typing indicators:** "X is typing..." via WebSocket, debounced 3s, cleared on channel switch - **Presence:** last_seen timestamp, 5-min stale detection, 60s heartbeat, sendBeacon offline - **WebSocket events:** message.created/updated/deleted, message.reacted, typing.start/stop ### Security hardening done: - Path traversal blocked in storage (canonicalization + base_path check) - Attachment get/delete checks channel membership - Delete button only shown to message author - message.get returns error for soft-deleted messages - WebSocket extracts X-Hero-User from upgrade request (Phase 1C — was deferred, now done) ### Ecosystem research on WebSocket auth: Checked hero_whiteboard and hero_proc — neither checks auth on WebSocket connections. The Hero pattern is: WebSocket relays are unauthenticated broadcast pipes, auth happens at hero_proxy edge. Our implementation (extracting X-Hero-User + logging) is ahead of the ecosystem standard. ### Remaining deferred (Phase 6): - N+1 unread queries (perf — needs server-side batch RPC) - Attachment ownership validation (needs schema change) - hero_proxy user lookup for invites (UX enhancement) - Claims-based auth (optional enhancement) **Ready for Phase 3 (search, notifications, pins, channel browse).**
sameh-farouk commented 2026-04-15 10:41:01 +00:00
Author
Member
Copy link

Deferred Items Resolved — Only 3 remain

Previously deferred 5 items from Phase 1+2 reviews. Resolved 2 more:

Now complete:

  • ✅ 1C: WebSocket auth — ws_handler extracts X-Hero-User, calls user.me + channel.member.list via rpc.sock to verify channel membership. Non-members get 403 Forbidden. Dev mode (no auth header) still allows connections for backward compat. Goes beyond hero_whiteboard and hero_proc which have zero WS auth. (88ec970, 43dd7f9)
  • ✅ Path traversal — LocalBackend.full_path() now canonicalizes paths and rejects traversal attempts. (9897044)
  • ✅ Attachment access control — attachment.get/delete check channel membership via check_attachment_access(). (9897044)
  • ✅ Delete button gated — Only shown to message author. (edd697f)
  • ✅ Soft-delete in message.get — Returns error for deleted messages. (edd697f)

Still deferred (Phase 6):

  • N+1 unread queries (perf — needs server-side batch RPC)
  • Attachment ownership validation (needs uploaded_by column + schema change)
  • Claims-based auth enhancement (optional, hero_collab's group/rights system handles workspace perms)

Starting Phase 3 (search, notifications, pins, channel browse).

## Deferred Items Resolved — Only 3 remain Previously deferred 5 items from Phase 1+2 reviews. Resolved 2 more: **Now complete:** - ✅ **1C: WebSocket auth** — `ws_handler` extracts `X-Hero-User`, calls `user.me` + `channel.member.list` via rpc.sock to verify channel membership. Non-members get 403 Forbidden. Dev mode (no auth header) still allows connections for backward compat. Goes beyond hero_whiteboard and hero_proc which have zero WS auth. (`88ec970`, `43dd7f9`) - ✅ **Path traversal** — `LocalBackend.full_path()` now canonicalizes paths and rejects traversal attempts. (`9897044`) - ✅ **Attachment access control** — `attachment.get/delete` check channel membership via `check_attachment_access()`. (`9897044`) - ✅ **Delete button gated** — Only shown to message author. (`edd697f`) - ✅ **Soft-delete in message.get** — Returns error for deleted messages. (`edd697f`) **Still deferred (Phase 6):** - N+1 unread queries (perf — needs server-side batch RPC) - Attachment ownership validation (needs `uploaded_by` column + schema change) - Claims-based auth enhancement (optional, hero_collab's group/rights system handles workspace perms) **Starting Phase 3 (search, notifications, pins, channel browse).**
sameh-farouk commented 2026-04-15 11:26:38 +00:00
Author
Member
Copy link

Phase 3 Complete — Pushed to development

Initial implementation:

  • 9f774be feat: Phase 3 — search, notifications, pins, channel browse

Production hardening (13 issues fixed from code review):

  • b6e3948 fix: Phase 3 production hardening — 13 issues from code review

OpenRPC spec updated:

  • 33e00b6 chore: update OpenRPC spec — 70 methods (was 65)

Phase 3 features:

3A. Full-text search:

  • SQLite FTS5 virtual table, synced on send/update/delete
  • FTS backfill for pre-existing messages on startup
  • Query sanitized (double-quote wrapping prevents FTS syntax injection)
  • Search joins channel_members for access control
  • Ctrl+K opens search modal with debounced results

3B. Mention notifications:

  • @username/@alias parsed on message.send via regex
  • Mentions stored in DB with sender_id, channel_id, content
  • mention.list returns sender_name (JOIN users), proper mention ID
  • Notification bell icon with unread badge (polls every 30s)
  • Dropdown with mention list, click to navigate, mark-read

3C. Pinned messages:

  • message.pin/unpin/list_pinned RPC methods
  • Pin/unpin requires channel membership
  • list_pinned uses json_extract SQL filter (not O(N) Rust scan)
  • Pin toggle in right-click context menu
  • Pin icon in channel header opens pinned messages panel

3D. Channel browse/discovery:

  • Browse modal shows all public channels with Join button
  • Correct membership comparison with Number() type safety

Review hardening (13 fixes):

FTS sync on edit/delete, FTS query sanitization, FTS backfill, notification interval leak, sender_name in mentions, mention ID from PK not blob, list_pinned SQL filter, pin permission check, search access control, mention LIMIT, json_extract for name lookup, ID type safety

OpenRPC spec: 70 methods. Ready for Phase 4 (Canvases & Documents).

## Phase 3 Complete — Pushed to development **Initial implementation:** - `9f774be` feat: Phase 3 — search, notifications, pins, channel browse **Production hardening (13 issues fixed from code review):** - `b6e3948` fix: Phase 3 production hardening — 13 issues from code review **OpenRPC spec updated:** - `33e00b6` chore: update OpenRPC spec — 70 methods (was 65) ### Phase 3 features: **3A. Full-text search:** - SQLite FTS5 virtual table, synced on send/update/delete - FTS backfill for pre-existing messages on startup - Query sanitized (double-quote wrapping prevents FTS syntax injection) - Search joins channel_members for access control - Ctrl+K opens search modal with debounced results **3B. Mention notifications:** - @username/@alias parsed on message.send via regex - Mentions stored in DB with sender_id, channel_id, content - mention.list returns sender_name (JOIN users), proper mention ID - Notification bell icon with unread badge (polls every 30s) - Dropdown with mention list, click to navigate, mark-read **3C. Pinned messages:** - message.pin/unpin/list_pinned RPC methods - Pin/unpin requires channel membership - list_pinned uses json_extract SQL filter (not O(N) Rust scan) - Pin toggle in right-click context menu - Pin icon in channel header opens pinned messages panel **3D. Channel browse/discovery:** - Browse modal shows all public channels with Join button - Correct membership comparison with Number() type safety ### Review hardening (13 fixes): FTS sync on edit/delete, FTS query sanitization, FTS backfill, notification interval leak, sender_name in mentions, mention ID from PK not blob, list_pinned SQL filter, pin permission check, search access control, mention LIMIT, json_extract for name lookup, ID type safety **OpenRPC spec: 70 methods.** Ready for Phase 4 (Canvases & Documents).
sameh-farouk commented 2026-04-15 13:33:38 +00:00
Author
Member
Copy link

UX Polish Round — Pushed

Multiple UX fixes after hands-on testing:

  • e0dc5fc Pin button in hover bar, editor 404 fix, thread composer buttons
  • e2ce56d Thread hover actions, emoji/attachment context-awareness
  • 56d2b73 Thread-aware emoji, attachments, pin, delete, reactions
  • 979c3c3 Soft-delete in thread.replies, pending attachment targeting
  • 613cd88 Proper separation of main and thread composer state (no workaround)
  • 70bbd11 Emoji picker positioned near triggering button
  • 5fd5d68 Thread reactions, inline editing, scroll to new message
  • d318235 Emoji picker viewport-aware positioning

Key improvements:

  • Pin button visible in message hover bar (was only in right-click menu)
  • Inline message editing (textarea + Save/Cancel) replaces separate editor tab
  • Thread composer has its own separate attachment and emoji state (no shared global)
  • Emoji picker positions near whichever button triggered it, clamped to viewport
  • Thread replies: soft-delete filtering, attachment rendering, reactions, hover actions
  • Scroll to bottom on new message send

OpenRPC: 70 methods. Starting Phase 4 (Canvases).

## UX Polish Round — Pushed Multiple UX fixes after hands-on testing: - `e0dc5fc` Pin button in hover bar, editor 404 fix, thread composer buttons - `e2ce56d` Thread hover actions, emoji/attachment context-awareness - `56d2b73` Thread-aware emoji, attachments, pin, delete, reactions - `979c3c3` Soft-delete in thread.replies, pending attachment targeting - `613cd88` Proper separation of main and thread composer state (no workaround) - `70bbd11` Emoji picker positioned near triggering button - `5fd5d68` Thread reactions, inline editing, scroll to new message - `d318235` Emoji picker viewport-aware positioning **Key improvements:** - Pin button visible in message hover bar (was only in right-click menu) - Inline message editing (textarea + Save/Cancel) replaces separate editor tab - Thread composer has its own separate attachment and emoji state (no shared global) - Emoji picker positions near whichever button triggered it, clamped to viewport - Thread replies: soft-delete filtering, attachment rendering, reactions, hover actions - Scroll to bottom on new message send **OpenRPC: 70 methods. Starting Phase 4 (Canvases).**
Sign in to join this conversation.
No Branch/Tag specified
Branches Tags
development
development_16
development_mik2
main
No results found.
Labels
Clear labels   
prio_critical

   
prio_low

   
type_bug

   
type_contact

   
type_issue

   
type_lead

   
type_question

   
type_story

   
type_task
No labels
prio_critical
prio_low
type_bug
type_contact
type_issue
type_lead
type_question
type_story
type_task
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
lhumina_code/hero_collab#9
No description provided.