Skip to content

1 feature messaging bot connection#2

Merged
maximedogawa merged 9 commits into
mainfrom
1-feature-messaging-bot-connection
Apr 8, 2026
Merged

1 feature messaging bot connection#2
maximedogawa merged 9 commits into
mainfrom
1-feature-messaging-bot-connection

Conversation

@maximedogawa
Copy link
Copy Markdown
Collaborator

@maximedogawa maximedogawa commented Apr 8, 2026

Summary by CodeRabbit

  • New Features

    • Local Telegram bot connect/disconnect with persisted session, health endpoints, and streamed runtime logs to the dashboard.
  • Improvements

    • Setup wizard uses real reachability checks and a real connect flow; offers a copyable connection command and clearer step gating.
    • Dashboard shows live Pengine/Ollama status, auto-resumes prior connections, and displays “No device connected” with an explicit setup link when disconnected.
    • Navigation simplified; startup flow refined to avoid unexpected redirects.
  • Bug Fixes

    • Prevented scrollbar layout shifts.

…zard with Pengine API integration. Add mock API for testing, improve service status checks, and update UI components for better user experience. Introduce connection handling and logging for the Telegram bot service.
…configuration. Update UI components to reflect device connection status, enhance navigation, and improve user experience. Introduce automatic redirection based on device connection state and streamline API calls across components.
…behavior. Update TopMenu to conditionally render the setup link with accessibility features, and add scrollbar-gutter property for better layout stability.
…oard access. Rename AutoForward to StartupDashboardRedirect for clarity, ensuring it only runs once on initial load. Update configuration to include Ollama API base URL and enhance error handling in DashboardPage for better service status reporting. Modify TopMenu to improve accessibility features for setup link.
…tests to mock Ollama API responses, enhancing the setup wizard with real-time model detection and status reporting. Refactor DashboardPage to display active model information and improve error handling for Ollama connectivity. Ensure consistent API usage across components for better maintainability.
@maximedogawa maximedogawa self-assigned this Apr 8, 2026
@maximedogawa maximedogawa linked an issue Apr 8, 2026 that may be closed by this pull request
5 tasks
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 8, 2026

Warning

Rate limit exceeded

@maximedogawa has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 12 minutes and 12 seconds before requesting another review.

Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 12 minutes and 12 seconds.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 3bb25bb0-7bb4-4989-8628-6bb593e80b6f

📥 Commits

Reviewing files that changed from the base of the PR and between d5ac216 and 420248b.

📒 Files selected for processing (7)
  • src-tauri/src/telegram_service.rs
  • src/App.tsx
  • src/components/SetupWizard.tsx
  • src/components/TerminalPreview.tsx
  • src/loopback.ts
  • src/pages/DashboardPage.tsx
  • src/stores/appSessionStore.ts
📝 Walkthrough

Walkthrough

Adds a local Axum HTTP loopback server and Telegram bot runtime in the Tauri backend with persistent connection state and SSE logs; exposes health/connect/logs endpoints. Frontend is rewired to probe Ollama/Pengine, perform real connect/disconnect flows, stream runtime logs, update session identity, and adjust tests to mock those APIs.

Changes

