Repository navigation
checkpoint #1 - #4
Merged
Merged
Conversation
Added expo app, fixed some bugs Expo app 52 SDK version required Expo app: 1. Power features such as sleep, power off and restart added. 2. Query can be given and be searched on the web by the LLM using playwright automation 2a. If pre-defined toolkit found -> uses that and provides answer to the user. 2b. If unsuccessful -> uses old toolkits. If still unsuccessful -> creates new automation toolkit from scratch by the LLM provider.
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThis PR adds an Expo React Native mobile client (UI, WebSocket hook, config, docs), a Tauri WebSocket server that forwards requests to a Python agent_router, multiple Playwright CLI tools with a JSON registry, and LLM-driven tool generalization and sufficiency checks plus related tests. ChangesMobile Client (React Native/Expo)
Backend Agent Router & Tools
Tauri WebSocket Server
Configuration and Testing
Sequence Diagram(s) sequenceDiagram
participant MobileApp
participant TauriServer
participant AgentRouter
participant PlaywrightTool
participant Browser
MobileApp->>TauriServer: ws send JSON query/request
TauriServer->>AgentRouter: write newline-terminated JSON to stdin
AgentRouter->>AgentRouter: route decision / execute tool(s)
AgentRouter->>PlaywrightTool: spawn tool subprocess with JSON args
PlaywrightTool->>Browser: Playwright navigations & DOM extraction
PlaywrightTool-->>AgentRouter: stdout JSON result
AgentRouter-->>TauriServer: stream synthesis/status lines to stdout
TauriServer-->>MobileApp: forward streamed JSON lines over WebSocket
Estimated code review effort:
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 18
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
python/tools/find_crypto_price.py (1)
1-104:⚠️ Potential issue | 🔴 CriticalCritical: Declare Playwright as a dependency
python/tools/find_crypto_price.pyimportsplaywright.async_api, butrequirements.txtdoesn’t includeplaywright, andimport playwrightfails withModuleNotFoundErrorin the current environment.📦 Add Playwright to requirements.txt
+playwright>=1.40.0After updating
requirements.txt, install browser binaries too:pip install playwright playwright install chromium🤖 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 `@python/tools/find_crypto_price.py` around lines 1 - 104, The module find_crypto_price imports from playwright.async_api (see async_playwright in find_crypto_price and the async function find_crypto_price), but Playwright is missing from dependencies; add "playwright" (optionally with a pinned version) to requirements.txt, update any packaging/CI configs to install it, and ensure post-installation step runs "playwright install chromium" (or include a setup script/CI step) so the browser binaries are available when running async_playwright; verify installation in CI before running the script.
🟡 Minor comments (4)
scratch/test_ws.py-31-51 (1)
31-51:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winTest sends real "sleep" command that will suspend the system.
Lines 33 and 50 construct and send a WebSocket frame containing the
"sleep"command, which will actually suspend the host machine if the server is running. This makes the test dangerous to run unintentionally.Recommendations:
- Use a harmless command (e.g.,
"test"or a query that returns quickly)- Add a confirmation prompt before sending power commands
- Document clearly in the script header that it will suspend the system
🛡️ Safer test payload
- # 3. Construct and send a masked WebSocket text frame containing "sleep" + # 3. Construct and send a masked WebSocket text frame containing a test query # For a client-to-server frame, payload MUST be masked. - payload = b"sleep" + payload = b'{"requestId":"test123","query":"hello"}' length = len(payload)Or add a warning comment at the top of the file:
# WARNING: This test sends a "sleep" command that will suspend the host system. # Only run when you intend to test the actual sleep functionality.🤖 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 `@scratch/test_ws.py` around lines 31 - 51, The test constructs and sends a real "sleep" WebSocket payload (payload = b"sleep") and transmits it via s.sendall(header + masked_payload), which can suspend the host; change payload to a harmless value (e.g., b"test" or a benign query) by updating the payload variable, add a user confirmation prompt (or require an environment variable flag) before calling s.sendall to guard against accidental runs, and add a clear top-of-file warning comment explaining the original behavior and that sending real power commands is dangerous.mobile/README.md-8-8 (1)
8-8:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winSDK version mismatch with AGENTS.md.
This line states "Expo SDK 54" but
mobile/AGENTS.md(line 3) references the v56.0.0 documentation. Based on learnings, the exact versioned Expo docs at v56.0.0 should be consulted before writing code. Please align the SDK version across documentation files.🤖 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 `@mobile/README.md` at line 8, The README line stating "*Note: This project is configured to run on **Expo SDK 54**.*" conflicts with AGENTS.md which references the v56.0.0 Expo docs; update the SDK version string so both documents match (either change the README note to "Expo SDK 56" to align with AGENTS.md or update AGENTS.md to 54 if you intend to stay on 54), and ensure any other README/AGENTS mentions or badges reference the same exact versioned Expo docs (e.g., "v56.0.0") for consistency.Source: Learnings
mobile/usePCWebSocket.ts-32-34 (1)
32-34:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winValidate IP format before appending port.
If the user enters just
:or:<port>, the port-defaulting logic will produce an invalid URL likews://:9001orws://:8080. Add validation to ensureformattedIpcontains actual host characters before the colon check.🛡️ Proposed validation
if (!formattedIp) { setStatus('error'); setErrorMsg('IP Address cannot be empty'); return; } + + // Validate format before appending port + if (formattedIp.startsWith(':')) { + setStatus('error'); + setErrorMsg('Invalid IP address format'); + return; + } if (!formattedIp.includes(':')) { formattedIp = `${formattedIp}:9001`; }🤖 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 `@mobile/usePCWebSocket.ts` around lines 32 - 34, The current port-defaulting logic appends ":9001" based only on the presence of ':' which allows invalid hosts like ":" or ":8080"; update the block around the formattedIp variable in usePCWebSocket.ts to first trim formattedIp, then validate the host portion before the colon (e.g., split on ':' and ensure the left side contains non-empty, non-whitespace characters and is not just empty), only append ":9001" when that host portion is valid, and if it is invalid fall back to a safe default (for example "localho 8000 st:9001") or surface an error so you don't produce "ws://:9001". Ensure references to formattedIp in this file use the validated value.python/tools/search_product_info.py-115-115 (1)
115-115:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUnused loop variable.
The loop variable
idxat line 115 is never used within the loop body. Per Python conventions, unused loop variables should be prefixed with an underscore.♻️ Rename unused variable
- for idx, block in enumerate(search_blocks[:6]): + for _idx, block in enumerate(search_blocks[:6]):🤖 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 `@python/tools/search_product_info.py` at line 115, The loop in python/tools/search_product_info.py uses an unused variable `idx` in "for idx, block in enumerate(search_blocks[:6]):"; rename `idx` to `_` (or `_idx`) to follow Python conventions for unused loop variables and ensure no other code references `idx` within the loop body (update any occurrences if present).
🧹 Nitpick comments (6)
src-tauri/Cargo.toml (1)
22-22: 💤 Low valueConsider using specific tokio features instead of "full".
The
"full"feature set includes many capabilities not used by this codebase. Based onwebsocket.rs, you need:net,process,io-util,sync,macros, andrt-multi-thread. Using specific features reduces compile time and binary size.♻️ More targeted feature set
-tokio = { version = "1.52.3", features = ["full"] } +tokio = { version = "1.52.3", features = ["net", "process", "io-util", "sync", "macros", "rt-multi-thread"] }🤖 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/Cargo.toml` at line 22, The dependency currently enables tokio's "full" feature set; restrict it to only the features used by the code (as indicated by websocket.rs) to reduce compile time and binary size—replace the existing tokio entry (the line with tokio = { version = "1.52.3", features = ["full"] }) with a feature list containing net, process, io-util, sync, macros, and rt-multi-thread while keeping the same version number.src-tauri/src/websocket.rs (2)
157-241: Concurrent query handling is serialized due to global daemon lock.The
daemon_mutexis held for the entire duration of query processing (lines 162-241), which means concurrent WebSocket requests will be queued and processed serially. This is necessary because the daemon uses stdin/stdout pipes that cannot handle interleaved requests, but it may create head-of-line blocking if queries take significant time.This is acceptable given the architecture, but consider documenting this limitation or adding a timeout on lock acquisition to prevent indefinite waits.
🤖 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/websocket.rs` around lines 157 - 241, forward_query_to_daemon currently holds the global daemon_mutex (obtained via get_daemon_mutex) for the entire lifecycle of sending a query and streaming the response, which serializes all concurrent WebSocket queries; either document this limitation in the forward_query_to_daemon function comment (mentioning stdin/stdout non-interleavability and potential head-of-line blocking) or implement a timed lock acquisition (e.g., attempt to lock daemon_mutex with a timeout before proceeding, return an error to the caller when lock acquisition times out) so callers don’t wait indefinitely; reference the daemon_mutex/get_daemon_mutex usage and the AgentDaemon::start/send_query/read_response flow when adding the comment or timeout logic.
105-116: 💤 Low valueConsider extracting request ID parsing into a helper function.
The nested conditionals for extracting
request_idfrom both"query:"format and JSON format are correct but reduce readability. A small helper function would clarify intent and enable reuse.♻️ Extract helper function
+fn extract_request_id(text: &str) -> String { + if text.starts_with("query:") { + let parts: Vec<&str> = text.splitn(3, ':').collect(); + if parts.len() >= 2 { + return parts[1].to_string(); + } + } else if let Ok(parsed) = serde_json::from_str::<serde_json::Value>(text) { + if let Some(id) = parsed.get("requestId").and_then(|r| r.as_str()) { + return id.to_string(); + } + } + "unknown".to_string() +} + async fn handle_connection(...) { // ... } else if trimmed.starts_with("query:") || trimmed.starts_with("{") { - let request_id = if trimmed.starts_with("query:") { - let parts: Vec<&str> = trimmed.splitn(3, ':').collect(); - if parts.len() == 3 { - parts[1].to_string() - } else { - "unknown".to_string() - } - } else if let Ok(parsed) = serde_json::from_str::<serde_json::Value>(trimmed) { - parsed.get("requestId").and_then(|r| r.as_str()).unwrap_or("unknown").to_string() - } else { - "unknown".to_string() - }; + let request_id = extract_request_id(trimmed);🤖 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/websocket.rs` around lines 105 - 116, Extract the nested logic that computes request_id into a small helper like fn extract_request_id(message: &str) -> String and replace the inline block that uses trimmed with a call to it; the helper should (1) check if message.starts_with("query:"), splitn(3, ':'), return parts[1].to_string() when parts.len()==3 or "unknown" otherwise, (2) otherwise attempt serde_json::from_str::<serde_json::Value>(message) and return parsed.get("requestId").and_then(|r| r.as_str()).unwrap_or("unknown").to_string(), and (3) default to "unknown". Ensure the new function is used wherever request_id is computed (referencing the request_id variable and trimmed input) to improve readability and reuse.mobile/usePCWebSocket.ts (2)
51-59: 💤 Low valueConsider updating error state on JSON parse failure.
Non-JSON messages are silently logged to the console but don't update any user-visible state. If the server sends malformed responses, users won't see any indication. Consider setting an error state or incrementing a debug counter when parse failures occur repeatedly.
🤖 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 `@mobile/usePCWebSocket.ts` around lines 51 - 59, The onmessage handler currently swallows JSON parse failures (inside ws.onmessage where wsRef.current is checked and setLatestResponse is called) and only console.logs the raw message; add/update a user-visible error state (e.g., setParseError) or increment a parse-failure counter state (e.g., incrementParseFailureCount) in the catch block so UI/components can show malformed-response errors or metrics; ensure you still check wsRef.current === ws before updating state and include the raw message or error details when setting the error state for easier debugging.
8-8: ⚡ Quick winReplace
anywith a typed interface.
latestResponseis typed asany, which reduces type safety. Based on the server response schema inpython/agent_router.py:38-46, define an interface matching{requestId: string, status: string, data?: any, error?: any}.✨ Suggested type definition
+interface WebSocketResponse { + requestId: string; + status: string; + data?: any; + error?: { code?: string; message?: string; details?: string }; +} + export function usePCWebSocket() { const [status, setStatus] = useState<ConnectionStatus>('disconnected'); const [errorMsg, setErrorMsg] = useState<string | null>(null); - const [latestResponse, setLatestResponse] = useState<any>(null); + const [latestResponse, setLatestResponse] = useState<WebSocketResponse | null>(null);🤖 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 `@mobile/usePCWebSocket.ts` at line 8, latestResponse is currently typed as any which weakens type-safety; define a Response interface (e.g. interface AgentRouterResponse { requestId: string; status: string; data?: unknown; error?: unknown }) based on the server schema and replace useState<any>(null) with useState<AgentRouterResponse | null>(null) (update usages of latestResponse and setLatestResponse accordingly); add the interface at the top of usePCWebSocket.ts (or export it if used elsewhere) and prefer unknown for data/error instead of any to preserve type-safety while allowing arbitrary payloads.mobile/App.tsx (1)
33-33: ⚡ Quick winConsider typing
agentResultmore specifically.
agentResultis typed asany, reducing type safety. Based on the server response schema and the UI's handling ofresponse(line 357) and key/value pairs (lines 360-371), define an interface or use a union type to capture the expected shapes.✨ Suggested type definition
+interface AgentResult { + response?: string; + raw_metadata?: any[]; + [key: string]: any; +} + export default function App() { const [ipAddress, setIpAddress] = useState(''); const { status, errorMsg, latestResponse, connect, disconnect, sendCommand, sendQuery } = usePCWebSocket(); const [actionFeedback, setActionFeedback] = useState<string | null>(null); const [queryText, setQueryText] = useState(''); const [agentStatus, setAgentStatus] = useState<'idle' | 'processing' | 'success' | 'error'>('idle'); const [agentProgressMsg, setAgentProgressMsg] = useState(''); - const [agentResult, setAgentResult] = useState<any>(null); + const [agentResult, setAgentResult] = useState<AgentResult | null>(null);🤖 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 `@mobile/App.tsx` at line 33, agentResult is currently typed as any; define a specific TypeScript type (e.g., an interface or union) that matches the server response shapes the UI expects (the top-level response field and the key/value pair list the UI iterates over) and use it as the useState generic for agentResult in place of any. Create types like an AgentResult with a response: string (or nullable) and either a metadata Record<string,string> or entries: Array<{key:string; value:string}> (or a union covering both response-only and response+entries shapes), then update the useState declaration (agentResult, setAgentResult) and any consumers to use that new type.
🤖 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 `@mobile/App.tsx`:
- Line 251: The TextInput in mobile/App.tsx is using keyboardType="numeric",
which prevents entering dots and colons for IP:port strings; update the
TextInput (the component where keyboardType="numeric" is set) to use
keyboardType="url" (or keyboardType="default") so the keyboard allows dots and
colons for addresses like "192.168.1.15:9001".
In `@mobile/package.json`:
- Line 7: The Expo SDK version in mobile/package.json is inconsistent with the
PR/docs (package.json has "expo": "~54.0.0" while PR/docs reference SDK 52 and
repo guidance points to SDK 56); pick a single target SDK (e.g., Expo SDK 56 or
whichever the project intends), update the "expo" entry in mobile/package.json
to that SDK string, and update the PR description and any docs/instructions to
reference the same SDK so tooling, React and RN baselines match; search for the
"expo" field in mobile/package.json and for any mentions of "Expo SDK", "SDK
52/54/56", or the Expo version in the PR text/docs and make them consistent.
In `@python/agent_router.py`:
- Around line 626-631: The code currently reuses combined_result even though
this branch only runs after check_sufficiency() returned False, which risks
returning a known-bad result; instead, when custom-tool verification fails do
not assign raw_result = combined_result — always call send_response(request_id,
"error", error={... error_details}) and return immediately. Locate the block
handling verification failure (references: combined_result, check_sufficiency(),
send_response, request_id, error_details) and remove the fallback that treats
combined_result as acceptable so the verification failure is surfaced.
- Around line 568-585: The code currently writes model-generated Python to
temp_tool_filepath and calls execute_script directly after audit_code passes,
which executes untrusted code with full daemon permissions; change the flow to
run generated scripts in a hardened sandbox instead: modify execute_script (or
replace its usage) so it launches the script in an isolated environment (e.g.,
unprivileged container or dedicated sandboxed worker process) with no network,
strict filesystem chroot/overlay, resource limits (CPU, memory, walltime), and
minimal environment variables, and ensure temp_tool_filepath is created with
restrictive permissions and removed after execution; update the call-site around
audit_code, temp_tool_filepath, send_response and execute_script to handle
sandbox failures and timeouts and to never execute untrusted code in-process.
- Around line 27-35: The save_registry_async function currently swallows all
exceptions which makes write failures invisible; change save_registry_async (and
its use of REGISTRY_LOCK/REGISTRY_PATH and loop.run_in_executor) to surface
errors instead: catch Exception as e, log the error with context via the
existing logger (or a new one) including REGISTRY_PATH and exception details,
and re-raise the exception (or return a failure indicator) so callers see the
persistence failure rather than treating it as success.
- Around line 559-577: The staging filename is built directly from request_id
(temp_tool_name/temp_tool_filepath) allowing path traversal; instead sanitize or
replace request_id with a safe identifier before creating
temp_candidate_{...}.py — e.g., validate and strip path separators, reject
unsafe characters, or better: generate a server-side safe name (UUID or hash of
request_id) or use a secure tempfile in the tools directory; then write new_code
to that sanitized/safe path and proceed with audit_code and send_response as
before.
- Around line 344-349: The code is making blocking synchronous LLM/HTTP calls
inside async handle_request(), e.g., ask_text_model(routing_prompt) ->
parse_routing_decision(...) and direct requests.post calls, which will block the
asyncio loop; fix by offloading these blocking calls to a thread executor (use
asyncio.to_thread or loop.run_in_executor) or convert the LLM client wrappers to
async, e.g., call await asyncio.to_thread(ask_text_model, routing_prompt) before
parse_routing_decision and before setting ROUTE_CACHE, and similarly wrap the
direct requests.post calls in the code-generation fallback and calls from
python/utils/generalizer.py; ensure exceptions are caught and passed to
send_response(request_id, "error", ...) unchanged.
In `@python/tests/test_routing_generalization.py`:
- Around line 29-47: The tests for handle_request still allow handle_request to
call stream_synthesis_llm, which can perform live Ollama/Groq I/O or block the
test; update the test(s) in python/tests/test_routing_generalization.py to
patch/mock stream_synthesis_llm (e.g., with
patch("agent_router.stream_synthesis_llm", return_value=None) or an async stub)
in the same context as the existing patches so the synthesis step is
short-circuited, and apply the same patch to the other test block around the
158-171 region to prevent real network/process I/O during successful-routing
assertions.
In `@python/tools/find_crypto_price.py`:
- Around line 38-50: coin_link_selector is built but unused and the selector
f'a[href*=".*/{crypto_name.replace(" ", "-")}"]' incorrectly embeds a regex-like
".*/" (CSS attribute *= does literal substring matching), causing timeouts; fix
by computing a slug (e.g., slug = crypto_name.replace(" ", "-").lower()) and
reuse coin_link_selector (or set coin_link_selector = f'a[href*="{slug}"]' or a
text/role-based Playwright locator) then call await
page.wait_for_selector(coin_link_selector, timeout=15000) and await
page.click(coin_link_selector), keeping the await
page.wait_for_load_state('networkidle') and error handling intact to ensure
robust navigation (references: coin_link_selector, page.wait_for_selector,
page.click, wait_for_load_state).
In `@python/tools/lookup_wikipedia_entity.py`:
- Around line 1-106: The implementation of lookup_wikipedia_entity is
mismatched: the file implements a Google search scraper (scrape_search_results)
while the tool name/registry claim it searches Wikipedia. Fix by choosing one
approach: either (A) rename the tool and registry entries to reflect Google
search (e.g., change file/name to search_general_entity or search_google_entity,
update registry.json name/description, and update results["source"] in
scrape_search_results), or (B) reimplement scrape_search_results to query
Wikipedia (navigate to https://en.wikipedia.org/wiki/{entity_name} or use
Wikipedia search, parse article structure for title, intro/snippet, and links)
and keep the current filename/registry; update main and returned fields
accordingly so function scrape_search_results, main, and results["source"] are
consistent with the chosen behavior.
In `@python/utils/generalizer.py`:
- Around line 199-225: The registry is written (via write_registry_safe using
registry_path) before the temp file is moved to final_filepath and before
original_filepath is removed, so a failed shutil.move leaves registry pointing
at a non-existent python/tools/{gen_name}.py; fix by delaying the registry
update until after the move and original file removal succeed: perform the
run_in_executor shutil.move(str(temp_filepath), str(final_filepath)) and delete
original_filepath (if != final_filepath) first, then
read_registry_safe(registry_path), update registry[gen_name] and delete
registry[original_tool_name] if present, and finally call
write_registry_safe(registry_path, registry) so the registry only reflects the
new file once the filesystem operation completed.
- Around line 181-195: The code uses a shared temp file ("temp_candidate.py")
for every generalize_task run (in generalize_tool where temp_filepath is
created, written, verified via loop.run_in_executor and passed to
run_verification_script), which causes races when main() schedules concurrent
generalize_tool() tasks; change temp_filepath to a unique per-task staging path
(e.g., incorporate a UUID, timestamp, or use tempfile to create a unique file
name in registry_path.parent) and pass that unique path to both the write and
run_verification_script calls, ensure you also use the same unique path when
moving to final_filepath or deleting the staging file so cleanup is safe and
atomic for each task.
- Around line 14-17: The REGISTRY_LOCK in utils.generalizer is local and
conflicts with the separate lock in agent_router.py, causing race conditions
between generalize_tool() saves and request-time registry updates; make the lock
shared by moving/declaring a single asyncio.Lock (e.g., REGISTRY_LOCK or
REGISTRY_LOCK_SHARED) into a common module (or a new module like
utils.registry_lock) and import that single symbol into both utils.generalizer
and agent_router.py, then wrap all registry access points (read_registry_safe,
write/save routines including generalize_tool() and the agent_router registry
update functions) with async with REGISTRY_LOCK to ensure exclusive,
synchronized reads/writes.
In `@python/utils/sufficiency_checker.py`:
- Around line 70-90: The current sufficiency fallback returns sufficient=True on
both JSON parse failures and on exceptions, which bypasses the generation
fallback; update the malformed-response branch (where you build decision after
json.loads fails) to set decision = {"sufficient": False, "reasoning": "Failed
to parse sufficiency auditor response, assuming insufficient"} (or similar) so
sufficient = decision.get("sufficient", False) yields False, and change the
outer exception handler (around ask_text_model) to return False, f"Sufficiency
audit failed: {str(e)}" instead of True so failures trigger the fallback; keep
ask_text_model, decision, sufficient, and reason variable usage and ensure you
still log or surface the original error text in the returned reason.
In `@src-tauri/src/websocket.rs`:
- Around line 98-103: The websocket handler currently invokes execute_power_off,
execute_restart, and execute_sleep directly when receiving matching messages;
protect these by requiring authentication and confirmation, implement
rate-limiting, and add logging: (1) verify a valid auth token or certificate
before executing (e.g., check token in the message or connection metadata in the
function handling messages), (2) require a two-step
confirmation/challenge-response before calling
execute_power_off/execute_restart/execute_sleep (send a challenge and only
perform the action when a matching confirmation message is received), (3)
enforce per-connection rate limiting/throttling to prevent repeated attempts,
and (4) log every attempted power command with the source address/connection id
and outcome; update the message handling code around the
execute_power_off/execute_restart/execute_sleep calls to perform these checks
and logging before executing the commands.
- Around line 66-75: The WebSocket server is binding to all interfaces via the
addr variable in start_websocket_server (TcpListener::bind("0.0.0.0:9001")),
exposing control endpoints without authentication; change the bind address to
localhost (e.g., "127.0.0.1:9001") in the start_websocket_server function where
TcpListener::bind is called so the server only listens on the loopback interface
and is not reachable from external networks.
- Around line 25-27: Replace the hardcoded "python3" and the to_str().unwrap()
usage: call the shared python_executable() helper (or duplicate its platform
logic) to obtain the Python binary name and use that when creating TokioCommand
(i.e., TokioCommand::new(python_executable())), and stop unwrapping
script_path.to_str() — pass the path safely via
cmd.arg_os(script_path.as_os_str()) or cmd.arg(script_path.to_string_lossy()) so
invalid UTF-8 won't panic; update the invocation that currently uses
cmd.args(&["-u", script_path.to_str().unwrap()]) accordingly.
- Around line 53-57: The read_response method currently calls
self.reader.read_line(&mut line).await with no timeout and can block forever;
wrap that await in tokio::time::timeout (e.g., Duration::from_secs(30) or 60) so
the call returns Err on timeout and map the timeout into an appropriate
std::io::Error (or propagate a custom error) before returning; update async fn
read_response(&mut self) to use tokio::time::timeout(Duration::from_secs(...),
self.reader.read_line(&mut line)).await, handle the Result<Option...,
JoinError/Elapsed> and ensure Ok(line) is only returned when read_line succeeded
within the timeout.
---
Outside diff comments:
In `@python/tools/find_crypto_price.py`:
- Around line 1-104: The module find_crypto_price imports from
playwright.async_api (see async_playwright in find_crypto_price and the async
function find_crypto_price), but Playwright is missing from dependencies; add
"playwright" (optionally with a pinned version) to requirements.txt, update any
packaging/CI configs to install it, and ensure post-installation step runs
"playwright install chromium" (or include a setup script/CI step) so the browser
binaries are available when running async_playwright; verify installation in CI
before running the script.
---
Minor comments:
In `@mobile/README.md`:
- Line 8: The README line stating "*Note: This project is configured to run on
**Expo SDK 54**.*" conflicts with AGENTS.md which references the v56.0.0 Expo
docs; update the SDK version string so both documents match (either change the
README note to "Expo SDK 56" to align with AGENTS.md or update AGENTS.md to 54
if you intend to stay on 54), and ensure any other README/AGENTS mentions or
badges reference the same exact versioned Expo docs (e.g., "v56.0.0") for
consistency.
In `@mobile/usePCWebSocket.ts`:
- Around line 32-34: The current port-defaulting logic appends ":9001" based
only on the presence of ':' which allows invalid hosts like ":" or ":8080";
update the block around the formattedIp variable in usePCWebSocket.ts to first
trim formattedIp, then validate the host portion before the colon (e.g., split
on ':' and ensure the left side contains non-empty, non-whitespace characters
and is not just empty), only append ":9001" when that host portion is valid, and
if it is invalid fall back to a safe default (for example "localhost:9001") or
surface an error so you don't produce "ws://:9001". Ensure references to
formattedIp in this file use the validated value.
In `@python/tools/search_product_info.py`:
- Line 115: The loop in python/tools/search_product_info.py uses an unused
variable `idx` in "for idx, block in enumerate(search_blocks[:6]):"; rename
`idx` to `_` (or `_idx`) to follow Python conventions for unused loop variables
and ensure no other code references `idx` within the loop body (update any
occurrences if present).
In `@scratch/test_ws.py`:
- Around line 31-51: The test constructs and sends a real "sleep" WebSocket
payload (payload = b"sleep") and transmits it via s.sendall(header +
masked_payload), which can suspend the host; change payload to a harmless value
(e.g., b"test" or a benign query) by updating the payload variable, add a user
confirmation prompt (or require an environment variable flag) before calling
s.sendall to guard against accidental runs, and add a clear top-of-file warning
comment explaining the original behavior and that sending real power commands is
dangerous.
---
Nitpick comments:
In `@mobile/App.tsx`:
- Line 33: agentResult is currently typed as any; define a specific TypeScript
type (e.g., an interface or union) that matches the server response shapes the
UI expects (the top-level response field and the key/value pair list the UI
iterates over) and use it as the useState generic for agentResult in place of
any. Create types like an AgentResult with a response: string (or nullable) and
either a metadata Record<string,string> or entries: Array<{key:string;
value:string}> (or a union covering both response-only and response+entries
shapes), then update the useState declaration (agentResult, setAgentResult) and
any consumers to use that new type.
In `@mobile/usePCWebSocket.ts`:
- Around line 51-59: The onmessage handler currently swallows JSON parse
failures (inside ws.onmessage where wsRef.current is checked and
setLatestResponse is called) and only console.logs the raw message; add/update a
user-visible error state (e.g., setParseError) or increment a parse-failure
counter state (e.g., incrementParseFailureCount) in the catch block so
UI/components can show malformed-response errors or metrics; ensure you still
check wsRef.current === ws before updating state and include the raw message or
error details when setting the error state for easier debugging.
- Line 8: latestResponse is currently typed as any which weakens type-safety;
define a Response interface (e.g. interface AgentRouterResponse { requestId:
string; status: string; data?: unknown; error?: unknown }) based on the server
schema and replace useState<any>(null) with useState<AgentRouterResponse |
null>(null) (update usages of latestResponse and setLatestResponse accordingly);
add the interface at the top of usePCWebSocket.ts (or export it if used
elsewhere) and prefer unknown for data/error instead of any to preserve
type-safety while allowing arbitrary payloads.
In `@src-tauri/Cargo.toml`:
- Line 22: The dependency currently enables tokio's "full" feature set; restrict
it to only the features used by the code (as indicated by websocket.rs) to
reduce compile time and binary size—replace the existing tokio entry (the line
with tokio = { version = "1.52.3", features = ["full"] }) with a feature list
containing net, process, io-util, sync, macros, and rt-multi-thread while
keeping the same version number.
In `@src-tauri/src/websocket.rs`:
- Around line 157-241: forward_query_to_daemon currently holds the global
daemon_mutex (obtained via get_daemon_mutex) for the entire lifecycle of sending
a query and streaming the response, which serializes all concurrent WebSocket
queries; either document this limitation in the forward_query_to_daemon function
comment (mentioning stdin/stdout non-interleavability and potential head-of-line
blocking) or implement a timed lock acquisition (e.g., attempt to lock
daemon_mutex with a timeout before proceeding, return an error to the caller
when lock acquisition times out) so callers don’t wait indefinitely; reference
the daemon_mutex/get_daemon_mutex usage and the
AgentDaemon::start/send_query/read_response flow when adding the comment or
timeout logic.
- Around line 105-116: Extract the nested logic that computes request_id into a
small helper like fn extract_request_id(message: &str) -> String and replace the
inline block that uses trimmed with a call to it; the helper should (1) check if
message.starts_with("query:"), splitn(3, ':'), return parts[1].to_string() when
parts.len()==3 or "unknown" otherwise, (2) otherwise attempt
serde_json::from_str::<serde_json::Value>(message) and return
parsed.get("requestId").and_then(|r|
r.as_str()).unwrap_or("unknown").to_string(), and (3) default to "unknown".
Ensure the new function is used wherever request_id is computed (referencing the
request_id variable and trimmed input) to improve readability and reuse.
🪄 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: aa39405c-343e-4888-853d-aa529ea3b8aa
⛔ Files ignored due to path filters (8)
mobile/assets/android-icon-background.pngis excluded by!**/*.pngmobile/assets/android-icon-foreground.pngis excluded by!**/*.pngmobile/assets/android-icon-monochrome.pngis excluded by!**/*.pngmobile/assets/favicon.pngis excluded by!**/*.pngmobile/assets/icon.pngis excluded by!**/*.pngmobile/assets/splash-icon.pngis excluded by!**/*.pngmobile/package-lock.jsonis excluded by!**/package-lock.jsonsrc-tauri/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (29)
.gitignoremobile/.claude/settings.jsonmobile/.gitignoremobile/AGENTS.mdmobile/App.tsxmobile/CLAUDE.mdmobile/LICENSEmobile/README.mdmobile/app.jsonmobile/index.tsmobile/package.jsonmobile/tsconfig.jsonmobile/usePCWebSocket.tspython/agent_router.pypython/ai/client.pypython/ai/groq_client.pypython/ai/ollama_client.pypython/tests/test_routing_generalization.pypython/tools/find_crypto_price.pypython/tools/lookup_wikipedia_entity.pypython/tools/lookup_youtube_stats.pypython/tools/registry.jsonpython/tools/search_product_info.pypython/utils/generalizer.pypython/utils/sufficiency_checker.pyscratch/test_ws.pysrc-tauri/Cargo.tomlsrc-tauri/src/lib.rssrc-tauri/src/websocket.rs
| value={ipAddress} | ||
| > | ||
| editable={!isConnected && status !== 'connecting'} | ||
| keyboardType="numeric" |
Contributor
There was a problem hiding this comment.
Incorrect keyboard type for IP address input.
keyboardType="numeric" restricts input to numbers only, but IP addresses require dots (.) and colons (:) for address and port notation (e.g., 192.168.1.15:9001). This makes it difficult or impossible for users to enter valid IP addresses on mobile keyboards.
🔧 Recommended fix
- keyboardType="numeric"
+ keyboardType="default"Or use keyboardType="url" for a keyboard optimized for entering network addresses.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| keyboardType="numeric" | |
| keyboardType="default" |
🤖 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 `@mobile/App.tsx` at line 251, The TextInput in mobile/App.tsx is using
keyboardType="numeric", which prevents entering dots and colons for IP:port
strings; update the TextInput (the component where keyboardType="numeric" is
set) to use keyboardType="url" (or keyboardType="default") so the keyboard
allows dots and colons for addresses like "192.168.1.15:9001".
| "main": "index.ts", | ||
| "dependencies": { | ||
| "@react-native-async-storage/async-storage": "2.2.0", | ||
| "expo": "~54.0.0", |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What React Native version and React version are officially paired with Expo SDK 54, and what are the upgrade differences from Expo SDK 52 and SDK 56?
💡 Result:
Expo SDK 54 is officially paired with React Native 0.81 and React 19.1.0 [1][2][3]. The following table summarizes the React Native and React versions for the requested SDKs: | Expo SDK Version | React Native Version | React Version | |:--- |:--- |:--- | | Expo SDK 56 | 0.85 [3][4][5] | 19.2.3 [3][4][6] | | Expo SDK 54 | 0.81 [1][3] | 19.1.0 [1][3] | | Expo SDK 52 | 0.76 [1][7] | 18.3.1 [1][7] | Upgrade Differences and Context: 1. From SDK 52 to SDK 54: The upgrade from SDK 52 to SDK 54 involved a significant bump in the underlying React and React Native versions, moving from React 18.3.1 to React 19.1.0 [1][3]. Notably, SDK 54 served as the final release to include Legacy Architecture support, with React Native 0.81 enforcing a shift toward the New Architecture [8]. Additionally, SDK 54 introduced precompiled React Native for iOS (XCFrameworks) to improve build workflows [8]. 2. From SDK 54 to SDK 56: The upgrade from SDK 54 to SDK 56 moved the ecosystem to React Native 0.85 and React 19.2.3 [3][4][6]. Key changes in this transition included Hermes v1 becoming the default JavaScript engine, a new animation backend designed for the New Architecture, and a minimum Node.js version requirement bump to v20.19.4 [5][6]. SDK 56 also included significant updates to Expo CLI performance, navigation APIs, and broader support for AI-friendly project scaffolding [9].
Citations:
- 1: https://docs.expo.dev/versions/v54.0.0/
- 2: https://expo.dev/sdk/54
- 3: https://docs.expo.dev/versions/latest/?redirected=
- 4: https://docs.expo.dev/versions/v56.0.0
- 5: https://expo.dev/changelog/sdk-56
- 6: https://expo.dev/changelog/sdk-56-beta
- 7: https://docs.expo.io/versions/v53.0.0
- 8: https://expo.dev/changelog/sdk-54
- 9: https://expo.dev/sdk/56
Align Expo SDK version targets across code + PR/docs.
mobile/package.json sets "expo": "~54.0.0", but the referenced PR objective says Expo SDK 52 and the repo guidance points to Expo v56 docs; these SDKs target different React Native/React baselines (SDK 54 → RN 0.81 / React 19.1.0, SDK 52 → RN 0.76 / React 18.3.1, SDK 56 → RN 0.85 / React 19.2.3). Update package.json and the corresponding PR/docs instructions to one consistent Expo SDK version to avoid dependency/tooling drift.
🤖 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 `@mobile/package.json` at line 7, The Expo SDK version in mobile/package.json
is inconsistent with the PR/docs (package.json has "expo": "~54.0.0" while
PR/docs reference SDK 52 and repo guidance points to SDK 56); pick a single
target SDK (e.g., Expo SDK 56 or whichever the project intends), update the
"expo" entry in mobile/package.json to that SDK string, and update the PR
description and any docs/instructions to reference the same SDK so tooling,
React and RN baselines match; search for the "expo" field in mobile/package.json
and for any mentions of "Expo SDK", "SDK 52/54/56", or the Expo version in the
PR text/docs and make them consistent.
Source: Learnings
Comment on lines
+27
to
+35
| async def save_registry_async(registry): | ||
| async with REGISTRY_LOCK: | ||
| try: | ||
| REGISTRY_PATH.parent.mkdir(parents=True, exist_ok=True) | ||
| loop = asyncio.get_running_loop() | ||
| content = json.dumps(registry, indent=2) | ||
| await loop.run_in_executor(None, REGISTRY_PATH.write_text, content) | ||
| except Exception: | ||
| pass |
Contributor
There was a problem hiding this comment.
Do not treat registry persistence failures as success.
save_registry_async() swallows every write error. When the generated-tool path hits this helper, the request can still return success even though registry.json was never updated, so the tool disappears on the next load/restart.
💡 Suggested fix
async def save_registry_async(registry):
async with REGISTRY_LOCK:
- try:
- REGISTRY_PATH.parent.mkdir(parents=True, exist_ok=True)
- loop = asyncio.get_running_loop()
- content = json.dumps(registry, indent=2)
- await loop.run_in_executor(None, REGISTRY_PATH.write_text, content)
- except Exception:
- pass
+ REGISTRY_PATH.parent.mkdir(parents=True, exist_ok=True)
+ loop = asyncio.get_running_loop()
+ content = json.dumps(registry, indent=2)
+ await loop.run_in_executor(None, REGISTRY_PATH.write_text, content)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| async def save_registry_async(registry): | |
| async with REGISTRY_LOCK: | |
| try: | |
| REGISTRY_PATH.parent.mkdir(parents=True, exist_ok=True) | |
| loop = asyncio.get_running_loop() | |
| content = json.dumps(registry, indent=2) | |
| await loop.run_in_executor(None, REGISTRY_PATH.write_text, content) | |
| except Exception: | |
| pass | |
| async def save_registry_async(registry): | |
| async with REGISTRY_LOCK: | |
| REGISTRY_PATH.parent.mkdir(parents=True, exist_ok=True) | |
| loop = asyncio.get_running_loop() | |
| content = json.dumps(registry, indent=2) | |
| await loop.run_in_executor(None, REGISTRY_PATH.write_text, content) |
🧰 Tools
🪛 Ruff (0.15.15)
[error] 34-35: try-except-pass detected, consider logging the exception
(S110)
[warning] 34-34: Do not catch blind exception: Exception
(BLE001)
🤖 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 `@python/agent_router.py` around lines 27 - 35, The save_registry_async
function currently swallows all exceptions which makes write failures invisible;
change save_registry_async (and its use of REGISTRY_LOCK/REGISTRY_PATH and
loop.run_in_executor) to surface errors instead: catch Exception as e, log the
error with context via the existing logger (or a new one) including
REGISTRY_PATH and exception details, and re-raise the exception (or return a
failure indicator) so callers see the persistence failure rather than treating
it as success.
Comment on lines
+344
to
+349
| try: | ||
| raw_decision = ask_text_model(routing_prompt) | ||
| route_decision = parse_routing_decision(raw_decision) | ||
| ROUTE_CACHE[query_sig] = route_decision | ||
| except Exception as e: | ||
| send_response(request_id, "error", error={"code": "LLM_ROUTING_ERROR", "message": "AI routing failed", "details": str(e)}) |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Checking synchronous model/network calls inside async flows..."
rg -n -C2 'ask_text_model\(|requests\.post\(' python/agent_router.py python/utils/generalizer.pyRepository: KingSahil/Blinky
Length of output: 2308
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== agent_router.py: check async context around lines 330-380 ==="
nl -ba python/agent_router.py | sed -n '300,390p'
echo "=== agent_router.py: check async context around lines 430-540 ==="
nl -ba python/agent_router.py | sed -n '430,540p'
echo "=== generalizer.py: inspect ask_text_model usage ==="
nl -ba python/utils/generalizer.py | sed -n '120,220p'
echo "=== agent_router.py: locate ask_text_model definition and inspect implementation ==="
rg -n "def ask_text_model|ask_text_model\(" python/agent_router.py python/utils/generalizer.pyRepository: KingSahil/Blinky
Length of output: 187
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== agent_router.py: locate handle_request and async context ==="
rg -n "async def handle_request|def handle_request|handle_request\(" -S python/agent_router.py
echo "=== agent_router.py: print lines 320-380 (cat -n) ==="
cat -n python/agent_router.py | sed -n '320,380p'
echo "=== agent_router.py: print lines 430-540 (cat -n) ==="
cat -n python/agent_router.py | sed -n '430,540p'
echo "=== agent_router.py: locate ask_text_model definition ==="
rg -n "def ask_text_model|ask_text_model\s*\(" python/agent_router.py
echo "=== generalizer.py: print lines around ask_text_model usage/definition ==="
cat -n python/utils/generalizer.py | sed -n '130,210p'
echo "=== generalizer.py: locate ask_text_model definition ==="
rg -n "def ask_text_model" python/utils/generalizer.pyRepository: KingSahil/Blinky
Length of output: 13196
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== locate ask_text_model definitions across repo ==="
rg -n --hidden --no-ignore-vcs "def ask_text_model\s*\(" .
echo "=== locate where ask_text_model is imported/assigned ==="
rg -n --hidden --no-ignore-vcs "ask_text_model\s*=" python/agent_router.py python/utils/generalizer.py
rg -n --hidden --no-ignore-vcs "from .* import .*ask_text_model|import .*ask_text_model" python/agent_router.py python/utils/generalizer.py
echo "=== inspect agent_router.py around handle_request start to see if sync calls are direct ==="
cat -n python/agent_router.py | sed -n '240,340p'
echo "=== inspect agent_router.py around the async task scheduler ==="
cat -n python/agent_router.py | sed -n '660,740p'
echo "=== locate requests.post usage and whether it's inside async def in agent_router.py ==="
rg -n "requests\.post\(" python/agent_router.pyRepository: KingSahil/Blinky
Length of output: 270
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== agent_router.py: print async handle_request body start (260-430) ==="
cat -n python/agent_router.py | sed -n '250,430p'
echo "=== agent_router.py: print scheduling/loop area (650-730) ==="
cat -n python/agent_router.py | sed -n '650,730p'
echo "=== agent_router.py: print earlier requests.post locations (90-220) ==="
cat -n python/agent_router.py | sed -n '90,220p'
echo "=== agent_router.py: print generation requests.post locations (450-520) ==="
cat -n python/agent_router.py | sed -n '450,520p'
echo "=== ai/client.py: inspect ask_text_model implementation ==="
cat -n python/ai/client.py | sed -n '1,220p'
echo "=== ai/client.py: check for any async/non-blocking HTTP clients used ==="
rg -n "async def|await |aiohttp|httpx|run_in_executor|requests\." python/ai/client.pyRepository: KingSahil/Blinky
Length of output: 22366
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== locate groq_client ask_groq_text implementation ==="
rg -n "def ask_groq_text|ask_groq_text" -S python/ai
ls -la python/ai || true
echo "=== print groq_client.py relevant section ==="
cat -n python/ai/groq_client.py | sed -n '1,240p'
echo "=== locate ollama_client ask_ollama_text implementation ==="
rg -n "def ask_ollama_text|ask_ollama_text" -S python/ai
echo "=== print ollama_client.py relevant section ==="
cat -n python/ai/ollama_client.py | sed -n '1,260p'
echo "=== search for requests usage in python/ai ==="
rg -n "requests\." python/ai
echo "=== search for aiohttp/httpx usage in python/ai ==="
rg -n "aiohttp|httpx" python/aiRepository: KingSahil/Blinky
Length of output: 15231
Fix blocking synchronous LLM/network calls inside async handle_request()
python/agent_router.py calls synchronous HTTP/model code directly from async def handle_request(), blocking the asyncio event loop and stalling other requests.
- Routing (lines 344-347):
raw_decision = ask_text_model(routing_prompt);ask_text_modelultimately uses Groq/Ollama clients backed byrequests.post. - Code generation fallback (lines 470-503): direct
requests.postcalls to Groq/Ollama also run insidehandle_request()(noawait/executor offload). - Also affected:
python/utils/generalizer.pycallsask_text_model(...)synchronously (same pattern).
try:
raw_decision = ask_text_model(routing_prompt)
route_decision = parse_routing_decision(raw_decision)
ROUTE_CACHE[query_sig] = route_decision
except Exception as e:
send_response(request_id, "error", error={"code": "LLM_ROUTING_ERROR", "message": "AI routing failed", "details": str(e)})🧰 Tools
🪛 Ruff (0.15.15)
[warning] 348-348: Do not catch blind exception: Exception
(BLE001)
🤖 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 `@python/agent_router.py` around lines 344 - 349, The code is making blocking
synchronous LLM/HTTP calls inside async handle_request(), e.g.,
ask_text_model(routing_prompt) -> parse_routing_decision(...) and direct
requests.post calls, which will block the asyncio loop; fix by offloading these
blocking calls to a thread executor (use asyncio.to_thread or
loop.run_in_executor) or convert the LLM client wrappers to async, e.g., call
await asyncio.to_thread(ask_text_model, routing_prompt) before
parse_routing_decision and before setting ROUTE_CACHE, and similarly wrap the
direct requests.post calls in the code-generation fallback and calls from
python/utils/generalizer.py; ensure exceptions are caught and passed to
send_response(request_id, "error", ...) unchanged.
Comment on lines
+559
to
+577
| # Unique staging filename: temp_candidate_{request_id}.py | ||
| temp_tool_name = f"temp_candidate_{request_id}" | ||
| temp_tool_filepath = Path(__file__).parent / "tools" / f"{temp_tool_name}.py" | ||
|
|
||
| # Diagnostic logging of generated code | ||
| from utils.logging import get_logger | ||
| logger = get_logger("blinky.router") | ||
| logger.info(f"Generated custom code for request {request_id}:\n{new_code}") | ||
|
|
||
| # Static Code Audit | ||
| is_safe, audit_msg = audit_code(new_code) | ||
| if not is_safe: | ||
| logger.warning(f"Safety audit rejected request {request_id}. Reason: {audit_msg}") | ||
| send_response(request_id, "error", error={"code": "SAFETY_AUDIT_REJECTED", "message": "Static safety audit rejected the generated script", "details": audit_msg}) | ||
| return | ||
|
|
||
| temp_tool_filepath.parent.mkdir(parents=True, exist_ok=True) | ||
| with open(temp_tool_filepath, "w") as f: | ||
| f.write(new_code) |
Contributor
There was a problem hiding this comment.
Do not derive the staging filename from requestId.
requestId comes from the inbound JSON, but it is interpolated directly into temp_candidate_{request_id}.py. A value containing / or .. can escape python/tools and overwrite arbitrary .py files before verification runs.
💡 Suggested fix
+import uuid
...
- temp_tool_name = f"temp_candidate_{request_id}"
- temp_tool_filepath = Path(__file__).parent / "tools" / f"{temp_tool_name}.py"
+ temp_tool_filepath = Path(__file__).parent / "tools" / f"temp_candidate_{uuid.uuid4().hex}.py"🤖 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 `@python/agent_router.py` around lines 559 - 577, The staging filename is built
directly from request_id (temp_tool_name/temp_tool_filepath) allowing path
traversal; instead sanitize or replace request_id with a safe identifier before
creating temp_candidate_{...}.py — e.g., validate and strip path separators,
reject unsafe characters, or better: generate a server-side safe name (UUID or
hash of request_id) or use a secure tempfile in the tools directory; then write
new_code to that sanitized/safe path and proceed with audit_code and
send_response as before.
Comment on lines
+70
to
+90
| try: | ||
| raw_decision = ask_text_model(prompt, max_tokens=200) | ||
| if isinstance(raw_decision, dict): | ||
| decision = raw_decision | ||
| else: | ||
| try: | ||
| decision = json.loads(raw_decision) | ||
| except Exception: | ||
| match_obj = re.search(r"\{.*\}", str(raw_decision), re.DOTALL) | ||
| if match_obj: | ||
| decision = json.loads(match_obj.group(0)) | ||
| else: | ||
| decision = {"sufficient": True, "reasoning": "Failed to parse sufficiency auditor response, assuming sufficient"} | ||
|
|
||
| sufficient = decision.get("sufficient", True) | ||
| reason = decision.get("reasoning", "LLM Audited") | ||
| return sufficient, reason | ||
| except Exception as e: | ||
| # If audit fails, default to True to avoid infinite loops, or False to be safe. | ||
| # Defaulting to True to be resilient, but logging the error. | ||
| return True, f"Sufficiency audit failed: {str(e)}" |
Contributor
There was a problem hiding this comment.
Audit failures should trigger fallback, not bypass it.
Both the malformed-response fallback and the outer exception path return sufficient=True. That makes the router skip custom generation exactly when the sufficiency check failed to validate the tool output.
💡 Suggested fix
- decision = {"sufficient": True, "reasoning": "Failed to parse sufficiency auditor response, assuming sufficient"}
+ decision = {"sufficient": False, "reasoning": "Failed to parse sufficiency auditor response"}
...
- return True, f"Sufficiency audit failed: {str(e)}"
+ return False, f"Sufficiency audit failed: {str(e)}"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| try: | |
| raw_decision = ask_text_model(prompt, max_tokens=200) | |
| if isinstance(raw_decision, dict): | |
| decision = raw_decision | |
| else: | |
| try: | |
| decision = json.loads(raw_decision) | |
| except Exception: | |
| match_obj = re.search(r"\{.*\}", str(raw_decision), re.DOTALL) | |
| if match_obj: | |
| decision = json.loads(match_obj.group(0)) | |
| else: | |
| decision = {"sufficient": True, "reasoning": "Failed to parse sufficiency auditor response, assuming sufficient"} | |
| sufficient = decision.get("sufficient", True) | |
| reason = decision.get("reasoning", "LLM Audited") | |
| return sufficient, reason | |
| except Exception as e: | |
| # If audit fails, default to True to avoid infinite loops, or False to be safe. | |
| # Defaulting to True to be resilient, but logging the error. | |
| return True, f"Sufficiency audit failed: {str(e)}" | |
| try: | |
| raw_decision = ask_text_model(prompt, max_tokens=200) | |
| if isinstance(raw_decision, dict): | |
| decision = raw_decision | |
| else: | |
| try: | |
| decision = json.loads(raw_decision) | |
| except Exception: | |
| match_obj = re.search(r"\{.*\}", str(raw_decision), re.DOTALL) | |
| if match_obj: | |
| decision = json.loads(match_obj.group(0)) | |
| else: | |
| decision = {"sufficient": False, "reasoning": "Failed to parse sufficiency auditor response"} | |
| sufficient = decision.get("sufficient", True) | |
| reason = decision.get("reasoning", "LLM Audited") | |
| return sufficient, reason | |
| except Exception as e: | |
| # If audit fails, default to True to avoid infinite loops, or False to be safe. | |
| # Defaulting to True to be resilient, but logging t 8 he error 10BC8 . | |
| return False, f"Sufficiency audit failed: {str(e)}" |
🧰 Tools
🪛 Ruff (0.15.15)
[warning] 77-77: Do not catch blind exception: Exception
(BLE001)
[warning] 87-87: Do not catch blind exception: Exception
(BLE001)
[warning] 90-90: Use explicit conversion flag
Replace with conversion flag
(RUF010)
🤖 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 `@python/utils/sufficiency_checker.py` around lines 70 - 90, The current
sufficiency fallback returns sufficient=True on both JSON parse failures and on
exceptions, which bypasses the generation fallback; update the
malformed-response branch (where you build decision after json.loads fails) to
set decision = {"sufficient": False, "reasoning": "Failed to parse sufficiency
auditor response, assuming insufficient"} (or similar) so sufficient =
decision.get("sufficient", False) yields False, and change the outer exception
handler (around ask_text_model) to return False, f"Sufficiency audit failed:
{str(e)}" instead of True so failures trigger the fallback; keep ask_text_model,
decision, sufficient, and reason variable usage and ensure you still log or
surface the original error text in the returned reason.
Comment on lines
+53
to
+57
| async fn read_response(&mut self) -> Result<String, std::io::Error> { | ||
| let mut line = String::new(); | ||
| self.reader.read_line(&mut line).await?; | ||
| Ok(line) | ||
| } |
Contributor
There was a problem hiding this comment.
Add timeout to prevent indefinite blocking on daemon reads.
read_line() has no timeout, so a hung or slow daemon will block the WebSocket handler indefinitely. Wrap the read operation with tokio::time::timeout to fail gracefully after a reasonable duration (e.g., 30-60 seconds).
⏱️ Proposed fix with timeout
async fn read_response(&mut self) -> Result<String, std::io::Error> {
let mut line = String::new();
- self.reader.read_line(&mut line).await?;
+ tokio::time::timeout(
+ std::time::Duration::from_secs(60),
+ self.reader.read_line(&mut line)
+ )
+ .await
+ .map_err(|_| std::io::Error::new(std::io::ErrorKind::TimedOut, "Daemon read timeout"))?
+ ?;
Ok(line)
}🤖 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/websocket.rs` around lines 53 - 57, The read_response method
currently calls self.reader.read_line(&mut line).await with no timeout and can
block forever; wrap that await in tokio::time::timeout (e.g.,
Duration::from_secs(30) or 60) so the call returns Err on timeout and map the
timeout into an appropriate std::io::Error (or propagate a custom error) before
returning; update async fn read_response(&mut self) to use
tokio::time::timeout(Duration::from_secs(...), self.reader.read_line(&mut
line)).await, handle the Result<Option..., JoinError/Elapsed> and ensure
Ok(line) is only returned when read_line succeeded within the timeout.
Comment on lines
+66
to
+75
| pub async fn start_websocket_server() { | ||
| let addr = "0.0.0.0:9001"; | ||
| let listener = match TcpListener::bind(addr).await { | ||
| Ok(l) => l, | ||
| Err(e) => { | ||
| eprintln!("Failed to bind WebSocket server to {}: {}", addr, e); | ||
| return; | ||
| } | ||
| }; | ||
| println!("WebSocket server listening on: {}", addr); |
Contributor
There was a problem hiding this comment.
Critical security issue: WebSocket server exposed to network without authentication.
Binding to 0.0.0.0:9001 exposes the server to all network interfaces, allowing any device on the local network (or internet if port-forwarded) to connect. Combined with the lack of authentication, this means anyone who can reach the port can execute power commands (shutdown, restart, sleep) or send arbitrary queries to the AI agent.
Security implications:
- Unauthorized system shutdown/restart/sleep
- Potential information disclosure through AI queries
- No rate limiting or access control
Bind to 127.0.0.1:9001 to restrict access to localhost only, since the mobile client connects through the same machine or via explicit tunneling.
🔒 Proposed fix to bind localhost only
- let addr = "0.0.0.0:9001";
+ let addr = "127.0.0.1:9001";
let listener = match TcpListener::bind(addr).await {🤖 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/websocket.rs` around lines 66 - 75, The WebSocket server is
binding to all interfaces via the addr variable in start_websocket_server
(TcpListener::bind("0.0.0.0:9001")), exposing control endpoints without
authentication; change the bind address to localhost (e.g., "127.0.0.1:9001") in
the start_websocket_server function where TcpListener::bind is called so the
server only listens on the loopback interface and is not reachable from external
networks.
Comment on lines
+98
to
+103
| if trimmed == "power_off" { | ||
| execute_power_off(); | ||
| } else if trimmed == "restart" { | ||
| execute_restart(); | ||
| } else if trimmed == "sleep" { | ||
| execute_sleep(); |
Contributor
There was a problem hiding this comment.
Power commands execute without authentication or confirmation.
Lines 98-103 immediately execute system power commands (power_off, restart, sleep) without any authentication, confirmation, or rate limiting. Combined with the network exposure issue (line 67), this allows any network client to shut down the host machine.
Recommended mitigations:
- Add authentication (token, API key, or certificate-based)
- Implement command confirmation/challenge-response
- Add rate limiting per connection
- Log all power command attempts with source IP
🤖 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/websocket.rs` around lines 98 - 103, The websocket handler
currently invokes execute_power_off, execute_restart, and execute_sleep directly
when receiving matching messages; protect these by requiring authentication and
confirmation, implement rate-limiting, and add logging: (1) verify a valid auth
token or certificate before executing (e.g., check token in the message or
connection metadata in the function handling messages), (2) require a two-step
confirmation/challenge-response before calling
execute_power_off/execute_restart/execute_sleep (send a challenge and only
perform the action when a matching confirmation message is received), (3)
enforce per-connection rate limiting/throttling to prevent repeated attempts,
and (4) log every attempted power command with the source address/connection id
and outcome; update the message handling code around the
execute_power_off/execute_restart/execute_sleep calls to perform these checks
and logging before executing the commands.
- Add fast-path routing for open/search commands from mobile - Open sites, Google searches, and YouTube searches in the PC browser - Avoid headless Playwright generation for simple browser actions - Fix Python sidecar startup to use the project venv - Support installed Tauri resources under _up_/python - Handle mobile WebSocket disconnects gracefully - Remove overlapping subtitle text from mobile cards - Add focused tests for open/search routing
KhannaSparsh0001
pushed a commit
to KhannaSparsh0001/Blinky
that referenced
this pull request
Sep 15, 2026
…c ABC, AT-SPI) GNOME/KDE implementation of the ComputerUseBackend contract — same ABC, auto- detected by DE. Correct-by-construction against standard mechanisms (untestable on Hyprland; verified by graceful degradation + full regression). NEW: - backend/atspi.py — AT-SPI a11y tree bridge: get_elements (UIElement boxes), get_active_window/list_windows (aggregate app bounds), is_available gate. Shared element/window source for GNOME + KDE; degrades to []/None when the registry is empty (Hyprland). Requires python-atspi pkg. - backend/gnome.py — GnomeBackend: portal capture, AT-SPI windows/elements, shared ydotool/wtype input, Shell.Eval focus. Gates on GNOME DBus ping. - backend/kde.py — KdeBackend: grim→portal capture, AT-SPI windows/elements, KWin Scripting.loadScript focus. Gates on KWin DBus ping. CHANGED: - backend/__init__.py — get_backend() auto-detects DE (hyprland/gnome/kde via system profile + env fallback); get_backend_name() exposes it. - backend/input.py — the only compositor-dependent pieces (cursor pos, move_to) dispatch per availability: xdotool (X11/XWayland) > hyprctl > ydotool REL w/ accel compensation. focus_window tries hyprctl → xdotool → portal activate. - backend/portal.py — added activate_window() per-DE DBus focus path. - linux_mcp_compat.get_app_state — pulls AT-SPI element tree when available. Verified on Hyprland: auto-detect → HyprlandBackend; AT-SPI degrades cleanly (0 elements, no crash); input dispatch compiled + move_to pixel-exact; python 235 passed / 10 pre-existing (no regressions); cargo check clean.
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
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.
Added expo app, fixed some bugs
Expo app 52 SDK version required
Expo app:
Summary by CodeRabbit
New Features
Improvements
Documentation