Staging

[ih8js-git]

No description provided.

[Copilot]

🟡 Changes recommended

There are confirmed correctness/UX issues (e.g., WS auth errors being swallowed and cursor positioning not being safely bounded) that should be fixed before approval.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR substantially expands VimStoat from a placeholder UI into a more functional “staging” client by adding DM flows, vim-like modal input/command mode, and WebSocket-driven real-time updates, alongside refactoring app state/event handling and documentation.

Changes:

• Added DM list + DM chat UIs and corresponding handlers, including vim-like insert/normal motions and command mode (:) support.
• Introduced WebSocket client + event model to receive real-time message events and populate server list data.
• Refactored core app state/store, API client (configurable base URL), caching behavior, and replaced ROADMAP reference with mdBook-style docs.

File summaries

File	Description
src/ui/server_list.rs	Implements a real server/DM entry list with selection highlighting and mode-colored borders.
src/ui/render.rs	Adds layout splitting for command mode, routes new app states to new UI renderers, and renders the command line.
src/ui/mod.rs	Registers new UI modules (dm, dm_list) and adjusts module ordering.
src/ui/input_token.rs	Adds cursor positioning for token entry.
src/ui/error.rs	Improves error titles based on message content and simplifies error text.
src/ui/dm.rs	New DM conversation view with message rendering, input box, scrolling, and cursor placement logic.
src/ui/dm_list.rs	New DM list view with unread indicator and message preview truncation.
src/notification.rs	Adds a notify-rust based notification helper.
src/models.rs	Introduces core models (Server, DirectMessageChannel, User, Message).
src/main.rs	Adds terminal keyboard enhancement flags + cursor style setup, WS auth + event handling, and cache dump on exit.
src/input.rs	Expands input modes, adds mode→color mapping, introduces richer keymaps, and adds tests.
src/handlers/mod.rs	New handlers module namespace for per-state input handling.
src/handlers/logged_in.rs	Implements key handling for the logged-in server list (including DM entry).
src/handlers/input_token.rs	Moves token entry key handling into a handler module with async validation/storage.
src/handlers/error.rs	Adds “press any key” handling to return to token input.
src/handlers/dm.rs	Adds vim-like text editing motions + message sending behavior for DM view.
src/handlers/dm_list.rs	Adds navigation/open behavior for DM list, including background message history fetch.
src/handlers/command.rs	Implements command-mode editing and command execution dispatch.
src/error.rs	Adds AuthError error taxonomy for auth-related failures.
src/command.rs	Introduces :q / :qa parsing and execution.
src/cache.rs	Switches to dump-on-request, adds temporary cache for tests, adds explicit dump and “get all users”.
src/app.rs	Major refactor: introduces AppStore/AppEvent, new states (DmList/Dm), WS client integration, and event handling.
src/api/ws.rs	Adds WS client connect/send/dispatch plus a minimal handler for Ready→servers population.
src/api/mod.rs	Adds new API modules and constants for default base URLs.
src/api/events.rs	Adds WS client/server event models for serde (including message events).
src/api/dm.rs	Adds REST helper to fetch DM channels and infer display names/usernames.
src/api/client.rs	Adds configurable base URL, adds POST support, and supports custom endpoints/paths.
src/api/channel.rs	Adds REST helper to fetch channel message history with optional query params.
src/api/auth.rs	Refactors auth to use unified Result/AuthError and validates token with configurable base URL.
src/action.rs	Expands the action enum to cover vim-like motions and command mode entry.
ROADMAP.md	Removes the roadmap document from the repository root.
README.md	Updates status link to point to the docs book instead of the removed roadmap.
docs/vimstoat/src/SUMMARY.md	Adds features page entry to the mdBook summary.
docs/vimstoat/src/keybinds.md	Expands documented keybinds and documents command-mode commands.
docs/vimstoat/src/features.md	Adds an initial features overview for implemented functionality.
Cargo.toml	Adds WS/notifications/ulid deps and adjusts reqwest features.
.gitignore	Adds mdBook output directory to ignore list.

Review details

Suppressed comments (2)

src/main.rs:109