Cohort / File(s) Summary
Tauri loopback server
src-tauri/src/connection_server.rs, src-tauri/Cargo.toml, src-tauri/capabilities/default.json
New Axum-based HTTP server (127.0.0.1:21516) with retry bind, CORS, endpoints: POST/DELETE /v1/connect, GET /v1/health, GET /v1/logs (SSE); new Rust deps and added event permissions.
Bot lifecycle & Telegram service
src-tauri/src/bot_lifecycle.rs, src-tauri/src/telegram_service.rs
Added bot shutdown helper and Teloxide-backed long-polling dispatcher. Includes token verification, message handling that queries Ollama endpoints, logging, and lifecycle start/stop logic.
Shared state & lib bootstrap
src-tauri/src/state.rs, src-tauri/src/lib.rs
Introduced AppState with persistence, broadcast log channel, emit_log; added Tauri commands (get_connection_status, disconnect_bot), resume persisted connection and spawn loopback server at startup.
Frontend loopback helpers
src/loopback.ts, src/config.ts
New client helpers for Ollama/Pengine (fetchOllamaModel, getPengineHealth, postConnect, deleteConnect); added PENGINE_API_BASE and OLLAMA_API_BASE constants.
App session & session wiring
src/stores/appSessionStore.ts, src/App.tsx
Session store now persists botUsername/botId; connectDevice accepts bot data and disconnectDevice calls loopback DELETE. Startup flow probes /v1/health and conditionally restores session and navigates to dashboard.
Setup & Connect UI
src/components/SetupWizard.tsx, e2e/setup-dashboard.spec.ts
Replaced mock demo steps with real Ollama/Pengine checks and a real connect POST flow; tests add mockApis(page) to intercept Ollama/Pengine and updated expectations for connect flow and dashboard state.
Dashboard, terminal & navigation
src/pages/DashboardPage.tsx, src/pages/SetupPage.tsx, src/pages/LandingPage.tsx, src/components/TopMenu.tsx, src/components/TerminalPreview.tsx, src/index.css
Dashboard polls health/ollama and shows dynamic service statuses; TerminalPreview streams logs via Tauri events or SSE with reconnect/backoff; TopMenu simplified to route/session-aware no-props component; minor CSS change (scrollbar-gutter).

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant Frontend
    participant Loopback as Tauri Server<br/>127.0.0.1:21516
    participant Telegram
    participant Ollama

    User->>Frontend: Submit bot token
    Frontend->>Loopback: POST /v1/connect { bot_token }
    Loopback->>Telegram: getMe(token)
    Telegram-->>Loopback: Me { bot_id, bot_username }
    Loopback->>Loopback: persist connection, spawn bot task
    Loopback-->>Frontend: 200 OK { bot_id, bot_username }
    Frontend->>Frontend: update session (connectDevice), navigate /dashboard

    Telegram->>Loopback: incoming update (long-poll)
    Loopback->>Ollama: GET /api/ps or GET /api/tags
    Ollama-->>Loopback: model info
    Loopback->>Ollama: POST /api/chat { model, prompt }
    Ollama-->>Loopback: inference result
    Loopback->>Telegram: send reply
    Loopback->>Loopback: emit_log() -> broadcast
    Loopback-->>Frontend: SSE /v1/logs event
    Frontend->>Frontend: append TerminalPreview
Loading
sequenceDiagram
    participant App as App.tsx
    participant Store as AppSessionStore
    participant Health as /v1/health

    App->>Health: GET PENGINE_API_BASE/v1/health (startup probe)
    Health-->>App: { bot_connected, bot_username?, bot_id? }
    alt bot_connected == true
        App->>Store: connectDevice({ bot_username, bot_id })
        App->>App: navigate to /dashboard (replace)
    else
        App->>App: no-op (remain or allow setup)
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Possibly related PRs

  • 1 feature messaging bot connection #2: Implements the same local connection server, Telegram bot lifecycle, loopback helpers, frontend wiring, and test updates — strong code-level overlap.

Poem

🐇 I hopped from mocks into the wild,

a loopback server, bot compiled.
Logs now stream and models talk,
Dashboard wakes — no more mock walk.
Hooray, the rabbit danced and smiled!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 28.95% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title '1 feature messaging bot connection' directly aligns with the main changes in this pull request, which implements a complete messaging bot connection system including backend HTTP server, Telegram bot service, state management, and frontend UI updates for device connection/disconnection workflows.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch 1-feature-messaging-bot-connection

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 8

🧹 Nitpick comments (3)
src/components/TerminalPreview.tsx (2)

70-72: Auto-scroll may disrupt users reading older logs.

