From 9042dc1c3b709cd4573aecba71f97ec5aa1c25e4 Mon Sep 17 00:00:00 2001
From: "warp-agent-staging[bot]"
 <240773466+warp-agent-staging[bot]@users.noreply.github.com>
Date: Sat, 15 Aug 2026 12:28:34 +0000
Subject: [PATCH 1/7] Bound SignatureCache key length to fix unbounded memory
 growth (APP-5431)

SignatureCache::get caches every distinct command token it is ever asked
to look up in an append-only memo_map::MemoMap, with no eviction, size
cap, or TTL. Since the completer looks up an arbitrary token from the
terminal input line on every keystroke/paste (see
CommandRegistry::signature_from_tokens and friends), a single
pathologically large token (e.g. a large blob of text pasted into the
terminal input) would be cloned into the cache and retained for the
lifetime of the process.

Fix: skip both the lookup and the cache entirely for tokens longer than
MAX_CACHEABLE_COMMAND_LEN (255, the common filesystem NAME_MAX). No real
command name can exceed this, so this cannot drop a legitimate lookup,
and it stops a single huge token from permanently growing the cache.

A true bounded/LRU cache was rejected: SignatureCache::get returns
Option<&Signature> borrowed from inside the map, and those references are
held across the parser (see SignatureAtTokenIndex), which the MemoMap's
append-only design exists specifically to support. Evicting entries while
references to them may be live would not compile; making it compile by
cloning Signature on every lookup would regress a hot path.

Co-Authored-By: Warp <agent@warp.dev>
---
 .../src/signatures/legacy/registry.rs         | 23 ++++++
 .../src/signatures/legacy/registry_tests.rs   | 78 ++++++++++++++++++-
 2 files changed, 100 insertions(+), 1 deletion(-)

diff --git a/crates/warp_completer/src/signatures/legacy/registry.rs b/crates/warp_completer/src/signatures/legacy/registry.rs
index 2ef0036001d..864eb5d5fac 100644
--- a/crates/warp_completer/src/signatures/legacy/registry.rs
+++ b/crates/warp_completer/src/signatures/legacy/registry.rs
@@ -18,6 +18,23 @@ pub enum SignatureResult<'a> {
 
 type SignatureLookupFn = dyn 'static + Send + Sync + Fn(&str) -> Option<Signature>;
 
+/// The longest a command name can be before we refuse to cache a lookup for it.
+///
+/// `SignatureCache::get` is called with arbitrary tokens taken from the terminal input line (see
+/// `CommandRegistry::signature_from_tokens` and friends), not just tokens we already know to be
+/// real command names. Since `SignatureCache::signatures` is append-only (see its doc comment for
+/// why), every distinct token ever looked up would otherwise be cached forever, which lets a
+/// single pathologically large token (e.g. a large blob of text pasted into the terminal input
+/// line) permanently retain an equally large `String` for the lifetime of the process. See
+/// APP-5431.
+///
+/// Real command names are executable names, which common filesystems cap at 255 bytes/characters
+/// (e.g. Linux `NAME_MAX`, macOS APFS/HFS+, and Windows NTFS all use this limit). No legitimate
+/// command name can exceed this, so a token longer than this cannot resolve to a real signature
+/// and is safe to skip caching (and looking up) entirely. As a sanity check, the longest name in
+/// the current embedded signature corpus is 43 characters, well under this cap.
+const MAX_CACHEABLE_COMMAND_LEN: usize = 255;
+
 /// A simple structure to cache parsed command signatures.  These are stored as
 /// JSON, so this makes it easy for us to lazily load and parse the JSON when
 /// a command signature is needed, and only need to do that parsing work once
@@ -49,6 +66,12 @@ impl SignatureCache {
         } else {
             command
         };
+        if command.len() > MAX_CACHEABLE_COMMAND_LEN {
+            // No real command name can be this long (see `MAX_CACHEABLE_COMMAND_LEN`), so this
+            // can't resolve to a signature. Bail out before caching (or even looking up) it to
+            // keep the cache bounded; see APP-5431.
+            return None;
+        }
         let command = command.to_lowercase();
         self.signatures
             .get_or_insert(&command, || (self.lookup_fn)(&command))
diff --git a/crates/warp_completer/src/signatures/legacy/registry_tests.rs b/crates/warp_completer/src/signatures/legacy/registry_tests.rs
index 2f5668a4f7a..c923db72aa5 100644
--- a/crates/warp_completer/src/signatures/legacy/registry_tests.rs
+++ b/crates/warp_completer/src/signatures/legacy/registry_tests.rs
@@ -1,6 +1,6 @@
 use crate::completer::testing::FakeCompletionContext;
 use crate::completer::{CompletionContext, TopLevelCommandCaseSensitivity};
-use crate::signatures::registry::SignatureResult;
+use crate::signatures::registry::{MAX_CACHEABLE_COMMAND_LEN, SignatureResult};
 use crate::signatures::testing::{create_test_command_registry, test_signature};
 
 #[test]
@@ -204,6 +204,82 @@ fn test_alias_expansion_path_skips_flag_with_value_before_subcommand() {
     assert_eq!(found_signature.token_index, 3);
 }
 
+#[test]
+fn test_oversized_command_is_not_cached_and_resolves_to_none() {
+    // Regression test for APP-5431: a pathologically large single "command" token (e.g. a large
+    // blob of text pasted into the terminal input line) must not grow the (append-only)
+    // SignatureCache.
+    let registry = create_test_command_registry([test_signature()]);
+    let len_before = registry.signatures.signatures.len();
+
+    let oversized_command = "a".repeat(MAX_CACHEABLE_COMMAND_LEN + 1);
+    assert_eq!(registry.signature(&oversized_command), None);
+    assert_eq!(
+        registry.signatures.signatures.len(),
+        len_before,
+        "looking up an oversized command should not add an entry to the cache"
+    );
+
+    // A command name right at the cap is still a legitimate lookup and should be cached as usual
+    // (as a negative-cache entry here, since it doesn't match any registered signature).
+    let max_length_command = "a".repeat(MAX_CACHEABLE_COMMAND_LEN);
+    assert_eq!(registry.signature(&max_length_command), None);
+    assert_eq!(registry.signatures.signatures.len(), len_before + 1);
+}
+
+#[test]
+fn test_ordinary_commands_still_resolve_and_are_cached() {
+    let registry = create_test_command_registry([test_signature()]);
+
+    // A registered command resolves correctly, case-insensitively, and repeated lookups hit the
+    // same cached entry rather than growing the cache.
+    let found = registry.signature("TEST");
+    assert_eq!(found.map(|s| s.name.as_str()), Some("test"));
+    let len_after_positive_lookup = registry.signatures.signatures.len();
+    assert_eq!(registry.signature("test"), found);
+    assert_eq!(
+        registry.signatures.signatures.len(),
+        len_after_positive_lookup
+    );
+
+    // Negative lookups are cached too, as before: the first miss adds an entry, and repeating it
+    // hits that cached entry rather than growing the cache further.
+    let len_before_negative_lookup = registry.signatures.signatures.len();
+    assert_eq!(registry.signature("not-a-real-command"), None);
+    assert_eq!(
+        registry.signatures.signatures.len(),
+        len_before_negative_lookup + 1
+    );
+    assert_eq!(registry.signature("not-a-real-command"), None);
+    assert_eq!(
+        registry.signatures.signatures.len(),
+        len_before_negative_lookup + 1
+    );
+}
+
+#[cfg(windows)]
+#[test]
+fn test_exe_suffix_is_trimmed_before_the_length_check() {
+    let registry = create_test_command_registry([test_signature()]);
+
+    assert_eq!(
+        registry.signature("test.exe").map(|s| s.name.as_str()),
+        Some("test")
+    );
+}
+
+#[test]
+fn test_registered_commands_unaffected_by_oversized_lookups() {
+    let registry = create_test_command_registry([test_signature()]);
+
+    let oversized_command = "a".repeat(MAX_CACHEABLE_COMMAND_LEN + 1);
+    assert_eq!(registry.signature(&oversized_command), None);
+    assert_eq!(registry.signature("not-a-real-command"), None);
+
+    let registered = registry.registered_commands().collect::<Vec<_>>();
+    assert_eq!(registered, vec!["test"]);
+}
+
 #[test]
 fn test_alias_expansion_path_skips_multiple_flags_before_subcommand() {
     // Exercises signature_with_alias_expansion with multiple flags (valued and

From 653efa4b59fae2fd21eedd9553031397addc8be6 Mon Sep 17 00:00:00 2001
From: "warp-agent-staging[bot]"
 <240773466+warp-agent-staging[bot]@users.noreply.github.com>
Date: Sat, 15 Aug 2026 12:53:21 +0000
Subject: [PATCH 2/7] Make SignatureCache a true positive-only bounded cache
 (review revision)

Address three review findings on the initial APP-5431 fix:

1. The length cap alone didn't bound the cache: every distinct <=255-byte
   miss still permanently stored a None entry. SignatureCache now only
   caches successful lookups (MemoMap<String, Signature>); a miss is
   never memoized. lookup_fn only probes the embedded, already-compiled
   signature corpus (a cheap binary search, per rust-embed >=8.0's
   "store file contents statically and use binary search" design) --
   JSON parsing only happens on a hit -- so redoing a miss on every
   keystroke is cheap, unlike memoizing it forever.

2. An explicitly insert()-ed signature whose name exceeds the cap would
   become unretrievable through get()/signature() while still being
   listed by registered_commands(). get() now falls back to a cheap,
   allocation-free linear scan (length-prechecked, so still cheap even
   for a huge token) for oversized tokens, so such a signature stays
   retrievable.

3. Added a regression test that exercises the fix for finding 2, and
   rewrote the .exe-trimming test to use a name exactly at the cap so it
   actually exercises trim-then-check ordering (the previous version
   passed regardless of ordering).

registered_commands() no longer needs to filter out negative-cache
entries, since every cache entry is now a real signature by construction.

Co-Authored-By: Warp <agent@warp.dev>
---
 .../src/signatures/legacy/registry.rs         | 72 ++++++++++------
 .../src/signatures/legacy/registry_tests.rs   | 82 +++++++++++++------
 2 files changed, 100 insertions(+), 54 deletions(-)

diff --git a/crates/warp_completer/src/signatures/legacy/registry.rs b/crates/warp_completer/src/signatures/legacy/registry.rs
index 864eb5d5fac..cb3903bf6a8 100644
--- a/crates/warp_completer/src/signatures/legacy/registry.rs
+++ b/crates/warp_completer/src/signatures/legacy/registry.rs
@@ -18,21 +18,24 @@ pub enum SignatureResult<'a> {
 
 type SignatureLookupFn = dyn 'static + Send + Sync + Fn(&str) -> Option<Signature>;
 
-/// The longest a command name can be before we refuse to cache a lookup for it.
+/// The longest a command name can be before `get` skips resolving it against the (uncached)
+/// `lookup_fn` and instead falls back to a linear scan of already-cached signatures.
 ///
 /// `SignatureCache::get` is called with arbitrary tokens taken from the terminal input line (see
 /// `CommandRegistry::signature_from_tokens` and friends), not just tokens we already know to be
-/// real command names. Since `SignatureCache::signatures` is append-only (see its doc comment for
-/// why), every distinct token ever looked up would otherwise be cached forever, which lets a
-/// single pathologically large token (e.g. a large blob of text pasted into the terminal input
-/// line) permanently retain an equally large `String` for the lifetime of the process. See
+/// real command names. `SignatureCache::signatures` only caches successful lookups (see its doc
+/// comment for why), so a single pathologically large token (e.g. a large blob of text pasted
+/// into the terminal input line) can't grow the cache -- but lowercasing such a token on every
+/// keystroke would still be wasted work, so `get` avoids that too for tokens this long. See
 /// APP-5431.
 ///
 /// Real command names are executable names, which common filesystems cap at 255 bytes/characters
-/// (e.g. Linux `NAME_MAX`, macOS APFS/HFS+, and Windows NTFS all use this limit). No legitimate
-/// command name can exceed this, so a token longer than this cannot resolve to a real signature
-/// and is safe to skip caching (and looking up) entirely. As a sanity check, the longest name in
-/// the current embedded signature corpus is 43 characters, well under this cap.
+/// (e.g. Linux `NAME_MAX`, macOS APFS/HFS+, and Windows NTFS all use this limit), so no
+/// legitimate *dynamically resolved* command name can exceed this. An explicitly `insert`-ed
+/// signature is not subject to that constraint, which is why `get` falls back to a scan instead
+/// of simply refusing to resolve tokens this long -- that keeps such a signature retrievable. As
+/// a sanity check, the longest name in the current embedded signature corpus is 43 characters,
+/// well under this cap.
 const MAX_CACHEABLE_COMMAND_LEN: usize = 255;
 
 /// A simple structure to cache parsed command signatures.  These are stored as
@@ -44,12 +47,14 @@ struct SignatureCache {
     /// for it.  Should return None if there is no signature available for the
     /// given command.
     lookup_fn: Box<SignatureLookupFn>,
-    /// A map from command name to the signature for the command, if any.  The
-    /// use of [`MemoMap`] here allows us to safely return references to the
-    /// contained signatures (as the map internally is an append-only
-    /// structure).  This stores an `Option<Signature>` in order to also store
-    /// our knowledge of commands for which we do _not_ have a signature.
-    signatures: MemoMap<String, Option<Signature>>,
+    /// A map from (lowercased) command name to its signature. The use of [`MemoMap`] here allows
+    /// us to safely return references to the contained signatures (as the map internally is an
+    /// append-only structure). Only lookups that actually resolve to a signature are stored (see
+    /// `get`), which keeps this bounded by the number of distinct command names that can ever
+    /// resolve to something -- the fixed embedded corpus, plus any explicitly `insert`-ed
+    /// signatures -- rather than by every distinct token a user has ever typed or pasted. See
+    /// APP-5431.
+    signatures: MemoMap<String, Signature>,
 }
 
 impl SignatureCache {
@@ -66,25 +71,37 @@ impl SignatureCache {
         } else {
             command
         };
+
         if command.len() > MAX_CACHEABLE_COMMAND_LEN {
-            // No real command name can be this long (see `MAX_CACHEABLE_COMMAND_LEN`), so this
-            // can't resolve to a signature. Bail out before caching (or even looking up) it to
-            // keep the cache bounded; see APP-5431.
-            return None;
+            // Avoid allocating a lowercase copy of a pathologically large token (see
+            // `MAX_CACHEABLE_COMMAND_LEN`) by scanning the cache directly instead of hashing into
+            // it. The length comparison short-circuits before any case folding, so this stays
+            // cheap even though `command` may be huge, and it keeps an `insert`-ed signature
+            // whose name happens to exceed the cap retrievable.
+            return self
+                .signatures
+                .iter()
+                .find(|(key, _)| key.len() == command.len() && key.eq_ignore_ascii_case(command))
+                .map(|(_, signature)| signature);
         }
+
         let command = command.to_lowercase();
+        // Only cache a lookup that actually resolves to a signature. `lookup_fn` probes the
+        // embedded, already-compiled signature corpus, which is cheap even on a miss (JSON
+        // parsing only happens once we know there's a hit), so there's no need to memoize a miss
+        // just to avoid redoing that work -- and doing so is what let every distinct token a user
+        // has ever typed grow this cache forever. See APP-5431.
         self.signatures
-            .get_or_insert(&command, || (self.lookup_fn)(&command))
-            .as_ref()
+            .get_or_try_insert(command.as_str(), || (self.lookup_fn)(&command).ok_or(()))
+            .ok()
     }
 
     /// Inserts the given `Signature` into the underlying map, keyed by `Signature::name`.
     ///
-    /// If there is already a cached value for the given `Signature::name`, this is a no-op (even
-    /// if the cached value is `None`).
+    /// If there is already a cached value for the given `Signature::name`, this is a no-op.
     fn insert(&self, signature: Signature) {
         self.signatures
-            .insert(signature.name.to_lowercase(), Some(signature));
+            .insert(signature.name.to_lowercase(), signature);
     }
 }
 
