justfile (1)

197-197: ⚠️ Potential issue | 🟠 Major

Create the sidecar destination directory before copying on Windows.

Copy-Item can fail on clean checkouts when tauri/src-tauri/binaries does not exist yet.

🛠️ Suggested patch

     & "{{ python }}" backend/build_binary.py; \
     if ($LASTEXITCODE -ne 0) { throw "build_binary.py failed with exit code $LASTEXITCODE" }; \
     $triple = (rustc --print host-tuple); \
+    New-Item -ItemType Directory -Path "{{ tauri_dir }}/src-tauri/binaries" -Force | Out-Null; \
     Copy-Item "backend/dist/voicebox-server.exe" "{{ tauri_dir }}/src-tauri/binaries/voicebox-server-$triple.exe" -Force; \
     Write-Host "Copied sidecar: voicebox-server-$triple.exe"

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@justfile` at line 197, The Copy-Item command in the justfile can fail if the
sidecar destination directory (tauri/src-tauri/binaries) doesn't exist; before
the Copy-Item that references "{{ tauri_dir
}}/src-tauri/binaries/voicebox-server-$triple.exe" ensure the destination
directory is created (e.g., check Test-Path or use New-Item -ItemType Directory
-Force) so the directory exists prior to copying the
backend/dist/voicebox-server.exe file.

tauri/src-tauri/src/main.rs (1)

528-537: ⚠️ Potential issue | 🟠 Major

Handle /shutdown failure on Windows and fall back to force-terminate.

stop_server currently ignores both request errors and non-success responses; restart can race with a still-running old server process.

🛠️ Suggested patch

-            let client = reqwest::blocking::Client::builder()
+            let client = reqwest::blocking::Client::builder()
                 .timeout(std::time::Duration::from_secs(2))
                 .build()
-                .unwrap();
+                .map_err(|e| format!("Failed to create shutdown client: {}", e))?;
 
-            let _ = client
+            let shutdown_ok = client
                 .post(&format!("http://127.0.0.1:{}/shutdown", SERVER_PORT))
-                .send();
+                .send()
+                .map(|r| r.status().is_success())
+                .unwrap_or(false);
+
+            if !shutdown_ok {
+                println!("Graceful shutdown failed, forcing process termination...");
+                let _ = std::process::Command::new("taskkill")
+                    .args(["/PID", &pid.to_string(), "/T", "/F"])
+                    .output();
+            }
 
             println!("Shutdown request sent (server watchdog will handle cleanup)");

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tauri/src-tauri/src/main.rs` around lines 528 - 537, In stop_server, don't
ignore the POST result to /shutdown; capture the result of
client.post(...).send() and check for Err or a non-success HTTP status, and if
either occurs perform a Windows-specific fallback to force-terminate the old
server process (e.g., use a Windows API/Command or kill-by-pid) to avoid races;
reference the existing client builder, the POST to
format!("http://127.0.0.1:{}/shutdown", SERVER_PORT) and the stop_server
function when adding this error handling and the conditional Windows-only
fallback.

backend/server.py (1)

151-156: ⚠️ Potential issue | 🟠 Major

Validate --parent-pid as strictly positive before enabling watchdog.

Current gating accepts 0/negative values, which can cause invalid parent-liveness behavior.

🛠️ Suggested patch

         args = parser.parse_args()
+        if args.parent_pid is not None and args.parent_pid <= 0:
+            parser.error("--parent-pid must be a positive integer")
@@
-        if args.parent_pid is not None:
+        if args.parent_pid is not None:
             _parent_pid = args.parent_pid
             _data_dir = args.data_dir
             `@app.on_event`("startup")
             async def _on_startup():
                 _start_parent_watchdog(_parent_pid, _data_dir)