The auto-scroll to bottom on every new line could be disruptive if a user is trying to read earlier entries. For a small preview component this is acceptable, but consider adding scroll-position detection if this becomes an issue.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/TerminalPreview.tsx` around lines 70 - 72, The current
useEffect in TerminalPreview always calls scrollRef.current?.scrollTo on lines
change, which forces scroll-to-bottom and can interrupt users reading older
logs; modify the effect to first read the scroll container's scrollTop,
clientHeight, and scrollHeight (via scrollRef.current) and only call scrollTo
when the user is already near the bottom (e.g., scrollHeight - (scrollTop +
clientHeight) < threshold), otherwise skip auto-scrolling so user scroll
position is preserved; keep the effect tied to lines and reference the existing
useEffect, scrollRef, lines, and scrollTo call when making this change.

44-59: SSE connection lacks error handling and reconnection.

The EventSource fallback doesn't handle connection errors or automatic reconnection. If the Pengine service starts after this component mounts, the SSE stream won't connect.

♻️ Optional: Add error handler with reconnection
       try {
         const es = new EventSource(`${PENGINE_API_BASE}/v1/logs`);
         es.onmessage = (event) => {
           if (cancelled) return;
           try {
             const entry: LogLine = JSON.parse(event.data);
             addLine(entry);
           } catch {
             // ignore malformed events
           }
         };
+        es.onerror = () => {
+          // Reconnect after delay if not cancelled
+          es.close();
+          if (!cancelled) {
+            setTimeout(() => { if (!cancelled) connect(); }, 3000);
+          }
+        };
         cleanup = () => es.close();
       } catch {
         // SSE not available
       }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/TerminalPreview.tsx` around lines 44 - 59, The SSE fallback in
TerminalPreview that constructs new EventSource(`${PENGINE_API_BASE}/v1/logs`)
(variable es) needs error handling and reconnection: add an es.onerror handler
that closes the current es, schedules a reconnect with exponential backoff
(retry timer) to recreate EventSource and reattach onmessage/onerror, ensure
cancelled is checked before reconnecting, and update cleanup to clear the retry
timer and close the es and remove handlers; keep using addLine for messages and
preserve the existing malformed-event try/catch.
src-tauri/Cargo.toml (1)

25-33: Consider updating teloxide to the latest stable version.

The added dependencies are appropriate for the feature set. However, teloxide 0.13 is significantly outdated—the latest stable is 0.17.0. This 4-version gap likely includes bug fixes and improvements. Similarly, reqwest 0.12 is one minor version behind (0.13.2). The other dependencies (axum 0.8.8, tower-http 0.6.8) are current. Recommend reviewing whether updating these versions is feasible, particularly teloxide.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src-tauri/Cargo.toml` around lines 25 - 33, Update the outdated dependencies
in Cargo.toml by bumping teloxide from "0.13" to the current stable (e.g.,
"0.17.0") and reqwest from "0.12" to the latest 0.13.x (e.g., "0.13.2"); locate
the dependency entries for teloxide and reqwest in Cargo.toml, change their
version strings, run cargo update and cargo build/test to resolve any breaking
changes (adjust feature flags or call sites if needed), and ensure the remaining
deps (tokio, axum, tower-http, etc.) remain compatible.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src-tauri/src/connection_server.rs`:
- Around line 138-165: The current shutdown uses notify_waiters() + sleep which
can race and allow telegram_service::start_bot to be spawned while the previous
dispatcher is still running; modify stop_existing_bot (and the other similar
call at the other location) to return/record a JoinHandle or await an explicit
"stopped" signal from the dispatcher (e.g., a oneshot or a state flag like
bot_running) and only proceed to set state.connection and tokio::spawn start_bot
after that wait completes (with a configurable timeout to avoid blocking
forever); update the spawn code to use the recorded handle or await the stopped
notification before cloning state and calling telegram_service::start_bot.
- Around line 43-53: The CorsLayer is currently configured with
allow_origin(Any) which exposes Router routes ("/v1/connect", DELETE
"/v1/connect", "/v1/logs") to any webpage; change this to a restrictive
allowlist (replace Any with a predefined allowed origins list such as the
desktop app and dev origins) and only register that CorsLayer for trusted
origins. Additionally add authentication middleware/guard that verifies an
app-scoped auth token header (e.g. check in a new middleware applied to the
Router or each handler: handle_connect, handle_disconnect, handle_logs_sse) so
requests from browsers without the token are rejected; ensure the token check is
enforced before streaming logs or accepting connect/disconnect operations.

