fix(audit r2): permission rules + markdown tables + /retry + bold streaming + atomic save + UX polish (#56)

* fix(audit): permission rules for apply_patch/lsp/question + markdown tables

Round 1 of the follow-up audit fixes.

## PermissionConfig: apply_patch, lsp, question (CRITICAL/HIGH)

Three tools called `check_perm(..., "apply_patch", _)` /
`check_perm(..., "lsp", _)` / `check_perm(..., "question", _)`
but their keys were never registered in `PermissionConfig`. Users
could only allow/deny them via the global `*` default — no
per-pattern rules.

Each added as `Option<ToolPerm>` on `PermissionConfig` and wired
into `checker.rs`'s tool-rule map. Users can now write rules like:

  "apply_patch": { "**/*.rs": "allow", "**": "ask" }
  "lsp": "allow"
  "question": "ask"

## Markdown tables now render (CRITICAL)

`Tag::Table` / `TableHead` / `TableRow` / `TableCell` had empty
match arms; pipe-delimited tables in agent replies silently
dropped. `pulldown_cmark::Parser::new` doesn't emit table events
by default either — needs `Options::ENABLE_TABLES`.

Now enables tables and renders as box-drawn cards:

  │ col1 │ col2 │ col3 │
  ├──────┼──────┼──────┤
  │ a    │ b    │ c    │
  │ d    │ e    │ f    │

Column widths computed from longest cell content, capped to fit
inside `max_width` so a runaway cell can't break alignment.
Header row paints in `theme::header()`, body in `theme::agent()`,
separator in `theme::dim()`. Long cells truncate with `…` to
preserve the right border.

## Test plan

- [x] `cargo test --features plugin` -> 612 pass, 0 fail.
- [x] Both build profiles -> 0 warnings.

* fix(audit r2): /retry, streaming bold, apply_patch cache + CRLF

Round 2 — HIGH fixes.

## /retry pops the previous assistant response

Previously /retry only restored the last user prompt into the
editor and left the failed assistant reply in the session. On
retry the agent saw its own bad answer as context. Now calls
`undo_last(session)` first to pop the trailing assistant message
(plus the user message if `undo_last`'s pair logic fires), then
restores the user prompt to the editor and re-renders.

## write_line + write now apply Bold for bright colors

`render_viewport` applied `Attribute::Bold` to bright tones for
the phosphor glow effect; the streaming paths (`write_line` and
`write`) didn't. Streamed agent text rendered flat until the
next full repaint shifted it to bold. Both streaming paths now
check `theme::is_bright` and wrap output in `Bold` /
`NormalIntensity`, matching the viewport's per-row paint.

## apply_patch CRLF normalization on update

`apply_patch`'s `update` operation did a raw substring match on
the file's on-disk bytes. The LLM almost always emits `\n` for
line breaks even when the file is CRLF on disk, so the substring
match failed on Windows-style files. `edit.rs` already had this
normalization; copying the same pattern here:

1. Read original.
2. If CRLF detected, build a `\n`-normalized working copy for
   matching.
3. Normalize `old_text` and `new_text` to `\n` for the match +
   replace.
4. Re-apply CRLF on write-back so the file's line endings aren't
   silently changed.

## apply_patch clears cache once after the batch

Per-op `cache.clear()` was wasteful — a 5-op batch cleared 5
times. Now the cache is cleared exactly once after all ops in the
batch finish (success or partial failure).

## Test plan

- [x] `cargo test --features plugin` -> 612 pass, 0 fail.
- [x] Both build profiles -> 0 warnings.

* fix(audit r3): atomic save, auto-compact UX, set_label init, bash fallback split

Round 3 — MEDIUM fixes.

## Atomic session save

`save_session` did `fs::write(path, json)` directly. A crash mid-
write left a truncated `.json` that `find_recent_sessions` would
skip silently — the user would notice their session was missing
without knowing why. Two concurrent dirge processes saving the
same id could also interleave bytes.

Now write-temp-then-rename:
1. Write to `dir/.{id}.json.tmp`.
2. `sync_all()` (best-effort; non-fatal on unsupported FS).
3. `rename(tmp, target)`.
4. Clean up the tmp if rename fails.

Rename is atomic on every OS we target as long as both paths
live on the same filesystem (they do — same dir).

## Auto-compact failure is visible now

The old auto-compact error was a single dim red line that
scrolled past unnoticed. Users kept typing into an over-full
context and got mysterious context-length errors next turn.
Replaced with a framed alert:

  ╭─ ⚠ AUTO-COMPACT FAILED ─...─╮
  │ cause: <error>
  │ context is over the threshold — replies may start
  │ hitting context-length errors. Try /compress
  │ manually, /clear to start fresh, or restart with
  │ a larger context_window in config.
  ╰─...─╯

Same style as the permission alert; impossible to miss.

The success-path banner also got a soft accent treatment
(`▒░ auto-compacting context ░▒`) so the user sees auto-compact
running rather than wondering why the next prompt is slow.

## set_label uses ensure_back_compat_initialized

Every other mutation method (`add_message`, `pop_last_message`,
`switch_to_leaf`, `fork_at`, `compress`) calls
`ensure_back_compat_initialized`. `set_label` was still using the
older split call. Switched for consistency so future label-aware
code reading `message_store` doesn't trip on an uninitialized store.

## Bash segment splitting without `semantic-bash` feature

Without the tree-sitter feature, complex compound commands like
`safe_cmd && rm -rf /` were checked as a single string against
the bash permission rules. If `safe_cmd && rm` didn't match any
deny pattern, the dangerous part squeaked through.

Now does a best-effort coarse split on `&&` / `;` / `||` and
checks each segment separately. Command substitution / subshell
constructs (`$(...)`, backticks, `<(...)`, `>(...)`) still need
the full parser, so when one is detected we fall back to
whole-command check — that surfaces the unfamiliar form before
any segment runs, letting the user explicitly allow or deny.

## Test plan

- [x] `cargo test --features plugin` -> 612 pass, 0 fail.
- [x] Both build profiles -> 0 warnings.

* fix(audit r4): bold streaming for prompt/input/search, avatar redraw on alert

Round 4 — LOW fixes (final audit round).

## Prompt + input + search bar get Bold-glow

The bottom row (prompt indicator, input text, search bar)
rendered without `Attribute::Bold` for their bright colors. Chat
above bloomed; the bottom area looked dimmer. Each site now
checks `theme::is_bright` and wraps the write in Bold /
NormalIntensity, matching the viewport's per-row paint.

Status row stays unbolded — `theme::dim()` is dim by design
(two-tone phosphor depth) so `is_bright` correctly returns false.

## Avatar updates instantly on permission alert

Setting the avatar state to `Alert` happened before the alert
box rendered, but the bottom-row repaint that actually shows the
new face waited for the next event (typically the user's
keystroke answering the prompt). The face still showed the
in-flight tool (Reading/Writing/Bash) while the alert was up.

Now `draw_bottom` runs explicitly right after the state change so
the `(O_O)` Alert face appears at the same moment the alert box
does.

## Test plan

- [x] `cargo test --features plugin` -> 612 pass, 0 fail.
- [x] Both build profiles -> 0 warnings.

Note: the previously-flagged "grep.rs glob_to_regex doesn't escape
dots" finding turned out to be a false positive — line 43 of
`grep.rs` does push `\\.` for `.` correctly. Skipped.

---------

Co-authored-by: Yogthos <yogthos@gmail.com>

Author: yogthos

src/agent/tools/apply_patch.rs

@@ -76,11 +76,24 @@ fn apply_create(path: &str, content: &str) -> Result<String, String> {
 fn apply_update(path: &str, old_text: &str, new_text: &str) -> Result<String, String> {
     let original = std::fs::read_to_string(path).map_err(|e| format!("read failed: {}", e))?;
 
-    if !original.contains(old_text) {
+    // CRLF normalization to match `edit.rs`. The LLM almost always
+    // generates `\n` in `old_text` even when the file is CRLF on
+    // disk; without normalization the literal substring match fails.
+    // We normalize a working copy for matching but preserve the
+    // original's line endings on the write-back.
+    let crlf = original.contains("\r\n");
+    let normalized = if crlf {
+        original.replace("\r\n", "\n")
+    } else {
+        original.clone()
+    };
+    let needle = old_text.replace("\r\n", "\n");
+
+    if !normalized.contains(&needle) {
         return Err(format!("text not found in {}", path));
     }
 
-    let matches: Vec<_> = original.match_indices(old_text).collect();
+    let matches: Vec<_> = normalized.match_indices(&needle).collect();
     if matches.len() > 1 {
         return Err(format!(
             "text matches {} locations in {} — provide more context to make unique",
@@ -89,8 +102,20 @@ fn apply_update(path: &str, old_text: &str, new_text: &str) -> Result<String, St
         ));
     }
 
-    let updated = original.replacen(old_text, new_text, 1);
-    std::fs::write(path, &updated).map_err(|e| format!("write failed: {}", e))?;
+    let replacement = if crlf {
+        new_text.replace("\r\n", "\n")
+    } else {
+        new_text.to_string()
+    };
+    let updated_normalized = normalized.replacen(&needle, &replacement, 1);
+    // Restore CRLF line endings on write-back so we don't silently
+    // re-format the user's file.
+    let to_write = if crlf {
+        updated_normalized.replace('\n', "\r\n")
+    } else {
+        updated_normalized
+    };
+    std::fs::write(path, &to_write).map_err(|e| format!("write failed: {}", e))?;
     Ok(format!("updated {}", path))
 }
 
@@ -206,9 +231,6 @@ impl Tool for ApplyPatchTool {
 
             match result {
                 Ok(msg) => {
-                    if let Some(ref cache) = self.cache {
-                        cache.clear();
-                    }
                     // Record the touched path(s) for the info panel. Rename
                     // adds the *new* path; delete still records the path the
                     // user/agent operated on so the panel reflects the action.
@@ -235,6 +257,14 @@ impl Tool for ApplyPatchTool {
             }
         }
 
+        // Clear the cache once after the batch instead of once per op.
+        // Per-op clearing was correct but wasteful — a 5-op batch
+        // would clear 5 times. Subsequent tool calls within the same
+        // turn now see a single clean cache.
+        if let Some(ref cache) = self.cache {
+            cache.clear();
+        }
+
         Ok(results.join("\n"))
     }
 }

src/agent/tools/bash.rs

@@ -133,6 +133,35 @@ async fn check_bash_segments(
     }
     #[cfg(not(feature = "semantic-bash"))]
     {
-        check_perm(permission, ask_tx, "bash", command).await
+        // Best-effort coarse split when tree-sitter isn't compiled in.
+        // Without it, a command like `safe_cmd && rm -rf /` would be
+        // checked as a single string against the bash rules and might
+        // squeak through if `safe_cmd && rm` doesn't match any deny.
+        // Split on the unambiguous compound separators (`&&`, `;`,
+        // `||`) so each segment is checked individually. This won't
+        // catch command substitution or subshells — those need the
+        // tree-sitter feature for correct parsing — but it covers the
+        // common compound case.
+        let segments = command
+            .split(|c| c == ';')
+            .flat_map(|s| s.split("&&"))
+            .flat_map(|s| s.split("||"))
+            .map(|s| s.trim())
+            .filter(|s| !s.is_empty())
+            .collect::<Vec<&str>>();
+        // Flag command substitution / subshell constructs that need a
+        // full parser. Surface as one whole-command check so the user
+        // sees the unfamiliar form before any segment runs.
+        let has_substitution = command.contains("$(")
+            || command.contains('`')
+            || command.contains("<(")
+            || command.contains(">(");
+        if has_substitution {
+            return check_perm(permission, ask_tx, "bash", command).await;
+        }
+        for segment in &segments {
+            check_perm(permission, ask_tx, "bash", segment).await?;
+        }
+        Ok(())
     }
 }

src/permission/checker.rs

@@ -60,6 +60,9 @@ impl PermissionChecker {
             ("find_files", &config.find_files),
             ("list_dir", &config.list_dir),
             ("write_todo_list", &config.write_todo_list),
+            ("apply_patch", &config.apply_patch),
+            ("lsp", &config.lsp),
+            ("question", &config.question),
         ] {
             let Some(tp) = tool_perm else { continue };
             let mut entries = Vec::new();

src/permission/mod.rs

@@ -32,6 +32,16 @@ pub struct PermissionConfig {
     pub find_files: Option<ToolPerm>,
     pub list_dir: Option<ToolPerm>,
     pub write_todo_list: Option<ToolPerm>,
+    /// `apply_patch` — bulk multi-file patch tool. Mutates the
+    /// filesystem like `write`/`edit`; deserves per-pattern rules.
+    pub apply_patch: Option<ToolPerm>,
+    /// `lsp` — language-server queries (definition, references,
+    /// hover, etc.). Reads project files via the language server.
+    pub lsp: Option<ToolPerm>,
+    /// `question` — interactive user-input solicitation tool. Per-
+    /// pattern rules let users restrict which kinds of questions
+    /// the agent can ask.
+    pub question: Option<ToolPerm>,
     pub external_directory: Option<HashMap<String, Action>>,
     pub doom_loop: Option<Action>,
 }

src/session/mod.rs

@@ -446,7 +446,10 @@ impl Session {
         entry_id: &CompactString,
         label: Option<String>,
     ) -> Result<(), String> {
-        self.ensure_tree_initialized();
+        // Mirror the other mutation methods — keep tree + store in
+        // lockstep even though set_label only touches the tree, in
+        // case a future label-aware code path inspects the store.
+        self.ensure_back_compat_initialized();
         let node = self
             .tree
             .entries

src/session/storage.rs

@@ -58,7 +58,22 @@ pub fn save_session(session: &Session) -> anyhow::Result<()> {
     std::fs::create_dir_all(&dir)?;
     let path = dir.join(format!("{}.json", session.id));
     let json = serde_json::to_string_pretty(session)?;
-    std::fs::write(path, json)?;
+    // Atomic write: write to a sibling temp file, fsync, then rename
+    // over the target. A crash mid-write leaves the temp behind but
+    // never a truncated `.json`. The rename is atomic on every OS we
+    // target. Use the same parent dir so rename stays on one filesystem.
+    let tmp = dir.join(format!(".{}.json.tmp", session.id));
+    {
+        use std::io::Write;
+        let mut f = std::fs::File::create(&tmp)?;
+        f.write_all(json.as_bytes())?;
+        // Best-effort fsync; non-fatal if the platform doesn't support it.
+        let _ = f.sync_all();
+    }
+    if let Err(e) = std::fs::rename(&tmp, &path) {
+        let _ = std::fs::remove_file(&tmp);
+        return Err(e.into());
+    }
     Ok(())
 }
 

src/ui/markdown.rs

@@ -64,12 +64,129 @@ fn bullet_prefix(in_blockquote: bool) -> &'static str {
     if in_blockquote { "  ┊ " } else { "  • " }
 }
 
+/// Render a markdown table as `| col | col |` rows with a separator
+/// line below the header. Columns are padded so the right borders
+/// align. Caps each cell's display at the available width so a
+/// long cell doesn't break alignment. No-ops when both header and
+/// rows are empty.
+fn render_table(
+    header: &[String],
+    rows: &[Vec<String>],
+    max_width: usize,
+    out: &mut Vec<LineEntry>,
+) {
+    if header.is_empty() && rows.is_empty() {
+        return;
+    }
+    // Compute per-column max char width.
+    let ncols = header
+        .len()
+        .max(rows.iter().map(|r| r.len()).max().unwrap_or(0));
+    if ncols == 0 {
+        return;
+    }
+    let mut widths = vec![0usize; ncols];
+    for (i, cell) in header.iter().enumerate() {
+        widths[i] = widths[i].max(cell.chars().count());
+    }
+    for row in rows {
+        for (i, cell) in row.iter().enumerate() {
+            widths[i] = widths[i].max(cell.chars().count());
+        }
+    }
+    // Cap any single column to avoid one runaway cell blowing the
+    // line width. Distribute available width: target inner width =
+    // max_width - 4 (for outer `| ` + ` |`), minus 3*(ncols-1) for
+    // ` | ` separators. Cells get clipped to fit.
+    let inner = max_width.saturating_sub(2 * 2);
+    let sep_overhead = if ncols > 1 { 3 * (ncols - 1) } else { 0 };
+    let cell_budget = inner.saturating_sub(sep_overhead);
+    let per_col = if ncols > 0 { cell_budget / ncols } else { 0 };
+    for w in widths.iter_mut() {
+        if per_col > 0 && *w > per_col {
+            *w = per_col;
+        }
+    }
+
+    let fit = |cell: &str, w: usize| -> String {
+        let chars: Vec<char> = cell.chars().collect();
+        if chars.len() <= w {
+            let mut s: String = chars.iter().collect();
+            for _ in chars.len()..w {
+                s.push(' ');
+            }
+            s
+        } else if w <= 1 {
+            chars.iter().take(w).collect()
+        } else {
+            let mut s: String = chars.iter().take(w - 1).collect();
+            s.push('…');
+            s
+        }
+    };
+
+    let render_row = |row: &[String], widths: &[usize]| -> String {
+        let mut s = String::with_capacity(max_width);
+        s.push_str("│ ");
+        for i in 0..widths.len() {
+            if i > 0 {
+                s.push_str(" │ ");
+            }
+            let cell = row.get(i).map(String::as_str).unwrap_or("");
+            s.push_str(&fit(cell, widths[i]));
+        }
+        s.push_str(" │");
+        s
+    };
+
+    let sep = {
+        let mut s = String::with_capacity(max_width);
+        s.push('├');
+        for (i, w) in widths.iter().enumerate() {
+            if i > 0 {
+                s.push('┼');
+            }
+            for _ in 0..(w + 2) {
+                s.push('─');
+            }
+        }
+        s.push('┤');
+        s
+    };
+
+    if !header.is_empty() {
+        out.push(LineEntry {
+            text: CompactString::new(&render_row(header, &widths)),
+            color: crate::ui::theme::header(),
+        });
+        out.push(LineEntry {
+            text: CompactString::new(&sep),
+            color: crate::ui::theme::dim(),
+        });
+    }
+    for row in rows {
+        out.push(LineEntry {
+            text: CompactString::new(&render_row(row, &widths)),
+            color: crate::ui::theme::agent(),
+        });
+    }
+    out.push(LineEntry {
+        text: CompactString::new(""),
+        color: crate::ui::theme::agent(),
+    });
+}
+
 pub fn markdown_to_styled(text: &str, max_width: usize) -> Vec<LineEntry> {
     if text.is_empty() {
         return Vec::new();
     }
 
-    let parser = pulldown_cmark::Parser::new(text);
+    // Enable GFM tables so `Tag::Table*` events actually fire.
+    // Without this, table syntax falls back to plain paragraphs and
+    // the table never reaches `render_table`.
+    let mut opts = pulldown_cmark::Options::empty();
+    opts.insert(pulldown_cmark::Options::ENABLE_TABLES);
+    let parser = pulldown_cmark::Parser::new_ext(text, opts);
     let mut result = Vec::new();
     let mut acc = String::new();
 
@@ -78,6 +195,17 @@ pub fn markdown_to_styled(text: &str, max_width: usize) -> Vec<LineEntry> {
     let mut in_blockquote = false;
     let mut ordered_list = false;
     let mut list_item_count: u64 = 0;
+    // Table accumulation: pulldown_cmark emits TableHead → (Row × N
+    // cells) for the header row, then more TableRow blocks for body.
+    // We collect cells into `current_cell`, rows into `current_row`,
+    // and the whole table into `table_header` + `table_rows`, then
+    // render with column-aligned padding when the table ends.
+    let mut in_table = false;
+    let mut in_table_head = false;
+    let mut current_cell = String::new();
+    let mut current_row: Vec<String> = Vec::new();
+    let mut table_header: Vec<String> = Vec::new();
+    let mut table_rows: Vec<Vec<String>> = Vec::new();
 
     for event in parser {
         match event {
@@ -108,10 +236,23 @@ pub fn markdown_to_styled(text: &str, max_width: usize) -> Vec<LineEntry> {
                     list_item_count += 1;
                 }
                 Tag::FootnoteDefinition(_) => {}
-                Tag::Table(_) => {}
-                Tag::TableHead => {}
-                Tag::TableRow => {}
-                Tag::TableCell => {}
+                Tag::Table(_) => {
+                    flush_acc(&acc, crate::ui::theme::agent(), max_width, &mut result);
+                    acc.clear();
+                    in_table = true;
+                    table_header.clear();
+                    table_rows.clear();
+                }
+                Tag::TableHead => {
+                    in_table_head = true;
+                    current_row.clear();
+                }
+                Tag::TableRow => {
+                    current_row.clear();
+                }
+                Tag::TableCell => {
+                    current_cell.clear();
+                }
                 _ => {}
             },
             Event::End(tag_end) => match tag_end {
@@ -226,21 +367,37 @@ pub fn markdown_to_styled(text: &str, max_width: usize) -> Vec<LineEntry> {
                     });
                 }
                 TagEnd::FootnoteDefinition => {}
-                TagEnd::Table => {}
-                TagEnd::TableHead => {}
-                TagEnd::TableRow => {}
-                TagEnd::TableCell => {}
+                TagEnd::Table => {
+                    render_table(&table_header, &table_rows, max_width, &mut result);
+                    in_table = false;
+                }
+                TagEnd::TableHead => {
+                    table_header = std::mem::take(&mut current_row);
+                    in_table_head = false;
+                }
+                TagEnd::TableRow => {
+                    if !in_table_head {
+                        table_rows.push(std::mem::take(&mut current_row));
+                    }
+                }
+                TagEnd::TableCell => {
+                    current_row.push(std::mem::take(&mut current_cell));
+                }
                 _ => {}
             },
             Event::Text(t) => {
-                if in_code_block {
+                if in_table {
+                    current_cell.push_str(&t);
+                } else if in_code_block {
                     acc.push_str(&t);
                 } else {
                     acc.push_str(&t);
                 }
             }
             Event::Code(t) => {
-                if in_code_block {
+                if in_table {
+                    current_cell.push_str(&t);
+                } else if in_code_block {
                     acc.push_str(&t);
                 } else {
                     acc.push_str(&t);