Also applies to: 176-177

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@backend/server.py` around lines 151 - 156, The --parent-pid argument is
currently accepted even when zero or negative, causing invalid parent-liveness
behavior; update validation so the parsed parent_pid is treated as None (or
rejected) unless strictly > 0: after parsing (where parser.add_argument defines
"--parent-pid") add a check for parent_pid = args.parent_pid and if parent_pid
is None or parent_pid <= 0 set parent_pid = None (or raise ArgumentTypeError)
and ensure the watchdog enabling logic (the code that checks parent_pid later,
e.g., the block referenced at lines ~176-177 that starts the
parent-monitor/watchdog) only runs when parent_pid is a positive int; refer to
the argument name "--parent-pid" and the variable parent_pid/watchdog enabling
code to locate and apply this validation.

backend/build_binary.py (1)

186-219: _get_cuda_dll_excludes() is currently dead code.

The helper is defined but never consumed in this script, so it adds maintenance overhead and can mislead readers about active exclusion behavior.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@backend/build_binary.py` around lines 186 - 219, The _get_cuda_dll_excludes()
helper is dead code—remove the entire function definition (named
_get_cuda_dll_excludes) from backend/build_binary.py and also delete any unused
local symbols that only supported it (e.g., exclude_dlls, cuda_prefixes) and any
imports that become unused as a result; if CUDA DLL exclusion is actually
intended, instead call _get_cuda_dll_excludes() from the build path that
configures PyInstaller excludes (replace the unused helper with an explicit
invocation in the packaging flow), but otherwise delete the function to avoid
confusion and update imports/linters accordingly.

backend/server.py (1)

75-76: Avoid silent exception swallowing in watchdog logger setup.

Catching and discarding all exceptions here makes production watchdog failures hard to diagnose.

🛠️ Proposed adjustment

-        except Exception:
-            pass
+        except Exception as e:
+            logging.getLogger(__name__).warning(
+                "Failed to initialize watchdog file logger: %s", e
+            )

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@backend/server.py` around lines 75 - 76, The except Exception: pass in the
watchdog logger setup silently swallows errors; replace it so exceptions are
logged and not discarded—catch the exception and call the module logger (or
processLogger) with logger.exception(...) or logger.error(..., exc_info=True) to
record the stacktrace and context (e.g., in the watchdog setup block where
except Exception: appears), and optionally re-raise or surface the error if
logger setup failure should stop startup.

backend/server.py (1)

69-76: Don't swallow watchdog log setup failures.

This drops permission/path errors even though the file logger is your only production breadcrumb for watchdog behavior. At least warn to the root logger and narrow the catch to OSError.

♻️ Proposed change

     if data_dir:
         try:
             log_dir = os.path.join(data_dir, "logs")
             os.makedirs(log_dir, exist_ok=True)
             fh = logging.FileHandler(os.path.join(log_dir, "watchdog.log"))
             fh.setFormatter(logging.Formatter('%(asctime)s - %(message)s'))
             watchdog_logger.addHandler(fh)
-        except Exception:
-            pass
+        except OSError as exc:
+            logger.warning("Failed to initialize watchdog log file in %s: %s", log_dir, exc)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@backend/server.py` around lines 69 - 76, The try/except around creating
log_dir and adding the FileHandler for watchdog_logger currently swallows all
exceptions; change it to catch only OSError (or PermissionError) and log a
warning to the root logger with the exception details instead of silently
passing. Specifically, update the block that creates log_dir, instantiates
logging.FileHandler and calls watchdog_logger.addHandler(...) to catch OSError
as e and call logging.getLogger().warning(...) (or .exception(...) if preferred)
with a clear message and the exception info so failures to set up the file
handler are visible.

backend/server.py (1)

116-118: ⚠️ Potential issue | 🟡 Minor

Exiting instead of returning would prevent orphan servers.

If the parent is already dead when the watchdog starts, returning here leaves the server running as an orphan. The server should exit in this case, consistent with the behavior when the parent dies later.

🛠️ Proposed fix

         if not alive:
             watchdog_logger.warning(f"Parent PID {parent_pid} not found on first check — disabling watchdog")
