fix: duplicate Ctrl+N/P/X handlers, write->edit allowlist mirroring, dead code removal

- Remove ~122 lines of unreachable duplicate Ctrl+N/P/X match arms in
  mod.rs — the early-block handler catches all three keys and continues,
  making the later match arms dead code. Fix Ctrl+X in the early block
  to call remove_chat + adjust chat_ui_states instead of computing
  'next chat' index.

- Fix session allowlist F2 alias re-prompt loop: when the user 'always
  allows' write/apply_patch, also register the pattern under the edit
  alias so enforce()'s most-restrictive merge doesn't return Ask every
  time. Mirrors edit→write/apply_patch in reverse direction too.
  load_session_allowlist routes through add_session_allowlist so
  persisted sessions also get the mirroring.

- Delete duplicate validate_write_path function body that was
  copy-pasted into mod tests (shadow copy — tests weren't testing
  production code).

- Rename validate_write_path → validate_path since it guards read/edit
  too. Fix error messages: 'Refusing to write to' → 'Refusing to use'.

- Remove dead code: draw_search_bar (31 lines), content_cols (8 lines),
  color + resolve_color import, scroll_to_line (7 lines),
  set_alert_overlay_with_title (8 lines), alert_overlay_active (4 lines).

- Remove 5 #[allow(dead_code)] annotations from items that are actually
  used. Replace block-level #[allow(dead_code)] on impl Renderer with
  targeted annotation on just the anchor marker method.

Author: Yogthos

src/permission/allowlist.rs

@@ -25,12 +25,6 @@ pub(crate) fn add(allowlist: &mut Vec<(String, Pattern)>, tool: &str, pattern_st
     allowlist.push((tool.to_string(), pattern));
 }
 