@@ -149,12 +166,13 @@ impl CommandRegistry {
     pub fn registered_commands(&self) -> impl Iterator<Item = &str> {
         // Note we need to collect the keys because MemoMap uses a mutex under the hood to control
         // access to the underlying signature data. This means the mutex is locked as long as the
-        // iterator returned from `keys()` lives, which means we need to collect keys into a vec
-        // and return an owned iterator.
+        // iterator returned from `iter()` lives, which means we need to collect keys into a vec
+        // and return an owned iterator. Every entry in `signatures` corresponds to a real
+        // signature (see its doc comment), so no filtering is needed here.
         self.signatures
             .signatures
             .iter()
-            .filter_map(|(key, signature)| signature.as_ref().map(|_| key.as_str()))
+            .map(|(key, _)| key.as_str())
             .collect::<Vec<_>>()
             .into_iter()
     }
diff --git a/crates/warp_completer/src/signatures/legacy/registry_tests.rs b/crates/warp_completer/src/signatures/legacy/registry_tests.rs
index c923db72aa5..5b42318a3cc 100644
--- a/crates/warp_completer/src/signatures/legacy/registry_tests.rs
+++ b/crates/warp_completer/src/signatures/legacy/registry_tests.rs
@@ -1,8 +1,25 @@
+use warp_command_signatures::{Priority, Signature};
+
 use crate::completer::testing::FakeCompletionContext;
 use crate::completer::{CompletionContext, TopLevelCommandCaseSensitivity};
 use crate::signatures::registry::{MAX_CACHEABLE_COMMAND_LEN, SignatureResult};
 use crate::signatures::testing::{create_test_command_registry, test_signature};
 
+/// A minimal signature with the given `name`, for exercising `SignatureCache` boundary
+/// conditions that don't care about arguments, subcommands, or options.
+fn signature_with_name(name: &str) -> Signature {
+    Signature {
+        name: name.to_string(),
+        alias_generator: None,
+        description: None,
+        arguments: None,
+        subcommands: None,
+        options: None,
+        priority: Priority::default(),
+        parser_directives: Default::default(),
+    }
+}
+
 #[test]
 fn test_find_command_from_a_top_level_signature() {
     let bundle = warp_command_signatures::signature_by_name("bundle")
@@ -208,7 +225,7 @@ fn test_alias_expansion_path_skips_flag_with_value_before_subcommand() {
 fn test_oversized_command_is_not_cached_and_resolves_to_none() {
     // Regression test for APP-5431: a pathologically large single "command" token (e.g. a large
     // blob of text pasted into the terminal input line) must not grow the (append-only)
-    // SignatureCache.
+    // SignatureCache, and doesn't match any registered signature.
     let registry = create_test_command_registry([test_signature()]);
     let len_before = registry.signatures.signatures.len();
 
@@ -219,52 +236,63 @@ fn test_oversized_command_is_not_cached_and_resolves_to_none() {
         len_before,
         "looking up an oversized command should not add an entry to the cache"
     );
+}
+
+#[test]
+fn test_misses_are_never_cached() {
+    // Regression test for APP-5431: the cache only grows via successful lookups (see
+    // `SignatureCache::signatures`'s doc comment), so a command that never resolves to a
+    // signature must not add an entry, however many times it's looked up.
+    let registry = create_test_command_registry([test_signature()]);
+    let len_before = registry.signatures.signatures.len();
 
-    // A command name right at the cap is still a legitimate lookup and should be cached as usual
-    // (as a negative-cache entry here, since it doesn't match any registered signature).
-    let max_length_command = "a".repeat(MAX_CACHEABLE_COMMAND_LEN);
-    assert_eq!(registry.signature(&max_length_command), None);
-    assert_eq!(registry.signatures.signatures.len(), len_before + 1);
+    assert_eq!(registry.signature("not-a-real-command"), None);
+    assert_eq!(registry.signature("not-a-real-command"), None);
+    assert_eq!(registry.signatures.signatures.len(), len_before);
 }
 
 #[test]
 fn test_ordinary_commands_still_resolve_and_are_cached() {
     let registry = create_test_command_registry([test_signature()]);
 
-    // A registered command resolves correctly, case-insensitively, and repeated lookups hit the
-    // same cached entry rather than growing the cache.
+    // A registered command resolves correctly, case-insensitively.
     let found = registry.signature("TEST");
     assert_eq!(found.map(|s| s.name.as_str()), Some("test"));
-    let len_after_positive_lookup = registry.signatures.signatures.len();
+
+    // Repeated lookups hit the same cached entry rather than growing the cache.
+    let len_after_first_lookup = registry.signatures.signatures.len();
     assert_eq!(registry.signature("test"), found);
-    assert_eq!(
-        registry.signatures.signatures.len(),
-        len_after_positive_lookup
-    );
+    assert_eq!(registry.signatures.signatures.len(), len_after_first_lookup);
+}
+
+#[test]
+fn test_registered_signature_longer_than_the_cap_still_resolves() {
+    // Regression test for APP-5431 (review finding): an explicitly registered signature must
+    // stay retrievable through `signature()` even if its name exceeds `MAX_CACHEABLE_COMMAND_LEN`
+    // -- the cap only governs whether a *dynamic* (uncached) lookup is attempted, not whether an
+    // already-registered signature can be found.
+    let long_name = "a".repeat(MAX_CACHEABLE_COMMAND_LEN + 1);
+    let registry = create_test_command_registry([signature_with_name(&long_name)]);
 
-    // Negative lookups are cached too, as before: the first miss adds an entry, and repeating it
-    // hits that cached entry rather than growing the cache further.
-    let len_before_negative_lookup = registry.signatures.signatures.len();
-    assert_eq!(registry.signature("not-a-real-command"), None);
-    assert_eq!(
-        registry.signatures.signatures.len(),
-        len_before_negative_lookup + 1
-    );
-    assert_eq!(registry.signature("not-a-real-command"), None);
     assert_eq!(
-        registry.signatures.signatures.len(),
-        len_before_negative_lookup + 1
+        registry.signature(&long_name).map(|s| s.name.as_str()),
+        Some(long_name.as_str())
     );
 }
 
 #[cfg(windows)]
 #[test]
 fn test_exe_suffix_is_trimmed_before_the_length_check() {
-    let registry = create_test_command_registry([test_signature()]);
+    // Regression test for APP-5431 (review finding): the ".exe" suffix must be trimmed *before*
+    // the length check runs. A command name exactly at the cap, looked up with ".exe" appended
+    // (so the raw token exceeds the cap), must still resolve via the normal lookup path.
+    let max_length_name = "a".repeat(MAX_CACHEABLE_COMMAND_LEN);
+    let registry = create_test_command_registry([signature_with_name(&max_length_name)]);
 
+    let token = format!("{max_length_name}.exe");
     assert_eq!(
-        registry.signature("test.exe").map(|s| s.name.as_str()),
-        Some("test")
+        registry.signature(&token).map(|s| s.name.as_str()),
+        Some(max_length_name.as_str())
     );
 }
 

From dfa2a04ae36409feef45b109b02e986ebf8ec2e3 Mon Sep 17 00:00:00 2001
From: "warp-agent-staging[bot]"
 <240773466+warp-agent-staging[bot]@users.noreply.github.com>
Date: Tue, 18 Aug 2026 00:57:20 +0000
Subject: [PATCH 3/7] Split SignatureCache into a bounded LRU miss cache plus
 the positive-only map (APP-5431)

The requester asked to keep the key-length cap from the previous revision,
but bound the cache with a true LRU instead of relying on "the set of
resolvable command names is finite". A follow-up from the requester then
changed direction away from an `Arc<Signature>`-based unified LRU (which
would have required `SignatureCache::get` to hand back an owned handle
instead of `Option<&Signature>`) towards keeping `signatures` untouched
and adding a second, bounded structure for misses only:

- `signatures: MemoMap<String, Signature>` is unchanged -- still
  append-only, still handing back `Option<&Signature>` borrowed from
  inside the map, so `SignatureAtTokenIndex`'s `&'a Signature` keeps
  working with no API change. It stays bounded by the number of distinct
  command names that can ever resolve to something (the embedded corpus
  of 1,167 top-level commands, plus explicitly `register_signature`-ed
  names), not by every token a user has ever typed.
- `misses: Mutex<LruCache<String, ()>>` is new: a bounded LRU set of
  tokens that recently failed to resolve. Because `get` only ever
  returns a `bool` from this set (no reference escapes), evicting from
  it is sound -- unlike `signatures`, there's no outstanding borrow that
  eviction could invalidate. Capacity is `MAX_CACHED_MISSES = 256`.
- The existing `MAX_CACHEABLE_COMMAND_LEN` length cap now also gates the
  negative cache: an oversized token skips both `signatures` and
  `misses` entirely, so the leak this PR exists to fix can't just move
  into the new negative cache.

Uses the `lru` crate, which was already vendored transitively (via
`ratatui-core`/`tantivy`) at 0.18.0, rather than adding a new dependency
version.

Co-Authored-By: Warp <agent@warp.dev>
---
 Cargo.lock                                    |  1 +
 Cargo.toml                                    |  1 +
 crates/warp_completer/Cargo.toml              |  1 +
 .../src/signatures/legacy/registry.rs         | 75 ++++++++++++++----
 .../src/signatures/legacy/registry_tests.rs   | 79 +++++++++++++++++--
 5 files changed, 134 insertions(+), 23 deletions(-)

diff --git a/Cargo.lock b/Cargo.lock
index b0cb1dd77a5..b77e385d3ce 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -15521,6 +15521,7 @@ dependencies = [
  "itertools 0.14.0",
  "lazy_static",
  "log",
+ "lru 0.18.0",
  "memo-map",
  "regex",
  "rquickjs",
diff --git a/Cargo.toml b/Cargo.toml
index 40ade89b60a..a7e4c729474 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -197,6 +197,7 @@ libc = "0.2.81"
 libtest-mimic = "0.8.2"
 line-ending = "1.4.0"
 log = { version = "0.4", features = ["serde", "std"] }
+lru = "0.18.0"
 mermaid_to_svg = { git = "https://github.com/warpdotdev/mermaid-to-svg.git", rev = "8d3f789c2eb49335d7bf247a06bb649f59b6d4ed" }
 mime_guess = "2.0"
 minimp4 = "0.1.2"
diff --git a/crates/warp_completer/Cargo.toml b/crates/warp_completer/Cargo.toml
index aae54e8474c..7e14c482e82 100644
--- a/crates/warp_completer/Cargo.toml
+++ b/crates/warp_completer/Cargo.toml
@@ -23,6 +23,7 @@ instant.workspace = true
 itertools.workspace = true
 lazy_static.workspace = true
 log.workspace = true
+lru.workspace = true
 memo-map.workspace = true
 regex.workspace = true
 serde.workspace = true
diff --git a/crates/warp_completer/src/signatures/legacy/registry.rs b/crates/warp_completer/src/signatures/legacy/registry.rs
index cb3903bf6a8..62d8aa17f48 100644
--- a/crates/warp_completer/src/signatures/legacy/registry.rs
+++ b/crates/warp_completer/src/signatures/legacy/registry.rs
@@ -1,6 +1,9 @@
 use std::collections::HashMap;
+use std::num::NonZeroUsize;
+use std::sync::Mutex;
 
 use itertools::Itertools;
+use lru::LruCache;
 use memo_map::MemoMap;
 use warp_command_signatures::{Argument, DynamicCompletionData, IsArgumentOptional, Signature};
 
@@ -25,9 +28,10 @@ type SignatureLookupFn = dyn 'static + Send + Sync + Fn(&str) -> Option<Signatur
 /// `CommandRegistry::signature_from_tokens` and friends), not just tokens we already know to be
 /// real command names. `SignatureCache::signatures` only caches successful lookups (see its doc
 /// comment for why), so a single pathologically large token (e.g. a large blob of text pasted
-/// into the terminal input line) can't grow the cache -- but lowercasing such a token on every
-/// keystroke would still be wasted work, so `get` avoids that too for tokens this long. See
-/// APP-5431.
+/// into the terminal input line) can't grow that map -- but lowercasing such a token on every
+/// keystroke would still be wasted work, and it must also never be admitted into
+/// `SignatureCache::misses` (see its doc comment for why), so `get` skips both for tokens this
+/// long. See APP-5431.
 ///
 /// Real command names are executable names, which common filesystems cap at 255 bytes/characters
 /// (e.g. Linux `NAME_MAX`, macOS APFS/HFS+, and Windows NTFS all use this limit), so no
@@ -38,6 +42,14 @@ type SignatureLookupFn = dyn 'static + Send + Sync + Fn(&str) -> Option<Signatur
 /// well under this cap.
 const MAX_CACHEABLE_COMMAND_LEN: usize = 255;
 
+/// Capacity of `SignatureCache::misses`. Kept small: the negative cache exists to save
+/// `lookup_fn` calls for a token that's been retried a handful of times in quick succession
+/// (e.g. a typo the user is correcting, or the same invalid token looked up again as the parser
+/// backtracks over the line), not to remember every miss for the life of the session. See
+/// `SignatureCache::misses`'s doc comment for why unbounded retention isn't needed for
+/// correctness.
+const MAX_CACHED_MISSES: usize = 256;
+
 /// A simple structure to cache parsed command signatures.  These are stored as
 /// JSON, so this makes it easy for us to lazily load and parse the JSON when
 /// a command signature is needed, and only need to do that parsing work once
@@ -51,10 +63,21 @@ struct SignatureCache {
     /// us to safely return references to the contained signatures (as the map internally is an
     /// append-only structure). Only lookups that actually resolve to a signature are stored (see
     /// `get`), which keeps this bounded by the number of distinct command names that can ever
-    /// resolve to something -- the fixed embedded corpus, plus any explicitly `insert`-ed
-    /// signatures -- rather than by every distinct token a user has ever typed or pasted. See
-    /// APP-5431.
+    /// resolve to something -- the fixed embedded corpus (1,167 top-level commands as of this
+    /// writing), plus any explicitly `insert`-ed signatures -- rather than by every distinct
+    /// token a user has ever typed or pasted. See APP-5431.
     signatures: MemoMap<String, Signature>,
+    /// A bounded LRU set of (lowercased) command names that recently failed to resolve to a
+    /// signature. Unlike `signatures`, `get` never hands out a reference into this set -- it only
+    /// ever returns a `bool` -- so eviction is sound: there's no borrow that could outlive an
+    /// evicted entry, which is exactly what keeps `signatures` itself append-only. That's also
+    /// what makes this bounded convenience rather than a fix for a performance problem: a miss is
+    /// already cheap to redo (`lookup_fn` probes the embedded, already-compiled signature corpus,
+    /// which does a binary search with no filesystem I/O or JSON parsing on a miss -- JSON
+    /// parsing only happens once we know there's a hit), so this cache exists to save a handful
+    /// of redundant lookups for a token that's retried in quick succession, not to avoid
+    /// expensive rework. See APP-5431.
+    misses: Mutex<LruCache<String, ()>>,
 }
 
 impl SignatureCache {
@@ -62,6 +85,9 @@ impl SignatureCache {
         Self {
             lookup_fn,
             signatures: Default::default(),
+            misses: Mutex::new(LruCache::new(
+                NonZeroUsize::new(MAX_CACHED_MISSES).expect("MAX_CACHED_MISSES is non-zero"),
+            )),
         }
     }
 
@@ -77,7 +103,9 @@ impl SignatureCache {
             // `MAX_CACHEABLE_COMMAND_LEN`) by scanning the cache directly instead of hashing into
             // it. The length comparison short-circuits before any case folding, so this stays
             // cheap even though `command` may be huge, and it keeps an `insert`-ed signature
-            // whose name happens to exceed the cap retrievable.
+            // whose name happens to exceed the cap retrievable. This never touches `misses`: an
+            // oversized token must never be admitted there either, or the leak this cap exists to
+            // prevent would just move to the negative cache.
             return self
                 .signatures
                 .iter()
@@ -86,14 +114,31 @@ impl SignatureCache {
         }
 
         let command = command.to_lowercase();
-        // Only cache a lookup that actually resolves to a signature. `lookup_fn` probes the
-        // embedded, already-compiled signature corpus, which is cheap even on a miss (JSON
-        // parsing only happens once we know there's a hit), so there's no need to memoize a miss
-        // just to avoid redoing that work -- and doing so is what let every distinct token a user
-        // has ever typed grow this cache forever. See APP-5431.
-        self.signatures
-            .get_or_try_insert(command.as_str(), || (self.lookup_fn)(&command).ok_or(()))
-            .ok()
+
+        if let Some(signature) = self.signatures.get(command.as_str()) {
+            return Some(signature);
+        }
+
+        if self.lock_misses().get(command.as_str()).is_some() {
+            return None;
+        }
+
+        match (self.lookup_fn)(&command) {
+            Some(signature) => Some(
+                self.signatures
+                    .get_or_insert(command.as_str(), || signature),
+            ),
+            None => {
+                self.lock_misses().put(command, ());
+                None
+            }
+        }
+    }
+
+    fn lock_misses(&self) -> std::sync::MutexGuard<'_, LruCache<String, ()>> {
+        self.misses
+            .lock()
+            .unwrap_or_else(|poisoned| poisoned.into_inner())
     }
 
     /// Inserts the given `Signature` into the underlying map, keyed by `Signature::name`.
diff --git a/crates/warp_completer/src/signatures/legacy/registry_tests.rs b/crates/warp_completer/src/signatures/legacy/registry_tests.rs
index 5b42318a3cc..09ec3a550aa 100644
--- a/crates/warp_completer/src/signatures/legacy/registry_tests.rs
+++ b/crates/warp_completer/src/signatures/legacy/registry_tests.rs
@@ -2,7 +2,7 @@ use warp_command_signatures::{Priority, Signature};
 
 use crate::completer::testing::FakeCompletionContext;
 use crate::completer::{CompletionContext, TopLevelCommandCaseSensitivity};
-use crate::signatures::registry::{MAX_CACHEABLE_COMMAND_LEN, SignatureResult};
+use crate::signatures::registry::{MAX_CACHEABLE_COMMAND_LEN, MAX_CACHED_MISSES, SignatureResult};
 use crate::signatures::testing::{create_test_command_registry, test_signature};
 
 /// A minimal signature with the given `name`, for exercising `SignatureCache` boundary
@@ -225,24 +225,32 @@ fn test_alias_expansion_path_skips_flag_with_value_before_subcommand() {
 fn test_oversized_command_is_not_cached_and_resolves_to_none() {
     // Regression test for APP-5431: a pathologically large single "command" token (e.g. a large
     // blob of text pasted into the terminal input line) must not grow the (append-only)
-    // SignatureCache, and doesn't match any registered signature.
+    // positive cache, nor the bounded negative cache, and doesn't match any registered
+    // signature.
     let registry = create_test_command_registry([test_signature()]);
-    let len_before = registry.signatures.signatures.len();
+    let positive_len_before = registry.signatures.signatures.len();
+    let negative_len_before = registry.signatures.misses.lock().unwrap().len();
 
     let oversized_command = "a".repeat(MAX_CACHEABLE_COMMAND_LEN + 1);
     assert_eq!(registry.signature(&oversized_command), None);
     assert_eq!(
         registry.signatures.signatures.len(),
-        len_before,
-        "looking up an oversized command should not add an entry to the cache"
+        positive_len_before,
+        "looking up an oversized command should not add an entry to the positive cache"
+    );
+    assert_eq!(
+        registry.signatures.misses.lock().unwrap().len(),
+        negative_len_before,
+        "looking up an oversized command should not add an entry to the negative cache either -- \
+         that would just move the leak this cap exists to fix into the negative cache"
     );
 }
 
 #[test]
-fn test_misses_are_never_cached() {
-    // Regression test for APP-5431: the cache only grows via successful lookups (see
+fn test_misses_are_never_cached_in_the_positive_cache() {
+    // Regression test for APP-5431: the positive cache only grows via successful lookups (see
     // `SignatureCache::signatures`'s doc comment), so a command that never resolves to a
-    // signature must not add an entry, however many times it's looked up.
+    // signature must not add an entry there, however many times it's looked up.
     let registry = create_test_command_registry([test_signature()]);
     let len_before = registry.signatures.signatures.len();
 
@@ -251,6 +259,61 @@ fn test_misses_are_never_cached() {
     assert_eq!(registry.signatures.signatures.len(), len_before);
 }
 
+#[test]
+fn test_negative_cache_stops_growing_at_capacity() {
+    // Regression test for APP-5431: the negative cache (`SignatureCache::misses`) is a bounded
+    // LRU set, so looking up more distinct misses than `MAX_CACHED_MISSES` must not grow it
+    // past that capacity.
+    let registry = create_test_command_registry([test_signature()]);
+
+    for i in 0..MAX_CACHED_MISSES * 2 {
+        assert_eq!(registry.signature(&format!("not-a-real-command-{i}")), None);
+        assert!(registry.signatures.misses.lock().unwrap().len() <= MAX_CACHED_MISSES);
+    }
+    assert_eq!(
+        registry.signatures.misses.lock().unwrap().len(),
+        MAX_CACHED_MISSES
+    );
+}
+
+#[test]
+fn test_negative_cache_evicts_in_lru_order() {
+    // Regression test for APP-5431: once the negative cache is at capacity, inserting a new
+    // miss must evict the least-recently-used entry, not an arbitrary one.
+    let registry = create_test_command_registry([test_signature()]);
+
+    // Fill the negative cache to capacity with "miss-0", .., "miss-{MAX_CACHED_MISSES - 1}".
+    for i in 0..MAX_CACHED_MISSES {
+        assert_eq!(registry.signature(&format!("miss-{i}")), None);
+    }
+
+    // Touch every entry except "miss-0" again, moving them all ahead of it in LRU order.
+    for i in 1..MAX_CACHED_MISSES {
+        assert_eq!(registry.signature(&format!("miss-{i}")), None);
+    }
+
+    // Inserting one more miss should evict "miss-0", the least-recently-used entry.
+    assert_eq!(registry.signature("miss-overflow"), None);
+    assert!(
+        !registry
+            .signatures
+            .misses
+            .lock()
+            .unwrap()
+            .contains("miss-0"),
+        "the least-recently-used entry should have been evicted"
+    );
+    assert!(
+        registry
+            .signatures
+            .misses
+            .lock()
+            .unwrap()
+            .contains("miss-1"),
+        "a recently-touched entry should not have been evicted"
+    );
+}
+
 #[test]
 fn test_ordinary_commands_still_resolve_and_are_cached() {
     let registry = create_test_command_registry([test_signature()]);

From 7a8ad8bbbc70a81446494df81f7688330e2af286 Mon Sep 17 00:00:00 2001
From: "warp-agent-staging[bot]"
 <240773466+warp-agent-staging[bot]@users.noreply.github.com>
Date: Tue, 18 Aug 2026 01:15:29 +0000
Subject: [PATCH 4/7] Address second-pass review findings on the split-cache
 design

- Add a regression test proving an oversized *later* token (e.g. the
  argument to `sudo`, resolved via `maybe_load_replacement_signature`)
  takes the same length-guarded path as an oversized top-level token.
  `self.signatures` there is `CommandRegistry`'s `SignatureCache` field,
  not the raw `MemoMap`, so it already routed through the guarded
  `SignatureCache::get` -- this locks that guarantee in with a test
  covering both `signature_from_tokens` and `signature_with_alias_expansion`.
- Fix `test_negative_cache_evicts_in_lru_order`, which previously only
  re-touched every entry except the oldest and then asserted the oldest
  was evicted -- indistinguishable from FIFO eviction. It now re-touches
  only the oldest entry before overflowing and asserts that entry
  survives while the next-oldest (now genuinely least-recently-used) is
  evicted instead, which a FIFO or arbitrary-eviction cache would fail.
- Fix the stale `register_signature` doc comment, which still described
  the old "cached `None` blocks registration" behavior from before
  misses were split into their own bounded structure.

Co-Authored-By: Warp <agent@warp.dev>
---
 .../src/signatures/legacy/registry.rs         | 10 ++-
 .../src/signatures/legacy/registry_tests.rs   | 83 ++++++++++++++++---
 2 files changed, 78 insertions(+), 15 deletions(-)

diff --git a/crates/warp_completer/src/signatures/legacy/registry.rs b/crates/warp_completer/src/signatures/legacy/registry.rs
index 62d8aa17f48..bbc35048ee0 100644
--- a/crates/warp_completer/src/signatures/legacy/registry.rs
+++ b/crates/warp_completer/src/signatures/legacy/registry.rs
@@ -497,11 +497,13 @@ impl CommandRegistry {
         self.signatures.get(name)
     }
 
-    /// Registers the given `Signature`.
+    /// Registers the given `Signature`, making it resolvable via `signature()` and friends.
     ///
-    /// Note the underlying map caches the lookup result for a given signature (regardless of
-    /// whether or not it is `Some` or `None`), which means that if there is already a cached
-    /// `None` value for the command corresponding to this signature, this is a no-op.
+    /// `get` always checks the positive cache before the negative one (see
+    /// `SignatureCache::misses`), so this takes effect immediately even if a lookup for this
+    /// name previously missed and is currently sitting in the negative cache -- there's no
+    /// stale `None` result to invalidate. If a signature is already registered for this name,
+    /// this is a no-op.
     pub fn register_signature(&self, signature: Signature) {
         self.signatures.insert(signature);
     }
diff --git a/crates/warp_completer/src/signatures/legacy/registry_tests.rs b/crates/warp_completer/src/signatures/legacy/registry_tests.rs
index 09ec3a550aa..5a1a1d64ace 100644
--- a/crates/warp_completer/src/signatures/legacy/registry_tests.rs
+++ b/crates/warp_completer/src/signatures/legacy/registry_tests.rs
@@ -279,41 +279,102 @@ fn test_negative_cache_stops_growing_at_capacity() {
 #[test]
 fn test_negative_cache_evicts_in_lru_order() {
     // Regression test for APP-5431: once the negative cache is at capacity, inserting a new
-    // miss must evict the least-recently-used entry, not an arbitrary one.
+    // miss must evict the least-recently-used entry specifically. Critically, this re-touches
+    // the *oldest-inserted* entry before overflowing so a FIFO (or arbitrary-eviction) cache
+    // would fail this test: a FIFO cache would still evict "miss-0" since it only tracks
+    // insertion order and ignores the re-touch, whereas an LRU cache must evict "miss-1"
+    // instead, since "miss-1" is now the actual least-recently-used entry.
     let registry = create_test_command_registry([test_signature()]);
 
-    // Fill the negative cache to capacity with "miss-0", .., "miss-{MAX_CACHED_MISSES - 1}".
+    // Fill the negative cache to capacity with "miss-0", .., "miss-{MAX_CACHED_MISSES - 1}",
+    // in insertion order.
     for i in 0..MAX_CACHED_MISSES {
         assert_eq!(registry.signature(&format!("miss-{i}")), None);
     }
 
-    // Touch every entry except "miss-0" again, moving them all ahead of it in LRU order.
-    for i in 1..MAX_CACHED_MISSES {
-        assert_eq!(registry.signature(&format!("miss-{i}")), None);
-    }
+    // Touch only "miss-0" -- the oldest, and thus the next entry a FIFO cache would evict --
+    // moving it to the most-recently-used position.
+    assert_eq!(registry.signature("miss-0"), None);
 
-    // Inserting one more miss should evict "miss-0", the least-recently-used entry.
+    // Inserting one more miss should evict "miss-1", the least-recently-used entry, not
+    // "miss-0", which was just touched.
     assert_eq!(registry.signature("miss-overflow"), None);
     assert!(
-        !registry
+        registry
             .signatures
             .misses
             .lock()
             .unwrap()
             .contains("miss-0"),
-        "the least-recently-used entry should have been evicted"
+        "the just-touched entry should not have been evicted -- a FIFO cache would incorrectly \
+         evict it instead of `miss-1`"
     );
     assert!(
-        registry
+        !registry
             .signatures
             .misses
             .lock()
             .unwrap()
             .contains("miss-1"),
-        "a recently-touched entry should not have been evicted"
+        "the least-recently-used entry should have been evicted"
     );
 }
 
+#[test]
+fn test_oversized_later_token_does_not_bypass_the_length_guard() {
+    // Regression test for APP-5431: `maybe_load_replacement_signature` resolves a *later*
+    // token (e.g. resolving `git` from `sudo git`) through the same `SignatureCache::get` the
+    // top-level token goes through (`self.signatures.get(token)`, where `self.signatures` is
+    // the `SignatureCache`, not the raw `MemoMap`). An oversized later token must take the same
+    // length-guarded path as an oversized top-level token: no lowercase allocation, and no
+    // growth of either cache.
+    let sudo = warp_command_signatures::signature_by_name("sudo")
+        .expect("global command signatures should include 'sudo'");
+    let registry = create_test_command_registry([sudo]);
+    let positive_len_before = registry.signatures.signatures.len();
+    let negative_len_before = registry.signatures.misses.lock().unwrap().len();
+
+    let oversized_token = "a".repeat(MAX_CACHEABLE_COMMAND_LEN + 1);
+    let tokens = ["sudo", oversized_token.as_str()];
+
+    let found_signature = registry
+        .signature_from_tokens(
+            &tokens,
+            false,
+            TopLevelCommandCaseSensitivity::CaseSensitive,
+        )
+        .expect("sudo signature from tokens should exist");
+    assert_eq!(
+        found_signature.signature.name(),
+        "sudo",
+        "an oversized later token shouldn't resolve to a replacement signature"
+    );
+    assert_eq!(
+        registry.signatures.signatures.len(),
+        positive_len_before,
+        "looking up an oversized later token should not add an entry to the positive cache"
+    );
+    assert_eq!(
+        registry.signatures.misses.lock().unwrap().len(),
+        negative_len_before,
+        "looking up an oversized later token should not add an entry to the negative cache"
+    );
+
+    // Exercise the alias-expansion path too, since it resolves later tokens the same way.
+    let sudo = warp_command_signatures::signature_by_name("sudo")
+        .expect("global command signatures should include 'sudo'");
+    let ctx =
+        FakeCompletionContext::new(create_test_command_registry([sudo])).with_case_sensitivity();
+    let result = warpui_core::r#async::block_on(
+        ctx.command_registry()
+            .signature_with_alias_expansion(&tokens, false, &ctx),
+    );
+    let SignatureResult::Success(found_signature) = result else {
+        panic!("expected SignatureResult::Success");
+    };
+    assert_eq!(found_signature.signature.name(), "sudo");
+}
+
 #[test]
 fn test_ordinary_commands_still_resolve_and_are_cached() {
     let registry = create_test_command_registry([test_signature()]);

From c04263ff3077e7e3fd81eb2f0f3725aa81a598bd Mon Sep 17 00:00:00 2001
From: "warp-agent-staging[bot]"
 <240773466+warp-agent-staging[bot]@users.noreply.github.com>
Date: Tue, 18 Aug 2026 19:00:24 +0000
Subject: [PATCH 5/7] Drop the oversized-token fallback scan; replace the
 misses Mutex with a FIFO RwLock set

Two follow-ups from the requester:

1. "Don't we know for sure none of our signatures have command names
   longer than MAX_CACHEABLE_COMMAND_LEN?" -- measured it rather than
   assuming: walked every embedded signature (recursively, including
   subcommands) plus the Warp CLI's own clap-derived command tree (the
   only other source of `register_signature`-ed names in production).
   The longest name found is 59 characters (a compound dig/dog
   subcommand name), well under the 255-byte cap. Added
   `test_all_known_signature_names_are_within_the_length_cap`, which
   walks both sources and fails loudly if a future signature ever
   violates this. On the strength of that invariant, `SignatureCache::get`
   now just returns `None` for an oversized token instead of falling
   back to a linear scan -- simpler, and sheds the "compare length
   before case-folding" machinery the scan needed. An explicitly
   `register_signature`-ed signature with an artificially oversized
   name would become unresolvable via lookup; that's a deliberate,
   tested tradeoff, not a real-world regression.

2. "I don't like the Mutex on the cached misses. can we achieve
   interior mutability some other way?" -- first checked whether
   `SignatureCache` is genuinely shared across threads: yes,
   `CommandRegistry::global_instance()` is a single `Arc` shared across
   every terminal session/pane, each generating completions on a
   background thread pool, so this needs real cross-thread
   synchronization, not just single-task interior mutability. But a
   negative cache is inherently approximate -- a false negative just
   costs one extra, cheap `lookup_fn` call, never a wrong answer -- so
   an LRU's strict recency tracking (which needs a write lock on every
   *hit*, to move the entry to the front) is more machinery than the
   requirement calls for. Replaced `Mutex<lru::LruCache<..>>` with a
   hand-rolled `MissCache`: a `RwLock`-guarded FIFO (a `VecDeque` for
   insertion order plus a `HashSet` for O(1) membership checks). A hit
   is now a pure read (shared read-lock, no mutation), and the write
   lock is only taken for a genuinely new miss. This also drops the
   `lru` crate as a dependency entirely. The tradeoff: eviction order is
   now FIFO instead of LRU, so a miss that's looked up again doesn't get
   a second life once the cache is full -- an accepted loss of precision
   for a cache that's explicitly a convenience, not a correctness
   mechanism.

Rewrote `test_negative_cache_evicts_in_lru_order` (which didn't
actually distinguish LRU from FIFO) into
`test_negative_cache_evicts_in_fifo_order`, replaced
`test_registered_signature_longer_than_the_cap_still_resolves` with
`test_registered_signature_longer_than_the_cap_is_unresolvable` to
match the new behavior, and updated the PR-touched doc comments
throughout to describe the current design.

Co-Authored-By: Warp <agent@warp.dev>
---
 Cargo.lock                                    |   1 -
 Cargo.toml                                    |   1 -
 crates/warp_completer/Cargo.toml              |   1 -
 .../src/signatures/legacy/registry.rs         | 163 ++++++++++++------
 .../src/signatures/legacy/registry_tests.rs   | 131 +++++++++-----
 5 files changed, 196 insertions(+), 101 deletions(-)

diff --git a/Cargo.lock b/Cargo.lock
index b77e385d3ce..b0cb1dd77a5 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -15521,7 +15521,6 @@ dependencies = [
  "itertools 0.14.0",
  "lazy_static",
  "log",
- "lru 0.18.0",
  "memo-map",
  "regex",
  "rquickjs",
diff --git a/Cargo.toml b/Cargo.toml
index a7e4c729474..40ade89b60a 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -197,7 +197,6 @@ libc = "0.2.81"
 libtest-mimic = "0.8.2"
 line-ending = "1.4.0"
 log = { version = "0.4", features = ["serde", "std"] }
-lru = "0.18.0"
 mermaid_to_svg = { git = "https://github.com/warpdotdev/mermaid-to-svg.git", rev = "8d3f789c2eb49335d7bf247a06bb649f59b6d4ed" }
 mime_guess = "2.0"
 minimp4 = "0.1.2"
diff --git a/crates/warp_completer/Cargo.toml b/crates/warp_completer/Cargo.toml
index 7e14c482e82..aae54e8474c 100644
--- a/crates/warp_completer/Cargo.toml
+++ b/crates/warp_completer/Cargo.toml
@@ -23,7 +23,6 @@ instant.workspace = true
 itertools.workspace = true
 lazy_static.workspace = true
 log.workspace = true
-lru.workspace = true
 memo-map.workspace = true
 regex.workspace = true
 serde.workspace = true
diff --git a/crates/warp_completer/src/signatures/legacy/registry.rs b/crates/warp_completer/src/signatures/legacy/registry.rs
index bbc35048ee0..e099c622e32 100644
--- a/crates/warp_completer/src/signatures/legacy/registry.rs
+++ b/crates/warp_completer/src/signatures/legacy/registry.rs
@@ -1,9 +1,7 @@
-use std::collections::HashMap;
-use std::num::NonZeroUsize;
-use std::sync::Mutex;
+use std::collections::{HashMap, HashSet, VecDeque};
+use std::sync::RwLock;
 
 use itertools::Itertools;
-use lru::LruCache;
 use memo_map::MemoMap;
 use warp_command_signatures::{Argument, DynamicCompletionData, IsArgumentOptional, Signature};
 
@@ -21,25 +19,24 @@ pub enum SignatureResult<'a> {
 
 type SignatureLookupFn = dyn 'static + Send + Sync + Fn(&str) -> Option<Signature>;
 
-/// The longest a command name can be before `get` skips resolving it against the (uncached)
-/// `lookup_fn` and instead falls back to a linear scan of already-cached signatures.
+/// The longest a command name can be before `get` gives up on it outright, without ever
+/// resolving it against the (uncached) `lookup_fn`, lowercasing it, or consulting either cache.
 ///
 /// `SignatureCache::get` is called with arbitrary tokens taken from the terminal input line (see
 /// `CommandRegistry::signature_from_tokens` and friends), not just tokens we already know to be
-/// real command names. `SignatureCache::signatures` only caches successful lookups (see its doc
-/// comment for why), so a single pathologically large token (e.g. a large blob of text pasted
-/// into the terminal input line) can't grow that map -- but lowercasing such a token on every
-/// keystroke would still be wasted work, and it must also never be admitted into
-/// `SignatureCache::misses` (see its doc comment for why), so `get` skips both for tokens this
-/// long. See APP-5431.
+/// real command names, so a single pathologically large token (e.g. a large blob of text pasted
+/// into the terminal input line) needs to be handled cheaply. See APP-5431.
 ///
 /// Real command names are executable names, which common filesystems cap at 255 bytes/characters
 /// (e.g. Linux `NAME_MAX`, macOS APFS/HFS+, and Windows NTFS all use this limit), so no
-/// legitimate *dynamically resolved* command name can exceed this. An explicitly `insert`-ed
-/// signature is not subject to that constraint, which is why `get` falls back to a scan instead
-/// of simply refusing to resolve tokens this long -- that keeps such a signature retrievable. As
-/// a sanity check, the longest name in the current embedded signature corpus is 43 characters,
-/// well under this cap.
+/// legitimate *dynamically resolved* command name can exceed this. This also holds for every
+/// name ever `register_signature`-ed in production (the Warp CLI's own command tree): see
+/// `test_all_known_signature_names_are_within_the_length_cap`, which walks both the embedded
+/// corpus and the Warp CLI's clap-derived signature and fails if that stops being true (the
+/// longest name found there is 59 characters, well under this cap). On the strength of that
+/// invariant, `get` simply returns `None` for a token this long -- an explicitly `insert`-ed
+/// signature with a name that happened to exceed this cap would become unresolvable via lookup,
+/// but that's a deliberate, tested tradeoff rather than a case that can occur today.
 const MAX_CACHEABLE_COMMAND_LEN: usize = 255;
 
 /// Capacity of `SignatureCache::misses`. Kept small: the negative cache exists to save
@@ -50,6 +47,83 @@ const MAX_CACHEABLE_COMMAND_LEN: usize = 255;
 /// correctness.
 const MAX_CACHED_MISSES: usize = 256;
 
+/// A bounded set of (lowercased) command names that recently failed to resolve to a signature,
+/// used by `SignatureCache::misses`.
+///
+/// This is a plain FIFO, not an LRU: once at capacity, inserting a new entry always evicts the
+/// *oldest-inserted* one, regardless of how recently any entry (including the one about to be
+/// evicted) was looked up again. `CommandRegistry` is a shared `Arc` behind a single global
+/// instance (see `CommandRegistry::global_instance`) that's called from multiple terminal
+/// sessions/panes concurrently, each generating completions on a background thread pool -- so
+/// this genuinely needs cross-thread synchronization, not just single-task interior mutability.
+/// But a negative cache is inherently approximate: a false negative (a miss this forgot) only
+/// costs one extra, cheap `lookup_fn` call, never a wrong answer. That's a much weaker
+/// requirement than an LRU implies, and dropping recency tracking is what makes a `RwLock` (an
+/// LRU would need every *hit* to also take a write lock, to move the entry to the front) the
+/// natural fit here, since a hit against `contains` becomes a pure read that never mutates
+/// anything -- the write lock is only ever needed for a genuinely new miss.
+struct MissCache {
+    capacity: usize,
+    entries: RwLock<MissCacheEntries>,
+}
+
+#[derive(Default)]
+struct MissCacheEntries {
+    /// Insertion order, oldest first, used to find the next entry to evict once at capacity.
+    order: VecDeque<String>,
+    /// The actual set of currently-remembered misses, for O(1) membership checks in `contains`.
+    set: HashSet<String>,
+}
+
+impl MissCache {
+    fn new(capacity: usize) -> Self {
+        Self {
+            capacity,
+            entries: RwLock::default(),
+        }
+    }
+
+    /// Returns `true` if `command` was recently recorded as a miss. A pure read: does not
+    /// affect eviction order.
+    fn contains(&self, command: &str) -> bool {
+        self.read().set.contains(command)
+    }
+
+    /// Returns the number of misses currently recorded, for tests.
+    #[cfg(test)]
+    fn len(&self) -> usize {
+        self.read().set.len()
+    }
+
+    /// Records `command` as a miss, evicting the oldest-recorded miss first if already at
+    /// capacity.
+    fn insert(&self, command: String) {
+        let mut entries = self.write();
+        if entries.set.contains(&command) {
+            return;
+        }
+        if entries.order.len() >= self.capacity
+            && let Some(oldest) = entries.order.pop_front()
+        {
+            entries.set.remove(&oldest);
+        }
+        entries.order.push_back(command.clone());
+        entries.set.insert(command);
+    }
+
+    fn read(&self) -> std::sync::RwLockReadGuard<'_, MissCacheEntries> {
+        self.entries
+            .read()
+            .unwrap_or_else(|poisoned| poisoned.into_inner())
+    }
+
+    fn write(&self) -> std::sync::RwLockWriteGuard<'_, MissCacheEntries> {
+        self.entries
+            .write()
+            .unwrap_or_else(|poisoned| poisoned.into_inner())
+    }
+}
+
 /// A simple structure to cache parsed command signatures.  These are stored as
 /// JSON, so this makes it easy for us to lazily load and parse the JSON when
 /// a command signature is needed, and only need to do that parsing work once
@@ -67,17 +141,18 @@ struct SignatureCache {
     /// writing), plus any explicitly `insert`-ed signatures -- rather than by every distinct
     /// token a user has ever typed or pasted. See APP-5431.
     signatures: MemoMap<String, Signature>,
-    /// A bounded LRU set of (lowercased) command names that recently failed to resolve to a
-    /// signature. Unlike `signatures`, `get` never hands out a reference into this set -- it only
-    /// ever returns a `bool` -- so eviction is sound: there's no borrow that could outlive an
-    /// evicted entry, which is exactly what keeps `signatures` itself append-only. That's also
-    /// what makes this bounded convenience rather than a fix for a performance problem: a miss is
-    /// already cheap to redo (`lookup_fn` probes the embedded, already-compiled signature corpus,
-    /// which does a binary search with no filesystem I/O or JSON parsing on a miss -- JSON
-    /// parsing only happens once we know there's a hit), so this cache exists to save a handful
-    /// of redundant lookups for a token that's retried in quick succession, not to avoid
-    /// expensive rework. See APP-5431.
-    misses: Mutex<LruCache<String, ()>>,
+    /// A bounded set of (lowercased) command names that recently failed to resolve to a
+    /// signature -- see `MissCache`'s doc comment for why it's a plain `RwLock`-guarded FIFO
+    /// rather than an LRU. Unlike `signatures`, `get` never hands out a reference into this set
+    /// -- it only ever returns a `bool` -- so eviction is sound: there's no borrow that could
+    /// outlive an evicted entry, which is exactly what keeps `signatures` itself append-only.
+    /// That's also what makes this bounded convenience rather than a fix for a performance
+    /// problem: a miss is already cheap to redo (`lookup_fn` probes the embedded,
+    /// already-compiled signature corpus, which does a binary search with no filesystem I/O or
+    /// JSON parsing on a miss -- JSON parsing only happens once we know there's a hit), so this
+    /// cache exists to save a handful of redundant lookups for a token that's retried in quick
+    /// succession, not to avoid expensive rework. See APP-5431.
+    misses: MissCache,
 }
 
 impl SignatureCache {
@@ -85,9 +160,7 @@ impl SignatureCache {
         Self {
             lookup_fn,
             signatures: Default::default(),
-            misses: Mutex::new(LruCache::new(
-                NonZeroUsize::new(MAX_CACHED_MISSES).expect("MAX_CACHED_MISSES is non-zero"),
-            )),
+            misses: MissCache::new(MAX_CACHED_MISSES),
         }
     }
 
@@ -99,18 +172,12 @@ impl SignatureCache {
         };
 
         if command.len() > MAX_CACHEABLE_COMMAND_LEN {
-            // Avoid allocating a lowercase copy of a pathologically large token (see
-            // `MAX_CACHEABLE_COMMAND_LEN`) by scanning the cache directly instead of hashing into
-            // it. The length comparison short-circuits before any case folding, so this stays
-            // cheap even though `command` may be huge, and it keeps an `insert`-ed signature
-            // whose name happens to exceed the cap retrievable. This never touches `misses`: an
-            // oversized token must never be admitted there either, or the leak this cap exists to
-            // prevent would just move to the negative cache.
-            return self
-                .signatures
-                .iter()
-                .find(|(key, _)| key.len() == command.len() && key.eq_ignore_ascii_case(command))
-                .map(|(_, signature)| signature);
+            // No known command/subcommand name comes anywhere close to this length (see
+            // `MAX_CACHEABLE_COMMAND_LEN`'s doc comment), so a token this long can never resolve
+            // to anything. Return before the lowercase allocation and before touching either
+            // cache -- in particular, an oversized token must never be admitted into `misses`,
+            // or the leak this cap exists to prevent would just move to the negative cache.
+            return None;
         }
 
         let command = command.to_lowercase();
@@ -119,7 +186,7 @@ impl SignatureCache {
             return Some(signature);
         }
 
-        if self.lock_misses().get(command.as_str()).is_some() {
+        if self.misses.contains(command.as_str()) {
             return None;
         }
 
@@ -129,18 +196,12 @@ impl SignatureCache {
                     .get_or_insert(command.as_str(), || signature),
             ),
             None => {
-                self.lock_misses().put(command, ());
+                self.misses.insert(command);
                 None
             }
         }
     }
 
-    fn lock_misses(&self) -> std::sync::MutexGuard<'_, LruCache<String, ()>> {
-        self.misses
-            .lock()
-            .unwrap_or_else(|poisoned| poisoned.into_inner())
-    }
-
     /// Inserts the given `Signature` into the underlying map, keyed by `Signature::name`.
     ///
     /// If there is already a cached value for the given `Signature::name`, this is a no-op.
diff --git a/crates/warp_completer/src/signatures/legacy/registry_tests.rs b/crates/warp_completer/src/signatures/legacy/registry_tests.rs
index 5a1a1d64ace..736536a4b42 100644
--- a/crates/warp_completer/src/signatures/legacy/registry_tests.rs
+++ b/crates/warp_completer/src/signatures/legacy/registry_tests.rs
@@ -1,4 +1,5 @@
 use warp_command_signatures::{Priority, Signature};
+use warp_core::channel::Channel;
 
 use crate::completer::testing::FakeCompletionContext;
 use crate::completer::{CompletionContext, TopLevelCommandCaseSensitivity};
@@ -20,6 +21,56 @@ fn signature_with_name(name: &str) -> Signature {
     }
 }
 
+/// Recursively finds the longest `Signature::name` in `signature` or any of its (possibly
+/// nested) subcommands, updating `longest` in place if a longer one is found.
+fn track_longest_name(signature: &Signature, longest: &mut (usize, String)) {
+    if signature.name.len() > longest.0 {
+        *longest = (signature.name.len(), signature.name.clone());
+    }
+    for subcommand in signature.subcommands() {
+        track_longest_name(subcommand, longest);
+    }
+}
+
+#[test]
+fn test_all_known_signature_names_are_within_the_length_cap() {
+    // `SignatureCache::get`'s oversized-token fast path (see `MAX_CACHEABLE_COMMAND_LEN`'s doc
+    // comment) assumes no name that can ever be dynamically resolved -- from the embedded
+    // signature corpus or from `CommandRegistry::register_signature` -- exceeds it. This test
+    // establishes that invariant by actually walking both sources, rather than assuming it: if
+    // a future signature (in the corpus, or a newly `register_signature`-ed one) is added with a
+    // name that violates it, this test fails loudly instead of that name silently becoming
+    // unresolvable via lookup.
+    let mut longest = (0, String::new());
+
+    for signature in warp_command_signatures::commands() {
+        track_longest_name(&signature, &mut longest);
+    }
+
+    // Mirrors `CommandRegistry::register_warp_signatures`: the only other names ever
+    // `register_signature`-ed in production. Uses the raw `CommandFactory::command()` rather
+    // than `Args::clap_command()`, since the latter only *hides* subcommands based on
+    // `FeatureFlag` state (which requires flags to be initialized, unavailable in this test
+    // environment) without changing any name -- so this still covers every name that
+    // `clap_command()` would produce, feature flags notwithstanding.
+    for channel in [Channel::Stable, Channel::Preview, Channel::Dev] {
+        let mut clap_cmd = <warp_cli::Args as clap::CommandFactory>::command();
+        let signature = crate::signatures::clap::signature_from_clap_command(
+            &mut clap_cmd,
+            channel.cli_command_name(),
+        );
+        track_longest_name(&signature, &mut longest);
+    }
+
+    let (max_len, longest_name) = longest;
+    assert!(
+        max_len <= MAX_CACHEABLE_COMMAND_LEN,
+        "found a command/subcommand name of length {max_len} ({longest_name:?}), which exceeds \
+         MAX_CACHEABLE_COMMAND_LEN ({MAX_CACHEABLE_COMMAND_LEN}) -- SignatureCache::get's \
+         oversized-token fast path assumes this can't happen"
+    );
+}
+
 #[test]
 fn test_find_command_from_a_top_level_signature() {
     let bundle = warp_command_signatures::signature_by_name("bundle")
@@ -229,7 +280,7 @@ fn test_oversized_command_is_not_cached_and_resolves_to_none() {
     // signature.
     let registry = create_test_command_registry([test_signature()]);
     let positive_len_before = registry.signatures.signatures.len();
-    let negative_len_before = registry.signatures.misses.lock().unwrap().len();
+    let negative_len_before = registry.signatures.misses.len();
 
     let oversized_command = "a".repeat(MAX_CACHEABLE_COMMAND_LEN + 1);
     assert_eq!(registry.signature(&oversized_command), None);
@@ -239,7 +290,7 @@ fn test_oversized_command_is_not_cached_and_resolves_to_none() {
         "looking up an oversized command should not add an entry to the positive cache"
     );
     assert_eq!(
-        registry.signatures.misses.lock().unwrap().len(),
+        registry.signatures.misses.len(),
         negative_len_before,
         "looking up an oversized command should not add an entry to the negative cache either -- \
          that would just move the leak this cap exists to fix into the negative cache"
@@ -262,28 +313,24 @@ fn test_misses_are_never_cached_in_the_positive_cache() {
 #[test]
 fn test_negative_cache_stops_growing_at_capacity() {
     // Regression test for APP-5431: the negative cache (`SignatureCache::misses`) is a bounded
-    // LRU set, so looking up more distinct misses than `MAX_CACHED_MISSES` must not grow it
-    // past that capacity.
+    // set, so looking up more distinct misses than `MAX_CACHED_MISSES` must not grow it past
+    // that capacity.
     let registry = create_test_command_registry([test_signature()]);
 
     for i in 0..MAX_CACHED_MISSES * 2 {
         assert_eq!(registry.signature(&format!("not-a-real-command-{i}")), None);
-        assert!(registry.signatures.misses.lock().unwrap().len() <= MAX_CACHED_MISSES);
+        assert!(registry.signatures.misses.len() <= MAX_CACHED_MISSES);
     }
-    assert_eq!(
-        registry.signatures.misses.lock().unwrap().len(),
-        MAX_CACHED_MISSES
-    );
+    assert_eq!(registry.signatures.misses.len(), MAX_CACHED_MISSES);
 }
 
 #[test]
-fn test_negative_cache_evicts_in_lru_order() {
-    // Regression test for APP-5431: once the negative cache is at capacity, inserting a new
-    // miss must evict the least-recently-used entry specifically. Critically, this re-touches
-    // the *oldest-inserted* entry before overflowing so a FIFO (or arbitrary-eviction) cache
-    // would fail this test: a FIFO cache would still evict "miss-0" since it only tracks
-    // insertion order and ignores the re-touch, whereas an LRU cache must evict "miss-1"
-    // instead, since "miss-1" is now the actual least-recently-used entry.
+fn test_negative_cache_evicts_in_fifo_order() {
+    // Regression test for APP-5431: the negative cache dropped strict LRU recency tracking for
+    // a simpler FIFO-order bounded set (see `MissCache`'s doc comment for why), so once at
+    // capacity, inserting a new miss must evict the *oldest-inserted* entry specifically --
+    // even if that entry was looked up again more recently than others, since a lookup against
+    // the negative cache is a pure read that never reorders anything.
     let registry = create_test_command_registry([test_signature()]);
 
     // Fill the negative cache to capacity with "miss-0", .., "miss-{MAX_CACHED_MISSES - 1}",
@@ -292,31 +339,21 @@ fn test_negative_cache_evicts_in_lru_order() {
         assert_eq!(registry.signature(&format!("miss-{i}")), None);
     }
 
-    // Touch only "miss-0" -- the oldest, and thus the next entry a FIFO cache would evict --
-    // moving it to the most-recently-used position.
-    assert_eq!(registry.signature("miss-0"), None);
+    // Repeatedly re-look-up the oldest entry. Under an LRU this would protect it from eviction,
+    // but FIFO eviction ignores lookups entirely.
+    for _ in 0..5 {
+        assert_eq!(registry.signature("miss-0"), None);
+    }
 
-    // Inserting one more miss should evict "miss-1", the least-recently-used entry, not
-    // "miss-0", which was just touched.
+    // Inserting one more miss should still evict "miss-0", not "miss-1".
     assert_eq!(registry.signature("miss-overflow"), None);
     assert!(
-        registry
-            .signatures
-            .misses
-            .lock()
-            .unwrap()
-            .contains("miss-0"),
-        "the just-touched entry should not have been evicted -- a FIFO cache would incorrectly \
-         evict it instead of `miss-1`"
+        !registry.signatures.misses.contains("miss-0"),
+        "the oldest-inserted entry should have been evicted regardless of being looked up again"
     );
     assert!(
-        !registry
-            .signatures
-            .misses
-            .lock()
-            .unwrap()
-            .contains("miss-1"),
-        "the least-recently-used entry should have been evicted"
+        registry.signatures.misses.contains("miss-1"),
+        "a newer entry should not have been evicted"
     );
 }
 
@@ -332,7 +369,7 @@ fn test_oversized_later_token_does_not_bypass_the_length_guard() {
         .expect("global command signatures should include 'sudo'");
     let registry = create_test_command_registry([sudo]);
     let positive_len_before = registry.signatures.signatures.len();
-    let negative_len_before = registry.signatures.misses.lock().unwrap().len();
+    let negative_len_before = registry.signatures.misses.len();
 
     let oversized_token = "a".repeat(MAX_CACHEABLE_COMMAND_LEN + 1);
     let tokens = ["sudo", oversized_token.as_str()];
@@ -355,7 +392,7 @@ fn test_oversized_later_token_does_not_bypass_the_length_guard() {
         "looking up an oversized later token should not add an entry to the positive cache"
     );
     assert_eq!(
-        registry.signatures.misses.lock().unwrap().len(),
+        registry.signatures.misses.len(),
         negative_len_before,
         "looking up an oversized later token should not add an entry to the negative cache"
     );
@@ -390,18 +427,18 @@ fn test_ordinary_commands_still_resolve_and_are_cached() {
 }
 
 #[test]
-fn test_registered_signature_longer_than_the_cap_still_resolves() {
-    // Regression test for APP-5431 (review finding): an explicitly registered signature must
-    // stay retrievable through `signature()` even if its name exceeds `MAX_CACHEABLE_COMMAND_LEN`
-    // -- the cap only governs whether a *dynamic* (uncached) lookup is attempted, not whether an
-    // already-registered signature can be found.
+fn test_registered_signature_longer_than_the_cap_is_unresolvable() {
+    // Regression test for APP-5431: `SignatureCache::get` now returns `None` outright for any
+    // token over `MAX_CACHEABLE_COMMAND_LEN`, on the strength of
+    // `test_all_known_signature_names_are_within_the_length_cap` establishing that no real
+    // signature ever has a name this long. An explicitly `register_signature`-ed signature with
+    // an artificially oversized name is therefore *not* retrievable through `signature()` --
+    // a deliberate, accepted tradeoff (see `MAX_CACHEABLE_COMMAND_LEN`'s doc comment), not a
+    // real-world regression, since that invariant test would fail first if this ever mattered.
     let long_name = "a".repeat(MAX_CACHEABLE_COMMAND_LEN + 1);
     let registry = create_test_command_registry([signature_with_name(&long_name)]);
 
-    assert_eq!(
-        registry.signature(&long_name).map(|s| s.name.as_str()),
-        Some(long_name.as_str())
-    );
+    assert_eq!(registry.signature(&long_name), None);
 }
 
 #[cfg(windows)]

From ddd825bde3d10b971a52ea47543b6019cbd400a1 Mon Sep 17 00:00:00 2001
From: "warp-agent-staging[bot]"
 <240773466+warp-agent-staging[bot]@users.noreply.github.com>
Date: Tue, 18 Aug 2026 19:34:19 +0000
Subject: [PATCH 6/7] Move MissCache into its own module; drop doc comments on
 the two consts

Reviewer request, mechanical only, no behavior change:

- `MissCache` (and `MissCacheEntries`) now live in their own
  `signatures/legacy/miss_cache.rs` module beside `registry.rs`, with a
  companion `miss_cache_tests.rs` wired the same way `registry.rs` wires
  `registry_tests.rs`. `MAX_CACHED_MISSES` moved with it; registry.rs no
  longer references it at all, since `MissCache` now exposes a `Default`
  impl that uses it internally. `MAX_CACHEABLE_COMMAND_LEN` stays in
  `registry.rs`, next to `SignatureCache`.
- The `MissCache`-specific tests (capacity bound, FIFO eviction order)
  moved out of `registry_tests.rs` into `miss_cache_tests.rs`, rewritten
  as direct unit tests against `MissCache` itself rather than driven
  through `CommandRegistry::signature()`. The tests that exercise
  `SignatureCache`/`CommandRegistry` behavior and only touch
  miss-caching indirectly (the two oversized-token tests, which check
  `misses.len()` before/after to confirm the length guard never touches
  it) stayed in `registry_tests.rs`.
- Deleted the doc comments on `MAX_CACHEABLE_COMMAND_LEN` and (now
  moved) `MAX_CACHED_MISSES`. Two comments elsewhere pointed at
  `MAX_CACHEABLE_COMMAND_LEN`'s doc comment specifically; both had that
  parenthetical dropped rather than having the deleted prose relocated
  into them. `MissCache`'s own type-level doc comment (why it's a
  `RwLock`-guarded FIFO) is unaffected -- it's not a const comment, and
  it moved with the type.
- Visibility kept as tight as the split allows: `MissCache` and the
  methods `registry.rs`/its tests need are `pub(super)` (visible within
  `legacy` and its descendants only); `MissCache::new` and the
  `MissCacheEntries` fields stay private to the new module.

Co-Authored-By: Warp <agent@warp.dev>
---
 .../src/signatures/legacy/miss_cache.rs       |  91 ++++++++++++++
 .../src/signatures/legacy/miss_cache_tests.rs |  60 +++++++++
 .../src/signatures/legacy/mod.rs              |   1 +
 .../src/signatures/legacy/registry.rs         | 119 ++----------------
 .../src/signatures/legacy/registry_tests.rs   |  61 ++-------
 5 files changed, 167 insertions(+), 165 deletions(-)
 create mode 100644 crates/warp_completer/src/signatures/legacy/miss_cache.rs
 create mode 100644 crates/warp_completer/src/signatures/legacy/miss_cache_tests.rs

diff --git a/crates/warp_completer/src/signatures/legacy/miss_cache.rs b/crates/warp_completer/src/signatures/legacy/miss_cache.rs
new file mode 100644
index 00000000000..2ec33144d8b
--- /dev/null
+++ b/crates/warp_completer/src/signatures/legacy/miss_cache.rs
@@ -0,0 +1,91 @@
+use std::collections::{HashSet, VecDeque};
+use std::sync::{RwLock, RwLockReadGuard, RwLockWriteGuard};
+
+const MAX_CACHED_MISSES: usize = 256;
+
+/// A bounded set of (lowercased) command names that recently failed to resolve to a signature,
+/// used by `SignatureCache::misses`.
+///
+/// This is a plain FIFO, not an LRU: once at capacity, inserting a new entry always evicts the
+/// *oldest-inserted* one, regardless of how recently any entry (including the one about to be
+/// evicted) was looked up again. `CommandRegistry` is a shared `Arc` behind a single global
+/// instance (see `CommandRegistry::global_instance`) that's called from multiple terminal
+/// sessions/panes concurrently, each generating completions on a background thread pool -- so
+/// this genuinely needs cross-thread synchronization, not just single-task interior mutability.
+/// But a negative cache is inherently approximate: a false negative (a miss this forgot) only
+/// costs one extra, cheap `lookup_fn` call, never a wrong answer. That's a much weaker
+/// requirement than an LRU implies, and dropping recency tracking is what makes a `RwLock` (an
+/// LRU would need every *hit* to also take a write lock, to move the entry to the front) the
+/// natural fit here, since a hit against `contains` becomes a pure read that never mutates
+/// anything -- the write lock is only ever needed for a genuinely new miss.
+pub(super) struct MissCache {
+    capacity: usize,
+    entries: RwLock<MissCacheEntries>,
+}
+
+#[derive(Default)]
+struct MissCacheEntries {
+    /// Insertion order, oldest first, used to find the next entry to evict once at capacity.
+    order: VecDeque<String>,
+    /// The actual set of currently-remembered misses, for O(1) membership checks in `contains`.
+    set: HashSet<String>,
+}
+
+impl Default for MissCache {
+    fn default() -> Self {
+        Self::new(MAX_CACHED_MISSES)
+    }
+}
+
+impl MissCache {
+    fn new(capacity: usize) -> Self {
+        Self {
+            capacity,
+            entries: RwLock::default(),
+        }
+    }
+
+    /// Returns `true` if `command` was recently recorded as a miss. A pure read: does not
+    /// affect eviction order.
+    pub(super) fn contains(&self, command: &str) -> bool {
+        self.read().set.contains(command)
+    }
+
+    /// Returns the number of misses currently recorded, for tests.
+    #[cfg(test)]
+    pub(super) fn len(&self) -> usize {
+        self.read().set.len()
+    }
+
+    /// Records `command` as a miss, evicting the oldest-recorded miss first if already at
+    /// capacity.
+    pub(super) fn insert(&self, command: String) {
+        let mut entries = self.write();
+        if entries.set.contains(&command) {
+            return;
+        }
+        if entries.order.len() >= self.capacity
+            && let Some(oldest) = entries.order.pop_front()
+        {
+            entries.set.remove(&oldest);
+        }
+        entries.order.push_back(command.clone());
+        entries.set.insert(command);
+    }
+
+    fn read(&self) -> RwLockReadGuard<'_, MissCacheEntries> {
+        self.entries
+            .read()
+            .unwrap_or_else(|poisoned| poisoned.into_inner())
+    }
+
+    fn write(&self) -> RwLockWriteGuard<'_, MissCacheEntries> {
+        self.entries
+            .write()
+            .unwrap_or_else(|poisoned| poisoned.into_inner())
+    }
+}
+
+#[cfg(test)]
+#[path = "miss_cache_tests.rs"]
+mod tests;
diff --git a/crates/warp_completer/src/signatures/legacy/miss_cache_tests.rs b/crates/warp_completer/src/signatures/legacy/miss_cache_tests.rs
new file mode 100644
index 00000000000..3589cae13cb
--- /dev/null
+++ b/crates/warp_completer/src/signatures/legacy/miss_cache_tests.rs
@@ -0,0 +1,60 @@
+use super::MissCache;
+
+#[test]
+fn test_contains_reflects_recorded_misses() {
+    let cache = MissCache::new(3);
+
+    assert!(!cache.contains("a"));
+    cache.insert("a".to_string());
+    assert!(cache.contains("a"));
+}
+
+#[test]
+fn test_inserting_an_existing_entry_is_a_no_op() {
+    let cache = MissCache::new(3);
+
+    cache.insert("a".to_string());
+    cache.insert("a".to_string());
+    assert_eq!(cache.len(), 1);
+}
+
+#[test]
+fn test_stops_growing_at_capacity() {
+    let capacity = 4;
+    let cache = MissCache::new(capacity);
+
+    for i in 0..capacity * 3 {
+        cache.insert(format!("miss-{i}"));
+        assert!(cache.len() <= capacity);
+    }
+    assert_eq!(cache.len(), capacity);
+}
+
+#[test]
+fn test_evicts_in_fifo_order_regardless_of_lookups() {
+    // Eviction is by insertion order, not recency: a hit against `contains` is a pure read and
+    // never protects an entry from eviction, unlike an LRU.
+    let cache = MissCache::new(3);
+    cache.insert("a".to_string());
+    cache.insert("b".to_string());
+    cache.insert("c".to_string());
+
+    // Repeatedly look up the oldest entry. Under an LRU this would move it to the front and
+    // protect it from eviction, but FIFO eviction ignores lookups entirely.
+    for _ in 0..5 {
+        assert!(cache.contains("a"));
+    }
+
+    // Inserting one more entry should still evict "a", the oldest-inserted entry, not "b".
+    cache.insert("d".to_string());
+    assert!(
+        !cache.contains("a"),
+        "the oldest-inserted entry should have been evicted regardless of being looked up again"
+    );
+    assert!(
+        cache.contains("b"),
+        "a newer entry should not have been evicted"
+    );
+    assert!(cache.contains("c"));
+    assert!(cache.contains("d"));
+}
diff --git a/crates/warp_completer/src/signatures/legacy/mod.rs b/crates/warp_completer/src/signatures/legacy/mod.rs
index 4096ee9654e..ae8dd8639e7 100644
--- a/crates/warp_completer/src/signatures/legacy/mod.rs
+++ b/crates/warp_completer/src/signatures/legacy/mod.rs
@@ -2,6 +2,7 @@ use std::sync::{Arc, OnceLock};
 
 use warp_core::channel::Channel;
 
+mod miss_cache;
 pub mod registry;
 
 pub use registry::CommandRegistry;
diff --git a/crates/warp_completer/src/signatures/legacy/registry.rs b/crates/warp_completer/src/signatures/legacy/registry.rs
index e099c622e32..b37871f796a 100644
--- a/crates/warp_completer/src/signatures/legacy/registry.rs
+++ b/crates/warp_completer/src/signatures/legacy/registry.rs
@@ -1,10 +1,10 @@
-use std::collections::{HashMap, HashSet, VecDeque};
-use std::sync::RwLock;
+use std::collections::HashMap;
 
 use itertools::Itertools;
 use memo_map::MemoMap;
 use warp_command_signatures::{Argument, DynamicCompletionData, IsArgumentOptional, Signature};
 
+use super::miss_cache::MissCache;
 use crate::completer::{CommandExitStatus, CompletionContext, TopLevelCommandCaseSensitivity};
 use crate::parsers::SignatureAtTokenIndex;
 
@@ -19,111 +19,8 @@ pub enum SignatureResult<'a> {
 
 type SignatureLookupFn = dyn 'static + Send + Sync + Fn(&str) -> Option<Signature>;
 
-/// The longest a command name can be before `get` gives up on it outright, without ever
-/// resolving it against the (uncached) `lookup_fn`, lowercasing it, or consulting either cache.
-///
-/// `SignatureCache::get` is called with arbitrary tokens taken from the terminal input line (see
-/// `CommandRegistry::signature_from_tokens` and friends), not just tokens we already know to be
-/// real command names, so a single pathologically large token (e.g. a large blob of text pasted
-/// into the terminal input line) needs to be handled cheaply. See APP-5431.
-///
-/// Real command names are executable names, which common filesystems cap at 255 bytes/characters
-/// (e.g. Linux `NAME_MAX`, macOS APFS/HFS+, and Windows NTFS all use this limit), so no
-/// legitimate *dynamically resolved* command name can exceed this. This also holds for every
-/// name ever `register_signature`-ed in production (the Warp CLI's own command tree): see
-/// `test_all_known_signature_names_are_within_the_length_cap`, which walks both the embedded
-/// corpus and the Warp CLI's clap-derived signature and fails if that stops being true (the
-/// longest name found there is 59 characters, well under this cap). On the strength of that
-/// invariant, `get` simply returns `None` for a token this long -- an explicitly `insert`-ed
-/// signature with a name that happened to exceed this cap would become unresolvable via lookup,
-/// but that's a deliberate, tested tradeoff rather than a case that can occur today.
 const MAX_CACHEABLE_COMMAND_LEN: usize = 255;
 
-/// Capacity of `SignatureCache::misses`. Kept small: the negative cache exists to save
-/// `lookup_fn` calls for a token that's been retried a handful of times in quick succession
-/// (e.g. a typo the user is correcting, or the same invalid token looked up again as the parser
-/// backtracks over the line), not to remember every miss for the life of the session. See
-/// `SignatureCache::misses`'s doc comment for why unbounded retention isn't needed for
-/// correctness.
-const MAX_CACHED_MISSES: usize = 256;
-
-/// A bounded set of (lowercased) command names that recently failed to resolve to a signature,
-/// used by `SignatureCache::misses`.
-///
-/// This is a plain FIFO, not an LRU: once at capacity, inserting a new entry always evicts the
-/// *oldest-inserted* one, regardless of how recently any entry (including the one about to be
-/// evicted) was looked up again. `CommandRegistry` is a shared `Arc` behind a single global
-/// instance (see `CommandRegistry::global_instance`) that's called from multiple terminal
-/// sessions/panes concurrently, each generating completions on a background thread pool -- so
-/// this genuinely needs cross-thread synchronization, not just single-task interior mutability.
-/// But a negative cache is inherently approximate: a false negative (a miss this forgot) only
-/// costs one extra, cheap `lookup_fn` call, never a wrong answer. That's a much weaker
-/// requirement than an LRU implies, and dropping recency tracking is what makes a `RwLock` (an
-/// LRU would need every *hit* to also take a write lock, to move the entry to the front) the
-/// natural fit here, since a hit against `contains` becomes a pure read that never mutates
-/// anything -- the write lock is only ever needed for a genuinely new miss.
-struct MissCache {
-    capacity: usize,
-    entries: RwLock<MissCacheEntries>,
-}
-
-#[derive(Default)]
-struct MissCacheEntries {
-    /// Insertion order, oldest first, used to find the next entry to evict once at capacity.
-    order: VecDeque<String>,
-    /// The actual set of currently-remembered misses, for O(1) membership checks in `contains`.
-    set: HashSet<String>,
-}
-
-impl MissCache {
-    fn new(capacity: usize) -> Self {
-        Self {
-            capacity,
-            entries: RwLock::default(),
-        }
-    }
-
-    /// Returns `true` if `command` was recently recorded as a miss. A pure read: does not
-    /// affect eviction order.
-    fn contains(&self, command: &str) -> bool {
-        self.read().set.contains(command)
-    }
-
-    /// Returns the number of misses currently recorded, for tests.
-    #[cfg(test)]
-    fn len(&self) -> usize {
-        self.read().set.len()
-    }
-
-    /// Records `command` as a miss, evicting the oldest-recorded miss first if already at
-    /// capacity.
-    fn insert(&self, command: String) {
-        let mut entries = self.write();
-        if entries.set.contains(&command) {
-            return;
-        }
-        if entries.order.len() >= self.capacity
-            && let Some(oldest) = entries.order.pop_front()
-        {
-            entries.set.remove(&oldest);
-        }
-        entries.order.push_back(command.clone());
-        entries.set.insert(command);
-    }
-
-    fn read(&self) -> std::sync::RwLockReadGuard<'_, MissCacheEntries> {
-        self.entries
-            .read()
-            .unwrap_or_else(|poisoned| poisoned.into_inner())
-    }
-
-    fn write(&self) -> std::sync::RwLockWriteGuard<'_, MissCacheEntries> {
-        self.entries
-            .write()
-            .unwrap_or_else(|poisoned| poisoned.into_inner())
-    }
-}
-
 /// A simple structure to cache parsed command signatures.  These are stored as
 /// JSON, so this makes it easy for us to lazily load and parse the JSON when
 /// a command signature is needed, and only need to do that parsing work once
@@ -160,7 +57,7 @@ impl SignatureCache {
         Self {
             lookup_fn,
             signatures: Default::default(),
-            misses: MissCache::new(MAX_CACHED_MISSES),
+            misses: MissCache::default(),
         }
     }
 
@@ -172,11 +69,11 @@ impl SignatureCache {
         };
 
         if command.len() > MAX_CACHEABLE_COMMAND_LEN {
-            // No known command/subcommand name comes anywhere close to this length (see
-            // `MAX_CACHEABLE_COMMAND_LEN`'s doc comment), so a token this long can never resolve
-            // to anything. Return before the lowercase allocation and before touching either
-            // cache -- in particular, an oversized token must never be admitted into `misses`,
-            // or the leak this cap exists to prevent would just move to the negative cache.
+            // No known command/subcommand name comes anywhere close to this length, so a token
+            // this long can never resolve to anything. Return before the lowercase allocation
+            // and before touching either cache -- in particular, an oversized token must never
+            // be admitted into `misses`, or the leak this cap exists to prevent would just move
+            // to the negative cache.
             return None;
         }
 
diff --git a/crates/warp_completer/src/signatures/legacy/registry_tests.rs b/crates/warp_completer/src/signatures/legacy/registry_tests.rs
index 736536a4b42..262dc632eb2 100644
--- a/crates/warp_completer/src/signatures/legacy/registry_tests.rs
+++ b/crates/warp_completer/src/signatures/legacy/registry_tests.rs
@@ -3,7 +3,7 @@ use warp_core::channel::Channel;
 
 use crate::completer::testing::FakeCompletionContext;
 use crate::completer::{CompletionContext, TopLevelCommandCaseSensitivity};
-use crate::signatures::registry::{MAX_CACHEABLE_COMMAND_LEN, MAX_CACHED_MISSES, SignatureResult};
+use crate::signatures::registry::{MAX_CACHEABLE_COMMAND_LEN, SignatureResult};
 use crate::signatures::testing::{create_test_command_registry, test_signature};
 
 /// A minimal signature with the given `name`, for exercising `SignatureCache` boundary
@@ -34,10 +34,10 @@ fn track_longest_name(signature: &Signature, longest: &mut (usize, String)) {
 
 #[test]
 fn test_all_known_signature_names_are_within_the_length_cap() {
-    // `SignatureCache::get`'s oversized-token fast path (see `MAX_CACHEABLE_COMMAND_LEN`'s doc
-    // comment) assumes no name that can ever be dynamically resolved -- from the embedded
-    // signature corpus or from `CommandRegistry::register_signature` -- exceeds it. This test
-    // establishes that invariant by actually walking both sources, rather than assuming it: if
+    // `SignatureCache::get`'s oversized-token fast path assumes no name that can ever be
+    // dynamically resolved -- from the embedded signature corpus or from
+    // `CommandRegistry::register_signature` -- exceeds it. This test establishes that invariant
+    // by actually walking both sources, rather than assuming it: if
     // a future signature (in the corpus, or a newly `register_signature`-ed one) is added with a
     // name that violates it, this test fails loudly instead of that name silently becoming
     // unresolvable via lookup.
@@ -310,53 +310,6 @@ fn test_misses_are_never_cached_in_the_positive_cache() {
     assert_eq!(registry.signatures.signatures.len(), len_before);
 }
 
-#[test]
-fn test_negative_cache_stops_growing_at_capacity() {
-    // Regression test for APP-5431: the negative cache (`SignatureCache::misses`) is a bounded
-    // set, so looking up more distinct misses than `MAX_CACHED_MISSES` must not grow it past
-    // that capacity.
-    let registry = create_test_command_registry([test_signature()]);
-
-    for i in 0..MAX_CACHED_MISSES * 2 {
-        assert_eq!(registry.signature(&format!("not-a-real-command-{i}")), None);
-        assert!(registry.signatures.misses.len() <= MAX_CACHED_MISSES);
-    }
-    assert_eq!(registry.signatures.misses.len(), MAX_CACHED_MISSES);
-}
-
-#[test]
-fn test_negative_cache_evicts_in_fifo_order() {
-    // Regression test for APP-5431: the negative cache dropped strict LRU recency tracking for
-    // a simpler FIFO-order bounded set (see `MissCache`'s doc comment for why), so once at
-    // capacity, inserting a new miss must evict the *oldest-inserted* entry specifically --
-    // even if that entry was looked up again more recently than others, since a lookup against
-    // the negative cache is a pure read that never reorders anything.
-    let registry = create_test_command_registry([test_signature()]);
-
-    // Fill the negative cache to capacity with "miss-0", .., "miss-{MAX_CACHED_MISSES - 1}",
-    // in insertion order.
-    for i in 0..MAX_CACHED_MISSES {
-        assert_eq!(registry.signature(&format!("miss-{i}")), None);
-    }
-
-    // Repeatedly re-look-up the oldest entry. Under an LRU this would protect it from eviction,
-    // but FIFO eviction ignores lookups entirely.
-    for _ in 0..5 {
-        assert_eq!(registry.signature("miss-0"), None);
-    }
-
-    // Inserting one more miss should still evict "miss-0", not "miss-1".
-    assert_eq!(registry.signature("miss-overflow"), None);
-    assert!(
-        !registry.signatures.misses.contains("miss-0"),
-        "the oldest-inserted entry should have been evicted regardless of being looked up again"
-    );
-    assert!(
-        registry.signatures.misses.contains("miss-1"),
-        "a newer entry should not have been evicted"
-    );
-}
-
 #[test]
 fn test_oversized_later_token_does_not_bypass_the_length_guard() {
     // Regression test for APP-5431: `maybe_load_replacement_signature` resolves a *later*
@@ -433,8 +386,8 @@ fn test_registered_signature_longer_than_the_cap_is_unresolvable() {
     // `test_all_known_signature_names_are_within_the_length_cap` establishing that no real
     // signature ever has a name this long. An explicitly `register_signature`-ed signature with
     // an artificially oversized name is therefore *not* retrievable through `signature()` --
-    // a deliberate, accepted tradeoff (see `MAX_CACHEABLE_COMMAND_LEN`'s doc comment), not a
-    // real-world regression, since that invariant test would fail first if this ever mattered.
+    // a deliberate, accepted tradeoff, not a real-world regression, since that invariant test
+    // would fail first if this ever mattered.
     let long_name = "a".repeat(MAX_CACHEABLE_COMMAND_LEN + 1);
     let registry = create_test_command_registry([signature_with_name(&long_name)]);
 

From 311d1355aeb0ea63ed82af2ba71690c3d30d9fb2 Mon Sep 17 00:00:00 2001
From: "warp-agent-staging[bot]"
 <240773466+warp-agent-staging[bot]@users.noreply.github.com>
Date: Tue, 18 Aug 2026 22:26:17 +0000
Subject: [PATCH 7/7] Strip comments from PR-added tests; trim field/type doc
 comments

Reviewer requests, comments-only, no behavior change:

- Removed every comment (block-above and inline) from the tests this
  PR introduced: in registry_tests.rs, the length-cap invariant test,
  both oversized-token tests, the never-cache-a-miss test, the
  ordinary-commands test, the registered-signature-longer-than-the-cap
  test, the .exe-trim-ordering test, and the registered_commands test;
  in miss_cache_tests.rs, all four MissCache tests. Tests that predate
  this PR are untouched. Where a removed comment was carrying real
  information, the test name already said it (no renames needed); for
  the length-cap invariant test specifically, folded what the comment
  explained -- what a failure means and what to do about it -- into
  the `assert!` message instead.
- Applied two suggested edits from the GitHub review, verbatim: the
  `signatures` field doc comment in registry.rs now ends at "...as the
  map internally is an append-only structure)."; the `misses` field
  doc comment collapses to two lines stating what it is, dropping the
  `MissCache` pointer, the eviction-soundness reasoning, and the
  bounded-convenience explanation.
- On my own initiative (not requested, but the same trend as the two
  review comments): trimmed `MissCache`'s own type doc comment in
  miss_cache.rs from ~15 lines to 3, stating what it is and why it's a
  FIFO behind a `RwLock` rather than an LRU behind a `Mutex`, without
  absorbing any of the text removed from registry.rs.

Co-Authored-By: Warp <agent@warp.dev>
---
 .../src/signatures/legacy/miss_cache.rs       | 18 ++-----
 .../src/signatures/legacy/miss_cache_tests.rs |  5 --
 .../src/signatures/legacy/registry.rs         | 17 +------
 .../src/signatures/legacy/registry_tests.rs   | 47 ++-----------------
 4 files changed, 10 insertions(+), 77 deletions(-)

diff --git a/crates/warp_completer/src/signatures/legacy/miss_cache.rs b/crates/warp_completer/src/signatures/legacy/miss_cache.rs
index 2ec33144d8b..f6cf419bf75 100644
--- a/crates/warp_completer/src/signatures/legacy/miss_cache.rs
+++ b/crates/warp_completer/src/signatures/legacy/miss_cache.rs
@@ -3,21 +3,9 @@ use std::sync::{RwLock, RwLockReadGuard, RwLockWriteGuard};
 
 const MAX_CACHED_MISSES: usize = 256;
 
-/// A bounded set of (lowercased) command names that recently failed to resolve to a signature,
-/// used by `SignatureCache::misses`.
-///
-/// This is a plain FIFO, not an LRU: once at capacity, inserting a new entry always evicts the
-/// *oldest-inserted* one, regardless of how recently any entry (including the one about to be
-/// evicted) was looked up again. `CommandRegistry` is a shared `Arc` behind a single global
-/// instance (see `CommandRegistry::global_instance`) that's called from multiple terminal
-/// sessions/panes concurrently, each generating completions on a background thread pool -- so
-/// this genuinely needs cross-thread synchronization, not just single-task interior mutability.
-/// But a negative cache is inherently approximate: a false negative (a miss this forgot) only
-/// costs one extra, cheap `lookup_fn` call, never a wrong answer. That's a much weaker
-/// requirement than an LRU implies, and dropping recency tracking is what makes a `RwLock` (an
-/// LRU would need every *hit* to also take a write lock, to move the entry to the front) the
-/// natural fit here, since a hit against `contains` becomes a pure read that never mutates
-/// anything -- the write lock is only ever needed for a genuinely new miss.
+/// A bounded, `RwLock`-guarded FIFO set of (lowercased) command names that recently failed to
+/// resolve to a signature. FIFO rather than LRU because a lookup here is a pure read that never
+/// needs to reorder anything, so the write lock is only needed for a genuinely new miss.
 pub(super) struct MissCache {
     capacity: usize,
     entries: RwLock<MissCacheEntries>,
diff --git a/crates/warp_completer/src/signatures/legacy/miss_cache_tests.rs b/crates/warp_completer/src/signatures/legacy/miss_cache_tests.rs
index 3589cae13cb..d04b0e8a580 100644
--- a/crates/warp_completer/src/signatures/legacy/miss_cache_tests.rs
+++ b/crates/warp_completer/src/signatures/legacy/miss_cache_tests.rs
@@ -32,20 +32,15 @@ fn test_stops_growing_at_capacity() {
 
 #[test]
 fn test_evicts_in_fifo_order_regardless_of_lookups() {
-    // Eviction is by insertion order, not recency: a hit against `contains` is a pure read and
-    // never protects an entry from eviction, unlike an LRU.
     let cache = MissCache::new(3);
     cache.insert("a".to_string());
     cache.insert("b".to_string());
     cache.insert("c".to_string());
 
-    // Repeatedly look up the oldest entry. Under an LRU this would move it to the front and
-    // protect it from eviction, but FIFO eviction ignores lookups entirely.
     for _ in 0..5 {
         assert!(cache.contains("a"));
     }
 
-    // Inserting one more entry should still evict "a", the oldest-inserted entry, not "b".
     cache.insert("d".to_string());
     assert!(
         !cache.contains("a"),
diff --git a/crates/warp_completer/src/signatures/legacy/registry.rs b/crates/warp_completer/src/signatures/legacy/registry.rs
index b37871f796a..59279f14d78 100644
--- a/crates/warp_completer/src/signatures/legacy/registry.rs
+++ b/crates/warp_completer/src/signatures/legacy/registry.rs
@@ -32,23 +32,10 @@ struct SignatureCache {
     lookup_fn: Box<SignatureLookupFn>,
     /// A map from (lowercased) command name to its signature. The use of [`MemoMap`] here allows
     /// us to safely return references to the contained signatures (as the map internally is an
-    /// append-only structure). Only lookups that actually resolve to a signature are stored (see
-    /// `get`), which keeps this bounded by the number of distinct command names that can ever
-    /// resolve to something -- the fixed embedded corpus (1,167 top-level commands as of this
-    /// writing), plus any explicitly `insert`-ed signatures -- rather than by every distinct
-    /// token a user has ever typed or pasted. See APP-5431.
+    /// append-only structure).
     signatures: MemoMap<String, Signature>,
     /// A bounded set of (lowercased) command names that recently failed to resolve to a
-    /// signature -- see `MissCache`'s doc comment for why it's a plain `RwLock`-guarded FIFO
-    /// rather than an LRU. Unlike `signatures`, `get` never hands out a reference into this set
-    /// -- it only ever returns a `bool` -- so eviction is sound: there's no borrow that could
-    /// outlive an evicted entry, which is exactly what keeps `signatures` itself append-only.
-    /// That's also what makes this bounded convenience rather than a fix for a performance
-    /// problem: a miss is already cheap to redo (`lookup_fn` probes the embedded,
-    /// already-compiled signature corpus, which does a binary search with no filesystem I/O or
-    /// JSON parsing on a miss -- JSON parsing only happens once we know there's a hit), so this
-    /// cache exists to save a handful of redundant lookups for a token that's retried in quick
-    /// succession, not to avoid expensive rework. See APP-5431.
+    /// signature.
     misses: MissCache,
 }
 
diff --git a/crates/warp_completer/src/signatures/legacy/registry_tests.rs b/crates/warp_completer/src/signatures/legacy/registry_tests.rs
index 262dc632eb2..a494d95be82 100644
--- a/crates/warp_completer/src/signatures/legacy/registry_tests.rs
+++ b/crates/warp_completer/src/signatures/legacy/registry_tests.rs
@@ -34,25 +34,12 @@ fn track_longest_name(signature: &Signature, longest: &mut (usize, String)) {
 
 #[test]
 fn test_all_known_signature_names_are_within_the_length_cap() {
-    // `SignatureCache::get`'s oversized-token fast path assumes no name that can ever be
-    // dynamically resolved -- from the embedded signature corpus or from
-    // `CommandRegistry::register_signature` -- exceeds it. This test establishes that invariant
-    // by actually walking both sources, rather than assuming it: if
-    // a future signature (in the corpus, or a newly `register_signature`-ed one) is added with a
-    // name that violates it, this test fails loudly instead of that name silently becoming
-    // unresolvable via lookup.
     let mut longest = (0, String::new());
 
     for signature in warp_command_signatures::commands() {
         track_longest_name(&signature, &mut longest);
     }
 
-    // Mirrors `CommandRegistry::register_warp_signatures`: the only other names ever
-    // `register_signature`-ed in production. Uses the raw `CommandFactory::command()` rather
-    // than `Args::clap_command()`, since the latter only *hides* subcommands based on
-    // `FeatureFlag` state (which requires flags to be initialized, unavailable in this test
-    // environment) without changing any name -- so this still covers every name that
-    // `clap_command()` would produce, feature flags notwithstanding.
     for channel in [Channel::Stable, Channel::Preview, Channel::Dev] {
         let mut clap_cmd = <warp_cli::Args as clap::CommandFactory>::command();
         let signature = crate::signatures::clap::signature_from_clap_command(
@@ -65,9 +52,11 @@ fn test_all_known_signature_names_are_within_the_length_cap() {
     let (max_len, longest_name) = longest;
     assert!(
         max_len <= MAX_CACHEABLE_COMMAND_LEN,
-        "found a command/subcommand name of length {max_len} ({longest_name:?}), which exceeds \
-         MAX_CACHEABLE_COMMAND_LEN ({MAX_CACHEABLE_COMMAND_LEN}) -- SignatureCache::get's \
-         oversized-token fast path assumes this can't happen"
+        "found a command/subcommand name of length {max_len} ({longest_name:?}), longer than \
+         MAX_CACHEABLE_COMMAND_LEN ({MAX_CACHEABLE_COMMAND_LEN}); SignatureCache::get's \
+         oversized-token fast path assumes no such name exists and would make this one \
+         unresolvable -- raise MAX_CACHEABLE_COMMAND_LEN or add back a fallback lookup path for \
+         oversized tokens"
     );
 }
 
@@ -274,10 +263,6 @@ fn test_alias_expansion_path_skips_flag_with_value_before_subcommand() {
 
 #[test]
 fn test_oversized_command_is_not_cached_and_resolves_to_none() {
-    // Regression test for APP-5431: a pathologically large single "command" token (e.g. a large
-    // blob of text pasted into the terminal input line) must not grow the (append-only)
-    // positive cache, nor the bounded negative cache, and doesn't match any registered
-    // signature.
     let registry = create_test_command_registry([test_signature()]);
     let positive_len_before = registry.signatures.signatures.len();
     let negative_len_before = registry.signatures.misses.len();
@@ -299,9 +284,6 @@ fn test_oversized_command_is_not_cached_and_resolves_to_none() {
 
 #[test]
 fn test_misses_are_never_cached_in_the_positive_cache() {
-    // Regression test for APP-5431: the positive cache only grows via successful lookups (see
-    // `SignatureCache::signatures`'s doc comment), so a command that never resolves to a
-    // signature must not add an entry there, however many times it's looked up.
     let registry = create_test_command_registry([test_signature()]);
     let len_before = registry.signatures.signatures.len();
 
@@ -312,12 +294,6 @@ fn test_misses_are_never_cached_in_the_positive_cache() {
 
 #[test]
 fn test_oversized_later_token_does_not_bypass_the_length_guard() {
-    // Regression test for APP-5431: `maybe_load_replacement_signature` resolves a *later*
-    // token (e.g. resolving `git` from `sudo git`) through the same `SignatureCache::get` the
-    // top-level token goes through (`self.signatures.get(token)`, where `self.signatures` is
-    // the `SignatureCache`, not the raw `MemoMap`). An oversized later token must take the same
-    // length-guarded path as an oversized top-level token: no lowercase allocation, and no
-    // growth of either cache.
     let sudo = warp_command_signatures::signature_by_name("sudo")
         .expect("global command signatures should include 'sudo'");
     let registry = create_test_command_registry([sudo]);
@@ -350,7 +326,6 @@ fn test_oversized_later_token_does_not_bypass_the_length_guard() {
         "looking up an oversized later token should not add an entry to the negative cache"
     );
 
-    // Exercise the alias-expansion path too, since it resolves later tokens the same way.
     let sudo = warp_command_signatures::signature_by_name("sudo")
         .expect("global command signatures should include 'sudo'");
     let ctx =
@@ -369,11 +344,9 @@ fn test_oversized_later_token_does_not_bypass_the_length_guard() {
 fn test_ordinary_commands_still_resolve_and_are_cached() {
     let registry = create_test_command_registry([test_signature()]);
 
-    // A registered command resolves correctly, case-insensitively.
     let found = registry.signature("TEST");
     assert_eq!(found.map(|s| s.name.as_str()), Some("test"));
 
-    // Repeated lookups hit the same cached entry rather than growing the cache.
     let len_after_first_lookup = registry.signatures.signatures.len();
     assert_eq!(registry.signature("test"), found);
     assert_eq!(registry.signatures.signatures.len(), len_after_first_lookup);
@@ -381,13 +354,6 @@ fn test_ordinary_commands_still_resolve_and_are_cached() {
 
 #[test]
 fn test_registered_signature_longer_than_the_cap_is_unresolvable() {
-    // Regression test for APP-5431: `SignatureCache::get` now returns `None` outright for any
-    // token over `MAX_CACHEABLE_COMMAND_LEN`, on the strength of
-    // `test_all_known_signature_names_are_within_the_length_cap` establishing that no real
-    // signature ever has a name this long. An explicitly `register_signature`-ed signature with
-    // an artificially oversized name is therefore *not* retrievable through `signature()` --
-    // a deliberate, accepted tradeoff, not a real-world regression, since that invariant test
-    // would fail first if this ever mattered.
     let long_name = "a".repeat(MAX_CACHEABLE_COMMAND_LEN + 1);
     let registry = create_test_command_registry([signature_with_name(&long_name)]);
 
@@ -397,9 +363,6 @@ fn test_registered_signature_longer_than_the_cap_is_unresolvable() {
 #[cfg(windows)]
 #[test]
 fn test_exe_suffix_is_trimmed_before_the_length_check() {
-    // Regression test for APP-5431 (review finding): the ".exe" suffix must be trimmed *before*
-    // the length check runs. A command name exactly at the cap, looked up with ".exe" appended
-    // (so the raw token exceeds the cap), must still resolve via the normal lookup path.
     let max_length_name = "a".repeat(MAX_CACHEABLE_COMMAND_LEN);
     let registry = create_test_command_registry([signature_with_name(&max_length_name)]);