-            return
+            watchdog_logger.info("Exiting to avoid orphan server")
+            if sys.platform == "win32":
+                os._exit(1)
+            else:
+                os.kill(os.getpid(), signal.SIGTERM)
+            return  # unreachable, but makes intent clear

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@backend/server.py` around lines 116 - 118, In the watchdog startup check, do
not simply return when the parent is already dead; replace the early "return"
with a process exit so the server does not become an orphan—use a clean exit
(e.g., sys.exit(1) or os._exit(1)) and keep the existing watchdog_logger.warning
that references parent_pid and alive to record the reason; update the code in
the function containing this block (the watchdog/startup check using variables
alive, parent_pid and watchdog_logger) to call the exit API instead of
returning.

backend/server.py (2)

68-76: Consider logging the exception instead of silently passing.

The bare except Exception: pass swallows all errors during log directory setup. While log setup failure shouldn't crash the watchdog, logging the exception would help debug production issues.

🔧 Proposed fix

         try:
             log_dir = os.path.join(data_dir, "logs")
             os.makedirs(log_dir, exist_ok=True)
             fh = logging.FileHandler(os.path.join(log_dir, "watchdog.log"))
             fh.setFormatter(logging.Formatter('%(asctime)s - %(message)s'))
             watchdog_logger.addHandler(fh)
-        except Exception:
-            pass
+        except Exception as e:
+            # Log to stderr since file handler failed
+            logging.getLogger(__name__).debug(f"Could not set up watchdog file logging: {e}")

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@backend/server.py` around lines 68 - 76, The current try/except around log
directory and FileHandler creation swallows errors; update the except in the
block that references data_dir, log_dir, FileHandler and watchdog_logger to
catch Exception as e and log the failure instead of passing — e.g., emit a clear
error via watchdog_logger.error or logging.exception including the exception
information (exc_info=True) and a contextual message like "failed to set up
watchdog log handler"; keep the function flow unchanged so failures don't raise
further.

---

122-128: Comment is misleading: os._exit(0) also bypasses cleanup handlers.

The comment explains why os.kill(SIGTERM) is unsuitable on Windows (it calls TerminateProcess), then uses os._exit(0) which also exits immediately without running cleanup handlers or atexit functions.

Using sys.exit(0) from a thread won't terminate the process (it only raises SystemExit in that thread). Since there's no clean cross-platform way to gracefully shutdown uvicorn from a background thread, os._exit(0) may be the pragmatic choice—but the comment should reflect that this is a hard exit, not a graceful one.

📝 Clarify the comment

                 if sys.platform == "win32":
-                    # sys.exit triggers SystemExit, allowing uvicorn to run
-                    # shutdown handlers. os.kill(SIGTERM) on Windows calls
-                    # TerminateProcess which hard-kills without cleanup.
+                    # On Windows, os.kill(SIGTERM) calls TerminateProcess which
+                    # hard-kills without cleanup. sys.exit() from a thread only
+                    # raises SystemExit in this thread, not the main thread.
+                    # os._exit(0) is a hard exit but at least terminates cleanly.
                     os._exit(0)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@backend/server.py` around lines 122 - 128, The comment above the
platform-specific exit branch is misleading: replace it to state that
os._exit(0) performs a hard process exit that bypasses cleanup handlers/atexit
(unlike sys.exit which raises SystemExit in the current thread), and clarify
that because there is no reliable cross-platform way to gracefully stop uvicorn
from a background thread the code deliberately chooses a hard exit on Windows
(os._exit(0)) while using os.kill(os.getpid(), signal.SIGTERM) on non-Windows;
update the comment adjacent to the if sys.platform == "win32": / os._exit(0) /
os.kill(os.getpid(), signal.SIGTERM) block to reflect this rationale and the
cleanup implications.

tauri/src-tauri/src/main.rs (1)

533-537: ⚠️ Potential issue | 🟠 Major

Handle failed Windows /shutdown and add force-stop fallback.

Line [533] discards the HTTP result, so stop_server can report success while the old server is still alive. Since PID/state is already cleared, restart_server can then reuse stale state/process behavior.

Suggested patch

         #[cfg(windows)]
         {
+            use std::process::Command;
             // Send graceful shutdown via HTTP — the server's parent-pid watchdog
             // will also handle cleanup if this app process exits.
             println!("Sending graceful shutdown via HTTP...");
             let client = reqwest::blocking::Client::builder()
                 .timeout(std::time::Duration::from_secs(2))
                 .build()
                 .unwrap();

-            let _ = client
+            let shutdown_ok = client
                 .post(&format!("http://127.0.0.1:{}/shutdown", SERVER_PORT))
-                .send();
+                .send()
+                .map(|r| r.status().is_success())
+                .unwrap_or(false);
+
+            if !shutdown_ok {
+                println!("Graceful shutdown failed, forcing process termination...");
+                let _ = Command::new("taskkill")
+                    .args(["/PID", &pid.to_string(), "/T", "/F"])
+                    .output();
+            }

             println!("Shutdown request sent (server watchdog will handle cleanup)");
         }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tauri/src-tauri/src/main.rs` around lines 533 - 537, The shutdown HTTP