-pub(crate) fn load(allowlist: &mut Vec<(String, Pattern)>, entries: &[(String, String)]) {
-    for (tool, pat) in entries {
-        add(allowlist, tool, pat);
-    }
-}
-
 pub(crate) fn entries(allowlist: &[(String, Pattern)]) -> Vec<(String, String)> {
     allowlist
         .iter()
@@ -90,19 +84,6 @@ mod tests {
         assert_eq!(e.len(), 2, "got: {:?}", e);
     }
 
-    #[test]
-    fn load_dedupes_against_existing() {
-        let mut al = Vec::new();
-        let entries_in = vec![
-            ("bash".to_string(), "cargo *".to_string()),
-            ("bash".to_string(), "cargo *".to_string()),
-        ];
-        load(&mut al, &entries_in);
-        assert_eq!(entries(&al).len(), 1);
-        load(&mut al, &entries_in);
-        assert_eq!(entries(&al).len(), 1);
-    }
-
     #[test]
     fn clear_empties_the_list() {
         let mut al = make_allowlist();

src/permission/checker.rs

@@ -473,7 +473,7 @@ impl PermissionChecker {
         // dialogs for non-existent files.  Absolute paths and
         // relative paths with directory components or file
         // extensions pass through to the normal check.
-        if let Err(reason) = path::validate_write_path(path) {
+        if let Err(reason) = path::validate_path(path) {
             return CheckResult::Denied(reason);
         }
 
@@ -606,10 +606,31 @@ impl PermissionChecker {
 
     pub fn add_session_allowlist(&mut self, tool: String, pattern_str: &str) {
         allowlist::add(&mut self.session_allowlist, &tool, pattern_str);
+        // F2 write↔edit↔apply_patch aliasing: when the user "always
+        // allows" any of these three, also register the pattern under
+        // the other two so the alias check in enforce() doesn't
+        // re-prompt. Without this, a user who "always allows" write
+        // gets asked again on the next write because the edit-alias
+        // check returns Ask with no allowlist match.
+        match tool.as_str() {
+            "write" | "apply_patch" => {
+                allowlist::add(&mut self.session_allowlist, "edit", pattern_str);
+            }
+            "edit" => {
+                allowlist::add(&mut self.session_allowlist, "write", pattern_str);
+                allowlist::add(&mut self.session_allowlist, "apply_patch", pattern_str);
+            }
+            _ => {}
+        }
     }
 
     pub fn load_session_allowlist(&mut self, entries: &[(String, String)]) {
-        allowlist::load(&mut self.session_allowlist, entries);
+        // Route through add_session_allowlist (not allowlist::add
+        // directly) so the write↔edit alias mirroring fires for
+        // persisted sessions too.
+        for (tool, pat) in entries {
+            self.add_session_allowlist(tool.clone(), pat);
+        }
     }
 
     pub fn allowlist_entries(&self) -> Vec<(String, String)> {
@@ -1460,6 +1481,98 @@ mod tests {
         );
     }
 
+    /// F2 write↔edit aliasing: when a user "always allows" a write
+    /// path, the alias check against "edit" must also match so the
+    /// most-restrictive merge doesn't re-prompt on every subsequent
+    /// call. Without this, `enforce()` sees Allowed from write rules
+    /// but Ask from edit (no session-allowlist entry), and the
+    /// combined result is Ask — infinite re-prompt loop.
+    #[test]
+    fn add_session_allowlist_mirrors_write_to_edit() {
+        let mut cfg = PermissionConfig::default();
+        cfg.default = Some(Action::Ask);
+        let mut checker = PermissionChecker::new(
+            &cfg,
+            SecurityMode::Standard,
+            Some(std::path::PathBuf::from("/cwd-off-test-axis")),
+        );
+        checker.add_session_allowlist("write".to_string(), "/probe/src/**");
+
+        // The write tool itself hits the allowlist.
+        assert!(matches!(
+            checker.check_path("write", "/probe/src/main.rs"),
+            CheckResult::Allowed
+        ));
+        // The edit alias MUST also match — this is what enforce() checks.
+        assert!(matches!(
+            checker.check_path("edit", "/probe/src/main.rs"),
+            CheckResult::Allowed,
+        ),
+            "edit alias must reflect write session-allowlist entry"
+        );
+
+        // Reverse direction: "always allow" edit → write must match.
+        let mut checker2 = PermissionChecker::new(
+            &cfg,
+            SecurityMode::Standard,
+            Some(std::path::PathBuf::from("/cwd-off-test-axis")),
+        );
+        checker2.add_session_allowlist("edit".to_string(), "/probe/src/**");
+        assert!(matches!(
+            checker2.check_path("write", "/probe/src/main.rs"),
+            CheckResult::Allowed,
+        ),
+            "write must reflect edit session-allowlist entry"
+        );
+        assert!(matches!(
+            checker2.check_path("apply_patch", "/probe/src/main.rs"),
+            CheckResult::Allowed,
+        ),
+            "apply_patch must reflect edit session-allowlist entry"
+        );
+
+        // apply_patch → edit mirroring too.
+        let mut checker3 = PermissionChecker::new(
+            &cfg,
+            SecurityMode::Standard,
+            Some(std::path::PathBuf::from("/cwd-off-test-axis")),
+        );
+        checker3.add_session_allowlist("apply_patch".to_string(), "/probe/src/**");
+        assert!(matches!(
+            checker3.check_path("edit", "/probe/src/main.rs"),
+            CheckResult::Allowed,
+        ),
+            "edit must reflect apply_patch session-allowlist entry"
+        );
+
+        // Via load_session_allowlist too (persisted-session path).
+        let mut checker4 = PermissionChecker::new(
+            &cfg,
+            SecurityMode::Standard,
+            Some(std::path::PathBuf::from("/cwd-off-test-axis")),
+        );
+        checker4.load_session_allowlist(&[("write".to_string(), "/probe/src/**".to_string())]);
+        assert!(matches!(
+            checker4.check_path("edit", "/probe/src/main.rs"),
+            CheckResult::Allowed,
+        ),
+            "load_session_allowlist must also mirror write→edit"
+        );
+
+        // Non-aliased tools are unaffected.
+        let mut checker5 = fresh_checker();
+        checker5.add_session_allowlist("read".to_string(), "/tmp/**");
+        assert!(matches!(
+            checker5.check_path("read", "/tmp/foo.txt"),
+            CheckResult::Allowed,
+        ));
+        // read doesn't alias to write/edit.
+        assert!(
+            !checker5.is_session_allowed("write", "/tmp/foo.txt"),
+            "read allowlist entry must not leak to write"
+        );
+    }
+
     // load_session_allowlist roundtrip: persisted patterns from a previous
     // session should match the way they did when saved.
     #[test]

src/permission/path.rs

@@ -162,13 +162,13 @@ fn lexical_normalize(p: &Path) -> std::path::PathBuf {
 /// trigger permission dialogs.  Relative single-segment paths
 /// that are purely numeric ("1", "42") or trivially short
 /// ("a", "x") are never valid file names a well-behaved
-/// agent would genuinely want to write to; the model is
-/// confusing a counter, index, or file-descriptor number with
-/// a file path.
+/// agent would genuinely want to use; the model is confusing
+/// a counter, index, or file-descriptor number with a file
+/// path.
 ///
 /// Returns `Ok(())` for plausible paths, `Err(reason)` for
 /// paths that should be hard-rejected.
-pub fn validate_write_path(path: &str) -> Result<(), String> {
+pub fn validate_path(path: &str) -> Result<(), String> {
     let p = Path::new(path);
     if p.is_absolute() {
         return Ok(());
@@ -186,13 +186,13 @@ pub fn validate_write_path(path: &str) -> Result<(), String> {
     // with no extension ("a", "xy").
     if path.chars().all(|c| c.is_ascii_digit()) {
         return Err(format!(
-            "Refusing to write to numeric path {:?}. Use an absolute path with a real file name.",
+            "Refusing to use numeric path {:?}. Use an absolute path with a real file name.",
             path,
         ));
     }
     if path.chars().count() <= 2 {
         return Err(format!(
-            "Refusing to write to trivial path {:?}. Use an absolute path with a real file name.",
+            "Refusing to use trivial path {:?}. Use an absolute path with a real file name.",
             path,
         ));
     }
@@ -286,89 +286,48 @@ mod tests {
         let _ = std::fs::remove_dir_all(&dir);
     }
 
-    // ── validate_write_path ──────────────────────────────────────
-
-    /// Reject paths that are clearly LLM hallucinations before they
-    /// trigger permission dialogs.  Relative single-segment paths
-    /// that are purely numeric (\"1\", \"42\") or trivially short
-    /// (\"a\", \"x\") are never valid file names a well-behaved
-    /// agent would genuinely want to write to; the model is
-    /// confusing a counter, index, or file-descriptor number with
-    /// a file path.
-    ///
-    /// Returns `Ok(())` for plausible paths, `Err(reason)` for
-    /// paths that should be hard-rejected.
-    pub fn validate_write_path(path: &str) -> Result<(), String> {
-        let p = Path::new(path);
-        if p.is_absolute() {
-            return Ok(());
-        }
-        // Has a directory component — plausible relative path.
-        if path.contains('/') || path.contains('\\') {
-            return Ok(());
-        }
-        // Has a file extension — plausible filename.
-        if path.contains('.') {
-            return Ok(());
-        }
-        // Just a bare name.  Reject single-segment names that are
-        // purely numeric (\"1\", \"42\") or a single short token
-        // with no extension (\"a\", \"xy\").
-        if path.chars().all(|c| c.is_ascii_digit()) {
-            return Err(format!(
-                "Refusing to write to numeric path {:?}. Use an absolute path with a real file name.",
-                path,
-            ));
-        }
-        if path.chars().count() <= 2 {
-            return Err(format!(
-                "Refusing to write to trivial path {:?}. Use an absolute path with a real file name.",
-                path,
-            ));
-        }
-        Ok(())
-    }
+    // ── validate_path ────────────────────────────────────────────
 
     #[test]
     fn validate_accepts_absolute_paths() {
-        assert!(validate_write_path("/etc/hosts").is_ok());
-        assert!(validate_write_path("/Users/bob/src/main.rs").is_ok());
+        assert!(validate_path("/etc/hosts").is_ok());
+        assert!(validate_path("/Users/bob/src/main.rs").is_ok());
     }
 
     #[test]
     fn validate_accepts_relative_paths_with_separator() {
-        assert!(validate_write_path("src/main.rs").is_ok());
-        assert!(validate_write_path("lib/core.js").is_ok());
-        assert!(validate_write_path("..\\windows\\path").is_ok());
+        assert!(validate_path("src/main.rs").is_ok());
+        assert!(validate_path("lib/core.js").is_ok());
+        assert!(validate_path("..\\windows\\path").is_ok());
     }
 
     #[test]
     fn validate_accepts_relative_names_with_extension() {
-        assert!(validate_write_path("Cargo.toml").is_ok());
-        assert!(validate_write_path("README.md").is_ok());
-        assert!(validate_write_path("build.sh").is_ok());
+        assert!(validate_path("Cargo.toml").is_ok());
+        assert!(validate_path("README.md").is_ok());
+        assert!(validate_path("build.sh").is_ok());
     }
 
     #[test]
     fn validate_accepts_extensionless_names_that_are_not_trivial() {
         // Common extensionless filenames.
-        assert!(validate_write_path("Makefile").is_ok());
-        assert!(validate_write_path("Dockerfile").is_ok());
-        assert!(validate_write_path("README").is_ok());
-        assert!(validate_write_path("LICENSE").is_ok());
-        assert!(validate_write_path("abc").is_ok());
+        assert!(validate_path("Makefile").is_ok());
+        assert!(validate_path("Dockerfile").is_ok());
+        assert!(validate_path("README").is_ok());
+        assert!(validate_path("LICENSE").is_ok());
+        assert!(validate_path("abc").is_ok());
     }
 
     #[test]
     fn validate_rejects_numeric_paths() {
-        assert!(validate_write_path("1").is_err());
-        assert!(validate_write_path("42").is_err());
-        assert!(validate_write_path("007").is_err());
+        assert!(validate_path("1").is_err());
+        assert!(validate_path("42").is_err());
+        assert!(validate_path("007").is_err());
     }
 
     #[test]
     fn validate_rejects_short_nonsense_paths() {
-        assert!(validate_write_path("a").is_err());
-        assert!(validate_write_path("xy").is_err());
+        assert!(validate_path("a").is_err());
+        assert!(validate_path("xy").is_err());
     }
 }