In `@src-tauri/src/state.rs`:
- Around line 8-14: ConnectionData currently stores bot_token and persist()
writes it to connection.json; remove bot_token from ConnectionData (keep bot_id,
bot_username, connected_at) and stop writing secrets to disk, instead
store/retrieve the bot token using OS-backed secure storage APIs (e.g., Windows
Credential Manager / macOS Keychain / Linux Secret Service) via a secure-storage
helper (e.g., get_secure_token(), set_secure_token()) called from the code paths
that create/load ConnectionData; update any code referencing
ConnectionData::bot_token to use the secure-storage helper and ensure
persist()/load() only handle non-secret metadata.

In `@src-tauri/src/telegram_service.rs`:
- Around line 115-133: The branches that handle Ollama failures (the Err arm of
active_ollama_model() and the Err arm of ask_ollama(&model, incoming)) only emit
logs and return, leaving the user without a reply; update both locations to send
a short fallback Telegram message before returning (e.g., use
bot.send_message(msg.chat.id, "Sorry, local inference is unavailable right now.
Please try again later.").await? or handle/send and log any send error), keeping
the existing state.emit_log calls (symbols: active_ollama_model, ask_ollama,
state.emit_log, bot.send_message, msg.chat.id) so the chat gets a clear
user-facing error before the function returns.

In `@src/App.tsx`:
- Around line 42-46: The health endpoint does not return bot_id but the code
calls connectDevice({ bot_username: data.bot_username, bot_id: "" }), which
stores an empty string; change the call in the component that handles the
/v1/health response to omit bot_id or pass null/undefined instead of "" so the
store reflects "not set" (e.g., call connectDevice with only bot_username or
with bot_id: data.bot_id ?? null), and if necessary update the connectDevice
signature (and any Device/store types like botId) to accept bot_id as optional
or nullable so downstream code can distinguish unset vs empty string.

In `@src/components/SetupWizard.tsx`:
- Around line 175-177: The component sets verifiedBot using setVerifiedBot on
success but never persists it, so update the happy-path to persist the verified
bot: after setVerifiedBot({ bot_id: data.bot_id, bot_username: data.bot_username
}) either call the provided onCompleteSetup callback with the verified bot
(e.g., onCompleteSetup({ bot_id: ..., bot_username: ... })) or call the
session/store updater used elsewhere (the same method connectDevice expects to
populate botId/botUsername in session) so the bot_id and bot_username are saved
for the rest of the app; modify the success branch where setVerifiedBot is
invoked (and the analogous block at lines ~512-516) to perform one of these
persistence actions.
- Around line 84-88: The step-completion check is allowing progress based on
ollamaAcknowledged even when no model is pulled; change the gating to require a
detected model (check ollamaModel truthiness, e.g. ollamaModel !== null or
!!ollamaModel) instead of ollamaAcknowledged for the Ollama step(s) (the
conditional that checks step and returns ollamaAcknowledged), and make the same
replace where the same logic appears later (the other block around lines
316-324) so the wizard only advances when ollamaModel is present; keep the other
checks (pengineReachable, connectStatus) unchanged and preserve the hook
dependencies ([step, status, ollamaAcknowledged, pengineReachable,
connectStatus]) as needed.

In `@src/stores/appSessionStore.ts`:
- Around line 27-36: The disconnectDevice implementation currently clears state
unconditionally; change it to await the DELETE response and only reset Zustand
state (isDeviceConnected, botUsername, botId) when the fetch succeeds (resp.ok),
otherwise throw or return the fetch error so callers can handle it. In other
words, in disconnectDevice make the request, capture the Response from fetch
(using AbortSignal.timeout(5000)), check resp.ok and only call set({...}) on
success; if resp is not ok or fetch throws, rethrow or propagate the error
instead of silently clearing state.

---

Nitpick comments:
In `@src-tauri/Cargo.toml`:
- Around line 25-33: Update the outdated dependencies in Cargo.toml by bumping
teloxide from "0.13" to the current stable (e.g., "0.17.0") and reqwest from
"0.12" to the latest 0.13.x (e.g., "0.13.2"); locate the dependency entries for
teloxide and reqwest in Cargo.toml, change their version strings, run cargo
update and cargo build/test to resolve any breaking changes (adjust feature
flags or call sites if needed), and ensure the remaining deps (tokio, axum,
tower-http, etc.) remain compatible.

In `@src/components/TerminalPreview.tsx`:
- Around line 70-72: The current useEffect in TerminalPreview always calls
scrollRef.current?.scrollTo on lines change, which forces scroll-to-bottom and
can interrupt users reading older logs; modify the effect to first read the
scroll container's scrollTop, clientHeight, and scrollHeight (via
scrollRef.current) and only call scrollTo when the user is already near the
bottom (e.g., scrollHeight - (scrollTop + clientHeight) < threshold), otherwise
skip auto-scrolling so user scroll position is preserved; keep the effect tied
to lines and reference the existing useEffect, scrollRef, lines, and scrollTo
call when making this change.
- Around line 44-59: The SSE fallback in TerminalPreview that constructs new
EventSource(`${PENGINE_API_BASE}/v1/logs`) (variable es) needs error handling
and reconnection: add an es.onerror handler that closes the current es,
schedules a reconnect with exponential backoff (retry timer) to recreate
EventSource and reattach onmessage/onerror, ensure cancelled is checked before
reconnecting, and update cleanup to clear the retry timer and close the es and
remove handlers; keep using addLine for messages and preserve the existing
malformed-event try/catch.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 973f5d58-572a-4054-b9ec-741e8d9922c1

📥 Commits

Reviewing files that changed from the base of the PR and between 9f7208b and 591e03d.

⛔ Files ignored due to path filters (1)
  • src-tauri/Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (17)
  • e2e/setup-dashboard.spec.ts
  • src-tauri/Cargo.toml
  • src-tauri/capabilities/default.json
  • src-tauri/src/connection_server.rs
  • src-tauri/src/lib.rs
  • src-tauri/src/state.rs
  • src-tauri/src/telegram_service.rs
  • src/App.tsx
  • src/components/SetupWizard.tsx
  • src/components/TerminalPreview.tsx
  • src/components/TopMenu.tsx
  • src/config.ts
  • src/index.css
  • src/pages/DashboardPage.tsx
  • src/pages/LandingPage.tsx
  • src/pages/SetupPage.tsx
  • src/stores/appSessionStore.ts

Comment on lines +43 to +53
let cors = CorsLayer::new()
.allow_origin(Any)
.allow_methods(Any)
.allow_headers(Any);

let app = Router::new()
.route("/v1/connect", post(handle_connect))
.route("/v1/connect", delete(handle_disconnect))
.route("/v1/health", get(handle_health))
.route("/v1/logs", get(handle_logs_sse))
.layer(cors)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

The loopback control API is open to any website right now.

allow_origin(Any) exposes /v1/connect, DELETE /v1/connect, and /v1/logs to arbitrary browser origins. That lets a malicious page script 127.0.0.1:21516 from the user’s browser and disconnect the bot, attach a different bot token, or read streamed logs. Restrict CORS to the app/dev origins and require an app-scoped auth token.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src-tauri/src/connection_server.rs` around lines 43 - 53, The CorsLayer is
currently configured with allow_origin(Any) which exposes Router routes
("/v1/connect", DELETE "/v1/connect", "/v1/logs") to any webpage; change this to
a restrictive allowlist (replace Any with a predefined allowed origins list such
as the desktop app and dev origins) and only register that CorsLayer for trusted
origins. Additionally add authentication middleware/guard that verifies an
app-scoped auth token header (e.g. check in a new middleware applied to the
Router or each handler: handle_connect, handle_disconnect, handle_logs_sse) so
requests from browsers without the token are rejected; ensure the token check is
enforced before streaming logs or accepting connect/disconnect operations.

Comment thread src-tauri/src/connection_server.rs Outdated
Comment thread src-tauri/src/state.rs
Comment on lines +8 to +14
#[derive(Debug, Clone, Serialize, Deserialize)]
pub struct ConnectionData {
pub bot_token: String,
pub bot_id: String,
pub bot_username: String,
pub connected_at: DateTime<Utc>,
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Avoid persisting the Telegram bot token in plaintext.

ConnectionData includes bot_token, and persist() writes it verbatim to connection.json. That leaves a long-lived Telegram credential unencrypted in app data. Keep only non-secret metadata in this file and move the token to OS-backed secure storage.

Also applies to: 62-68

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src-tauri/src/state.rs` around lines 8 - 14, ConnectionData currently stores
bot_token and persist() writes it to connection.json; remove bot_token from
ConnectionData (keep bot_id, bot_username, connected_at) and stop writing
secrets to disk, instead store/retrieve the bot token using OS-backed secure
storage APIs (e.g., Windows Credential Manager / macOS Keychain / Linux Secret
Service) via a secure-storage helper (e.g., get_secure_token(),
set_secure_token()) called from the code paths that create/load ConnectionData;
update any code referencing ConnectionData::bot_token to use the secure-storage
helper and ensure persist()/load() only handle non-secret metadata.

Comment thread src-tauri/src/telegram_service.rs
Comment thread src/App.tsx Outdated
Comment thread src/components/SetupWizard.tsx Outdated
Comment thread src/components/SetupWizard.tsx Outdated
Comment thread src/stores/appSessionStore.ts
…and error handling. Update E2E tests to mock new log streaming API responses, ensuring real-time log updates in the TerminalPreview component. Refactor SetupWizard to utilize bot ID and streamline connection logic. Improve disconnect handling in DashboardPage with error reporting for better user experience.
…components to utilize a centralized fetchOllamaModel function for improved code maintainability. Update StartupDashboardRedirect documentation for clarity. Enhance error handling in Telegram service response.
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/components/SetupWizard.tsx (1)

241-245: ⚠️ Potential issue | 🟠 Major

Invalidate the verified connection state on token edits.

After a successful connect, editing the Step 1 token leaves connectStatus === "connected" and verifiedBot intact. Step 4 can then keep showing the old bot as connected and never offer a fresh connect for the new token.

Suggested fix
-                onChange={(event) => setBotToken(event.target.value)}
+                onChange={(event) => {
+                  setBotToken(event.target.value);
+                  setConnectStatus("idle");
+                  setConnectError("");
+                  setVerifiedBot(null);
+                  setBotUsername("");
+                }}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/SetupWizard.tsx` around lines 241 - 245, When the token input
changes, invalidate the previous verified connection by clearing verifiedBot and
resetting connectStatus; update the onChange handler used by the token input
(where setBotToken is called) to also call setVerifiedBot(null) (or equivalent)
and setConnectStatus("idle" or "disconnected") so editing the token will force a
fresh connect via the connect/verify flow instead of leaving connectStatus ===
"connected".
♻️ Duplicate comments (2)
src-tauri/src/connection_server.rs (2)

45-56: ⚠️ Potential issue | 🔴 Critical

Restrict CORS on the loopback control API.

allow_origin(Any) lets arbitrary websites script POST /v1/connect, DELETE /v1/connect, and GET /v1/logs against the user's local runtime. That is enough to replace the configured bot, disconnect it, or read live logs from the browser. Allowlist only the packaged app/dev origins and narrow the allowed methods/headers to what the frontend actually uses.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src-tauri/src/connection_server.rs` around lines 45 - 56, The CORS
configuration is too permissive: replace the
CorsLayer::new().allow_origin(Any).allow_methods(Any).allow_headers(Any) usage
with a restricted policy that only allowlists the packaged app and dev origins
(e.g., the Tauri/webview origin and localhost dev origin) and limits methods and
headers to what's actually needed by our endpoints; update the CorsLayer
referenced where Router is built (the cors variable used with
Router::with_state) to allow only POST for "/v1/connect", DELETE for
"/v1/connect", GET for "/v1/health" and "/v1/logs", and restrict headers to the
minimal set the frontend sends (e.g., Content-Type and any auth header used,
plus any CORS-safelisted headers), and ensure the SSE "/v1/logs" supports the
necessary accept/content types for event-streams.

140-166: ⚠️ Potential issue | 🔴 Critical

Use per-bot lifecycle state instead of a shared Notify + bot_running flag.

stop_and_wait_for_bot(&state).await does not give these handlers a definitive handoff. It can time out and return, and a just-spawned bot has not set bot_running yet because that only happens inside src-tauri/src/telegram_service.rs, Lines 59-62. A fast second connect/disconnect can therefore skip the stop entirely or proceed while the previous poller is still alive. Track the spawned task/cancellation handle behind a lifecycle mutex and fail the request if shutdown never completes.

Also applies to: 182-185

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src-tauri/src/connection_server.rs` around lines 140 - 166, The current flow
uses a shared Notify and bot_running flag and races between
stop_and_wait_for_bot(&state).await and telegram_service::start_bot; instead,
add a per-bot lifecycle slot (e.g., behind a lifecycle Mutex in the same state
struct) that stores the spawned task's cancellation handle/JoinHandle or a
CancellationToken, change stop_and_wait_for_bot to acquire that lifecycle mutex,
cancel and await the previous handle with a timeout and return an error if
shutdown never completes, and when spawning via telegram_service::start_bot
store the new handle/token in that lifecycle slot and set the running flag
there; update the connect path around state.persist, state.connection and
tokio::spawn to use this lifecycle mutex so fast reconnects deterministically
fail if the previous bot didn't stop.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src-tauri/src/connection_server.rs`:
- Around line 31-37: The HealthResponse currently exposes bot_connected (derived
from state.connection) which reflects stored connection state but not runtime
polling; update the HealthResponse struct to add a new boolean field (e.g.,
bot_running or polling) and populate it from state.bot_running so the response
distinguishes stored connection vs active long-polling, then update any handlers
that construct HealthResponse (including the code around the existing
HealthResponse usage at lines ~207-214) to set the new field appropriately;
ensure front-end (DashboardPage.tsx) is switched to use the new
bot_running/polling field for service status rendering.

In `@src/components/SetupWizard.tsx`:
- Around line 194-204: handleCopyUri currently writes a full curl command with
the raw Telegram token to the clipboard; change it so the default copy action
writes a template curl command with a placeholder (e.g.,
"BOT_TOKEN_PLACEHOLDER") instead of inserting connectionUri/connectionPayload,
and add an explicit separate action (e.g., handleCopyRawUri or a confirmation
modal triggered from handleCopyUri) that only after explicit user confirmation
copies the full raw command containing connectionUri/connectionPayload to the
clipboard; update the useCallback named handleCopyUri and any UI wiring (copy
button) to use the template by default and require explicit confirmation before
calling the raw copy path, and keep existing setCopiedUri behavior for UX
feedback.

---

Outside diff comments:
In `@src/components/SetupWizard.tsx`:
- Around line 241-245: When the token input changes, invalidate the previous
verified connection by clearing verifiedBot and resetting connectStatus; update
the onChange handler used by the token input (where setBotToken is called) to
also call setVerifiedBot(null) (or equivalent) and setConnectStatus("idle" or
"disconnected") so editing the token will force a fresh connect via the
connect/verify flow instead of leaving connectStatus === "connected".

---

Duplicate comments:
In `@src-tauri/src/connection_server.rs`:
- Around line 45-56: The CORS configuration is too permissive: replace the
CorsLayer::new().allow_origin(Any).allow_methods(Any).allow_headers(Any) usage
with a restricted policy that only allowlists the packaged app and dev origins
(e.g., the Tauri/webview origin and localhost dev origin) and limits methods and
headers to what's actually needed by our endpoints; update the CorsLayer
referenced where Router is built (the cors variable used with
Router::with_state) to allow only POST for "/v1/connect", DELETE for
"/v1/connect", GET for "/v1/health" and "/v1/logs", and restrict headers to the
minimal set the frontend sends (e.g., Content-Type and any auth header used,
plus any CORS-safelisted headers), and ensure the SSE "/v1/logs" supports the
necessary accept/content types for event-streams.
- Around line 140-166: The current flow uses a shared Notify and bot_running
flag and races between stop_and_wait_for_bot(&state).await and
telegram_service::start_bot; instead, add a per-bot lifecycle slot (e.g., behind
a lifecycle Mutex in the same state struct) that stores the spawned task's
cancellation handle/JoinHandle or a CancellationToken, change
stop_and_wait_for_bot to acquire that lifecycle mutex, cancel and await the
previous handle with a timeout and return an error if shutdown never completes,
and when spawning via telegram_service::start_bot store the new handle/token in
that lifecycle slot and set the running flag there; update the connect path
around state.persist, state.connection and tokio::spawn to use this lifecycle
mutex so fast reconnects deterministically fail if the previous bot didn't stop.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 0c0bfedd-c148-42b8-8d21-076391941612

📥 Commits

Reviewing files that changed from the base of the PR and between 591e03d and d5ac216.

⛔ Files ignored due to path filters (1)
  • src-tauri/Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (12)
  • e2e/setup-dashboard.spec.ts
  • src-tauri/Cargo.toml
  • src-tauri/src/bot_lifecycle.rs
  • src-tauri/src/connection_server.rs
  • src-tauri/src/lib.rs
  • src-tauri/src/telegram_service.rs
  • src/App.tsx
  • src/components/SetupWizard.tsx
  • src/components/TerminalPreview.tsx
  • src/pages/DashboardPage.tsx
  • src/pages/SetupPage.tsx
  • src/stores/appSessionStore.ts
✅ Files skipped from review due to trivial changes (2)
  • src-tauri/Cargo.toml
  • src-tauri/src/lib.rs
🚧 Files skipped from review as they are similar to previous changes (3)
  • src/pages/SetupPage.tsx
  • src/App.tsx
  • e2e/setup-dashboard.spec.ts

Comment on lines +31 to +37
#[derive(Serialize)]
pub struct HealthResponse {
pub status: String,
pub bot_connected: bool,
pub bot_username: Option<String>,
pub bot_id: Option<String>,
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Expose runtime liveness separately from stored connection state.

bot_connected is derived only from state.connection.is_some(), but that value is already set before start_bot reaches the polling loop and it stays set if the dispatcher exits unexpectedly. src/pages/DashboardPage.tsx renders this as "@user long poll active", so the UI can say "running" when no bot is polling. Add a bot_running/polling field sourced from state.bot_running and have the dashboard use that for service status.

Also applies to: 207-214

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src-tauri/src/connection_server.rs` around lines 31 - 37, The HealthResponse
currently exposes bot_connected (derived from state.connection) which reflects
stored connection state but not runtime polling; update the HealthResponse
struct to add a new boolean field (e.g., bot_running or polling) and populate it
from state.bot_running so the response distinguishes stored connection vs active
long-polling, then update any handlers that construct HealthResponse (including
the code around the existing HealthResponse usage at lines ~207-214) to set the
new field appropriately; ensure front-end (DashboardPage.tsx) is switched to use
the new bot_running/polling field for service status rendering.

Comment on lines +194 to +204
const handleCopyUri = useCallback(async () => {
try {
await navigator.clipboard.writeText(
`curl -X POST ${connectionUri} -H "Content-Type: application/json" -d '${connectionPayload}'`,
);
setCopiedUri(true);
setTimeout(() => setCopiedUri(false), 2000);
} catch {
/* clipboard not available */
}
}, [connectionUri, connectionPayload]);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Don't copy a live bot token into the system clipboard.

This writes a fully usable curl command with the raw Telegram token embedded. Clipboard history/sync managers turn that into a long-lived secret leak. Prefer copying a template with a placeholder, or require an explicit "copy secret" confirmation if you really need the raw command.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/SetupWizard.tsx` around lines 194 - 204, handleCopyUri
currently writes a full curl command with the raw Telegram token to the
clipboard; change it so the default copy action writes a template curl command
with a placeholder (e.g., "BOT_TOKEN_PLACEHOLDER") instead of inserting
connectionUri/connectionPayload, and add an explicit separate action (e.g.,
handleCopyRawUri or a confirmation modal triggered from handleCopyUri) that only
after explicit user confirmation copies the full raw command containing
connectionUri/connectionPayload to the clipboard; update the useCallback named
handleCopyUri and any UI wiring (copy button) to use the template by default and
require explicit confirmation before calling the raw copy path, and keep
existing setCopiedUri behavior for UX feedback.

…nnection logic in loopback module. Update SetupWizard and DashboardPage components to utilize new functions for improved maintainability and error handling. Remove obsolete ollamaStatus module to streamline codebase.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature] Messaging bot connection

1 participant