watcher correctness + perf: graceful shutdown, mtime-cached scans, single-row UPDATEs, pcr_dir fail-loud

crates/pcr-cli/Cargo.toml

@@ -23,3 +23,7 @@ serde_json = { workspace = true }
 # Already a workspace dep — reused here so the migration smoke test can
 # build a v1-schema fixture DB and re-query it after migrations run.
 rusqlite = { workspace = true }
+# SIGINT delivery for the graceful-shutdown integration test (Unix-only,
+# guarded by `#![cfg(unix)]` in the test file).
+[target.'cfg(unix)'.dev-dependencies]
+libc = "0.2"

crates/pcr-cli/tests/start_graceful_shutdown.rs

@@ -0,0 +1,103 @@
+//! Integration test for the audit's task 1: `pcr start` must shut
+//! down gracefully on SIGINT.
+//!
+//! Before this fix the source watcher threads ran a bare `loop {
+//! sleep; scan() }` with no cooperative shutdown signal. Ctrl-C
+//! killed the process mid-scan and the PID file was racey to remove.
+//! Now `pcr start` installs a Ctrl-C handler that flips a process-
+//! wide `crate::shutdown` flag; scan loops poll it at the top of
+//! every iteration; `wait_for_shutdown` returns; the `PidFileGuard`
+//! drop removes the PID file before the binary exits cleanly with
+//! code 0.
+//!
+//! Unix-only — `nix`/`libc::kill(pid, SIGINT)` is the natural way to
+//! send the signal, and the `ctrlc` crate's Windows path is a
+//! console-mode handler that doesn't translate to a clean test
+//! shape. Windows ungraceful-shutdown is a separate concern.
+
+#![cfg(unix)]
+
+mod common;
+
+use std::time::{Duration, Instant};
+
+use common::home_fixture;
+
+#[test]
+fn pcr_start_exits_zero_and_removes_pid_file_on_sigint() {
+    let fx = home_fixture();
+    let pid_file = fx.pcr_dir().join("watcher.pid");
+    assert!(!pid_file.exists(), "fresh fixture has no PID file yet");
+
+    // Spawn `pcr start --plain` headless in the fixture's $HOME. We
+    // need a `std::process::Child` (not `assert_cmd::Command`) so we
+    // can grab the PID and send a signal to it directly.
+    let bin = assert_cmd::cargo::cargo_bin("pcr");
+    let mut child = std::process::Command::new(&bin)
+        .arg("start")
+        .env("HOME", fx.home_path())
+        .env("USERPROFILE", fx.home_path())
+        // Disable TUI: `is_tui_eligible` already returns false because
+        // stderr is piped, but set NO_COLOR for belt-and-braces in case
+        // the binary's env carries a real CI/terminal value through.
+        .env("NO_COLOR", "1")
+        .env_remove("CI")
+        .env_remove("CURSOR_AGENT")
+        .current_dir(fx.cwd_path())
+        .stdin(std::process::Stdio::null())
+        .stdout(std::process::Stdio::piped())
+        .stderr(std::process::Stdio::piped())
+        .spawn()
+        .expect("spawn pcr start");
+
+    // Wait for the PID file to appear, capped at 10 s. The binary
+    // writes it before `wait_for_shutdown()` so it's the simplest
+    // readiness signal — without it, we might send SIGINT during
+    // startup, before the ctrlc handler is installed.
+    let pid = child.id() as i32;
+    let deadline = Instant::now() + Duration::from_secs(10);
+    while !pid_file.exists() {
+        if Instant::now() >= deadline {
+            let _ = child.kill();
+            panic!("PID file never appeared at {pid_file:?} — pcr start may have crashed");
+        }
+        std::thread::sleep(Duration::from_millis(50));
+    }
+
+    // Send SIGINT, exactly as `kill -INT <pid>` would.
+    // SAFETY: `pid` is the child we just spawned; kill(2) with SIGINT
+    // is safe and has no other observable side effects in this test.
+    let rc = unsafe { libc::kill(pid, libc::SIGINT) };
+    assert_eq!(
+        rc,
+        0,
+        "kill(2) syscall failed: {}",
+        std::io::Error::last_os_error()
+    );
+
+    // Wait for graceful exit. 15 s gives the 200 ms ctrlc-poll loop +
+    // any scan-in-progress + final flush plenty of headroom.
+    let exit_deadline = Instant::now() + Duration::from_secs(15);
+    let status = loop {
+        match child.try_wait().expect("try_wait") {
+            Some(s) => break s,
+            None => {
+                if Instant::now() >= exit_deadline {
+                    let _ = child.kill();
+                    panic!("pcr start did not exit within 15s after SIGINT — graceful shutdown is broken");
+                }
+                std::thread::sleep(Duration::from_millis(50));
+            }
+        }
+    };
+
+    assert!(
+        status.success(),
+        "expected exit code 0, got {status:?} — SIGINT must produce a clean exit"
+    );
+    assert!(
+        !pid_file.exists(),
+        "PidFileGuard::drop must have removed {pid_file:?}; otherwise a stale PID file \
+         confuses the next `pcr start` into prompting about an already-dead watcher"
+    );
+}