• Similarly, only one queued WebSocket event is processed per tick. Switching this to while let drains the receiver each frame and reduces UI lag during bursty traffic.

        if let Ok(event) = app.ws_rx.try_recv() {

src/api/ws.rs:44

• Typo in the log message: "Connexion" should be "Connection".

                        info!("WS Connexion closed by server.");

• Files reviewed: 36/37 changed files
• Comments generated: 7
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟡 Changes recommended

There are multiple confirmed runtime-impacting bugs (notably unconditional WS auth at startup and missing WS auth after first-time login, plus cursor reset and UTF-8 truncation issues) that should be fixed before approval.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (4)

Previously missed (2) — in code that hasn't changed since the last review.

src/handlers/dm_list.rs:39

• Entering a DM clears input_text but does not reset input_cursor. If the cursor was previously > 0, subsequent insert/remove operations in handlers/dm.rs can no-op because the cursor is past the end of the buffer.

            app.store.current_dm_messages.clear();
            app.input_text.clear();

src/input.rs:282

• Test assertion message has a typo: “appened” → “appended”.

This issue also appears in the following locations of the same file:

• line 287
• line 292

            "Should have done appened q"

src/input.rs:292

• Test assertion message has a typo: “appened” → “appended”.

            "Should have appened R"

src/input.rs:287

• Test assertion message has a typo: “appened” → “appended”.

            "Should have appened r"

• Files reviewed: 36/37 changed files
• Comments generated: 3
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

There are confirmed compile-blocking borrow/move issues in the new async WebSocket/auth handling paths (temporary String borrows across .await and moved event reuse).

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (4)

Previously missed (2) — in code that hasn't changed since the last review.

src/ui/render.rs:33

• Rendering the Error screen allocates a new String every frame via message.to_string(). Since the error UI already formats the message into another String, this adds avoidable per-frame allocation while in AppState::Error.
  src/input.rs:282
• Typo in test assertion message: "appened" should be "appended".

This issue also appears in the following locations of the same file:

• line 287
• line 292

            "Should have done appened q"

src/input.rs:292

• Typo in test assertion message: "appened" should be "appended".

            "Should have appened R"

src/input.rs:287

• Typo in test assertion message: "appened" should be "appended".

            "Should have appened r"

• Files reviewed: 36/37 changed files
• Comments generated: 4
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

It introduces broad architectural and async/WebSocket behavior changes across core app, input handling, and rendering, and includes reliability/cursor-positioning issues that should be validated end-to-end before approval.

Review details

Suppressed comments (8)

Previously missed (6) — in code that hasn't changed since the last review.

src/ui/render.rs:33

• error::render(f, &message.to_string()) allocates a new String every frame while the error screen is shown. Consider changing error::render to accept an impl std::fmt::Display (or &anyhow::Error) so you can format inside error::render without forcing a separate allocation here.
  src/input.rs:312
• Grammar in test failure message is off: "Should not called" reads incorrectly. Consider changing it to "Should not have called action early".

            "Should not called action early"

src/input.rs:282

• Typo in test failure message: "appened" → "appended".

This issue also appears in the following locations of the same file:

• line 287
• line 292

            "Should have done appened q"

src/ui/render.rs:54

• Cursor X uses as u16 on the command text length, which truncates on long input and can also overflow during addition, leading to incorrect cursor placement. Clamp/convert safely and use saturating arithmetic before the final min().
  src/ui/input_token.rs:34
• Cursor X uses app.input_cursor as u16, which truncates for long tokens and can overflow during addition. Convert/clamp safely and use saturating arithmetic before clamping to the box interior.
  src/app.rs:85
• Auth::new() already returns crate::Result, so the map_err(|e| anyhow::anyhow!(e))? wrapping is redundant and can lose error context. This can be simplified to propagate the error directly.

        let auth = Auth::new().map_err(|e| anyhow::anyhow!(e))?;

src/input.rs:287

• Typo in test failure message: "appened" → "appended".

            "Should have appened r"

src/input.rs:292

• Typo in test failure message: "appened" → "appended".

            "Should have appened R"

• Files reviewed: 36/37 changed files
• Comments generated: 1
• Review effort level: Lite

[YetAnotherMechanicusEnjoyer]

👍