Reliability P1s: audio deadlock, bounded capture queue, download/script/provider hardening - #71
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 73a4781ca8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if cancelled.load(Ordering::Relaxed) { | ||
| return Err(anyhow!("download cancelled while reading")); |
There was a problem hiding this comment.
When the user clicks Cancel during a model download, cancel_download only flips this flag; the active loops then call next_chunk(...).await? (for example in ModelManager) and this Err propagates to commands::models::download_model, which emits model-download-failed and rejects the promise. Previous code returned Ok(()) on cancellation and preserved the partial file for resume, so normal cancels now surface as failed downloads; the Android and local-LLM download paths use the same helper and hit the same behavior.
Useful? React with 👍 / 👎.
| label | ||
| )); | ||
| } | ||
| validate_post_process_base_url(&base_url, allow_insecure_lan)?; |
There was a problem hiding this comment.
This validation rejects http://192.168... custom provider URLs unless allow_insecure_lan_post_process is already true, but this commit does not add the new setting/command to src/bindings.ts or any React settings UI (rg finds no frontend references). The existing base URL field calls changePostProcessBaseUrlSetting, so users of LAN LLM servers have no UI path to enable the opt-in before saving the URL and are effectively blocked unless they hand-edit persisted settings.
Useful? React with 👍 / 👎.
check:public-hygiene flags RFC 1918 private IPs (192.168.x) left in source. The P1-06 provider-TLS tests only need a NON-LOOPBACK host; swap 192.168.1.20 for 203.0.113.20 (TEST-NET-3, reserved for documentation) to keep the test semantics while passing the published-surface hygiene gate. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… setting P1-06 added the change_post_process_insecure_lan_setting command and the allow_insecure_lan_post_process AppSettings field; regenerate the specta TypeScript bindings so check:bindings drift passes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 participant
Before Submitting This PR
Please confirm you have done the following:
Human Written Description
After the main reliability pass I did a full A-to-Z audit of the codebase and it turned up a handful of "this will bite someone eventually" issues — a lock-ordering bug in the mic mute path that could deadlock recording, downloads that ignore Cancel because they never set a timeout, an external paste script that runs on the UI thread and can freeze the app, and provider URLs that would send your transcript and API key in plaintext over the network. None are everyday crashes, but each is the kind of latent fault that turns into a bad bug report. This finishes the hardening by closing them.
Related Issues/Discussions
Bug-fix work (top priority per CONTRIBUTING). Addresses the P1 findings from an internal full-codebase assessment (the two P0s from that same assessment already shipped in #69).
Fixes #
Discussion:
Testing
TDD throughout; full Rust library suite passes 700/700. Notable coverage:
http://is rejected unless an explicit insecure-LAN opt-in is set, and no API key is attached over insecure transport.Screenshots/Videos (if applicable)
N/A (backend/reliability changes).
AI Assistance
If AI was used:
What's in this PR
Final phase of the reliability effort (builds on #69 and #70).
🤖 Generated with Claude Code