crates/pcr-core/src/auth.rs

@@ -16,21 +16,21 @@ pub struct Auth {
     pub user_id: String,
 }
 
-fn auth_file_path() -> PathBuf {
-    config::pcr_dir().join("auth.json")
+fn auth_file_path() -> anyhow::Result<PathBuf> {
+    Ok(config::pcr_dir()?.join("auth.json"))
 }
 
 /// Load the saved auth credentials. Returns `None` if the file doesn't
-/// exist or can't be parsed — mirrors the Go `Load() *Auth` behavior which
-/// returns `nil` on any error.
+/// exist, can't be parsed, or if `$HOME` couldn't be resolved — mirrors
+/// the Go `Load() *Auth` behavior which returns `nil` on any error.
 pub fn load() -> Option<Auth> {
-    let data = fs::read(auth_file_path()).ok()?;
+    let data = fs::read(auth_file_path().ok()?).ok()?;
     serde_json::from_slice(&data).ok()
 }
 
 /// Persist auth credentials to disk with 0600 permissions.
 pub fn save(auth: &Auth) -> anyhow::Result<()> {
-    let path = auth_file_path();
+    let path = auth_file_path()?;
     if let Some(parent) = path.parent() {
         fs::create_dir_all(parent)?;
     }
@@ -46,7 +46,11 @@ pub fn save(auth: &Auth) -> anyhow::Result<()> {
     Ok(())
 }
 
-/// Clear saved credentials. Silently succeeds if no file exists.
+/// Clear saved credentials. Silently succeeds if no file exists. If
+/// `$HOME` can't be resolved there's nothing to clear in the first
+/// place, so we still no-op rather than surfacing an error.
 pub fn clear() {
-    let _ = fs::remove_file(auth_file_path());
+    if let Ok(p) = auth_file_path() {
+        let _ = fs::remove_file(p);
+    }
 }

crates/pcr-core/src/commands/hook.rs

@@ -7,8 +7,13 @@ use crate::exit::ExitCode;
 use crate::sources::claudecode::hook::run_hook as run_claude_hook;
 
 pub fn run(_mode: OutputMode) -> ExitCode {
-    // Only act if `pcr start` is currently running.
-    if read_existing_pid(&pid_file_path()).is_none() {
+    // Only act if `pcr start` is currently running. If `$HOME`
+    // can't even be resolved we can't have a live watcher anyway —
+    // exit 0 so the hook doesn't re-engage the model.
+    let Ok(path) = pid_file_path() else {
+        return ExitCode::Success;
+    };
+    if read_existing_pid(&path).is_none() {
         return ExitCode::Success;
     }
     let ctx = project_context::resolve();

crates/pcr-core/src/commands/log.rs

@@ -32,8 +32,14 @@ fn short_sha(sha: &str) -> String {
     if sha.starts_with("manual-") {
         return "[manual]".to_string();
     }
-    if sha.len() >= 7 {
-        return sha[..7].to_string();
+    // Real SHAs are hex (ASCII), but `short_sha` is called on whatever
+    // sha string the store handed us — including the "manual-*"
+    // sentinels and any future non-ASCII tags. `chars().take(7)`
+    // matches the truncate-on-char-boundary fix in `util::text` and
+    // never panics on multi-byte input.
+    let short: String = sha.chars().take(7).collect();
+    if short.chars().count() == 7 {
+        return short;
     }
     sha.to_string()
 }

crates/pcr-core/src/commands/start.rs

@@ -1,8 +1,6 @@
 //! `pcr start`. Mirrors `cli/cmd/start.go`.
 
-use std::path::PathBuf;
-use std::sync::atomic::{AtomicBool, Ordering};
-use std::sync::Arc;
+use std::path::{Path, PathBuf};
 use std::thread;
 use std::time::Duration;
 
@@ -12,13 +10,14 @@ use crate::display;
 use crate::entry::StartArgs;
 use crate::exit::ExitCode;
 use crate::projects;
+use crate::shutdown;
 use crate::sources;
 
-pub fn pid_file_path() -> PathBuf {
-    config::pcr_dir().join("watcher.pid")
+pub fn pid_file_path() -> anyhow::Result<PathBuf> {
+    Ok(config::pcr_dir()?.join("watcher.pid"))
 }
 
-pub fn read_existing_pid(pid_file: &PathBuf) -> Option<i32> {
+pub fn read_existing_pid(pid_file: &Path) -> Option<i32> {
     let data = std::fs::read_to_string(pid_file).ok()?;
     let pid: i32 = data.trim().parse().ok()?;
     #[cfg(unix)]
@@ -44,7 +43,27 @@ pub fn read_existing_pid(pid_file: &PathBuf) -> Option<i32> {
 }
 
 pub fn run(mode: OutputMode, args: StartArgs) -> ExitCode {
-    let pid_file = pid_file_path();
+    // Install the Ctrl-C handler before doing any setup work (PID
+    // file write, watcher spawn, etc.). The default SIGINT handler
+    // would otherwise kill the process during that window — racing
+    // with PID file cleanup and exiting with `W_TERMSIG(SIGINT)`
+    // instead of the clean code 0 the integration test asserts on.
+    //
+    // `set_handler` returns Err if a handler is already installed
+    // (re-entry in tests via the same process); the existing
+    // handler already routes to `request_shutdown`, so ignore Err.
+    let _ = ctrlc::set_handler(shutdown::request_shutdown);
+
+    let pid_file = match pid_file_path() {
+        Ok(p) => p,
+        Err(e) => {
+            // Surface the $HOME-missing case immediately rather
+            // than letting downstream singletons (`store::open()`,
+            // etc.) panic with the same error mid-watcher-spawn.
+            display::print_error("start", &format!("{e}"));
+            return ExitCode::GenericError;
+        }
+    };
 
     if let Some(pid) = read_existing_pid(&pid_file) {
         if !agent::is_interactive_terminal() {
@@ -208,12 +227,14 @@ fn spawn_all_sources() -> Vec<thread::JoinHandle<()>> {
 }
 
 fn wait_for_shutdown() {
-    let flag = Arc::new(AtomicBool::new(false));
-    let flag_handler = flag.clone();
-    let _ = ctrlc::set_handler(move || {
-        flag_handler.store(true, Ordering::SeqCst);
-    });
-    while !flag.load(Ordering::SeqCst) {
+    // The ctrlc handler is installed at the top of `run()` so the
+    // SIGINT-arrives-before-watchers-are-ready window doesn't kill
+    // the process with the default signal handler. Here we just
+    // park the main thread until the handler flips the flag (which
+    // every long-running scan loop in `crate::shutdown` is also
+    // polling). PID file cleanup runs via `PidFileGuard::drop` —
+    // `remove_file` is wrapped in `let _ =` so it's idempotent.
+    while !shutdown::is_shutting_down() {
         thread::sleep(Duration::from_millis(200));
     }
 }

crates/pcr-core/src/config.rs

@@ -1,5 +1,7 @@
 //! Compile-time constants. Mirrors `cli/internal/config/constants.go`.
 
+use std::path::PathBuf;
+
 /// Supabase project URL used for all RPC calls.
 pub const SUPABASE_URL: &str = "https://icbsvwffcykzimjjonad.supabase.co";
 
@@ -12,9 +14,90 @@ pub const APP_URL: &str = "https://pcr.dev";
 /// Name of the per-user data directory under `$HOME`.
 pub const PCR_DIR: &str = ".pcr-dev";
 
-/// Returns the absolute path of `$HOME/.pcr-dev`, creating nothing on disk.
-pub fn pcr_dir() -> std::path::PathBuf {
-    dirs::home_dir()
-        .unwrap_or_else(std::env::temp_dir)
-        .join(PCR_DIR)
+/// Returns the absolute path of `$HOME/.pcr-dev`, creating nothing on
+/// disk.
+///
+/// Previously this fell back to `std::env::temp_dir()` when
+/// `dirs::home_dir()` returned `None` — a silent failure mode that
+/// wrote auth credentials and the SQLite store to `/tmp`, where they
+/// vanished on reboot. The new contract: callers see the error and
+/// can decide whether to fail loudly (CLI entry points) or skip
+/// soft-state writes (background loops). No silent fallback ever.
+///
+/// On well-configured Unix systems and on Windows with `USERPROFILE`
+/// set this returns `Ok(...)` essentially unconditionally; the
+/// `Err` path only fires inside sandboxes / containers / cron jobs
+/// where neither `HOME` nor `USERPROFILE` is set.
+pub fn pcr_dir() -> anyhow::Result<PathBuf> {
+    let home = dirs::home_dir().ok_or_else(|| {
+        anyhow::anyhow!(
+            "could not determine $HOME (or %USERPROFILE% on Windows). PCR refuses to \
+             silently fall back to a temp directory because that would lose auth + \
+             local drafts on reboot. Set HOME and re-run."
+        )
+    })?;
+    Ok(home.join(PCR_DIR))
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    /// `dirs::home_dir()` reads `HOME` (Unix) / `USERPROFILE` (Win)
+    /// from the env at call time, so we can exercise the error
+    /// path by removing both before calling. Restore them before
+    /// the test ends so the rest of the test binary stays sane.
+    ///
+    /// Skipped on platforms where `dirs::home_dir()` falls back to
+    /// something else (e.g. `getpwuid_r` on Unix): if the
+    /// unset-everything-snapshot still resolves, we can't drive
+    /// the error branch from a test, but the production code is
+    /// still strictly safer than the previous `unwrap_or_else(
+    /// env::temp_dir)`.
+    #[test]
+    fn pcr_dir_returns_err_when_home_is_unresolvable() {
+        let prev_home = std::env::var_os("HOME");
+        let prev_userprofile = std::env::var_os("USERPROFILE");
+        // SAFETY: this test process is single-threaded inside its
+        // own binary (the suite spawns each #[test] sequentially
+        // by default), and we restore the env before returning.
+        unsafe {
+            std::env::remove_var("HOME");
+            std::env::remove_var("USERPROFILE");
+        }
+
+        let result = pcr_dir();
+
+        // Restore before any assertion can panic.
+        unsafe {
+            if let Some(v) = prev_home {
+                std::env::set_var("HOME", v);
+            }
+            if let Some(v) = prev_userprofile {
+                std::env::set_var("USERPROFILE", v);
+            }
+        }
+
+        if dirs::home_dir().is_some() {
+            eprintln!(
+                "skipping: dirs::home_dir() still resolves with HOME/USERPROFILE unset \
+                 (likely getpwuid_r on Unix); error branch isn't reachable from a test \
+                 on this platform"
+            );
+            return;
+        }
+        let err = match result {
+            Ok(p) => panic!(
+                "expected an error when HOME is unset; got Ok({}). The previous \
+                 implementation would silently fall back to /tmp here — that's exactly \
+                 the regression we're guarding against.",
+                p.display(),
+            ),
+            Err(e) => e.to_string(),
+        };
+        assert!(
+            err.contains("$HOME") || err.to_lowercase().contains("home"),
+            "error message must mention $HOME so users know what to fix; got: {err}"
+        );
+    }
 }

crates/pcr-core/src/lib.rs

@@ -35,6 +35,7 @@ pub mod exit;
 pub mod help;
 pub mod mcp;
 pub mod projects;
+pub mod shutdown;
 pub mod sources;
 pub mod store;
 pub mod supabase;