request result is being ignored causing stop_server to report success even if
the server didn't stop; update the code around the client.post(...).send() call
in stop_server to check the Result/Response, treat non-success or errors
(especially on Windows) as a failure, and implement a force-stop fallback that
kills the process by PID when the HTTP shutdown fails; ensure stop_server only
clears state after a successful shutdown or after the force-stop path completes,
and make restart_server rely on the cleared state only after stop_server
confirms termination.

backend/server.py (1)

125-141: ⚠️ Potential issue | 🟠 Major

Unify parent-death handling; avoid orphaning on first check and hard-exit on Windows.

Line [127] returns when the parent is already gone, which leaves the server running orphaned.
Line [138] uses os._exit(0), which bypasses normal cleanup/shutdown hooks. Use one consistent shutdown path for both the initial and loop checks.

In CPython on Windows, what cleanup/shutdown hooks (atexit, finally blocks, FastAPI/Uvicorn shutdown events) run when calling os._exit(0) versus sys.exit() from a background thread?

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@backend/server.py` around lines 125 - 141, The early return when the parent
PID is missing and the hard os._exit(0) call on Windows create inconsistent
shutdown behavior; update the parent-death handling so the initial check follows
the same graceful shutdown path as the loop: instead of returning when not
alive, log the parent-gone message and invoke the same shutdown branch used
inside the while (use sys.exit(0) on Windows to allow cleanup and
uvicorn/FastAPI shutdown handlers, and os.kill(os.getpid(), signal.SIGTERM) on
non-Windows), and remove/replace any os._exit(0) usage; keep references to
_is_pid_alive, watchdog_logger, _watchdog_disabled and ensure both the initial
check and the loop use the identical shutdown sequence.

backend/server.py (1)

84-85: Don’t silently swallow watchdog file-logger setup failures.

Catching broad Exception and pass makes production watchdog failures hard to diagnose. Prefer narrowing the exception and logging once.

Suggested patch

-        except Exception:
-            pass
+        except OSError as e:
+            watchdog_logger.warning("Failed to initialize watchdog file logger: %s", e)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@backend/server.py` around lines 84 - 85, Replace the silent broad except in
the watchdog file-logger setup with a narrowed exception handler that logs the
error; specifically, instead of "except Exception: pass" around the watchdog
file-logger setup in backend/server.py, catch likely OS/file errors (e.g.,
OSError, IOError, PermissionError) as e and call the module logger (e.g.,
logger.error or logger.exception) with a clear message like "watchdog
file-logger setup failed" and the exception details so the failure is recorded;
do not swallow other unexpected exceptions (let them propagate) and ensure the
logging call uses the existing logger instance used elsewhere in server.py.