Companion: mobile cockpit endpoints#125
Merged
Merged
Conversation
System Monitor backend: CPU usage/brand/cores, memory, per-volume storage, uptime, and optional battery via sysinfo (disk feature) + starship-battery.
list/read(text)/write/mkdir/delete/download(bytes)/upload(bytes)/bounded search, home-dir default, authed via the /v1 bearer middleware.
spawn_shell_pty now takes an optional output Channel; spawn_companion_shell
spawns a headless shell whose writer lands in TerminalState, so the existing
/v1/term/{id}/ws mirrors output and forwards input unchanged.
Port enumeration (netstat2 + sysinfo process names) and a best-effort HTTP
reverse proxy to 127.0.0.1:{port} via reqwest for the embedded browser.
The mobile browser loads localhost ports as .../v1/proxy/<port>/ (trailing slash for
correct relative-asset resolution). axum's {*path} wildcard doesn't match the empty
path after the slash, so that exact URL 404'd → 'webpage not available'. Add the
trailing-slash route mapping to proxy_root (empty path → upstream root).
The idle-prompt attention sweep treated every idle SSH shell prompt as 'needs input' and re-fired each idle stretch (post-auth, MOTD, after each command). Gate the sweep to agent terminals; SSH's only legitimate attention will come from real keyboard-interactive auth rounds (handled separately).
Phone-cancelled opens were leaking real desktop tabs: the open-session event is queued to the (possibly suspended) frontend, so a lidded desktop opened the tab on wake regardless of the phone giving up. Add POST /v1/sessions/cancel + a cancelled set in LifecycleState; spawn endpoints accept a client request_id so the phone can target the cancel, and companion_report_opened closes the tab if its request was cancelled.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughAdds filesystem, port, and system-metrics companion endpoints, a headless shell session flow, and cancellation-aware lifecycle handling for pending session opens. Shell terminals gain their own kind and routing behavior. ChangesMobile Companion API Expansion
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src-tauri/src/companion/api.rs`:
- Around line 398-400: The client-supplied requestId values used by the session
lifecycle need validation and deduplication before they are stored or reused as
map keys. Update the request handling in api.rs and the lifecycle path around
LifecycleState::register_pending so empty/blank IDs are rejected and duplicate
IDs are not allowed to overwrite an existing pending entry. If a requestId is
already pending, return a 409 Conflict instead of inserting it, and ensure the
cancel/open flows continue to use the normalized, validated requestId
consistently.
In `@src-tauri/src/companion/files.rs`:
- Around line 233-271: The recursive file search in search and walk can still
block the async request worker while scanning large trees, even when
SEARCH_MAX_RESULTS is never reached. Move the traversal work into spawn_blocking
from search, and update walk to enforce a visited-entry budget and/or
elapsed-time cutoff in addition to the existing depth and result limits. Use the
existing search, walk, SEARCH_MAX_RESULTS, and SEARCH_MAX_DEPTH symbols to place
the fix and keep the response shape unchanged.
- Around line 45-49: The path handling in resolve and the upload-related
handlers still accepts relative or traversal-prone inputs, which violates the
absolute-host-path contract. Centralize parsing into a shared helper used by
resolve and the handlers around upload/list/delete so only absolute paths are
accepted, and make upload name validation require exactly one normal filename
component with no separators or “..”. Update the affected functions in files.rs
to route all path inputs through this parser and reject anything outside the
selected directory.
- Around line 137-157: The download handler in download currently buffers the
entire file with std::fs::read, which can spike memory for large redirected
downloads. Update download to use a streamed file response body instead of
loading bytes into memory, or enforce an explicit size cap before serving; keep
the existing header handling and error path intact while changing the
file-serving logic around PathBuf::from, std::fs::read, and the final
into_response.
- Around line 198-200: The upload handler in upload currently buffers the full
request body as Bytes, so large files can be rejected before the route is
reached. Update the /v1/fs/upload path by either adding a route-specific body
limit in server.rs or changing upload in files.rs to stream directly to disk
instead of collecting the entire payload, and make sure the fix is applied
specifically to the upload route.
In `@src-tauri/src/companion/lifecycle.rs`:
- Around line 49-51: The cancel() logic in LifecycleState currently records
every request_id in cancelled even when there is no matching pending entry,
which can leave stale cancellations that affect later spawns. Update cancel() to
only mark a request as cancelled if it actually existed in pending, and make the
pending removal and cancellation recording happen together using the existing
pending/cancelled state in LifecycleState. This should be fixed in the cancel()
method and its interaction with companion_report_opened so reused IDs do not
inherit old tombstones.
In `@src-tauri/src/companion/ports.rs`:
- Around line 99-105: The header forwarding loop in the proxy path is leaking
the companion bearer token upstream because it only skips host and connection;
update the logic in the header iteration within the port forwarding code to also
drop Authorization before calling rb.header, alongside the existing hop-by-hop
exclusions. Use the header-processing block in the proxy request builder to
ensure no bearer-auth credentials are forwarded to the local service.
- Around line 94-111: The proxied request built in the request flow for ports.rs
currently uses reqwest::Client::new() without any explicit timeout, so it can
hang indefinitely. Update the request setup around the client/request builder
path to apply a timeout to the proxied call before send(), matching the timeout
behavior used by other HTTP clients in the codebase.
- Around line 74-93: The proxy handlers in proxy_root and proxy_path drop any
incoming query string before calling forward, so proxied requests lose
parameters. Update forward (and its callers) to preserve the original URI query
component when building the target URL, appending it to the path from Path and
keeping the existing method, headers, and body flow intact.
In `@src-tauri/src/companion/sysmon.rs`:
- Around line 88-108: The volume collection in sysmon is only doing adjacent
deduplication, so duplicate mount points can still remain in the metrics
response when they are not consecutive. Update the logic around the
Disks::new_with_refreshed_list flow and the volumes Vec<Volume> processing to
perform set-based deduplication by mount_point instead of relying on dedup_by,
so each mount point is kept only once regardless of ordering.
In `@src-tauri/src/modes/agent/terminal.rs`:
- Around line 294-297: The shell PTY setup in spawn_shell_pty is still
registering all spawned terminals as fanout::TermKind::Agent, which makes
headless/plain shells participate in attention pushes. Thread the terminal kind
through spawn_shell_pty (and the spawn_companion_shell call path) or introduce a
separate Shell kind, then update the fanout registration and sweep_attention
filtering so only real agent shells are included and non-agent terminal prompts
are excluded.
🪄 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 Plus
Run ID: 8665d3c7-f2f5-46ee-880d-7ae20b401383
⛔ Files ignored due to path filters (1)
src-tauri/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (9)
src-tauri/Cargo.tomlsrc-tauri/src/companion/api.rssrc-tauri/src/companion/fanout.rssrc-tauri/src/companion/files.rssrc-tauri/src/companion/lifecycle.rssrc-tauri/src/companion/mod.rssrc-tauri/src/companion/ports.rssrc-tauri/src/companion/sysmon.rssrc-tauri/src/modes/agent/terminal.rs
Comment on lines
+137
to
+157
| pub async fn download(Query(q): Query<PathQuery>) -> Response { | ||
| let path = PathBuf::from(&q.path); | ||
| let bytes = match std::fs::read(&path) { | ||
| Ok(b) => b, | ||
| Err(e) => return err(StatusCode::NOT_FOUND, &format!("not found: {e}")), | ||
| }; | ||
| let name = path | ||
| .file_name() | ||
| .map(|n| n.to_string_lossy().to_string()) | ||
| .unwrap_or_else(|| "download".to_string()); | ||
| ( | ||
| [ | ||
| (header::CONTENT_TYPE, "application/octet-stream".to_string()), | ||
| ( | ||
| header::CONTENT_DISPOSITION, | ||
| format!("attachment; filename=\"{name}\""), | ||
| ), | ||
| ], | ||
| bytes, | ||
| ) | ||
| .into_response() |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Stream downloads instead of buffering the whole file.
std::fs::read loads arbitrary downloads fully into memory. Since /fs/read redirects large files to download, a large file can spike memory or stall the companion server. Use a streamed file body or add an explicit download size cap.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src-tauri/src/companion/files.rs` around lines 137 - 157, The download
handler in download currently buffers the entire file with std::fs::read, which
can spike memory for large redirected downloads. Update download to use a
streamed file response body instead of loading bytes into memory, or enforce an
explicit size cap before serving; keep the existing header handling and error
path intact while changing the file-serving logic around PathBuf::from,
std::fs::read, and the final into_response.
- proxy: drop upstream Authorization, preserve query string, add request timeout - fs: require absolute paths, sanitize upload filename (no traversal), cap download size, run search off the async worker with a visited budget, raise upload body limit - lifecycle: reject duplicate requestIds (409), only tombstone a real pending cancel - shells register as TermKind::Shell so the idle-prompt sweep skips them (no false 'needs input' for plain Terminal-tab shells) - sysmon: set-based mount-point dedup
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Backend for the mobile companion app.
GET /v1/sys/metrics/v1/fs/{list,read,download,search,mkdir,write,upload,delete}POST /v1/sessions/shell/v1/ports,/v1/proxy/{port}POST /v1/sessions/cancel/v1/proxy/{port}/(trailing slash) routes correctlySummary by CodeRabbit