feat: Improve user experience with errors

Author: math280h

.github/ISSUE_TEMPLATE/ai_missed_tool.md

@@ -0,0 +1,44 @@
+---
+name: AI skipped safe-pkgs
+about: Report a case where an AI agent installed packages without calling check_package or check_lockfile first
+title: 'AI skipped safe-pkgs: <brief description>'
+labels: ai-tool-selection
+assignees: ''
+
+---
+
+**Which AI / client were you using?**
+e.g. Claude (Sonnet 4.5), GPT-4o, Cursor, Continue, etc.
+
+**What was the prompt or instruction you gave the AI?**
+
+<!-- Paste the exact message or instruction that triggered the package install -->
+
+```
+<your prompt here>
+```
+
+**What did the AI do instead?**
+
+<!-- Paste the relevant part of the AI's response or actions — e.g. it ran `npm install` directly without calling check_package -->
+
+```
+<AI response or tool calls here>
+```
+
+**What did you expect it to do?**
+
+<!-- e.g. "Call check_package for each dependency before installing" -->
+
+**MCP / tool configuration**
+
+- How is safe-pkgs registered? (Claude Desktop config, Cursor settings, etc.)
+- Paste your MCP server config if relevant:
+
+```json
+
+```
+
+**Additional context**
+
+<!-- Anything else that might help — system prompt, tool list shown to the model, etc. -->

README.md

@@ -50,7 +50,7 @@ cargo install --path . --locked
 Run MCP server:
 
 ```bash
-safe-pkgs serve --mcp
+safe-pkgs serve
 ```
 
 Run a one-off audit:
@@ -127,7 +127,7 @@ macOS/Linux:
     "safe-pkgs": {
       "type": "stdio",
       "command": "/path/to/safe-pkgs",
-      "args": ["serve", "--mcp"]
+      "args": ["serve"]
     }
   },
   "inputs": []
@@ -258,12 +258,11 @@ For direct dependencies, `dependency_ancestry` is omitted.
 - Deterministic policy context: responses include `policy_snapshot_version`, config and policy fingerprints, and enabled check set.
 - Local cache: SQLite cache keyed by policy fingerprint + package tuple with TTL expiry.
 
-## Docs Map
+## Disclaimer
 
-- Getting started: `docs/getting-started.md`
-- Full config schema: `docs/configuration-spec.md`
-- Registry check matrix: `docs/check-support-map.md`
-- Cache and policy fingerprinting: `docs/cache-deep-dive.md`
+`safe-pkgs` works as an MCP tool that AI agents can call before installing packages. However, **we cannot guarantee that an AI agent will always choose to call this tool** — agentic models that autonomously select tools may proceed with package installation without invoking `check_package` or `check_lockfile` first, depending on the model, prompt context, and system prompt configuration.
+
+If your AI agent skipped `safe-pkgs` when it should have called it, please [open an issue](https://github.com/math280h/safe-pkgs/issues/new?template=ai_missed_tool.md) with the prompt and response so we can improve tool descriptions and usage guidance.
 
 ## Roadmap
 

crates/checks/advisory/src/lib.rs

@@ -43,13 +43,12 @@ impl Check for AdvisoryCheck {
             &package.latest,
             context.advisories,
         )
-        .await
         .into_iter()
         .collect())
     }
 }
 
-async fn run(
+fn run(
     package_name: &str,
     requested_version: &str,
     latest_version: &str,
@@ -149,39 +148,35 @@ fn best_fixed_version(candidates: &[String]) -> Option<&str> {
 mod tests {
     use super::*;
 
-    #[tokio::test]
-    async fn empty_advisories_has_no_finding() {
-        let finding = run("demo", "1.0.0", "1.2.0", &[]).await;
+    #[test]
+    fn empty_advisories_has_no_finding() {
+        let finding = run("demo", "1.0.0", "1.2.0", &[]);
         assert!(finding.is_none());
     }
 
-    #[tokio::test]
-    async fn advisory_with_cve_alias_and_fixed_version_is_high_risk() {
+    #[test]
+    fn advisory_with_cve_alias_and_fixed_version_is_high_risk() {
         let advisories = vec![PackageAdvisory {
             id: "OSV-123".to_string(),
             aliases: vec!["CVE-2025-1234".to_string()],
             fixed_versions: vec!["1.1.0".to_string(), "2.0.0".to_string()],
         }];
 
-        let finding = run("demo", "1.0.0", "2.0.0", &advisories)
-            .await
-            .expect("finding");
+        let finding = run("demo", "1.0.0", "2.0.0", &advisories).expect("finding");
         assert_eq!(finding.severity, Severity::High);
         assert!(finding.reason.contains("CVE-2025-1234"));
         assert!(finding.reason.contains("newer version 1.1.0"));
     }
 
-    #[tokio::test]
-    async fn advisory_without_alias_uses_advisory_id() {
+    #[test]
+    fn advisory_without_alias_uses_advisory_id() {
         let advisories = vec![PackageAdvisory {
             id: "OSV-999".to_string(),
             aliases: Vec::new(),
             fixed_versions: Vec::new(),
         }];
 
-        let finding = run("demo", "1.0.0", "1.0.0", &advisories)
-            .await
-            .expect("finding");
+        let finding = run("demo", "1.0.0", "1.0.0", &advisories).expect("finding");
         assert!(finding.reason.contains("OSV-999"));
     }
 }

crates/checks/typosquat/src/lib.rs

@@ -28,6 +28,10 @@ impl Check for TyposquatCheck {
         true
     }
 
+    fn needs_popular_package_names(&self) -> bool {
+        true
+    }
+
     async fn run(
         &self,
         context: &CheckExecutionContext<'_>,
@@ -102,11 +106,15 @@ async fn run(
     ))
 }
 
+/// Computes the Levenshtein distance between two strings, returning `None` early
+/// when the distance provably exceeds `max_distance`.
+///
+/// Package names are ASCII, so byte comparison is both correct and allocation-free.
 fn bounded_levenshtein(lhs: &str, rhs: &str, max_distance: usize) -> Option<usize> {
-    let lhs_chars = lhs.chars().collect::<Vec<_>>();
-    let rhs_chars = rhs.chars().collect::<Vec<_>>();
-    let lhs_len = lhs_chars.len();
-    let rhs_len = rhs_chars.len();
+    let lhs_bytes = lhs.as_bytes();
+    let rhs_bytes = rhs.as_bytes();
+    let lhs_len = lhs_bytes.len();
+    let rhs_len = rhs_bytes.len();
 
     if lhs_len.abs_diff(rhs_len) > max_distance {
         return None;
@@ -115,12 +123,12 @@ fn bounded_levenshtein(lhs: &str, rhs: &str, max_distance: usize) -> Option<usiz
     let mut previous = (0..=rhs_len).collect::<Vec<_>>();
     let mut current = vec![0usize; rhs_len + 1];
 
-    for (i, lhs_char) in lhs_chars.iter().enumerate() {
+    for (i, &lhs_byte) in lhs_bytes.iter().enumerate() {
         current[0] = i + 1;
         let mut row_min = current[0];
 
-        for (j, rhs_char) in rhs_chars.iter().enumerate() {
-            let substitution_cost = usize::from(lhs_char != rhs_char);
+        for (j, &rhs_byte) in rhs_bytes.iter().enumerate() {
+            let substitution_cost = usize::from(lhs_byte != rhs_byte);
             let deletion = previous[j + 1] + 1;
             let insertion = current[j] + 1;
             let substitution = previous[j] + substitution_cost;

crates/core/src/lib.rs

@@ -176,6 +176,9 @@ pub trait Check: Send + Sync {
     fn needs_advisories(&self) -> bool {
         false
     }
+    fn needs_popular_package_names(&self) -> bool {
+        false
+    }
     async fn run(
         &self,
         context: &CheckExecutionContext<'_>,
@@ -606,6 +609,9 @@ pub trait RegistryClient: Send + Sync {
     async fn fetch_weekly_downloads(&self, _package: &str) -> Result<Option<u64>, RegistryError> {
         Ok(None)
     }
+    async fn prefetch_popular_package_names(&self) -> Result<(), RegistryError> {
+        Ok(())
+    }
     async fn fetch_popular_package_names(
         &self,
         _limit: usize,

crates/http/src/lib.rs

@@ -39,7 +39,7 @@ pub fn build_http_client() -> Client {
         .connect_timeout(Duration::from_secs(DEFAULT_CONNECT_TIMEOUT_SECS))
         .timeout(Duration::from_secs(DEFAULT_REQUEST_TIMEOUT_SECS))
         .build()
-        .unwrap_or_else(|_| Client::new())
+        .expect("HTTP client construction should not fail with supported TLS and valid timeouts")
 }
 
 pub async fn send_with_retry<F>(
@@ -128,18 +128,10 @@ fn compute_retry_delay(
 ) -> Duration {
     let fallback = exponential_backoff(attempt, policy.initial_backoff, policy.max_backoff);
     match retry_after {
-        Some(delay) => {
-            let bounded = if delay > policy.max_backoff {
-                policy.max_backoff
-            } else {
-                delay
-            };
-            if bounded.is_zero() {
-                Duration::from_millis(1)
-            } else {
-                bounded
-            }
-        }
+        // Retry-After is a server directive; respect it exactly rather than capping it.
+        // Capping can cause immediate rate-limiting on the next attempt.
+        Some(delay) if delay.is_zero() => Duration::from_millis(1),
+        Some(delay) => delay,
         None => fallback,
     }
 }
@@ -179,6 +171,20 @@ mod tests {
         assert_eq!(delay, Duration::from_secs(2));
     }
 
+    #[test]
+    fn compute_retry_delay_respects_retry_after_even_when_larger_than_max_backoff() {
+        let policy = RetryPolicy {
+            max_attempts: 3,
+            initial_backoff: Duration::from_millis(100),
+            max_backoff: Duration::from_secs(5),
+        };
+
+        // Retry-After: 60 must be honoured; ignoring it and retrying after 5s
+        // would almost certainly trigger another rate-limit response.
+        let delay = compute_retry_delay(1, policy, Some(Duration::from_secs(60)));
+        assert_eq!(delay, Duration::from_secs(60));
+    }
+
     #[tokio::test]
     async fn send_with_retry_retries_retryable_statuses() {
         let server = MockServer::start().await;

crates/registry/npm/src/registry.rs

@@ -19,6 +19,10 @@ use safe_pkgs_registry_http::{
 const NPMS_POPULAR_QUERY: &str = "not:deprecated";
 const NPMS_PAGE_SIZE: usize = 250;
 const NPM_BULK_DOWNLOAD_MAX_PACKAGES: usize = 128;
+/// Number of popular packages to warm into the cache during lockfile prefetch.
+/// Chosen to match the typosquat check's sample size so subsequent per-package
+/// calls always hit the in-process cache.
+const POPULAR_PACKAGE_PREFETCH_SIZE: usize = 5000;
 
 #[derive(Clone)]
 pub struct NpmRegistryClient {
@@ -125,6 +129,12 @@ impl RegistryClient for NpmRegistryClient {
         self.prefetch_weekly_downloads_bulk(packages).await
     }
 
+    async fn prefetch_popular_package_names(&self) -> Result<(), RegistryError> {
+        self.fetch_popular_package_names(POPULAR_PACKAGE_PREFETCH_SIZE)
+            .await
+            .map(|_| ())
+    }
+
     async fn fetch_package(&self, package: &str) -> Result<PackageRecord, RegistryError> {
         let encoded_name = Self::encode_package_name(package);
         let url = format!("{}/{}", self.base_url.trim_end_matches('/'), encoded_name);

src/cache.rs

@@ -31,16 +31,22 @@ impl SqliteCache {
         }
         let conn = Connection::open(&db_path)
             .with_context(|| format!("failed to open sqlite cache at {}", db_path.display()))?;
-        Self::from_connection(conn, ttl_minutes)
+        Self::from_connection(conn, Duration::from_secs(ttl_minutes.max(1) * 60))
     }
 
     #[cfg(test)]
     pub fn in_memory(ttl_minutes: u64) -> anyhow::Result<Self> {
         let conn = Connection::open_in_memory().context("failed to open in-memory sqlite cache")?;
-        Self::from_connection(conn, ttl_minutes)
+        Self::from_connection(conn, Duration::from_secs(ttl_minutes.max(1) * 60))
     }
 
-    fn from_connection(conn: Connection, ttl_minutes: u64) -> anyhow::Result<Self> {
+    #[cfg(test)]
+    pub fn in_memory_with_ttl(ttl: Duration) -> anyhow::Result<Self> {
+        let conn = Connection::open_in_memory().context("failed to open in-memory sqlite cache")?;
+        Self::from_connection(conn, ttl)
+    }
+
+    fn from_connection(conn: Connection, ttl: Duration) -> anyhow::Result<Self> {
         conn.execute_batch(
             r#"
 CREATE TABLE IF NOT EXISTS cache_entries (
@@ -55,7 +61,7 @@ CREATE INDEX IF NOT EXISTS idx_cache_entries_expires_at ON cache_entries (expire
 
         Ok(Self {
             conn: Mutex::new(conn),
-            ttl: Duration::from_secs(ttl_minutes.max(1) * 60),
+            ttl,
         })
     }
 
@@ -168,8 +174,8 @@ mod tests {
 
     #[test]
     fn expired_entries_are_treated_as_cache_miss() {
-        let mut cache = SqliteCache::in_memory(1).expect("in-memory cache");
-        cache.ttl = Duration::from_secs(1);
+        let cache =
+            SqliteCache::in_memory_with_ttl(Duration::from_secs(1)).expect("in-memory cache");
         cache
             .set("expiring-key", "{\"ok\":true}")
             .expect("set cache value");
@@ -180,8 +186,8 @@ mod tests {
 
     #[test]
     fn set_returns_error_when_ttl_math_overflows() {
-        let mut cache = SqliteCache::in_memory(1).expect("in-memory cache");
-        cache.ttl = Duration::from_secs(u64::MAX);
+        let cache =
+            SqliteCache::in_memory_with_ttl(Duration::from_secs(u64::MAX)).expect("in-memory cache");
         let err = cache
             .set("overflow", "{\"ok\":true}")
             .expect_err("expected ttl overflow error");

src/checks.rs

@@ -19,8 +19,6 @@ use crate::types::{Evidence, EvidenceKind};
 pub struct CheckDescriptor {
     /// Stable check id (for config and support maps).
     pub id: CheckId,
-    /// Human-facing key shown in CLI output.
-    pub key: &'static str,
     /// Short description of what the check does.
     pub description: &'static str,
     /// Whether the check needs weekly download data.
@@ -36,6 +34,8 @@ pub struct CheckRuntimeRequirements {
     pub needs_weekly_downloads: bool,
     /// True when at least one enabled check needs advisories.
     pub needs_advisories: bool,
+    /// True when at least one enabled check needs popular package name data.
+    pub needs_popular_package_names: bool,
 }
 
 /// Final result produced by running all enabled checks.
@@ -60,7 +60,6 @@ pub fn check_descriptors() -> Vec<CheckDescriptor> {
         .iter()
         .map(|check| CheckDescriptor {
             id: check.id(),
-            key: check.id(),
             description: check.description(),
             needs_weekly_downloads: check.needs_weekly_downloads(),
             needs_advisories: check.needs_advisories(),
@@ -85,6 +84,9 @@ pub fn runtime_requirements_for_registry(
     CheckRuntimeRequirements {
         needs_weekly_downloads: checks.iter().any(|check| check.needs_weekly_downloads()),
         needs_advisories: checks.iter().any(|check| check.needs_advisories()),
+        needs_popular_package_names: checks
+            .iter()
+            .any(|check| check.needs_popular_package_names()),
     }
     .merge(custom_requirements)
 }
@@ -276,6 +278,9 @@ pub async fn run_all_checks_at_time(
     let requirements = CheckRuntimeRequirements {
         needs_weekly_downloads: checks.iter().any(|check| check.needs_weekly_downloads()),
         needs_advisories: checks.iter().any(|check| check.needs_advisories()),
+        needs_popular_package_names: checks
+            .iter()
+            .any(|check| check.needs_popular_package_names()),
     }
     .merge(custom_rules::runtime_requirements_for_registry(
         config,
@@ -386,6 +391,7 @@ impl CheckRuntimeRequirements {
         Self {
             needs_weekly_downloads: self.needs_weekly_downloads || custom.needs_weekly_downloads,
             needs_advisories: self.needs_advisories || custom.needs_advisories,
+            needs_popular_package_names: self.needs_popular_package_names,
         }
     }
 }