Per-turn read/grep/list cache with mutation invalidation by yogthos · Pull Request #3 · dirge-code/dirge

yogthos · 2026-05-19T03:48:08Z

Summary

New `ToolCache` (Arc<Mutex> + atomic generation counter), cleared at the start of every agent turn
Read-only tools (read/grep/find_files/list_dir) check the cache before doing filesystem work
Write/edit/bash clear the cache on success so a re-read after mutation returns fresh content (review fix)
Provider builder refactored: macro-ized variant construction in `AnyAgent::build`

Test plan

Unit tests for cache hit/miss, clear, clone-sharing, generation invalidation
Manual: model that does read → edit → read same file in one turn returns updated content on the second read
Manual: repeated identical read in one turn returns instantly (cache hit)

- Generation-based ToolCache shared across read-only tools (read, grep, find_files, list_dir) - Cache clears automatically before each new prompt via spawn_agent - Repeated identical tool calls within a turn return cached results - AnyAgent refactored from enum to struct to hold ToolCache alongside inner agent - 4 unit tests for cache hit/miss/clear/clone sharing

Without this, a model that reads a file, mutates it, and re-reads in the same turn gets the stale pre-mutation content back from the per-turn cache. Plumb the ToolCache into WriteTool/EditTool/BashTool via new with_cache constructors and call cache.clear() after a successful mutation. Bash conservatively clears unconditionally since we can't tell from outside whether the command touched the filesystem.

…t tool count + token parity (#66) TDD: tests first, all 6 new tests failed initially, then implemented. ## Round A — hook error sanitize + dedup Two bugs in PR #64's hook-error notification path: 1. Multi-line / tab-containing Janet errors broke the `level\tmsg\n` wire format. A `(error "trace\n at file:42")` produced multiple malformed notification entries (one per source line), the first with truncated content and the rest filtered out as malformed. drain_notifications splits raw on `\n` per entry and on first `\t` per level/msg — both control chars now sanitized in Janet before push. 2. A buggy `on-message-update` hook (fires ~every 16 streamed tokens) flooded the chat with thousands of identical "[plugin] hook X.Y errored: ..." banners during a single long response. Now deduped: two new Janet vars track the most-recent sanitized error msg + a consecutive-repeat count; identical errors just bump the count instead of pushing. On drain, any outstanding count is flushed as a "(repeated N times)" summary entry. Implementation: - `harness/sanitize-hook-err` (new) normalizes `\t` → space and `\n`/`\r\n` → ` | `. Distinct hook errors stay separate; only consecutive identical ones collapse. Wrote with explicit nested `string/replace-all` calls — Janet's `->` threading macro would pass the string in the wrong arg position (string/replace-all expects `(patt subst str)`). - `harness/push-hook-err` (new) does the dedup check using `harness-last-hook-err-msg` + `harness-last-hook-err-count` module-level vars. - `drain_notifications` flushes pending dedup count before reading the notif list so a 50× repeat shows up as a single "(repeated 50 times)" entry in the next drain. - The catch arm in `dispatch` calls these instead of appending directly. Wrapped in explicit `(do ...)` for Janet's single-form catch-body semantics. ## Round B — partial-on-abort trailer notes tool calls PR #65 saved the streamed assistant text on abort but didn't indicate that tool calls had also run in the same turn (whose results aren't in `response_buf` — only Token events accumulate there). The LLM on next turn would see the partial as a definitive "this was my reply" and could re-run side-effecting tools. `capture_partial_on_abort` now takes a `tool_calls_in_turn: u32` parameter. When non-zero, the trailer reads: [interrupted by user (Ctrl+C); 2 tool calls ran in this turn — results not preserved] Singular case ("1 tool call ran") uses the right noun. UI loop tracks `tool_calls_this_run: u32`, incremented on every `AgentEvent::ToolCall`, reset on `Done`/`Interjected`/both abort sites (since each marks the end of one agent run). ## Round C — token-accumulator parity on abort `Done` and `Interjected` branches both update `session.total_tokens` alongside the message add. The abort path didn't — made aborted turns look like zero-token contributions in the placeholder field. Fixed with an explicit `session.total_tokens.saturating_add(Session::estimate_tokens(&stashed))` inside `capture_partial_on_abort`. Both fields stay under the `TODO(cost-tracking)` comment but at least they're now internally consistent. ## Test plan - [x] 6 new tests (3 plugin dispatch + 3 capture_partial_on_abort + 2 updated existing tests with new signature). - [x] `cargo test --features plugin` -> 622 pass, 0 fail. - [x] `cargo build --all-features` -> compiles. ## Skipped (observational, not bugs) - #3 print/loop mode notifications never drained: print mode is non-interactive; tracing::warn (via `--verbose`) is the right channel. - #4 Janet `err` non-string-coerced: `(string ...)` calls Janet's `tostring` which handles any value type. Documented behavior. - #7 markdown rendering of `[interrupted by user (Ctrl+C)]`: not a link by pulldown-cmark's rules; visually acceptable inline. Co-authored-by: Yogthos <yogthos@gmail.com>

Track F-HIGH #3 from ROADMAP.md. ## Problem `handle_compress` (`ui/slash.rs:115-123`) did a reverse token-budget scan to pick the cut index, then drained `messages[..cut_idx]` into the summary. If that scan landed on an Assistant message, the kept tail started with [Assistant, …]. After compress, the session became [SystemSummary, Assistant, …] — broken role sequence. Anthropic and OpenAI reject with HTTP 400. The reverse-scan was content-only: it accumulated tokens until the threshold without considering role-boundary constraints. ## Fix New `align_cut_to_user_boundary(messages, cut_idx) -> usize` helper. Walks `cut_idx` forward until the message at that index is a `User` message (or the index reaches end-of-array). Called right after the existing reverse-scan, before any state mutation. Matches opencode's `splitTurn` discipline in `session/compaction.ts:161-184`. ## Tests 5 new tests in `ui::slash::tests`: - `align_cut_advances_past_assistant_to_next_user`: cut_idx landed on Assistant → advance to next User. - `align_cut_idempotent_when_already_on_user`: User-boundary cut unchanged. - `align_cut_past_end_clamps`: out-of-range index stays in range. - `align_cut_returns_end_when_no_user_in_tail`: no User after cut → return messages.len() (caller surfaces "nothing to compress"). - `align_cut_skips_system_to_user`: System messages (prior summaries) are also skipped — only User starts a kept tail. 656 pass (was 651). All build profiles clean. Co-authored-by: Yogthos <yogthos@gmail.com>

…aths (#111) 23 audit findings verified REAL via parallel agent verification + cross-check against opencode/pi reference patterns. Shipping the 10 most concrete fixes here; the rest go in a follow-up docs/test batch. ## Security - **#9 bash quote_aware_split missed bare `|`** — `safe_cmd | rm -rf /` was treated as one segment; only the LHS got permission-checked. Pipe RHS rode in unchecked under the fallback (non-semantic-bash) path. Added single-byte `|` split after `||` is matched. The tree-sitter path was already correct. - **#4 read.rs no binary detection** — feeding a PDF/ELF/.pyc into the LLM as lossy UTF-8 wasted tokens and confused the model. Ported opencode `read.ts:153-198`: reject by extension list (zip/exe/.o/.pdf/.png/etc.), then sniff the first 4 KiB — null byte = binary, >30% non-printable = binary. Clear error message tells the agent to use bash + xxd instead. ## Correctness - **#2 skill override inverted** — README contract: "Project skills override global skills by name". Code used `map.entry(name).or_insert(skill)` which KEEPS the first (global) value and silently drops project overrides. Switch to `map.insert` (last-write-wins) since globals iterate first and project iterates second. - **#37 skill empty name** — frontmatter `name:` with empty value parsed to "", which then matched any `skill ""` call silently. Fall back to directory name when frontmatter name is empty/whitespace-only. - **#1 session_tree.janet hook never fired** — plugin defined `(defn on-message ...)` but `(def hooks [])` was empty AND the hook name doesn't exist (dirge uses `on-message-update`). `/label` was permanently broken ("no entry yet"). Fix: rename to `on-message-update` + register in hooks vector. - **#7 workflow.janet hooks vector missing entries** — plugin defined `workflow-on-tool-end`, `-on-error`, `-on-complete` but only registered the first four hook names. Three hooks were dead. Added them. - **#26 MCP malformed JSON silently empty args** — `serde_json::from_str(&args).unwrap_or_default()` turned bad JSON into None, sending the server an empty argument set. Server then errored with confusing "missing required field" instead of dirge surfacing the actual parse error. Now returns ToolError with the parse error message + first 200 chars of the offending JSON. - **#22 /prompt default unreachable** — README documents `default` as a built-in prompt (prompts/default.md exists), but `/prompt default` was intercepted as a magic "clear" keyword. If `default` is registered in `context.prompts`, the new branch falls through to the normal name-lookup. Only acts as clear-keyword when no `default` prompt is present (legacy fallback). - **#23 /allow add accepted invalid tools** — typo `/allow add bsah ...` silently created an inert rule the user couldn't debug. Added a known-tools whitelist matching PermissionConfig fields; unknown tools error with the valid list. ## Performance + correctness - **#11 grep loaded whole files into memory** — no size cap meant a 9MB file got fully buffered. Added 10 MiB per-file cap via metadata pre-check. - **#15 Python dunder methods marked non-exported** — `!name.starts_with('_')` treats `__init__`/`__call__`/etc. as private, even though they're Python's standard public protocol. Recognize `__x__` dunder pattern as exported. ## UI - **#36 panel char-count truncation vs Unicode width** — panel truncation used `chars().count()` while wide emoji and CJK take 2 cells. A status line with an emoji overflowed the right border by one cell. Switched to `UnicodeWidthStr::width` for both truncation and padding. ## Tests 4 new regression tests: - `test_is_binary_extension_known` — pdf/tgz/.so/.jpg/.pyc - `test_is_binary_content_null_byte` — null byte trigger, UTF-8 Japanese stays clean, all-non-printable triggers - `quote_aware_split_splits_on_bare_pipe` — pipe security - `quote_aware_split_or_and_pipe_distinct` — `a || b | c` produces 3 segments, not 2 725 plugin / 599 default pass. All build profiles clean. ## Verified false positives (not fixed, audit was wrong) - #3 cache.rs clear() race — generation counter gating in `get` makes stale entries invisible, no correctness impact. - #17 DeepSeek auto-detect priority — auto-detect only fires when env vars present; default-default is still OpenRouter. - #19 semantic tools in collision filter — semantic tools added separately, can't be shadowed by MCP. - #20 glob global gitignore — intentionally disabled to match grep behavior. - #28 nearest_root blocking std::fs — function doesn't exist in current code. - #32 ReadArgs.path vs GrepArgs.path — semantically different by design (file vs dir), documented in schema. - #33 install_plugin_providers dead-without-feature — gated with explicit `#[cfg_attr(not(feature), allow(dead_code))]`. - #34 websearch double-gated — config + API key serve distinct purposes (enable + auth). ## Deferred to follow-up batches Docs-only fixes (#6 CONFIG.md tools, #12 temperature, #13 --api-key, #14 acp_host/port), MCP/LSP architecture (#8, #25, #27), test gaps (#38-40), and lower-priority polish — all in a follow-up PR. Co-authored-by: Yogthos <yogthos@gmail.com>

…d exports (#117) Four cross-cutting cleanups identified in the adapter review, applied across all 9 language adapters (bash + clojure + go + ruby + rust + java + c + cpp + python + typescript). ## 1. Shared helpers — `semantic/common.rs` Three helpers previously duplicated 7+ times across adapters now live in one place: - `node_text(node, source)` — UTF-8 decode with structured-log fallback (see #3). - `find_node_at_range(root, start, end)` — depth-first range match used by every `find_callees_in_range` impl. - `signature_first_line(node, source)` and `signature_up_to_body(node, source)` — the two signature builders. Per-adapter rules choose between them based on the language's body-field convention. `ByteRange::from(tree_sitter::Node)` (in `types.rs`) replaces the 7 hand-written constructors. Each adapter's local `text()` / `range()` shim is preserved as a one-liner that delegates, keeping the call-site shape (`self.text(n, s)`) so the bulk of each adapter reads unchanged. Net effect: ~150 lines of duplication removed; any future fix to range arithmetic / signature truncation lands in one place. ## 2. `ImportKind` normalization `Import` gained a `kind: ImportKind` field with three variants: - `Header` — C / C++ `#include` - `Module` — single-token namespaces (Go `"fmt"`, Python `os.path`, Ruby `'json'`, Clojure `clojure.string`) - `Qualified` — fully-qualified names with explicit scoping (Java `java.util.List`, Rust `std::sync::Arc`, TypeScript module specifiers) Lets cross-language queries ("what files import this module?") normalize without re-parsing the source string. Each adapter sets `kind` on every `Import` it constructs. ## 3. UTF-8 fallback → `tracing::debug!` `node.utf8_text(s).unwrap_or("")` (the pattern every adapter used) was silent. The fallback is now logged at `debug` level via `common::node_text` — visible under `--verbose` (which enables `dirge=debug`), quiet by default. Chose `debug` over `warn` deliberately: UTF-8 failures aren't load-bearing (the resulting empty symbol name is filtered out downstream) and the log line exists for diagnostic purposes only. Default-mode users running on a single oddball file shouldn't see a noisy warning. ## 4. `exports` field populated The field on `ExtractedFile` was `Vec::new()` in every adapter; now backfilled from `is_exported=true` symbols at the end of each `extract()`. TypeScript + Python preserve their existing explicit-exports list (TS index re-exports are load-bearing); other adapters fall through to the is_exported-derived backfill when their per-language exports vec is empty. Consumers asking "what does this file export?" can now read `extracted.exports` directly instead of iterating the symbol vec. ## Tests 3 new regression tests: - `byte_range_from_node_uses_1_based_lines` — pins the new `From<Node>` impl's line-number contract (the 7 hand-written copies all used `row + 1`; the shared converter has to match). - `imports_are_tagged_module_kind` (Clojure) — verifies the kind classification. - `exports_mirror_is_exported_symbols` (Clojure) — verifies the backfill picks up `defn` while filtering `defn-`. 812 all-features / 729 plugin / 603 default pass (was 809 / 729 / 603). All build profiles + `cargo fmt --check` clean. ## Files touched - `src/semantic/common.rs` (new) - `src/semantic/types.rs` — `ImportKind` enum, `Import.kind` field, `ByteRange::From<Node>` impl, `exports` docstring. - `src/semantic/mod.rs` — wire `common` module. - `src/semantic/adapters/{bash,clojure,go,ruby,rust,java,c,cpp,python,typescript}.rs` — refactored to use shared helpers + new Import shape + populate exports. Co-authored-by: Yogthos <yogthos@gmail.com>

…nd hygiene Self-review of the chamber + collapse work found 8 actionable issues. Addressing all in one round. - **#1 CRITICAL — ContextOverflow infinite loop on compress no-op** (`ui/slash.rs`, `ui/mod.rs`): `handle_compress` had three `Ok` paths that returned without compacting (within-limits, nothing-to-cut, summary-too-large). The auto-recovery branch treated any `Ok` as success and respawned the run against the SAME history, which immediately re-emitted `ContextOverflow` → loop. Added `CompressOutcome::{Compacted, NoOp{reason}}`; auto-recovery now respawns only on `Compacted` and surfaces a "made no progress" error otherwise. The interactive `/compress` and auto-Done paths ignore the discriminant — only the success/error path matters there. - **#2 HIGH — ContextOverflow re-runs side-effecting tools**: the interactive retry loop already refuses to retry once `had_tool_calls=true`; the new auto path bypassed that safety. Now gates respawn on `tool_calls_this_run == 0`; if any tool ran in the failed turn, compact happens but auto-retry is refused and the user must re-issue. - **#3 HIGH — char-truncated body wasn't stashed for Ctrl+O**: `render_tool_output` returned `None` when only `chars_truncated > 0` (no line truncation) — Ctrl+O reported "nothing to expand" despite the visible `+N chars truncated` footer. Now stashes whenever EITHER signal indicates hidden content. Regression test: `render_tool_output_stashes_on_char_truncation_alone`. - **#4 HIGH — `last_collapsed` persisted across turns**: a Ctrl+O press would expand a collapsed result from any prior unrelated turn. Cleared on prompt-send (every user submission starts a new turn) and on ContextOverflow respawn. - **#5 MEDIUM — Ctrl+O was one-shot via `.take()`**: switched to `.as_ref().cloned()` so a second Ctrl+O re-emits the same expand; stash overwrites on the next collapse or clears on next turn. - **#6 MEDIUM — empty `resolved_name` painted unnamed chamber**: added an early-out: when name resolution drops to empty (no `last_tool_name`, no buffered call by id), emit a dim `(unresolved tool)` trailer + chamber bottom and skip the body paint. - **#7 MEDIUM — edit colorized diff lost when `last_tool_name` drained**: `is_edit` was gated on `last_tool_name`, falling through to plain `render_tool_output` when the slot drained (same shape as the chamber-orphan bug). Now gates on `resolved_name`. - **#8 DESIGN/SEC — `banner_value` unsanitized at expand time**: the initial chamber TOP sanitizes the banner; the Ctrl+O reprint did not. ANSI-bearing tool args (MCP, plugin, attacker-shaped filename) could paint raw at expand. Sanitize once at stash time in `render_tool_output`. Also (review #9, reviewer recommendation): swapped `apply_patch` out of `tool_skips_collapse` and `read` in. `read` is what the user/LLM explicitly asked for — defaulting to 4 lines defeated the request. `apply_patch` output is usually a short "N ops" summary; the rare per-op-failure spew is the right place for Ctrl+O to engage. Tests: 625 pass (3 added — char-truncation stash, apply_patch collapses, exempt-set without apply_patch). Updated 2 existing tests for the exempt-set change. Fmt clean.

…view Adversarial review of the per-prompt deny-list architecture flagged two real bypasses + several defense-in-depth gaps. Addressing. - **#1 CRITICAL — MCP tools bypassed the deny-list entirely**: `McpTool::call` passes the umbrella name `"mcp_tool"` to `check_perm`. The deny-list match is literal `==` (now case- insensitive), so a prompt declaring `deny_tools: [edit]` would NOT match an MCP server's `edit` tool — the LLM could route filesystem writes through any MCP server unscathed. Added `PermissionChecker::any_prompt_denied(&[name1, name2, ...])` public probe; `McpTool::call` now checks (concrete tool name, `mcp_tool:<server>:<name>` qualified form, umbrella `mcp_tool`) before invoking `check_perm`. Any hit returns a hard denial. - **#2 CRITICAL — ACP never installed the prompt deny-list**: the ACP bridge built a fresh `PermissionChecker` per session and never wired in `context.current_prompt_deny_tools`. Plan mode was a no-op for editor clients. Mirror the `apply_prompt_deny` call from `main.rs::build_channels` into the ACP `run_prompt` path right after `build_acp_permission`. - **#5 MEDIUM — `glob` / `repo_overview` added to PermissionConfig**: both were filesystem walkers reachable via the perm checker but not declared as user-configurable in `PermissionConfig`. User- level `permission.glob = "deny"` would silently fall through to the `*` default. Added the fields + the per-tool rule loop entry. - **#6 MEDIUM — `plan_enter` / `plan_exit` now consult deny-list**: both intentionally skip `check_perm` (the confirmation dialog IS the user-prompt). But the prompt deny-list should still apply — a strict-mode prompt that says `deny_tools: [plan_exit]` should refuse the call WITHOUT opening the dialog. Added a thin `check_prompt_deny` helper that queries `any_prompt_denied` before opening the channel. - **#7 MEDIUM — case-insensitive tool-name matching**: `deny_tools: [Edit]` (typo capitalization) used to silently no-op. `is_prompt_denied` now uses `eq_ignore_ascii_case`; the frontmatter parser also lowercases at load so the stored list is canonical in every consumer (status line, UI, etc.). - **#9 LOW — warn on unknown tool names in `deny_tools`**: at prompt load time, cross-check every `deny_tools` entry against a `KNOWN_TOOLS` list. Warns once per unknown entry, with the full known set printed for guidance. MCP-server-exported tool names will trigger this benignly; documented inline. - **#3 HIGH — pin order with tests**: three new checker tests pin the contract: - `prompt_deny_any_matches_concrete_and_qualified_mcp_names` (locks the MCP bypass fix) - `prompt_deny_is_case_insensitive` - existing tests continue passing - **#4 plugin trust boundary documented**: per the review's recommendation, added a "Plugin trust boundary" section to CONFIG.md acknowledging that plugins are inside the trust boundary and not sandboxed. Not addressed (intentional): - #8 doom-loop UI nudge on repeated deny-list hits — UX polish. - #10 `/prompt default` clear confirmation — user-typed; would have to confirm every clear, including the legitimate ones. 634 tests pass; fmt clean.

…ry, repo_overview walk Self-review of the last 5 commits surfaced 9 findings. Addressing all. - **#1 MEDIUM — `repo_overview` permission patterns got command-glob semantics**: `is_path_tool_name` was updated for `glob` but not `repo_overview`, so a config rule like `"repo_overview": { "/etc/**": "deny" }` parsed `**` as command-glob (no path-spanning match). Added `repo_overview` to the path-tool arm. - **#2 MEDIUM — `providers.<name>` override silently missed on capitalised provider names**: `parse_provider` is case-insensitive but `HashMap::get(provider)` is not, so `--provider Anthropic` built the client fine but the `providers.anthropic` chunk-timeout override was a no-op. Lowercased the lookup key in `resolve_stream_chunk_timeout` and `resolve_provider_info` (with the original key as a fallback so existing exact-case configs keep working). - **#3 MEDIUM — `Prompt.description` parsed but never displayed**: the frontmatter `description: "..."` was being captured into a `#[allow(dead_code)]` field. A user writing a description expected it to appear somewhere. Wired into the `/prompt` list output — each prompt now renders as `name description` (padded for alignment) when a description is set, falling back to the bare name otherwise. - **#4 MEDIUM — `stream_chunk_timeout_secs` undocumented**: the knob landed in 6c395d0 with the error message pointing users at it, but neither CONFIG.md nor README explained the resolution ladder. Added a "Streaming timeouts" section to CONFIG.md covering the precedence order (custom_providers > providers > top-level > 300s default) and noting the case-insensitive matching. - **#5 LOW — Unicode lowercase mismatch with ASCII-only matcher**: the frontmatter parser called `to_lowercase()` (Unicode-aware) while `is_prompt_denied` used `eq_ignore_ascii_case`. Swapped the parser to `make_ascii_lowercase` so both ends share the same contract — non-ASCII bytes pass through unchanged on both sides. - **#6 LOW — MCP bare-name deny semantics undocumented**: the 3-name probe `any_prompt_denied(&[concrete, qualified, "mcp_tool"])` matches an MCP server's `edit` tool when the prompt denies `edit` (intended for the built-in). README now spells this out and recommends the qualified `mcp_tool:<server>:<name>` form for surgical denies. - **#7 LOW — `KNOWN_TOOLS` duplicated `BUILTIN_TOOL_NAMES`**: two hand-maintained lists of every built-in tool, used for the MCP collision filter and the frontmatter `deny_tools` warn respectively. Drift would produce either spurious warnings or (worse) an unsafely shadowable name. Extracted a single `pub const BUILTIN_TOOL_NAMES` in `agent/tools/mod.rs`; both sites now import it. - **#8 DESIGN — ACP can't switch prompts mid-session**: the 30604a3 fix installed the deny-list correctly per-request but ACP has no protocol message for `/prompt <name>`, so the deny- list is effectively locked at boot. Documented in the README ACP bullet — recommends `--prompt <name>` at launch for restricted modes. - **#9 LOW — `repo_overview` ancestor walk went above crawl root**: `compute_dir_file_counts` bumped every parent up to `/`, including dirs that would never be printed. Now stops at the first ancestor not in the printed-dir set. 634 tests pass; fmt clean.

…rf cache Self-review of the last 4 commits flagged 11 findings. Addressing all in one batch. - **#1 HIGH — leading whitespace dropped on first row of soft_wrap**: `current.is_empty()` at row start unconditionally dropped `token.leading_ws`, including on the first row of a logical line. Option lines `" ▶ label …"` lost their ` ` margin; the green ` allowed …` confirmation lost its indent too. First-row branch now preserves leading_ws (with a ws-overflow fallback to keep the token). - **#2 HIGH — drain_events early-broke before reader quiesced**: the Ok(false) shortcut fired on the first quiet poll, which was often "the background reader currently holds crossterm's internal mutex" rather than "terminal is quiet". A delayed OSC 11 / DA1 response could still escape past our drain. Now requires at least one observed event before the Ok(false) shortcut; honors the full budget otherwise. - **#3 HIGH — MODIFIED section clipped its own bottom border at `available == 4`**: row_budget=1, footer + 1 file = 2 items, +3 frame rows = 5 total in a 4-row budget. draw_panel clipped the `╰────╯`. Bumped MIN_MOD_SECTION_ROWS from 4 → 5 so the bottom border is always painted. - **#4 MEDIUM — `\r` not stripped from CRLF input**: Windows / some-MCP tool output left `\r` in tokens, producing terminal redraw artifacts. `soft_wrap` now strips a trailing `\r` per logical line. - **#5 MEDIUM — Show cursor on alt screen was a no-op**: `Show` was issued while still on the alt screen; `LeaveAlternateScreen` restores the main screen's saved DECTCEM state, discarding the Show. Moved Show to AFTER LeaveAlternateScreen + disable_raw_mode. - **#6 MEDIUM — recent(256) clones + locks on every redraw**: panel redraws on every streamed token. Added `modified::version()` monotonic counter (bumped on mark / clear); panel-side cache in `panel_modified_cached` keyed by (version, cwd) skips the lock + 256-PathBuf clone + path-strip when nothing changed. - **#7 MEDIUM — break_long_token could emit wide-glyph row > budget at max_width<2**: floored max_width at 2 inside soft_wrap. A 1-cell terminal is unusable anyway; this just removes a sharp edge. - **#8 MEDIUM — all-whitespace first row collapsed to empty**: same root cause as #1, fixed by the same change. Indented blank separators now preserve their indentation. - **#9 LOW — `allowed …` confirmation flush against alert `╰─╯`**: added a blank-line breathing row before the green confirmation so the alert's bottom border and the confirmation don't read as one block. - **#10 LOW — head_w used chars().count() not display width**: switched to `UnicodeWidthStr::width(head)` so future wide-glyph markers won't under-pad the continuation indent. - **#11 LOW — single-select marker width inconsistent**: cursor marker was `▶` (w=1), non-cursor was ` ` (w=2), so wrapped tails of adjacent options drifted by one column. Cursor marker padded to `▶ ` so all markers in a question share display width. 5 new tests: - preserves_leading_whitespace_on_first_row - strips_carriage_returns_from_crlf_input - wide_glyph_respects_max_width_at_floor - preserves_leading_whitespace_only_line - version_bumps_on_mark_and_clear 651 tests pass; fmt clean.

…e migration, model-shrink, unicode tokens Self-review of the last 6 substantive commits flagged 7 findings. Addressing all in one batch. - **#1 HIGH — Accept mode silently bypassed mcp_tool's default-Ask**: `SecurityMode::Accept` coerced `Ask → Allow` for every non-path tool, so the new default-Ask rule for `mcp_tool` (27bd70a) was effective only in Standard mode. `dirge --accept` + an MCP server still allowed every call silently. Added `is_high_risk_non_path_tool(tool)` (matches `mcp_tool` and `bash`) which forces the Ask to survive the coercion. The general non-path coercion still applies to `question`, etc. Regression test pins both directions. - **#2 MEDIUM — Pre-9a044ce sessions resumed with under-counted `estimated_tokens`**: stored values were computed under the old text-only logic. Bumped `SCHEMA_VERSION` to 2 and added a v1 → v2 migration step that calls a new `Session::recompute_all_estimates` (also exposed `estimate_message_tokens` as the per-message helper). Tested via `v1_to_v2_recomputes_under_counted_estimates`. - **#3 MEDIUM — `/model` switch to a smaller window didn't warn about over-capacity**: switching from a 1M to a 200k model with the session already exceeding the new budget would have errored mid-stream on the next prompt. The /model handler now surfaces a warning recommending `/compress` when `total_estimated_tokens > new_ctx - reserve`. - **#4 LOW — Highlight tokenizer mis-split non-ASCII identifiers**: `bytes[i] as char` for a UTF-8 lead byte produced a Latin-1 char that failed `is_ascii_alphanumeric`, terminating the identifier mid-word (`naïve` → `na` + punctuation + `ve`). Walk via `line[i..].chars().next()` and broadened `is_ident_cont` to accept any non-ASCII non-control letter. ASCII path unchanged. - **#5 LOW — `looks_like_type` colored short capitalized words as types**: `Ok`/`No`/`Hi`/`Id` all hit. Tightened the floor to ≥3 chars. Added `Ok`/`Err`/`Some`/`None` to the Rust types table so idiomatic 2-char Rust constructors still get type color via the explicit table. - **#6 INFO — Yolo bypass documented**: README permission section now says explicitly that `--yolo` skips rule eval, the per-tool default-Ask, and the doom-loop detector — but that `deny_tools` frontmatter STILL applies (that gate runs BEFORE the yolo short-circuit by design). Accept-mode bullet also updated to note `bash` and `mcp_tool` keep their Ask. - **#7 + #8 CLEARED**: doom-loop Allow + `"*": "allow"` default config both verified NOT to bypass mcp_tool's Ask. 684 tests pass; fmt clean.

…nds, error masking Adversarial review of `1c341e9`/`8e60553`/`69318b7` flagged 14 findings. Addressing all. - **#1 HIGH — C1 controls bypassed MCP stderr sanitizer**: the original filter was `b == 0x09 || (0x20..0x7f).contains(&b) || b >= 0x80`. That second clause let through every non-ASCII byte including the C1 control range (U+0080..=U+009F). In particular U+009B is single-byte CSI, behaves identically to `\x1b[` on iTerm2/xterm in 8-bit mode. A misbehaving MCP child could write `\u{9b}2J` and repaint the screen — exactly the smuggling vector the commit message claimed to close. Filter now blocks C0 controls (except `\t`), DEL, and the full C1 range. - **#2 HIGH — MCP stderr was silently dropped at default verbosity**: emit was `tracing::info!`, but dirge's default EnvFilter is `warn,rig=off`. Users diagnosing MCP server panics or init errors saw nothing — real regression vs the old `Stdio::inherit()`. Raised to `tracing::warn!` so it surfaces on the default config. - **#3 HIGH — `parse_ddg_html` panicked on truncated input**: `&html[tag_start..abs_start + 32]` blew up when `abs_start + 32 > html.len()` or landed mid-codepoint. Rewrote the scanner to anchor on `<a ` tags and walk only within bounded slices via `tag_end.min(html.len())`. Added regression test for the truncated case. - **#5 MEDIUM — MCP stderr forwarder had no per-line cap**: `BufReader::lines()` buffers until `\n`. A buggy child writing a GB without newline would OOM dirge. Replaced with a manual read loop, 16 KiB per-line cap, emit `…[truncated]` past the cap and skip until next `\n`. - **#6 MEDIUM — provider rotation race on first call**: two concurrent first-callers both saw `AtomicU8 = 0`, both rolled a fresh entropy pick, both stored. Last writer won — inconsistent contract. Switched to `compare_exchange` from 0 to candidate; loser re-reads the winner's value. - **#7 MEDIUM — both-providers-fail error masked secondary + DDG errors**: only `primary_err` was returned. Now concatenates all three failures so the user can diagnose without chasing the wrong cause. - **#8 MEDIUM — `parse_ddg_html` false-positives on substring match**: previously matched `class="result__a"` anywhere in the HTML, including inside `<script>` blocks or quoted text. Walked backward via `rfind("<a ")` could grab an unrelated anchor. Now anchors on `<a ` first and inspects the tag's attributes — proper containment check. - **#9 MEDIUM — DDG snippet control bytes flowed into LLM prompt**: `strip_tags_and_decode` decoded entities but didn't filter ESC / C1 controls. A malicious or mojibake search result could ship ANSI styling into the agent's context. Added control-byte filter to the decoder's output pass. - **#10 LOW — `PARALLEL_API_KEY` read per-call**: was `std::env::var` inside `call`, inconsistent with how `EXA_API_KEY` was captured at construction. Moved to `WebSearchTool::new`. - **#11 LOW — whitespace-only key passed empty-filter**: `EXA_API_KEY=" "` produced a malformed `?exaApiKey=%20%20` URL. Now trims keys at construction. - **#12 LOW — tool description out of date**: still mentioned Exa-only + DDG fallback, missed Parallel.ai rotation and the keyless default. Rewritten to reflect the actual contract. - **#13 LOW — `urlencode_query` renamed to `percent_encode`**: the function is a generic percent-encoder (RFC 3986 unreserved set), not a form-encoder. Misleading name. - **#14 DESIGN — no tests for new code paths**: added 11 regression tests covering ddg parser bounds + happy path + anti-script-block, control-byte filter, key trim, MCP response parser (plain JSON + SSE + malformed), DDG redirect unwrap, percent encoding, provider env override. - **#15 LOW — bot-identifying DDG User-Agent**: swapped `compatible; dirge-agent/1.0` for a real Firefox 133 UA. DDG aggressively rate-limits identifiable scrapers. 695 tests pass (684 + 11 new); fmt clean.

Audit response covering the user's 6-item duplication list. Three substantive modules + thin wrappers; two items deferred as non-issues; one already complete from earlier commits. **#4 — ANSI / control-byte filter (new `src/ui/ansi.rs`)**: Previously three independent filters: MCP forwarder (`emit_mcp_line`), websearch (`strip_tags_and_decode`), chat (`sanitize_output`). Each drifted in coverage — one blocked C0 but not C1, another stripped `\r` only, etc. New `ansi::strip_controls(s, policy)` with a `StripPolicy { keep_newline, keep_tab }` knob is the single source of truth. MCP forwarder + websearch routed through it. Chat `sanitize_output` left as-is — it does ANSI-escape PARSING (consumes `\x1b[…m` as a unit so the payload disappears) which is more specific than what `strip_controls` does (drops just ESC, leaving `[31m` as visible text). The two coexist intentionally. **#1 — Reusable box component (new `src/ui/box_render.rs`)**: Three implementations of chamber/box math previously: tool chambers (`chamber_row` / `chamber_row_with_bg` / `chamber_bottom`), permission alert (inline `row` closure), panel sections (`push_section` closure). Inconsistent — `chamber_row` was display-width-aware, `chamber_row_with_bg` was char-count-based; each treated tabs and width math differently. New module: - `BoxStyle` enum (currently only `Rounded`) - `top(style, title, total_w)`, `bottom(style, total_w)`, `divider(style, total_w)` — frame primitives - `row(style, content, total_w)` — display-width-aware content row with tab expansion + truncate-with-`…` - `row_with_bg(style, content, total_w, bg_idx)` — for diff backgrounds - `expand_tabs(s, tab_stop)` — moved here from `mod.rs` - `BoxBuilder` — fluent API for callers that build a box all-at-once (notifications, alerts, panel sections). Long rows soft-wrap via `wrap::soft_wrap` instead of truncating. `chamber_row`, `chamber_row_with_bg`, `chamber_bottom` in `mod.rs` are now thin wrappers around `box_render`'s primitives — existing call sites unchanged, but the underlying math is shared. 7 new unit tests covering frame width invariants, tab handling, CJK, builder construction, and soft-wrap. **#2 — Single output chokepoint** — already done in commit `dc21de7` (the `ui::notifications` module + channel). The remaining stray `eprintln!` sites all run during STARTUP (config parsing, skill discovery, MCP connect_all) BEFORE `TerminalGuard` is installed, so they don't paint over the UI. Audited and confirmed clean. **#3 — Permission check chokepoint** — already centralized. `check_perm` / `check_perm_path` / `check_perm_path_resolve` in `agent/tools/mod.rs` cover every tool's input-checked path; `any_prompt_denied` covers MCP-style multi-name lookup; `check_prompt_deny` covers plan tools. No duplication worth extracting. **#5 — Layout module** — deferred. `chamber_widths`, `Renderer::content_width`, `Renderer::line_width`, `Renderer::max_line_width` are already in their proper homes; forcing them into a new module would be churn, not clarification. **#6 — Soft-wrap inside chambers** — deferred. The current chamber rows truncate with `…` (matches user expectation for single-row tool result lines); the `BoxBuilder` path soft-wraps when the caller wants that explicitly. Forcing all chamber rows to soft-wrap would change the visual feel of tool output and needs a separate UX call. 709 tests pass (702 + 7 box_render); fmt clean.

…h_bg width, sanitization Correctness + security review of `dc21de7`/`cd701a8` flagged 15 findings. Addressing all 15. - **#1 HIGH — startup race: MCP forwarders fired before `install()`**. `connect_all` spawns stderr forwarders in `main` BEFORE `run_interactive` reached the old `install()` call. Lines emitted during MCP-server handshake hit `sender() == None` and were silently dropped. Moved `install()` to the very top of `main()` so the channel is live by the time any forwarder starts. Split the API: `install()` (creates channel) + `take_receiver()` (UI loop claims the rx). - **#2 HIGH — orphaned-sender footgun on UI restart**. `OnceLock` meant a re-entry could never replace the sender; producers holding clones would send into a dead channel forever. Switched to `RwLock<Option<Sender>>`. Producers also self-heal: when `try_send` returns Err because the receiver was dropped, `notify_send` clears the slot so subsequent producers see `None` and skip. - **#3 HIGH — `row_with_bg` was still char-count-based**. The refactor claim was "unifies display-width vs char-count" but the bg-tinted variant still used `chars().count()`. A diff row with CJK / emoji drifted the right border. Now uses the same display-width budget as the plain `row`. Regression test `row_with_bg_width_invariant` pins it. - **#4 HIGH — unbounded channel + no backpressure → OOM**. A buggy / hostile MCP child spamming stderr would grow the queue unboundedly. Switched to `mpsc::channel(1024)` (bounded) with `try_send` so the producer drops on overflow rather than unboundedly queuing. Test `bounded_channel_drops_on_full` pins the contract. - **#5 MEDIUM — multi-colon MCP tool names**. `splitn(3, ':')` on `mcp_tool:server:do:thing` parsed correctly but the comment explanation was off. Clarified; behavior unchanged (the wildcarded server pattern is the desired semantics). - **#6 MEDIUM — mcp_tool umbrella check case-sensitive**. `umbrella == "mcp_tool"` would miss `MCP_TOOL:…` if a future caller surfaces uppercase. Switched to `eq_ignore_ascii_case`. - **#7 MEDIUM — receiver-side sanitization for ALL Notification variants**. MCP variant was pre-sanitized at the producer, but Info/Warn/Error had no producer-side contract. Adding receiver-side `ansi::strip_controls(KEEP_NEWLINE)` makes the rule un-bypassable: nothing reaches `write_line` carrying escape bytes regardless of how careful a future producer is. - **#8 MEDIUM — websearch `KEEP_BOTH` + `\n` broke chamber border**. Tabs survived into chamber rows where they interacted poorly with the wrap math. Switched to `KEEP_NEWLINE` and replace `\t` with single space. - **#9 MEDIUM — whitespace-only MCP lines dropped**. The blank-line collapse used `trim().is_empty()` which also ate legitimate indented continuation lines. Now uses `is_empty()` post-sanitize. - **#10 LOW — `top()` with empty title rendered `╭─ ─…─╮`** (two spaces with no glyph between). Empty title now matches the bottom-border shape `╭{horizontals}╮`. Test pins it. - **#11 LOW — `expand_tabs` precondition undocumented**. Added comment that input should be control-byte free; callers must sanitize first. - **#12 LOW — `BoxBuilder::row("a\nb")` produced one row containing a literal `\n`**. Now splits on `\n` and emits one row per logical line. Test `builder_splits_embedded_newlines` pins it. - **#13 LOW — `BoxBuilder` had no labelled-row variant**. Added `row_labelled(label, sep, value)` that indents wrapped tails under the value column. Mirrors the alert chamber's `labelled_rows` shape so a future alert migration to BoxBuilder is unblocked. Test pins continuation indent. - **#14 LOW — `strip_controls` allocated on no-op path**. Fast path returns the input unchanged when no chars would be filtered. - **#15 DESIGN — sender caching deferred**. Per-call `sender()` is the right semantics for the orphan-detection case (#2); caching would skip the slot-clear behavior. Kept as is. 8 new tests; 715 total. Two-test serialisation via TEST_GATE for the notification tests since they mutate global TX/RX_HOLDER state. fmt clean.

…ser_rx writes Code review of `b508658`/`8b48bfd`/`f9285ad` flagged 15 findings, including one **active regression** I shipped: `write_outside_chamber` reused `close_tool_chamber_if_open` which always painted "⚠ tool denied · aborted · no result". So every notification arriving while a tool was in-flight would falsely brand that tool as denied. Fixed. Headline: of 9 tokio::select! arms, only 2 were using the new chokepoint. 3 others (question_rx, dialog_rx, plan_rx) carried the SAME X-inside-chamber bug the helper was built to eliminate. Migrated them. - **#4 HIGH (regression I shipped)**: split chamber-close into two variants: - `close_tool_chamber_abort` — paints the "⚠ tool denied" row + bottom border. Used by permission-deny / agent error / interjection / context-overflow paths (the tool is being actively rejected). - `close_tool_chamber_passive` — emits ONLY the bottom border. Used by `write_outside_chamber` (the tool isn't being denied; we just need to terminate the visual frame so notification text doesn't land inside). - `close_tool_chamber_if_open` kept as back-compat alias for the abort variant — existing call sites (4 of them, all in abort-shaped contexts) keep their previous behavior. - **#1 / #2 / #3 CRITICAL — three arms migrated**: - `question_rx` (3537): a `question` tool's chamber was open when the prompt header was painted; header + stem + option grid landed inside. - `dialog_rx` (3811): plugin `harness/confirm` / `harness/select` fires from inside on-tool-start hooks while a tool chamber is open; the dialog rendered inside. - `plan_rx` (3955): plan-switch prompt could be delivered while a tool chamber was open; prompt landed inside. - **#5 HIGH — user_rx interactive writes migrated**: - Ctrl+C interrupt msg (1135) - "copied selection" (1150) - Ctrl+X dropped-interjection trailer (1168) - "agent is busy" × 2 (1533, 1598) - **#7 MEDIUM — defense-in-depth sanitization**: `write_outside_chamber` now runs `strip_controls(KEEP_NEWLINE)` on `text` before writing. A future caller that forgets producer-side sanitization can't smuggle ANSI escapes. - **#12 LOW — notification amplification cap**: the bounded channel limits NOTIFICATIONS but not ROWS per notification. A single `Notification::McpLog` carrying 10k `\n`s would expand to 10k chamber rows. After 200 lines we truncate and emit a `[N more lines suppressed]` marker. - **#6 audit** revealed the 4 remaining manual sites (1984/2602/2713/2884) all ARE abort-shaped and correctly use the abort variant via the back-compat alias. No migration needed. - **#8 / #9 / #10 / #13 / #14 / #15** noted as design trade-offs or already verified clean. 3 new regression tests: - `close_passive_does_not_paint_abort_row` pins the new no-abort-label contract - `close_abort_paints_warning_and_bottom` pins the abort variant still emits 2 rows - existing `write_outside_chamber_closes_chamber_first` still passes; helper now uses passive close 718 tests pass (716 + 2 new); fmt clean.

…rupt on crash External review flagged that `write.rs:105`, `edit.rs:234`, and `apply_patch.rs:93,155` all called `tokio::fs::write` directly, which opens with O_TRUNC and writes in-place. A crash between the truncation and the final byte (power loss / OOM-kill / SIGKILL / panic) leaves the file corrupted with no recovery. The irony: `session/storage.rs` already had the correct pattern — temp + fsync + rename — but it wasn't shared. Extracted the pattern into new top-level module `src/fs_atomic.rs`: - `atomic_write_sync(path, content)` — sync, used by storage - `atomic_write(path, content)` — async, used by tools (delegates to spawn_blocking so the create + fsync + chmod + rename sequence runs atomically in one blocking task) - `next_temp(target)` — hidden sibling temp path with pid+nanos+counter nonce so two concurrent saves don't collide on the temp filename (counter is the load-bearing piece — same-nanosecond firings still get distinct names) - Unix mode preservation: stat the existing target's perms BEFORE rename, chmod the temp to match. Without this, an atomic overwrite of an executable script would silently drop the +x bit (default temp perms are 0644 minus umask). Migrated four call sites: - `agent/tools/write.rs:105` — full-file write - `agent/tools/edit.rs:234` — edit-tool output - `agent/tools/apply_patch.rs:93` — apply_create - `agent/tools/apply_patch.rs:155` — apply_update Plus `session/storage.rs` now also uses the shared helper (was the original site with the pattern; now consolidated). Return type: `io::Result<()>` so existing `From<io::Error>` impls on `ToolError` / `anyhow::Error` continue to work. The async variant maps spawn_blocking join failures to `io::Error::other`. Six new regression tests: - `atomic_write_creates_new_file` - `atomic_write_overwrites_existing` - `temp_is_hidden_sibling` — verifies same-fs + dot-prefix - `next_temp_is_unique` — 1000-call distinct-name check - `atomic_write_preserves_mode` (Unix) — +x stays on across overwrite - `target_untouched_on_failed_rename` — atomicity guarantee Tests use `std::env::temp_dir` + a `TestDir` RAII helper (matches the codebase convention; dirge doesn't pull in `tempfile`). 724 tests pass (718 + 6); fmt clean. #2 (ToolStarted event), #3 (prepareNextTurn hook), #4 (structured tool output) follow in separate commits. #5 from the review was a false positive — `skill` IS registered at `builder.rs:239`.

External-review #3 and #4 implemented as minimal, honestly-scoped versions. **#3 — `prepare-next-run` plugin hook** New hook fires AFTER `Done` (run complete) and BEFORE the next user prompt is processed. Plugins read this to signal session- level state changes for the next run. Currently the only supported mutation slot is `harness-next-model` (Janet: `(harness/set-next-model "claude-opus-4.7")`). Scope honesty: the request is SURFACED to the user as a notification (`"[plugin] requested model swap to 'X' — apply with /model X"`) rather than auto-applied. Auto-apply is deferred because: - The agent rebuild path is non-trivial across cfg-feature combinations. - The existing `/model` slash already does it correctly. - "Plugins propose, user disposes" is the safer default — a plugin can't silently swap to a more expensive model without the user noticing. Mid-stream model swap is explicitly UNSUPPORTED — rig's multi-turn stream owns state that doesn't survive a swap. The hook is scoped to between-runs only, documented in the comment next to the slot. Slot infrastructure: - `harness-next-model` declared in `worker.rs` startup blob - `harness/set-next-model` helper in the same blob - `take_pending_next_model()` on `PluginManager` clears the slot and returns its value **#4 — `ToolContent` classification on `ToolResult`** New enum `event::ToolContent { Text, File }`. Added as an additive field on `AgentEvent::ToolResult { id, output, kind }`. `output: CompactString` remains the authoritative payload for the LLM and the default UI rendering path — `kind` is purely metadata for richer consumers (ACP resource links, future UI file-card components). The runner classifies by tool name: `read` / `find_files` / `list_dir` produce `File`, everything else `Text`. Tracked via a per-stream `id → name` HashMap populated at each `ToolCall`, drained at the matching `ToolResult` (1:1 call/result pairing within a turn). Coarse on purpose — no per-tool `type Output` change required across ~20 tools. A future refactor could thread the variant through the rig `Tool` trait for finer-grained control. Consumers: - `extras/acp/mod.rs` reads `kind` (currently no-op; comment flags `ResourceLink` migration as a follow-up) - `ui/mod.rs` uses `{ .. }` rest pattern; ignores `kind` for now **#5 from the review remains a false positive** — `skill` IS registered at `builder.rs:239`. 724 tests pass; all-features build clean.

…ap on #3) User asked to close the gap with pi's \`prepareNextTurn\` for issue #3. Compared against pi's implementation at \`packages/agent/src/agent-loop.ts:220-239\`: | Aspect | pi | dirge before | dirge after | |--------------------|---------------------------------|------------------|-----------------| | Frequency | per TURN (mid-run) | per RUN (Done) | per RUN (Done) | | Model swap | auto-applied | notification | **auto-applied**| | thinkingLevel | auto-applied | n/a | n/a | | context replace | auto-applied | n/a | n/a | Closed in this commit: **auto-apply of the model swap**. When a plugin sets \`harness-next-model\` from \`prepare-next-run\`, dirge now rebuilds the agent inline (same logic as \`/model X\`) so the next user prompt runs against the new model without any user intervention. Plus updates \`session.model\` / \`session.context_window\` / \`session.provider\` to keep the status indicator + recovery paths in sync. Validation guards on the swap: - Empty / whitespace-only \`next_model\` string ignored (mis- configuration shouldn't silently nuke the active model). - Same-model swap is a no-op (don't pay the rebuild cost when the plugin "swaps" to the current model). Remaining deltas vs pi (documented in the comment): 1. **Per-turn frequency**: pi fires \`prepareNextTurn\` between turns within a single agent run; dirge fires \`prepare-next-run\` only at run boundaries (after Done). Closing this requires breaking rig's multi-turn stream and restarting with a new agent — would lose partial assistant state, so we keep the swap at run boundaries. 2. **thinkingLevel**: dirge has no equivalent config concept (the \`/reasoning\` slash is UI-only — visibility toggle, not a knob into the model's reasoning budget). Adding it would require provider-side request-param plumbing across every supported backend. Skip until a real use case. 3. **context replace**: dirge has \`/clear\` and \`/compress\` for this; per-plugin wholesale replacement is niche. 724 tests pass; all-features build clean.

Three issues from the post-cutover code review against pi: **Bug #1**: stream.rs:186-194 — defensive fallback (stream closed without Done/Error) skipped emitting message_start / message_end. Pi at agent-loop.ts:359-366 emits both. Fix: route the fallback through `finalize()` so it follows the same emit path as Done/Error. Updated the existing test that documented the wrong behavior as "intentional Rust deviation" — it's now pi-faithful. **Bug #4**: integration.rs:411 — orphaned inner loop task. `spawn_loop_runner` spawned `run_agent_loop` as a NESTED `tokio::spawn`. A `task.abort()` on the outer task would kill it but leave the nested task running silently — tools could keep executing after the user thought they'd cancelled. Fix: collapse to `tokio::join!(loop_future, pump_future)` in the same outer task. Shared fate; outer abort drops both futures at their next .await. Tools that poll the AbortSignal still observe cancellation cooperatively. **Gap #3**: run.rs prepareNextTurn — pi at agent-loop.ts:229-238 rebuilds config with the new model / reasoning. We accepted the fields but silently ignored them. Surfacing a tracing warning per ignored swap so users wiring the hook know their change didn't take effect. Full fix requires the StreamFn to be a factory `Fn(Context) -> StreamFn` (so the loop can rebuild it on swap) — flagged for follow-up when a real consumer demands it. Items NOT addressed (documented in review): - #2 get_api_key receives empty string (no production caller) - #5/#6 timing / ordering changes (observable but not bugs) - #7-9 efficiency micro-optimizations - #10/#11 UI-side wiring + Agent.preamble defensiveness Gates: - cargo build (default) clean - cargo build --all-features clean - cargo test (default) 841 green (unchanged) - cargo fmt clean

Surface every pi loop hook (prepareNextTurn, shouldStopAfterTurn, getSteeringMessages, getFollowUpMessages) to Janet plugins via dedicated slots. Auto-wired into spawn_loop_runner when a plugin manager is supplied. **Janet helpers added** (plugin/worker.rs): ; prepareNextTurn → next-turn config swap (harness/set-next-thinking-level "high") ; low/medium/high/xhigh/off/minimal ; harness/set-next-model already exists — repurposed ; shouldStopAfterTurn → graceful exit (harness/request-stop-after-turn) ; getSteeringMessages → mid-run user injection (harness/add-steering "wait, also do X") ; getFollowUpMessages → outer-loop continuation (harness/add-followup "do this next") **Rust slot accessors** (plugin/mod.rs): - take_pending_next_thinking_level() -> Option<String> - take_pending_stop_after_turn() -> bool - drain_steering_messages() -> Vec<String> - drain_followup_messages() -> Vec<String> Steering + followup use a newline-blob format (`msg\n`) so a single eval round-trip drains the queue cleanly. **Hook factories** (plugin_hooks.rs): - prepare_next_turn_from_plugin_manager(pm) -> PrepareNextTurnFn - should_stop_after_turn_from_plugin_manager(pm) -> ShouldStopAfterTurnFn - get_steering_messages_from_plugin_manager(pm) -> GetSteeringMessagesFn - get_followup_messages_from_plugin_manager(pm) -> GetFollowupMessagesFn Each follows the same lock-then-sync pattern as before/after_tool_call hooks: acquire mutex, eval slot, release. No `.await` while held. **Wired into spawn_loop_runner** (integration.rs): when `cfg.plugin_mgr` is set, all four hooks are installed alongside the existing before/after_tool_call. Caller-provided steering_queue still wins if both are set (explicit beats global). **Tests** (6 new integration tests with real Janet VM): - prepare_next_turn_reads_thinking_level - prepare_next_turn_returns_none_when_no_slot_set - prepare_next_turn_ignores_unknown_thinking_level (typo safety) - should_stop_after_turn_drains_slot - get_steering_messages_drains_queue (multiple add + drain) - get_followup_messages_drains_queue **Pi reference**: PLAN.md phase 5. Each slot maps 1:1 to a pi hook from runLoop. Slot mechanism (Janet `var` + `defn` helper + Rust `take_*` reader) was already established by the pre-existing harness-next-model / harness-block / harness-mutate-input slots; phase 5 extends the pattern to the remaining pi hooks. **Composition with phase 4.6**: prepareNextTurn's thinking_level field is now actively populated by plugins. The full chain works: plugin sets harness-next-thinking-level "high" in on-tool-end ↓ loop polls prepare_next_turn between turns ↓ TurnUpdate.thinking_level = Some(High) ↓ (currently surfaces tracing warn — code review #3; full model-swap apply pending rig API growth, see h-7 deferred) Gates: - cargo build (default) clean - cargo build --features plugin clean - cargo build --all-features clean - cargo test (default 846) green - cargo test --features plugin (985 = 979 pre-existing + 6 new phase 5 tests) green - cargo test --ignored 6 h-7 still green (no regression after Janet slot additions) - cargo fmt clean Phase 6 next: recovery / interjection / abort hardening under the new loop. Or phase 7 (custom message types). Phase 5 risk was low as predicted; landed clean.

…gnal threading, #2 provider name Three fixes from the phase-4.6/phase-5 code review. **R1**: `prepare_next_turn_from_plugin_manager` was draining `harness-next-model` alongside `harness-next-thinking-level`. The model slot has pre-existing dirge semantics — read by the UI at end-of-run (`ui/mod.rs:2359`) to spawn a fresh agent against the new model. With the prepareNextTurn hook draining it first, the UI's consumer saw None and `harness/set-next-model` silently failed. Fix: only drain the thinking-level slot in the hook. The model slot stays for the UI consumer. Mid-run model swap isn't supported anyway (run.rs already logs a warning when TurnUpdate.model is set — code review #3); a separate API for real mid-run model swap waits on rig API growth. **R3**: `opts.signal` in StreamOptions was silently ignored by the rig stream adapter. Mid-stream cancellation against the rig request had no effect — signal only took effect at the next turn boundary. (Old runner.rs path had the same limitation; not a regression but a real gap.) Fix: thread `Option<AbortSignal>` into wrap_streamed_assistant. Per-chunk pre-poll check: if signal is cancelled, emit an Error event with "aborted" substring and exit. Mid-LLM-call cancel now actually stops the rig request promptly. **#2**: `get_api_key` hook was called with `""` instead of the provider name. Pi contract: `getApiKey(provider: string) => key`. Provider-aware hooks couldn't dispatch. Fix: add `provider_name: Option<String>` to LoopConfig (and LoopSpawnConfig, threaded through). `AnyAgent::provider_name()` returns the canonical name per variant ("anthropic", "glm", etc.). spawn_runner sets it. stream_assistant_response passes it to the hook. **Tests** (3 new): - prepare_next_turn_does_not_drain_next_model_slot (R1) - signal_cancels_stream_mid_flight (R3) - signal_none_does_not_affect_stream (R3 negative case) - test_get_api_key_receives_provider_name (#2) **Not fixed in this commit** (documented as deferred): - R2: opts.api_key silently ignored by rig adapter (rig's client carries the key at construction; per-request override would need a rig API change) - R4: opts.request_timeout silently ignored (same reason) - Per-provider reasoning mappers (separate larger commit) Gates: - cargo build (default) clean - cargo build --features plugin clean - cargo build --all-features clean - cargo test 849 passed; 6 ignored - cargo fmt clean

Three follow-ups to PR #126 (experimental tab completion) flagged in the code review: #1 — mid-word cursor produced corrupt buffer `try_complete` replaced `[word_start..cursor]` with the candidate command and appended `buffer[cursor..]` as a tail. With the cursor inside the first word (e.g. Tab pressed after Home, or after moving Left into `/mod`), the unreplaced suffix leaked into the result: `/mod` cursor=2 → `/mcpod` (replacement `/mcp` + residual `od`). Cursor=0 → `/allow/mod` (replacement + the entire word as tail). Fix: anchor replacement to the WHOLE-WORD boundary (`word_start..word_end`) so cursor position inside the command name no longer matters. New buffer is exactly one of the matching commands followed by the args tail (if any). Also: `cursor > word_end` now returns None — the user is past the command name typing args, completion shouldn't fire. #3 — Tab raced the file picker The `@`-file-picker has its own keystroke handling (Enter / Ctrl+J guard it explicitly). Tab in the new completion path didn't. In practice the picker's buffer doesn't start with `/`, but guarding explicitly closes the race for future changes — mirrors the existing Enter/Ctrl+J gates. #2 — single source of truth for slash command names Previously `builtin_commands()` (feature-gated, used by tab completion) and `handle_slash`'s match arms (always compiled, the actual dispatch) were two independent lists. Adding a command required updating both — or the command worked but wasn't tab-completable, or appeared in completion but errored on use. Restructure: * `slash_command_names()` — always-compiled canonical list. The one place to add a command name when wiring it up. * `is_known_slash_command(name)` — wrapper over the canonical list. Single implementation, no second match to drift from. * `builtin_commands()` — now just `slash_command_names()`, kept as a feature-gated alias so the public API doesn't break. * `handle_slash`'s `_` default arm — now consults `is_known_slash_command(parts[0])`. A name listed in the canonical list but with no matching dispatch arm surfaces as `internal error: X is listed in slash_command_names() but has no dispatch arm in handle_slash` instead of silently falling through to "unknown command" or a plugin lookup. Loud failure in dev/test. The remaining drift direction (dispatch arm but missing from the list) only costs tab completion — accepted as the lesser failure mode. Tests (9 new, 22 slash tests pass total) * complete_with_cursor_mid_word_produces_clean_buffer — #1 * complete_with_cursor_at_start_produces_clean_buffer — #1 * complete_preserves_trailing_args — #1 edge * no_completion_when_cursor_in_args — #1 edge * is_known_slash_command_agrees_with_canonical_list — #2 * slash_command_names_is_sorted — #2 (preview ordering) * always_on_commands_appear_in_canonical_list — #2 drift guard: pins the 22 always-on dispatch arms against the canonical list Verified * cargo test --bin dirge → 1011 passed * cargo test --bin dirge --features experimental-ui-tab-slash → 1025 passed * cargo test --bin dirge --features "experimental-ui-tab-slash plugin" → 1239 passed * cargo fmt --all --check → clean

…tests SESS-2 follow-up #1 — UI session-mutation on ContextCompacted: ContextCompacted event now carries summary + first_kept_index. The UI consumer mutates session.id in-place, calls Session::compress_reporting() to push a Compaction entry, and runs save_session() so the rotated id and summary are persisted on disk. Mirrors hermes-agent/conversation_compression.py lines 380-397. Without this the on-disk session kept the OLD id and the compaction was lost on next resume. SESS-2 follow-up #4 — /compress <focus> argument wire-through: - build_summary_prompt now honors focus_topic: when supplied, the Hermes-style "FOCUS TOPIC: …" framing is appended to the prompt, asking the model to allocate ~60-70% of its summary budget to the topic (verbatim port of hermes context_compressor.py:1050-1054). - compress_messages (existing slash-command path) gets the same treatment: any free-form text after /compress is wrapped in the FOCUS TOPIC framing instead of the generic "Additional instructions" placeholder. - run_compaction_pass exposes the focus parameter via a new with_focus wrapper (auto-trigger path still uses None). - Slash help text updated: "/compress [focus] compress; focus text guides what to preserve". SESS-2 follow-ups #2 (background spawn) and #3 (multi-generation chaining) closed as "matches reference impl": hermes is also inline (no background spawn) and only chains the most-recent prior summary via _find_latest_context_summary. Our implementation already matches. H7_SMOKE: remove the 6 #[ignore] markers from the real-API integration tests. Each test already has a runtime `detect_provider()` check that bails with `[skipped]` + Ok when no provider key is set; #[ignore] was blocking that check from ever running. Removing the markers means: in CI without keys, the tests run, hit the skip path, pass (1713 pass / 0 fail / 0 ignored). With keys present, they exercise the real provider as designed. Header doc updated. Tests: 1713 pass / 0 fail / 0 ignored (was 1707 / 0 / 6).

Independent verification turned up 6 gaps in the Phase 2.5 parity work. This commit closes all of them. #1 HIGH — `truncations_fixed` now bumps on hard-fallback too. Reasonix counts both success (`repair/index.ts:105`) and unrecoverable (`repair/index.ts:99`) under the same counter; dirge was dropping the latter, under-reporting exactly the cases operators need most. `apply_truncation_repair` now records the kind whenever the closer ran, not just on successful repair. #2 MEDIUM — closer notes are now surfaced to the model. Reasonix pushes `r.notes` into `report.notes` with `[<tool>]` prefix on success and `[<tool>] ⚠️ TRUNCATION UNRECOVERABLE: ...` on fallback (`repair/index.ts:100-101, :106`), then carries them into the next-turn assistant input. Dirge now stashes them per-call-id on a new `LoopConfig.truncation_notes` shared map; `prepare_tool_call` drains them and appends to `repair_notes`, which `prepend_notes_to_result` (already in place for relational-default notes) prepends to the tool result content so the model sees the repair in the same turn. #3 MEDIUM — added end-to-end wiring tests through `run_agent_loop`. The prior 7 tests proved the helpers worked in isolation; they did not prove the loop calls them in the right order. Two new tests drive the full canned-stream loop: - `dirge_7bwx_end_to_end_storm_dedupes_after_truncation_repair`: three tool calls with different truncated raw strings that heal identically. Storm threshold=3 → the third must be suppressed (only possible if truncation runs before storm). - `dirge_ngic_end_to_end_orphan_dsml_in_text_dispatches`: DSML invoke in `ContentBlock::Text` ONLY (no Thinking, no declared ToolCall) must dispatch (only possible if `build_scavenge_source` includes Text). #4 MEDIUM — removed dead `try_truncation_repair`. It was kept as "defense in depth" but marked `#[allow(dead_code)]`, so the safety claim was illusory. Now actually gone; direct callers can use `repair_truncated_json` for the brace-closer if needed. The `validate_and_repair` block-comment was updated to reflect the new contract. #5 LOW — `truncation_repair_canonicalizes_divergent_streams_before_storm` tested canonicalization in isolation; the new end-to-end #3 tests exercise the actual storm dedupe path that depends on the String→Object promotion. The promotion itself is now also covered with an explicit note in `apply_truncation_repair`'s doc — it has no Reasonix analog (their args are always strings) and is dirge-specific compensation for mixed arg representations. #6 LOW — added a comment near `storm.rs::inspect` documenting the implicit dependency on `serde_json` being built without the `preserve_order` feature. If feature unification ever enables it, storm dedupe regresses silently; the comment points at the workaround (`run::canonical_json`) and notes Reasonix has the same fragility at `repair/index.ts:127`. `LoopConfig` gained the new `truncation_notes` field; all constructors (production + tests) were updated. `Clone` impl threaded through. 1632 tests pass with `-D warnings` (was 1630; +3 new, -1 removed).

…e#78) Track F-HIGH #3 from ROADMAP.md. ## Problem `handle_compress` (`ui/slash.rs:115-123`) did a reverse token-budget scan to pick the cut index, then drained `messages[..cut_idx]` into the summary. If that scan landed on an Assistant message, the kept tail started with [Assistant, …]. After compress, the session became [SystemSummary, Assistant, …] — broken role sequence. Anthropic and OpenAI reject with HTTP 400. The reverse-scan was content-only: it accumulated tokens until the threshold without considering role-boundary constraints. ## Fix New `align_cut_to_user_boundary(messages, cut_idx) -> usize` helper. Walks `cut_idx` forward until the message at that index is a `User` message (or the index reaches end-of-array). Called right after the existing reverse-scan, before any state mutation. Matches opencode's `splitTurn` discipline in `session/compaction.ts:161-184`. ## Tests 5 new tests in `ui::slash::tests`: - `align_cut_advances_past_assistant_to_next_user`: cut_idx landed on Assistant → advance to next User. - `align_cut_idempotent_when_already_on_user`: User-boundary cut unchanged. - `align_cut_past_end_clamps`: out-of-range index stays in range. - `align_cut_returns_end_when_no_user_in_tail`: no User after cut → return messages.len() (caller surfaces "nothing to compress"). - `align_cut_skips_system_to_user`: System messages (prior summaries) are also skipped — only User starts a kept tail. 656 pass (was 651). All build profiles clean. Co-authored-by: Yogthos <yogthos@gmail.com>

…aths (dirge-code#111) 23 audit findings verified REAL via parallel agent verification + cross-check against opencode/pi reference patterns. Shipping the 10 most concrete fixes here; the rest go in a follow-up docs/test batch. ## Security - **dirge-code#9 bash quote_aware_split missed bare `|`** — `safe_cmd | rm -rf /` was treated as one segment; only the LHS got permission-checked. Pipe RHS rode in unchecked under the fallback (non-semantic-bash) path. Added single-byte `|` split after `||` is matched. The tree-sitter path was already correct. - **#4 read.rs no binary detection** — feeding a PDF/ELF/.pyc into the LLM as lossy UTF-8 wasted tokens and confused the model. Ported opencode `read.ts:153-198`: reject by extension list (zip/exe/.o/.pdf/.png/etc.), then sniff the first 4 KiB — null byte = binary, >30% non-printable = binary. Clear error message tells the agent to use bash + xxd instead. ## Correctness - **#2 skill override inverted** — README contract: "Project skills override global skills by name". Code used `map.entry(name).or_insert(skill)` which KEEPS the first (global) value and silently drops project overrides. Switch to `map.insert` (last-write-wins) since globals iterate first and project iterates second. - **dirge-code#37 skill empty name** — frontmatter `name:` with empty value parsed to "", which then matched any `skill ""` call silently. Fall back to directory name when frontmatter name is empty/whitespace-only. - **#1 session_tree.janet hook never fired** — plugin defined `(defn on-message ...)` but `(def hooks [])` was empty AND the hook name doesn't exist (dirge uses `on-message-update`). `/label` was permanently broken ("no entry yet"). Fix: rename to `on-message-update` + register in hooks vector. - **dirge-code#7 workflow.janet hooks vector missing entries** — plugin defined `workflow-on-tool-end`, `-on-error`, `-on-complete` but only registered the first four hook names. Three hooks were dead. Added them. - **dirge-code#26 MCP malformed JSON silently empty args** — `serde_json::from_str(&args).unwrap_or_default()` turned bad JSON into None, sending the server an empty argument set. Server then errored with confusing "missing required field" instead of dirge surfacing the actual parse error. Now returns ToolError with the parse error message + first 200 chars of the offending JSON. - **dirge-code#22 /prompt default unreachable** — README documents `default` as a built-in prompt (prompts/default.md exists), but `/prompt default` was intercepted as a magic "clear" keyword. If `default` is registered in `context.prompts`, the new branch falls through to the normal name-lookup. Only acts as clear-keyword when no `default` prompt is present (legacy fallback). - **dirge-code#23 /allow add accepted invalid tools** — typo `/allow add bsah ...` silently created an inert rule the user couldn't debug. Added a known-tools whitelist matching PermissionConfig fields; unknown tools error with the valid list. ## Performance + correctness - **dirge-code#11 grep loaded whole files into memory** — no size cap meant a 9MB file got fully buffered. Added 10 MiB per-file cap via metadata pre-check. - **dirge-code#15 Python dunder methods marked non-exported** — `!name.starts_with('_')` treats `__init__`/`__call__`/etc. as private, even though they're Python's standard public protocol. Recognize `__x__` dunder pattern as exported. ## UI - **dirge-code#36 panel char-count truncation vs Unicode width** — panel truncation used `chars().count()` while wide emoji and CJK take 2 cells. A status line with an emoji overflowed the right border by one cell. Switched to `UnicodeWidthStr::width` for both truncation and padding. ## Tests 4 new regression tests: - `test_is_binary_extension_known` — pdf/tgz/.so/.jpg/.pyc - `test_is_binary_content_null_byte` — null byte trigger, UTF-8 Japanese stays clean, all-non-printable triggers - `quote_aware_split_splits_on_bare_pipe` — pipe security - `quote_aware_split_or_and_pipe_distinct` — `a || b | c` produces 3 segments, not 2 725 plugin / 599 default pass. All build profiles clean. ## Verified false positives (not fixed, audit was wrong) - #3 cache.rs clear() race — generation counter gating in `get` makes stale entries invisible, no correctness impact. - dirge-code#17 DeepSeek auto-detect priority — auto-detect only fires when env vars present; default-default is still OpenRouter. - dirge-code#19 semantic tools in collision filter — semantic tools added separately, can't be shadowed by MCP. - dirge-code#20 glob global gitignore — intentionally disabled to match grep behavior. - dirge-code#28 nearest_root blocking std::fs — function doesn't exist in current code. - dirge-code#32 ReadArgs.path vs GrepArgs.path — semantically different by design (file vs dir), documented in schema. - dirge-code#33 install_plugin_providers dead-without-feature — gated with explicit `#[cfg_attr(not(feature), allow(dead_code))]`. - dirge-code#34 websearch double-gated — config + API key serve distinct purposes (enable + auth). ## Deferred to follow-up batches Docs-only fixes (dirge-code#6 CONFIG.md tools, dirge-code#12 temperature, dirge-code#13 --api-key, dirge-code#14 acp_host/port), MCP/LSP architecture (dirge-code#8, dirge-code#25, dirge-code#27), test gaps (dirge-code#38-40), and lower-priority polish — all in a follow-up PR. Co-authored-by: Yogthos <yogthos@gmail.com>

…d exports (dirge-code#117) Four cross-cutting cleanups identified in the adapter review, applied across all 9 language adapters (bash + clojure + go + ruby + rust + java + c + cpp + python + typescript). ## 1. Shared helpers — `semantic/common.rs` Three helpers previously duplicated 7+ times across adapters now live in one place: - `node_text(node, source)` — UTF-8 decode with structured-log fallback (see #3). - `find_node_at_range(root, start, end)` — depth-first range match used by every `find_callees_in_range` impl. - `signature_first_line(node, source)` and `signature_up_to_body(node, source)` — the two signature builders. Per-adapter rules choose between them based on the language's body-field convention. `ByteRange::from(tree_sitter::Node)` (in `types.rs`) replaces the 7 hand-written constructors. Each adapter's local `text()` / `range()` shim is preserved as a one-liner that delegates, keeping the call-site shape (`self.text(n, s)`) so the bulk of each adapter reads unchanged. Net effect: ~150 lines of duplication removed; any future fix to range arithmetic / signature truncation lands in one place. ## 2. `ImportKind` normalization `Import` gained a `kind: ImportKind` field with three variants: - `Header` — C / C++ `#include` - `Module` — single-token namespaces (Go `"fmt"`, Python `os.path`, Ruby `'json'`, Clojure `clojure.string`) - `Qualified` — fully-qualified names with explicit scoping (Java `java.util.List`, Rust `std::sync::Arc`, TypeScript module specifiers) Lets cross-language queries ("what files import this module?") normalize without re-parsing the source string. Each adapter sets `kind` on every `Import` it constructs. ## 3. UTF-8 fallback → `tracing::debug!` `node.utf8_text(s).unwrap_or("")` (the pattern every adapter used) was silent. The fallback is now logged at `debug` level via `common::node_text` — visible under `--verbose` (which enables `dirge=debug`), quiet by default. Chose `debug` over `warn` deliberately: UTF-8 failures aren't load-bearing (the resulting empty symbol name is filtered out downstream) and the log line exists for diagnostic purposes only. Default-mode users running on a single oddball file shouldn't see a noisy warning. ## 4. `exports` field populated The field on `ExtractedFile` was `Vec::new()` in every adapter; now backfilled from `is_exported=true` symbols at the end of each `extract()`. TypeScript + Python preserve their existing explicit-exports list (TS index re-exports are load-bearing); other adapters fall through to the is_exported-derived backfill when their per-language exports vec is empty. Consumers asking "what does this file export?" can now read `extracted.exports` directly instead of iterating the symbol vec. ## Tests 3 new regression tests: - `byte_range_from_node_uses_1_based_lines` — pins the new `From<Node>` impl's line-number contract (the 7 hand-written copies all used `row + 1`; the shared converter has to match). - `imports_are_tagged_module_kind` (Clojure) — verifies the kind classification. - `exports_mirror_is_exported_symbols` (Clojure) — verifies the backfill picks up `defn` while filtering `defn-`. 812 all-features / 729 plugin / 603 default pass (was 809 / 729 / 603). All build profiles + `cargo fmt --check` clean. ## Files touched - `src/semantic/common.rs` (new) - `src/semantic/types.rs` — `ImportKind` enum, `Import.kind` field, `ByteRange::From<Node>` impl, `exports` docstring. - `src/semantic/mod.rs` — wire `common` module. - `src/semantic/adapters/{bash,clojure,go,ruby,rust,java,c,cpp,python,typescript}.rs` — refactored to use shared helpers + new Import shape + populate exports. Co-authored-by: Yogthos <yogthos@gmail.com>

…nd hygiene Self-review of the chamber + collapse work found 8 actionable issues. Addressing all in one round. - **#1 CRITICAL — ContextOverflow infinite loop on compress no-op** (`ui/slash.rs`, `ui/mod.rs`): `handle_compress` had three `Ok` paths that returned without compacting (within-limits, nothing-to-cut, summary-too-large). The auto-recovery branch treated any `Ok` as success and respawned the run against the SAME history, which immediately re-emitted `ContextOverflow` → loop. Added `CompressOutcome::{Compacted, NoOp{reason}}`; auto-recovery now respawns only on `Compacted` and surfaces a "made no progress" error otherwise. The interactive `/compress` and auto-Done paths ignore the discriminant — only the success/error path matters there. - **#2 HIGH — ContextOverflow re-runs side-effecting tools**: the interactive retry loop already refuses to retry once `had_tool_calls=true`; the new auto path bypassed that safety. Now gates respawn on `tool_calls_this_run == 0`; if any tool ran in the failed turn, compact happens but auto-retry is refused and the user must re-issue. - **#3 HIGH — char-truncated body wasn't stashed for Ctrl+O**: `render_tool_output` returned `None` when only `chars_truncated > 0` (no line truncation) — Ctrl+O reported "nothing to expand" despite the visible `+N chars truncated` footer. Now stashes whenever EITHER signal indicates hidden content. Regression test: `render_tool_output_stashes_on_char_truncation_alone`. - **#4 HIGH — `last_collapsed` persisted across turns**: a Ctrl+O press would expand a collapsed result from any prior unrelated turn. Cleared on prompt-send (every user submission starts a new turn) and on ContextOverflow respawn. - **dirge-code#5 MEDIUM — Ctrl+O was one-shot via `.take()`**: switched to `.as_ref().cloned()` so a second Ctrl+O re-emits the same expand; stash overwrites on the next collapse or clears on next turn. - **dirge-code#6 MEDIUM — empty `resolved_name` painted unnamed chamber**: added an early-out: when name resolution drops to empty (no `last_tool_name`, no buffered call by id), emit a dim `(unresolved tool)` trailer + chamber bottom and skip the body paint. - **dirge-code#7 MEDIUM — edit colorized diff lost when `last_tool_name` drained**: `is_edit` was gated on `last_tool_name`, falling through to plain `render_tool_output` when the slot drained (same shape as the chamber-orphan bug). Now gates on `resolved_name`. - **dirge-code#8 DESIGN/SEC — `banner_value` unsanitized at expand time**: the initial chamber TOP sanitizes the banner; the Ctrl+O reprint did not. ANSI-bearing tool args (MCP, plugin, attacker-shaped filename) could paint raw at expand. Sanitize once at stash time in `render_tool_output`. Also (review dirge-code#9, reviewer recommendation): swapped `apply_patch` out of `tool_skips_collapse` and `read` in. `read` is what the user/LLM explicitly asked for — defaulting to 4 lines defeated the request. `apply_patch` output is usually a short "N ops" summary; the rare per-op-failure spew is the right place for Ctrl+O to engage. Tests: 625 pass (3 added — char-truncation stash, apply_patch collapses, exempt-set without apply_patch). Updated 2 existing tests for the exempt-set change. Fmt clean.

…view Adversarial review of the per-prompt deny-list architecture flagged two real bypasses + several defense-in-depth gaps. Addressing. - **#1 CRITICAL — MCP tools bypassed the deny-list entirely**: `McpTool::call` passes the umbrella name `"mcp_tool"` to `check_perm`. The deny-list match is literal `==` (now case- insensitive), so a prompt declaring `deny_tools: [edit]` would NOT match an MCP server's `edit` tool — the LLM could route filesystem writes through any MCP server unscathed. Added `PermissionChecker::any_prompt_denied(&[name1, name2, ...])` public probe; `McpTool::call` now checks (concrete tool name, `mcp_tool:<server>:<name>` qualified form, umbrella `mcp_tool`) before invoking `check_perm`. Any hit returns a hard denial. - **#2 CRITICAL — ACP never installed the prompt deny-list**: the ACP bridge built a fresh `PermissionChecker` per session and never wired in `context.current_prompt_deny_tools`. Plan mode was a no-op for editor clients. Mirror the `apply_prompt_deny` call from `main.rs::build_channels` into the ACP `run_prompt` path right after `build_acp_permission`. - **dirge-code#5 MEDIUM — `glob` / `repo_overview` added to PermissionConfig**: both were filesystem walkers reachable via the perm checker but not declared as user-configurable in `PermissionConfig`. User- level `permission.glob = "deny"` would silently fall through to the `*` default. Added the fields + the per-tool rule loop entry. - **dirge-code#6 MEDIUM — `plan_enter` / `plan_exit` now consult deny-list**: both intentionally skip `check_perm` (the confirmation dialog IS the user-prompt). But the prompt deny-list should still apply — a strict-mode prompt that says `deny_tools: [plan_exit]` should refuse the call WITHOUT opening the dialog. Added a thin `check_prompt_deny` helper that queries `any_prompt_denied` before opening the channel. - **dirge-code#7 MEDIUM — case-insensitive tool-name matching**: `deny_tools: [Edit]` (typo capitalization) used to silently no-op. `is_prompt_denied` now uses `eq_ignore_ascii_case`; the frontmatter parser also lowercases at load so the stored list is canonical in every consumer (status line, UI, etc.). - **dirge-code#9 LOW — warn on unknown tool names in `deny_tools`**: at prompt load time, cross-check every `deny_tools` entry against a `KNOWN_TOOLS` list. Warns once per unknown entry, with the full known set printed for guidance. MCP-server-exported tool names will trigger this benignly; documented inline. - **#3 HIGH — pin order with tests**: three new checker tests pin the contract: - `prompt_deny_any_matches_concrete_and_qualified_mcp_names` (locks the MCP bypass fix) - `prompt_deny_is_case_insensitive` - existing tests continue passing - **#4 plugin trust boundary documented**: per the review's recommendation, added a "Plugin trust boundary" section to CONFIG.md acknowledging that plugins are inside the trust boundary and not sandboxed. Not addressed (intentional): - dirge-code#8 doom-loop UI nudge on repeated deny-list hits — UX polish. - dirge-code#10 `/prompt default` clear confirmation — user-typed; would have to confirm every clear, including the legitimate ones. 634 tests pass; fmt clean.

…ry, repo_overview walk Self-review of the last 5 commits surfaced 9 findings. Addressing all. - **#1 MEDIUM — `repo_overview` permission patterns got command-glob semantics**: `is_path_tool_name` was updated for `glob` but not `repo_overview`, so a config rule like `"repo_overview": { "/etc/**": "deny" }` parsed `**` as command-glob (no path-spanning match). Added `repo_overview` to the path-tool arm. - **#2 MEDIUM — `providers.<name>` override silently missed on capitalised provider names**: `parse_provider` is case-insensitive but `HashMap::get(provider)` is not, so `--provider Anthropic` built the client fine but the `providers.anthropic` chunk-timeout override was a no-op. Lowercased the lookup key in `resolve_stream_chunk_timeout` and `resolve_provider_info` (with the original key as a fallback so existing exact-case configs keep working). - **#3 MEDIUM — `Prompt.description` parsed but never displayed**: the frontmatter `description: "..."` was being captured into a `#[allow(dead_code)]` field. A user writing a description expected it to appear somewhere. Wired into the `/prompt` list output — each prompt now renders as `name description` (padded for alignment) when a description is set, falling back to the bare name otherwise. - **#4 MEDIUM — `stream_chunk_timeout_secs` undocumented**: the knob landed in 186628b with the error message pointing users at it, but neither CONFIG.md nor README explained the resolution ladder. Added a "Streaming timeouts" section to CONFIG.md covering the precedence order (custom_providers > providers > top-level > 300s default) and noting the case-insensitive matching. - **dirge-code#5 LOW — Unicode lowercase mismatch with ASCII-only matcher**: the frontmatter parser called `to_lowercase()` (Unicode-aware) while `is_prompt_denied` used `eq_ignore_ascii_case`. Swapped the parser to `make_ascii_lowercase` so both ends share the same contract — non-ASCII bytes pass through unchanged on both sides. - **dirge-code#6 LOW — MCP bare-name deny semantics undocumented**: the 3-name probe `any_prompt_denied(&[concrete, qualified, "mcp_tool"])` matches an MCP server's `edit` tool when the prompt denies `edit` (intended for the built-in). README now spells this out and recommends the qualified `mcp_tool:<server>:<name>` form for surgical denies. - **dirge-code#7 LOW — `KNOWN_TOOLS` duplicated `BUILTIN_TOOL_NAMES`**: two hand-maintained lists of every built-in tool, used for the MCP collision filter and the frontmatter `deny_tools` warn respectively. Drift would produce either spurious warnings or (worse) an unsafely shadowable name. Extracted a single `pub const BUILTIN_TOOL_NAMES` in `agent/tools/mod.rs`; both sites now import it. - **dirge-code#8 DESIGN — ACP can't switch prompts mid-session**: the 4dd17d7 fix installed the deny-list correctly per-request but ACP has no protocol message for `/prompt <name>`, so the deny- list is effectively locked at boot. Documented in the README ACP bullet — recommends `--prompt <name>` at launch for restricted modes. - **dirge-code#9 LOW — `repo_overview` ancestor walk went above crawl root**: `compute_dir_file_counts` bumped every parent up to `/`, including dirs that would never be printed. Now stops at the first ancestor not in the printed-dir set. 634 tests pass; fmt clean.

…rf cache Self-review of the last 4 commits flagged 11 findings. Addressing all in one batch. - **#1 HIGH — leading whitespace dropped on first row of soft_wrap**: `current.is_empty()` at row start unconditionally dropped `token.leading_ws`, including on the first row of a logical line. Option lines `" ▶ label …"` lost their ` ` margin; the green ` allowed …` confirmation lost its indent too. First-row branch now preserves leading_ws (with a ws-overflow fallback to keep the token). - **#2 HIGH — drain_events early-broke before reader quiesced**: the Ok(false) shortcut fired on the first quiet poll, which was often "the background reader currently holds crossterm's internal mutex" rather than "terminal is quiet". A delayed OSC 11 / DA1 response could still escape past our drain. Now requires at least one observed event before the Ok(false) shortcut; honors the full budget otherwise. - **#3 HIGH — MODIFIED section clipped its own bottom border at `available == 4`**: row_budget=1, footer + 1 file = 2 items, +3 frame rows = 5 total in a 4-row budget. draw_panel clipped the `╰────╯`. Bumped MIN_MOD_SECTION_ROWS from 4 → 5 so the bottom border is always painted. - **#4 MEDIUM — `\r` not stripped from CRLF input**: Windows / some-MCP tool output left `\r` in tokens, producing terminal redraw artifacts. `soft_wrap` now strips a trailing `\r` per logical line. - **dirge-code#5 MEDIUM — Show cursor on alt screen was a no-op**: `Show` was issued while still on the alt screen; `LeaveAlternateScreen` restores the main screen's saved DECTCEM state, discarding the Show. Moved Show to AFTER LeaveAlternateScreen + disable_raw_mode. - **dirge-code#6 MEDIUM — recent(256) clones + locks on every redraw**: panel redraws on every streamed token. Added `modified::version()` monotonic counter (bumped on mark / clear); panel-side cache in `panel_modified_cached` keyed by (version, cwd) skips the lock + 256-PathBuf clone + path-strip when nothing changed. - **dirge-code#7 MEDIUM — break_long_token could emit wide-glyph row > budget at max_width<2**: floored max_width at 2 inside soft_wrap. A 1-cell terminal is unusable anyway; this just removes a sharp edge. - **dirge-code#8 MEDIUM — all-whitespace first row collapsed to empty**: same root cause as #1, fixed by the same change. Indented blank separators now preserve their indentation. - **dirge-code#9 LOW — `allowed …` confirmation flush against alert `╰─╯`**: added a blank-line breathing row before the green confirmation so the alert's bottom border and the confirmation don't read as one block. - **dirge-code#10 LOW — head_w used chars().count() not display width**: switched to `UnicodeWidthStr::width(head)` so future wide-glyph markers won't under-pad the continuation indent. - **dirge-code#11 LOW — single-select marker width inconsistent**: cursor marker was `▶` (w=1), non-cursor was ` ` (w=2), so wrapped tails of adjacent options drifted by one column. Cursor marker padded to `▶ ` so all markers in a question share display width. 5 new tests: - preserves_leading_whitespace_on_first_row - strips_carriage_returns_from_crlf_input - wide_glyph_respects_max_width_at_floor - preserves_leading_whitespace_only_line - version_bumps_on_mark_and_clear 651 tests pass; fmt clean.

…e migration, model-shrink, unicode tokens Self-review of the last 6 substantive commits flagged 7 findings. Addressing all in one batch. - **#1 HIGH — Accept mode silently bypassed mcp_tool's default-Ask**: `SecurityMode::Accept` coerced `Ask → Allow` for every non-path tool, so the new default-Ask rule for `mcp_tool` (f62a89b) was effective only in Standard mode. `dirge --accept` + an MCP server still allowed every call silently. Added `is_high_risk_non_path_tool(tool)` (matches `mcp_tool` and `bash`) which forces the Ask to survive the coercion. The general non-path coercion still applies to `question`, etc. Regression test pins both directions. - **#2 MEDIUM — Pre-5dc036d sessions resumed with under-counted `estimated_tokens`**: stored values were computed under the old text-only logic. Bumped `SCHEMA_VERSION` to 2 and added a v1 → v2 migration step that calls a new `Session::recompute_all_estimates` (also exposed `estimate_message_tokens` as the per-message helper). Tested via `v1_to_v2_recomputes_under_counted_estimates`. - **#3 MEDIUM — `/model` switch to a smaller window didn't warn about over-capacity**: switching from a 1M to a 200k model with the session already exceeding the new budget would have errored mid-stream on the next prompt. The /model handler now surfaces a warning recommending `/compress` when `total_estimated_tokens > new_ctx - reserve`. - **#4 LOW — Highlight tokenizer mis-split non-ASCII identifiers**: `bytes[i] as char` for a UTF-8 lead byte produced a Latin-1 char that failed `is_ascii_alphanumeric`, terminating the identifier mid-word (`naïve` → `na` + punctuation + `ve`). Walk via `line[i..].chars().next()` and broadened `is_ident_cont` to accept any non-ASCII non-control letter. ASCII path unchanged. - **dirge-code#5 LOW — `looks_like_type` colored short capitalized words as types**: `Ok`/`No`/`Hi`/`Id` all hit. Tightened the floor to ≥3 chars. Added `Ok`/`Err`/`Some`/`None` to the Rust types table so idiomatic 2-char Rust constructors still get type color via the explicit table. - **dirge-code#6 INFO — Yolo bypass documented**: README permission section now says explicitly that `--yolo` skips rule eval, the per-tool default-Ask, and the doom-loop detector — but that `deny_tools` frontmatter STILL applies (that gate runs BEFORE the yolo short-circuit by design). Accept-mode bullet also updated to note `bash` and `mcp_tool` keep their Ask. - **dirge-code#7 + dirge-code#8 CLEARED**: doom-loop Allow + `"*": "allow"` default config both verified NOT to bypass mcp_tool's Ask. 684 tests pass; fmt clean.

…nds, error masking Adversarial review of `349afb6`/`b9de3d5`/`2fac609` flagged 14 findings. Addressing all. - **#1 HIGH — C1 controls bypassed MCP stderr sanitizer**: the original filter was `b == 0x09 || (0x20..0x7f).contains(&b) || b >= 0x80`. That second clause let through every non-ASCII byte including the C1 control range (U+0080..=U+009F). In particular U+009B is single-byte CSI, behaves identically to `\x1b[` on iTerm2/xterm in 8-bit mode. A misbehaving MCP child could write `\u{9b}2J` and repaint the screen — exactly the smuggling vector the commit message claimed to close. Filter now blocks C0 controls (except `\t`), DEL, and the full C1 range. - **#2 HIGH — MCP stderr was silently dropped at default verbosity**: emit was `tracing::info!`, but dirge's default EnvFilter is `warn,rig=off`. Users diagnosing MCP server panics or init errors saw nothing — real regression vs the old `Stdio::inherit()`. Raised to `tracing::warn!` so it surfaces on the default config. - **#3 HIGH — `parse_ddg_html` panicked on truncated input**: `&html[tag_start..abs_start + 32]` blew up when `abs_start + 32 > html.len()` or landed mid-codepoint. Rewrote the scanner to anchor on `<a ` tags and walk only within bounded slices via `tag_end.min(html.len())`. Added regression test for the truncated case. - **dirge-code#5 MEDIUM — MCP stderr forwarder had no per-line cap**: `BufReader::lines()` buffers until `\n`. A buggy child writing a GB without newline would OOM dirge. Replaced with a manual read loop, 16 KiB per-line cap, emit `…[truncated]` past the cap and skip until next `\n`. - **dirge-code#6 MEDIUM — provider rotation race on first call**: two concurrent first-callers both saw `AtomicU8 = 0`, both rolled a fresh entropy pick, both stored. Last writer won — inconsistent contract. Switched to `compare_exchange` from 0 to candidate; loser re-reads the winner's value. - **dirge-code#7 MEDIUM — both-providers-fail error masked secondary + DDG errors**: only `primary_err` was returned. Now concatenates all three failures so the user can diagnose without chasing the wrong cause. - **dirge-code#8 MEDIUM — `parse_ddg_html` false-positives on substring match**: previously matched `class="result__a"` anywhere in the HTML, including inside `<script>` blocks or quoted text. Walked backward via `rfind("<a ")` could grab an unrelated anchor. Now anchors on `<a ` first and inspects the tag's attributes — proper containment check. - **dirge-code#9 MEDIUM — DDG snippet control bytes flowed into LLM prompt**: `strip_tags_and_decode` decoded entities but didn't filter ESC / C1 controls. A malicious or mojibake search result could ship ANSI styling into the agent's context. Added control-byte filter to the decoder's output pass. - **dirge-code#10 LOW — `PARALLEL_API_KEY` read per-call**: was `std::env::var` inside `call`, inconsistent with how `EXA_API_KEY` was captured at construction. Moved to `WebSearchTool::new`. - **dirge-code#11 LOW — whitespace-only key passed empty-filter**: `EXA_API_KEY=" "` produced a malformed `?exaApiKey=%20%20` URL. Now trims keys at construction. - **dirge-code#12 LOW — tool description out of date**: still mentioned Exa-only + DDG fallback, missed Parallel.ai rotation and the keyless default. Rewritten to reflect the actual contract. - **dirge-code#13 LOW — `urlencode_query` renamed to `percent_encode`**: the function is a generic percent-encoder (RFC 3986 unreserved set), not a form-encoder. Misleading name. - **dirge-code#14 DESIGN — no tests for new code paths**: added 11 regression tests covering ddg parser bounds + happy path + anti-script-block, control-byte filter, key trim, MCP response parser (plain JSON + SSE + malformed), DDG redirect unwrap, percent encoding, provider env override. - **dirge-code#15 LOW — bot-identifying DDG User-Agent**: swapped `compatible; dirge-agent/1.0` for a real Firefox 133 UA. DDG aggressively rate-limits identifiable scrapers. 695 tests pass (684 + 11 new); fmt clean.

Audit response covering the user's 6-item duplication list. Three substantive modules + thin wrappers; two items deferred as non-issues; one already complete from earlier commits. **#4 — ANSI / control-byte filter (new `src/ui/ansi.rs`)**: Previously three independent filters: MCP forwarder (`emit_mcp_line`), websearch (`strip_tags_and_decode`), chat (`sanitize_output`). Each drifted in coverage — one blocked C0 but not C1, another stripped `\r` only, etc. New `ansi::strip_controls(s, policy)` with a `StripPolicy { keep_newline, keep_tab }` knob is the single source of truth. MCP forwarder + websearch routed through it. Chat `sanitize_output` left as-is — it does ANSI-escape PARSING (consumes `\x1b[…m` as a unit so the payload disappears) which is more specific than what `strip_controls` does (drops just ESC, leaving `[31m` as visible text). The two coexist intentionally. **#1 — Reusable box component (new `src/ui/box_render.rs`)**: Three implementations of chamber/box math previously: tool chambers (`chamber_row` / `chamber_row_with_bg` / `chamber_bottom`), permission alert (inline `row` closure), panel sections (`push_section` closure). Inconsistent — `chamber_row` was display-width-aware, `chamber_row_with_bg` was char-count-based; each treated tabs and width math differently. New module: - `BoxStyle` enum (currently only `Rounded`) - `top(style, title, total_w)`, `bottom(style, total_w)`, `divider(style, total_w)` — frame primitives - `row(style, content, total_w)` — display-width-aware content row with tab expansion + truncate-with-`…` - `row_with_bg(style, content, total_w, bg_idx)` — for diff backgrounds - `expand_tabs(s, tab_stop)` — moved here from `mod.rs` - `BoxBuilder` — fluent API for callers that build a box all-at-once (notifications, alerts, panel sections). Long rows soft-wrap via `wrap::soft_wrap` instead of truncating. `chamber_row`, `chamber_row_with_bg`, `chamber_bottom` in `mod.rs` are now thin wrappers around `box_render`'s primitives — existing call sites unchanged, but the underlying math is shared. 7 new unit tests covering frame width invariants, tab handling, CJK, builder construction, and soft-wrap. **#2 — Single output chokepoint** — already done in commit `ea042b1` (the `ui::notifications` module + channel). The remaining stray `eprintln!` sites all run during STARTUP (config parsing, skill discovery, MCP connect_all) BEFORE `TerminalGuard` is installed, so they don't paint over the UI. Audited and confirmed clean. **#3 — Permission check chokepoint** — already centralized. `check_perm` / `check_perm_path` / `check_perm_path_resolve` in `agent/tools/mod.rs` cover every tool's input-checked path; `any_prompt_denied` covers MCP-style multi-name lookup; `check_prompt_deny` covers plan tools. No duplication worth extracting. **dirge-code#5 — Layout module** — deferred. `chamber_widths`, `Renderer::content_width`, `Renderer::line_width`, `Renderer::max_line_width` are already in their proper homes; forcing them into a new module would be churn, not clarification. **dirge-code#6 — Soft-wrap inside chambers** — deferred. The current chamber rows truncate with `…` (matches user expectation for single-row tool result lines); the `BoxBuilder` path soft-wraps when the caller wants that explicitly. Forcing all chamber rows to soft-wrap would change the visual feel of tool output and needs a separate UX call. 709 tests pass (702 + 7 box_render); fmt clean.

…h_bg width, sanitization Correctness + security review of `ea042b1`/`524c90c` flagged 15 findings. Addressing all 15. - **#1 HIGH — startup race: MCP forwarders fired before `install()`**. `connect_all` spawns stderr forwarders in `main` BEFORE `run_interactive` reached the old `install()` call. Lines emitted during MCP-server handshake hit `sender() == None` and were silently dropped. Moved `install()` to the very top of `main()` so the channel is live by the time any forwarder starts. Split the API: `install()` (creates channel) + `take_receiver()` (UI loop claims the rx). - **#2 HIGH — orphaned-sender footgun on UI restart**. `OnceLock` meant a re-entry could never replace the sender; producers holding clones would send into a dead channel forever. Switched to `RwLock<Option<Sender>>`. Producers also self-heal: when `try_send` returns Err because the receiver was dropped, `notify_send` clears the slot so subsequent producers see `None` and skip. - **#3 HIGH — `row_with_bg` was still char-count-based**. The refactor claim was "unifies display-width vs char-count" but the bg-tinted variant still used `chars().count()`. A diff row with CJK / emoji drifted the right border. Now uses the same display-width budget as the plain `row`. Regression test `row_with_bg_width_invariant` pins it. - **#4 HIGH — unbounded channel + no backpressure → OOM**. A buggy / hostile MCP child spamming stderr would grow the queue unboundedly. Switched to `mpsc::channel(1024)` (bounded) with `try_send` so the producer drops on overflow rather than unboundedly queuing. Test `bounded_channel_drops_on_full` pins the contract. - **dirge-code#5 MEDIUM — multi-colon MCP tool names**. `splitn(3, ':')` on `mcp_tool:server:do:thing` parsed correctly but the comment explanation was off. Clarified; behavior unchanged (the wildcarded server pattern is the desired semantics). - **dirge-code#6 MEDIUM — mcp_tool umbrella check case-sensitive**. `umbrella == "mcp_tool"` would miss `MCP_TOOL:…` if a future caller surfaces uppercase. Switched to `eq_ignore_ascii_case`. - **dirge-code#7 MEDIUM — receiver-side sanitization for ALL Notification variants**. MCP variant was pre-sanitized at the producer, but Info/Warn/Error had no producer-side contract. Adding receiver-side `ansi::strip_controls(KEEP_NEWLINE)` makes the rule un-bypassable: nothing reaches `write_line` carrying escape bytes regardless of how careful a future producer is. - **dirge-code#8 MEDIUM — websearch `KEEP_BOTH` + `\n` broke chamber border**. Tabs survived into chamber rows where they interacted poorly with the wrap math. Switched to `KEEP_NEWLINE` and replace `\t` with single space. - **dirge-code#9 MEDIUM — whitespace-only MCP lines dropped**. The blank-line collapse used `trim().is_empty()` which also ate legitimate indented continuation lines. Now uses `is_empty()` post-sanitize. - **dirge-code#10 LOW — `top()` with empty title rendered `╭─ ─…─╮`** (two spaces with no glyph between). Empty title now matches the bottom-border shape `╭{horizontals}╮`. Test pins it. - **dirge-code#11 LOW — `expand_tabs` precondition undocumented**. Added comment that input should be control-byte free; callers must sanitize first. - **dirge-code#12 LOW — `BoxBuilder::row("a\nb")` produced one row containing a literal `\n`**. Now splits on `\n` and emits one row per logical line. Test `builder_splits_embedded_newlines` pins it. - **dirge-code#13 LOW — `BoxBuilder` had no labelled-row variant**. Added `row_labelled(label, sep, value)` that indents wrapped tails under the value column. Mirrors the alert chamber's `labelled_rows` shape so a future alert migration to BoxBuilder is unblocked. Test pins continuation indent. - **dirge-code#14 LOW — `strip_controls` allocated on no-op path**. Fast path returns the input unchanged when no chars would be filtered. - **dirge-code#15 DESIGN — sender caching deferred**. Per-call `sender()` is the right semantics for the orphan-detection case (#2); caching would skip the slot-clear behavior. Kept as is. 8 new tests; 715 total. Two-test serialisation via TEST_GATE for the notification tests since they mutate global TX/RX_HOLDER state. fmt clean.

…ser_rx writes Code review of `a70fb03`/`8b0688c`/`afc76eb` flagged 15 findings, including one **active regression** I shipped: `write_outside_chamber` reused `close_tool_chamber_if_open` which always painted "⚠ tool denied · aborted · no result". So every notification arriving while a tool was in-flight would falsely brand that tool as denied. Fixed. Headline: of 9 tokio::select! arms, only 2 were using the new chokepoint. 3 others (question_rx, dialog_rx, plan_rx) carried the SAME X-inside-chamber bug the helper was built to eliminate. Migrated them. - **#4 HIGH (regression I shipped)**: split chamber-close into two variants: - `close_tool_chamber_abort` — paints the "⚠ tool denied" row + bottom border. Used by permission-deny / agent error / interjection / context-overflow paths (the tool is being actively rejected). - `close_tool_chamber_passive` — emits ONLY the bottom border. Used by `write_outside_chamber` (the tool isn't being denied; we just need to terminate the visual frame so notification text doesn't land inside). - `close_tool_chamber_if_open` kept as back-compat alias for the abort variant — existing call sites (4 of them, all in abort-shaped contexts) keep their previous behavior. - **#1 / #2 / #3 CRITICAL — three arms migrated**: - `question_rx` (3537): a `question` tool's chamber was open when the prompt header was painted; header + stem + option grid landed inside. - `dialog_rx` (3811): plugin `harness/confirm` / `harness/select` fires from inside on-tool-start hooks while a tool chamber is open; the dialog rendered inside. - `plan_rx` (3955): plan-switch prompt could be delivered while a tool chamber was open; prompt landed inside. - **dirge-code#5 HIGH — user_rx interactive writes migrated**: - Ctrl+C interrupt msg (1135) - "copied selection" (1150) - Ctrl+X dropped-interjection trailer (1168) - "agent is busy" × 2 (1533, 1598) - **dirge-code#7 MEDIUM — defense-in-depth sanitization**: `write_outside_chamber` now runs `strip_controls(KEEP_NEWLINE)` on `text` before writing. A future caller that forgets producer-side sanitization can't smuggle ANSI escapes. - **dirge-code#12 LOW — notification amplification cap**: the bounded channel limits NOTIFICATIONS but not ROWS per notification. A single `Notification::McpLog` carrying 10k `\n`s would expand to 10k chamber rows. After 200 lines we truncate and emit a `[N more lines suppressed]` marker. - **dirge-code#6 audit** revealed the 4 remaining manual sites (1984/2602/2713/2884) all ARE abort-shaped and correctly use the abort variant via the back-compat alias. No migration needed. - **dirge-code#8 / dirge-code#9 / dirge-code#10 / dirge-code#13 / dirge-code#14 / dirge-code#15** noted as design trade-offs or already verified clean. 3 new regression tests: - `close_passive_does_not_paint_abort_row` pins the new no-abort-label contract - `close_abort_paints_warning_and_bottom` pins the abort variant still emits 2 rows - existing `write_outside_chamber_closes_chamber_first` still passes; helper now uses passive close 718 tests pass (716 + 2 new); fmt clean.

…rupt on crash External review flagged that `write.rs:105`, `edit.rs:234`, and `apply_patch.rs:93,155` all called `tokio::fs::write` directly, which opens with O_TRUNC and writes in-place. A crash between the truncation and the final byte (power loss / OOM-kill / SIGKILL / panic) leaves the file corrupted with no recovery. The irony: `session/storage.rs` already had the correct pattern — temp + fsync + rename — but it wasn't shared. Extracted the pattern into new top-level module `src/fs_atomic.rs`: - `atomic_write_sync(path, content)` — sync, used by storage - `atomic_write(path, content)` — async, used by tools (delegates to spawn_blocking so the create + fsync + chmod + rename sequence runs atomically in one blocking task) - `next_temp(target)` — hidden sibling temp path with pid+nanos+counter nonce so two concurrent saves don't collide on the temp filename (counter is the load-bearing piece — same-nanosecond firings still get distinct names) - Unix mode preservation: stat the existing target's perms BEFORE rename, chmod the temp to match. Without this, an atomic overwrite of an executable script would silently drop the +x bit (default temp perms are 0644 minus umask). Migrated four call sites: - `agent/tools/write.rs:105` — full-file write - `agent/tools/edit.rs:234` — edit-tool output - `agent/tools/apply_patch.rs:93` — apply_create - `agent/tools/apply_patch.rs:155` — apply_update Plus `session/storage.rs` now also uses the shared helper (was the original site with the pattern; now consolidated). Return type: `io::Result<()>` so existing `From<io::Error>` impls on `ToolError` / `anyhow::Error` continue to work. The async variant maps spawn_blocking join failures to `io::Error::other`. Six new regression tests: - `atomic_write_creates_new_file` - `atomic_write_overwrites_existing` - `temp_is_hidden_sibling` — verifies same-fs + dot-prefix - `next_temp_is_unique` — 1000-call distinct-name check - `atomic_write_preserves_mode` (Unix) — +x stays on across overwrite - `target_untouched_on_failed_rename` — atomicity guarantee Tests use `std::env::temp_dir` + a `TestDir` RAII helper (matches the codebase convention; dirge doesn't pull in `tempfile`). 724 tests pass (718 + 6); fmt clean. #2 (ToolStarted event), #3 (prepareNextTurn hook), #4 (structured tool output) follow in separate commits. dirge-code#5 from the review was a false positive — `skill` IS registered at `builder.rs:239`.

External-review #3 and #4 implemented as minimal, honestly-scoped versions. **#3 — `prepare-next-run` plugin hook** New hook fires AFTER `Done` (run complete) and BEFORE the next user prompt is processed. Plugins read this to signal session- level state changes for the next run. Currently the only supported mutation slot is `harness-next-model` (Janet: `(harness/set-next-model "claude-opus-4.7")`). Scope honesty: the request is SURFACED to the user as a notification (`"[plugin] requested model swap to 'X' — apply with /model X"`) rather than auto-applied. Auto-apply is deferred because: - The agent rebuild path is non-trivial across cfg-feature combinations. - The existing `/model` slash already does it correctly. - "Plugins propose, user disposes" is the safer default — a plugin can't silently swap to a more expensive model without the user noticing. Mid-stream model swap is explicitly UNSUPPORTED — rig's multi-turn stream owns state that doesn't survive a swap. The hook is scoped to between-runs only, documented in the comment next to the slot. Slot infrastructure: - `harness-next-model` declared in `worker.rs` startup blob - `harness/set-next-model` helper in the same blob - `take_pending_next_model()` on `PluginManager` clears the slot and returns its value **#4 — `ToolContent` classification on `ToolResult`** New enum `event::ToolContent { Text, File }`. Added as an additive field on `AgentEvent::ToolResult { id, output, kind }`. `output: CompactString` remains the authoritative payload for the LLM and the default UI rendering path — `kind` is purely metadata for richer consumers (ACP resource links, future UI file-card components). The runner classifies by tool name: `read` / `find_files` / `list_dir` produce `File`, everything else `Text`. Tracked via a per-stream `id → name` HashMap populated at each `ToolCall`, drained at the matching `ToolResult` (1:1 call/result pairing within a turn). Coarse on purpose — no per-tool `type Output` change required across ~20 tools. A future refactor could thread the variant through the rig `Tool` trait for finer-grained control. Consumers: - `extras/acp/mod.rs` reads `kind` (currently no-op; comment flags `ResourceLink` migration as a follow-up) - `ui/mod.rs` uses `{ .. }` rest pattern; ignores `kind` for now **dirge-code#5 from the review remains a false positive** — `skill` IS registered at `builder.rs:239`. 724 tests pass; all-features build clean.

…ap on #3) User asked to close the gap with pi's \`prepareNextTurn\` for issue #3. Compared against pi's implementation at \`packages/agent/src/agent-loop.ts:220-239\`: | Aspect | pi | dirge before | dirge after | |--------------------|---------------------------------|------------------|-----------------| | Frequency | per TURN (mid-run) | per RUN (Done) | per RUN (Done) | | Model swap | auto-applied | notification | **auto-applied**| | thinkingLevel | auto-applied | n/a | n/a | | context replace | auto-applied | n/a | n/a | Closed in this commit: **auto-apply of the model swap**. When a plugin sets \`harness-next-model\` from \`prepare-next-run\`, dirge now rebuilds the agent inline (same logic as \`/model X\`) so the next user prompt runs against the new model without any user intervention. Plus updates \`session.model\` / \`session.context_window\` / \`session.provider\` to keep the status indicator + recovery paths in sync. Validation guards on the swap: - Empty / whitespace-only \`next_model\` string ignored (mis- configuration shouldn't silently nuke the active model). - Same-model swap is a no-op (don't pay the rebuild cost when the plugin "swaps" to the current model). Remaining deltas vs pi (documented in the comment): 1. **Per-turn frequency**: pi fires \`prepareNextTurn\` between turns within a single agent run; dirge fires \`prepare-next-run\` only at run boundaries (after Done). Closing this requires breaking rig's multi-turn stream and restarting with a new agent — would lose partial assistant state, so we keep the swap at run boundaries. 2. **thinkingLevel**: dirge has no equivalent config concept (the \`/reasoning\` slash is UI-only — visibility toggle, not a knob into the model's reasoning budget). Adding it would require provider-side request-param plumbing across every supported backend. Skip until a real use case. 3. **context replace**: dirge has \`/clear\` and \`/compress\` for this; per-plugin wholesale replacement is niche. 724 tests pass; all-features build clean.

Three issues from the post-cutover code review against pi: **Bug #1**: stream.rs:186-194 — defensive fallback (stream closed without Done/Error) skipped emitting message_start / message_end. Pi at agent-loop.ts:359-366 emits both. Fix: route the fallback through `finalize()` so it follows the same emit path as Done/Error. Updated the existing test that documented the wrong behavior as "intentional Rust deviation" — it's now pi-faithful. **Bug #4**: integration.rs:411 — orphaned inner loop task. `spawn_loop_runner` spawned `run_agent_loop` as a NESTED `tokio::spawn`. A `task.abort()` on the outer task would kill it but leave the nested task running silently — tools could keep executing after the user thought they'd cancelled. Fix: collapse to `tokio::join!(loop_future, pump_future)` in the same outer task. Shared fate; outer abort drops both futures at their next .await. Tools that poll the AbortSignal still observe cancellation cooperatively. **Gap #3**: run.rs prepareNextTurn — pi at agent-loop.ts:229-238 rebuilds config with the new model / reasoning. We accepted the fields but silently ignored them. Surfacing a tracing warning per ignored swap so users wiring the hook know their change didn't take effect. Full fix requires the StreamFn to be a factory `Fn(Context) -> StreamFn` (so the loop can rebuild it on swap) — flagged for follow-up when a real consumer demands it. Items NOT addressed (documented in review): - #2 get_api_key receives empty string (no production caller) - dirge-code#5/dirge-code#6 timing / ordering changes (observable but not bugs) - dirge-code#7-9 efficiency micro-optimizations - dirge-code#10/dirge-code#11 UI-side wiring + Agent.preamble defensiveness Gates: - cargo build (default) clean - cargo build --all-features clean - cargo test (default) 841 green (unchanged) - cargo fmt clean

Surface every pi loop hook (prepareNextTurn, shouldStopAfterTurn, getSteeringMessages, getFollowUpMessages) to Janet plugins via dedicated slots. Auto-wired into spawn_loop_runner when a plugin manager is supplied. **Janet helpers added** (plugin/worker.rs): ; prepareNextTurn → next-turn config swap (harness/set-next-thinking-level "high") ; low/medium/high/xhigh/off/minimal ; harness/set-next-model already exists — repurposed ; shouldStopAfterTurn → graceful exit (harness/request-stop-after-turn) ; getSteeringMessages → mid-run user injection (harness/add-steering "wait, also do X") ; getFollowUpMessages → outer-loop continuation (harness/add-followup "do this next") **Rust slot accessors** (plugin/mod.rs): - take_pending_next_thinking_level() -> Option<String> - take_pending_stop_after_turn() -> bool - drain_steering_messages() -> Vec<String> - drain_followup_messages() -> Vec<String> Steering + followup use a newline-blob format (`msg\n`) so a single eval round-trip drains the queue cleanly. **Hook factories** (plugin_hooks.rs): - prepare_next_turn_from_plugin_manager(pm) -> PrepareNextTurnFn - should_stop_after_turn_from_plugin_manager(pm) -> ShouldStopAfterTurnFn - get_steering_messages_from_plugin_manager(pm) -> GetSteeringMessagesFn - get_followup_messages_from_plugin_manager(pm) -> GetFollowupMessagesFn Each follows the same lock-then-sync pattern as before/after_tool_call hooks: acquire mutex, eval slot, release. No `.await` while held. **Wired into spawn_loop_runner** (integration.rs): when `cfg.plugin_mgr` is set, all four hooks are installed alongside the existing before/after_tool_call. Caller-provided steering_queue still wins if both are set (explicit beats global). **Tests** (6 new integration tests with real Janet VM): - prepare_next_turn_reads_thinking_level - prepare_next_turn_returns_none_when_no_slot_set - prepare_next_turn_ignores_unknown_thinking_level (typo safety) - should_stop_after_turn_drains_slot - get_steering_messages_drains_queue (multiple add + drain) - get_followup_messages_drains_queue **Pi reference**: PLAN.md phase 5. Each slot maps 1:1 to a pi hook from runLoop. Slot mechanism (Janet `var` + `defn` helper + Rust `take_*` reader) was already established by the pre-existing harness-next-model / harness-block / harness-mutate-input slots; phase 5 extends the pattern to the remaining pi hooks. **Composition with phase 4.6**: prepareNextTurn's thinking_level field is now actively populated by plugins. The full chain works: plugin sets harness-next-thinking-level "high" in on-tool-end ↓ loop polls prepare_next_turn between turns ↓ TurnUpdate.thinking_level = Some(High) ↓ (currently surfaces tracing warn — code review #3; full model-swap apply pending rig API growth, see h-7 deferred) Gates: - cargo build (default) clean - cargo build --features plugin clean - cargo build --all-features clean - cargo test (default 846) green - cargo test --features plugin (985 = 979 pre-existing + 6 new phase 5 tests) green - cargo test --ignored 6 h-7 still green (no regression after Janet slot additions) - cargo fmt clean Phase 6 next: recovery / interjection / abort hardening under the new loop. Or phase 7 (custom message types). Phase 5 risk was low as predicted; landed clean.

…gnal threading, #2 provider name Three fixes from the phase-4.6/phase-5 code review. **R1**: `prepare_next_turn_from_plugin_manager` was draining `harness-next-model` alongside `harness-next-thinking-level`. The model slot has pre-existing dirge semantics — read by the UI at end-of-run (`ui/mod.rs:2359`) to spawn a fresh agent against the new model. With the prepareNextTurn hook draining it first, the UI's consumer saw None and `harness/set-next-model` silently failed. Fix: only drain the thinking-level slot in the hook. The model slot stays for the UI consumer. Mid-run model swap isn't supported anyway (run.rs already logs a warning when TurnUpdate.model is set — code review #3); a separate API for real mid-run model swap waits on rig API growth. **R3**: `opts.signal` in StreamOptions was silently ignored by the rig stream adapter. Mid-stream cancellation against the rig request had no effect — signal only took effect at the next turn boundary. (Old runner.rs path had the same limitation; not a regression but a real gap.) Fix: thread `Option<AbortSignal>` into wrap_streamed_assistant. Per-chunk pre-poll check: if signal is cancelled, emit an Error event with "aborted" substring and exit. Mid-LLM-call cancel now actually stops the rig request promptly. **#2**: `get_api_key` hook was called with `""` instead of the provider name. Pi contract: `getApiKey(provider: string) => key`. Provider-aware hooks couldn't dispatch. Fix: add `provider_name: Option<String>` to LoopConfig (and LoopSpawnConfig, threaded through). `AnyAgent::provider_name()` returns the canonical name per variant ("anthropic", "glm", etc.). spawn_runner sets it. stream_assistant_response passes it to the hook. **Tests** (3 new): - prepare_next_turn_does_not_drain_next_model_slot (R1) - signal_cancels_stream_mid_flight (R3) - signal_none_does_not_affect_stream (R3 negative case) - test_get_api_key_receives_provider_name (#2) **Not fixed in this commit** (documented as deferred): - R2: opts.api_key silently ignored by rig adapter (rig's client carries the key at construction; per-request override would need a rig API change) - R4: opts.request_timeout silently ignored (same reason) - Per-provider reasoning mappers (separate larger commit) Gates: - cargo build (default) clean - cargo build --features plugin clean - cargo build --all-features clean - cargo test 849 passed; 6 ignored - cargo fmt clean

Three follow-ups to PR dirge-code#126 (experimental tab completion) flagged in the code review: #1 — mid-word cursor produced corrupt buffer `try_complete` replaced `[word_start..cursor]` with the candidate command and appended `buffer[cursor..]` as a tail. With the cursor inside the first word (e.g. Tab pressed after Home, or after moving Left into `/mod`), the unreplaced suffix leaked into the result: `/mod` cursor=2 → `/mcpod` (replacement `/mcp` + residual `od`). Cursor=0 → `/allow/mod` (replacement + the entire word as tail). Fix: anchor replacement to the WHOLE-WORD boundary (`word_start..word_end`) so cursor position inside the command name no longer matters. New buffer is exactly one of the matching commands followed by the args tail (if any). Also: `cursor > word_end` now returns None — the user is past the command name typing args, completion shouldn't fire. #3 — Tab raced the file picker The `@`-file-picker has its own keystroke handling (Enter / Ctrl+J guard it explicitly). Tab in the new completion path didn't. In practice the picker's buffer doesn't start with `/`, but guarding explicitly closes the race for future changes — mirrors the existing Enter/Ctrl+J gates. #2 — single source of truth for slash command names Previously `builtin_commands()` (feature-gated, used by tab completion) and `handle_slash`'s match arms (always compiled, the actual dispatch) were two independent lists. Adding a command required updating both — or the command worked but wasn't tab-completable, or appeared in completion but errored on use. Restructure: * `slash_command_names()` — always-compiled canonical list. The one place to add a command name when wiring it up. * `is_known_slash_command(name)` — wrapper over the canonical list. Single implementation, no second match to drift from. * `builtin_commands()` — now just `slash_command_names()`, kept as a feature-gated alias so the public API doesn't break. * `handle_slash`'s `_` default arm — now consults `is_known_slash_command(parts[0])`. A name listed in the canonical list but with no matching dispatch arm surfaces as `internal error: X is listed in slash_command_names() but has no dispatch arm in handle_slash` instead of silently falling through to "unknown command" or a plugin lookup. Loud failure in dev/test. The remaining drift direction (dispatch arm but missing from the list) only costs tab completion — accepted as the lesser failure mode. Tests (9 new, 22 slash tests pass total) * complete_with_cursor_mid_word_produces_clean_buffer — #1 * complete_with_cursor_at_start_produces_clean_buffer — #1 * complete_preserves_trailing_args — #1 edge * no_completion_when_cursor_in_args — #1 edge * is_known_slash_command_agrees_with_canonical_list — #2 * slash_command_names_is_sorted — #2 (preview ordering) * always_on_commands_appear_in_canonical_list — #2 drift guard: pins the 22 always-on dispatch arms against the canonical list Verified * cargo test --bin dirge → 1011 passed * cargo test --bin dirge --features experimental-ui-tab-slash → 1025 passed * cargo test --bin dirge --features "experimental-ui-tab-slash plugin" → 1239 passed * cargo fmt --all --check → clean

…tests SESS-2 follow-up #1 — UI session-mutation on ContextCompacted: ContextCompacted event now carries summary + first_kept_index. The UI consumer mutates session.id in-place, calls Session::compress_reporting() to push a Compaction entry, and runs save_session() so the rotated id and summary are persisted on disk. Mirrors hermes-agent/conversation_compression.py lines 380-397. Without this the on-disk session kept the OLD id and the compaction was lost on next resume. SESS-2 follow-up #4 — /compress <focus> argument wire-through: - build_summary_prompt now honors focus_topic: when supplied, the Hermes-style "FOCUS TOPIC: …" framing is appended to the prompt, asking the model to allocate ~60-70% of its summary budget to the topic (verbatim port of hermes context_compressor.py:1050-1054). - compress_messages (existing slash-command path) gets the same treatment: any free-form text after /compress is wrapped in the FOCUS TOPIC framing instead of the generic "Additional instructions" placeholder. - run_compaction_pass exposes the focus parameter via a new with_focus wrapper (auto-trigger path still uses None). - Slash help text updated: "/compress [focus] compress; focus text guides what to preserve". SESS-2 follow-ups #2 (background spawn) and #3 (multi-generation chaining) closed as "matches reference impl": hermes is also inline (no background spawn) and only chains the most-recent prior summary via _find_latest_context_summary. Our implementation already matches. H7_SMOKE: remove the 6 #[ignore] markers from the real-API integration tests. Each test already has a runtime `detect_provider()` check that bails with `[skipped]` + Ok when no provider key is set; #[ignore] was blocking that check from ever running. Removing the markers means: in CI without keys, the tests run, hit the skip path, pass (1713 pass / 0 fail / 0 ignored). With keys present, they exercise the real provider as designed. Header doc updated. Tests: 1713 pass / 0 fail / 0 ignored (was 1707 / 0 / 6).

…asonix parity Independent verification turned up 6 gaps in the Phase 2.5 parity work. This commit closes all of them. #1 HIGH — `truncations_fixed` now bumps on hard-fallback too. Reasonix counts both success (`repair/index.ts:105`) and unrecoverable (`repair/index.ts:99`) under the same counter; dirge was dropping the latter, under-reporting exactly the cases operators need most. `apply_truncation_repair` now records the kind whenever the closer ran, not just on successful repair. #2 MEDIUM — closer notes are now surfaced to the model. Reasonix pushes `r.notes` into `report.notes` with `[<tool>]` prefix on success and `[<tool>] ⚠️ TRUNCATION UNRECOVERABLE: ...` on fallback (`repair/index.ts:100-101, :106`), then carries them into the next-turn assistant input. Dirge now stashes them per-call-id on a new `LoopConfig.truncation_notes` shared map; `prepare_tool_call` drains them and appends to `repair_notes`, which `prepend_notes_to_result` (already in place for relational-default notes) prepends to the tool result content so the model sees the repair in the same turn. #3 MEDIUM — added end-to-end wiring tests through `run_agent_loop`. The prior 7 tests proved the helpers worked in isolation; they did not prove the loop calls them in the right order. Two new tests drive the full canned-stream loop: - `dirge_7bwx_end_to_end_storm_dedupes_after_truncation_repair`: three tool calls with different truncated raw strings that heal identically. Storm threshold=3 → the third must be suppressed (only possible if truncation runs before storm). - `dirge_ngic_end_to_end_orphan_dsml_in_text_dispatches`: DSML invoke in `ContentBlock::Text` ONLY (no Thinking, no declared ToolCall) must dispatch (only possible if `build_scavenge_source` includes Text). #4 MEDIUM — removed dead `try_truncation_repair`. It was kept as "defense in depth" but marked `#[allow(dead_code)]`, so the safety claim was illusory. Now actually gone; direct callers can use `repair_truncated_json` for the brace-closer if needed. The `validate_and_repair` block-comment was updated to reflect the new contract. dirge-code#5 LOW — `truncation_repair_canonicalizes_divergent_streams_before_storm` tested canonicalization in isolation; the new end-to-end #3 tests exercise the actual storm dedupe path that depends on the String→Object promotion. The promotion itself is now also covered with an explicit note in `apply_truncation_repair`'s doc — it has no Reasonix analog (their args are always strings) and is dirge-specific compensation for mixed arg representations. dirge-code#6 LOW — added a comment near `storm.rs::inspect` documenting the implicit dependency on `serde_json` being built without the `preserve_order` feature. If feature unification ever enables it, storm dedupe regresses silently; the comment points at the workaround (`run::canonical_json`) and notes Reasonix has the same fragility at `repair/index.ts:127`. `LoopConfig` gained the new `truncation_notes` field; all constructors (production + tests) were updated. `Clone` impl threaded through. 1632 tests pass with `-D warnings` (was 1630; +3 new, -1 removed).

…PLAN #3) The per-result cap (cap_oversized_tool_results) was a flat 3000 tokens regardless of context pressure. Near the limit, a single uncapped tool result could push the NEXT request over before the reactive 75% post-response fold fires. Add a tiered cap: above AGGRESSIVE_CAP_THRESHOLD (60% estimated context) the per-result cap tightens to AGGRESSIVE_RESULT_CAP_TOKENS (1000) via a pure `tiered_result_cap(estimate, ctx_max)` helper; below it stays at 3000. The 60% threshold sits below the 75% fold trigger so the tighter cap has room to work first. Wired at the pre-send cap site in run_loop. Unit test for the tiering (normal below 60%, strict boundary at 60%, aggressive above). 2136 pass at -D warnings. Stacked on the circuit-breaker branch (PR dirge-code#220).

…essive-prune-tier feat(agent-loop): aggressive prune tier at 60% context (#3)

From the review of PR dirge-code#231: - #1 (run.rs): the cap notice's new_messages.push is a legitimate part of run_agent_loop's returned message list, but the comment overclaimed that headless/subagent collectors and resumed models read it — they drive display from the LoopEvent stream and discard the return value. Comment corrected to describe it as a return-value contract nicety, not the display mechanism. - #2 (provider/mod.rs): headless run_print dropped SystemNotice into its catch-all, so a --print run hitting the turn cap reported a clean success with no truncation signal. It now prints the notice to stderr. - #3 (text_output.rs): documented the deliberate divergence between the live <system>/warning-color notice and persisted <sys>/system-color session history, cross-referencing render_session. - #4 + altitude (text_output.rs): extracted write_prefixed_lines() shared by write_user_lines and write_system_lines, removing the copy-paste and making blank/empty-line handling identical by construction.

Addresses the six findings from the post-merge review of the harness/lsp API (PR dirge-code#245). #1 (dirge-5qqo) Load-time deadlock. The LSP responder used to spawn after the plugin-load loop, so a plugin querying LSP at load time blocked the worker forever on a reply with no drainer. Build the LspManager standalone (`build_lsp_manager`) and wire the responder BEFORE plugins load. On the current_thread runtime a load-time call still can't be serviced while the loader blocks, but it now falls back (see dirge-code#6) instead of hanging forever, and the disabled case returns nil instantly. Documented the load-time caveat alongside dialogs. #2 (dirge-m0zm) Honest availability. `(harness/lsp?)` checked only compile-time symbol existence, so with LSP disabled at runtime it reported available and queries silently returned nil (crashing plugins that then json/decode). New `harness/__lsp-live` C-fn reports liveness via the request channel's `is_closed()`, so the predicate reflects a real, wired bridge. #3 (dirge-38q7) Coordinate validation. `harness/lsp` now asserts line/char are positive 1-based integers (surfacing a plugin bug) rather than `read_uint_arg` silently clamping negatives/NaN to line 0. #4 (dirge-d098) Shared dispatch. Extracted `lsp::query` (Operation + parse + run) so the `lsp` tool and the harness share one op→method match and one coordinate-conversion point. Removes ~140 lines of duplication and the drift that left the harness without the tool's goToDefinition/findReferences aliases. dirge-code#5 (dirge-lfxd) Cheap diagnostics. Added `LspManager::diagnostics_for` (O(one file)); the harness no longer clones the whole project diagnostic map per single-file query. dirge-code#6 (dirge-eehk) Query timeout. `send_lsp` is now bounded by a 30s `LSP_QUERY_TIMEOUT` (via the testable `lsp_should_abort` helper) so a wedged language server returns nil instead of freezing the worker thread. Tests: liveness predicate false on dropped receiver; query nil on dropped receiver; nonpositive-coordinate rejection; abort-decision helper; operation-alias acceptance; Operation parse/needs_position. Full feature matrix green at -D warnings; default 2193, all-features 2214 tests pass.

…_and_resolve preamble [dirge-ac9k] (dirge-code#355) #1 (ANSI): sanitize_output (ui/events.rs) and strip_escapes (ui/ansi.rs) were two byte-identical DoS-capped ESC state machines guarding untrusted LLM/bash output to the terminal — a security-relevant duplication the codebase itself flagged as intended-but-unfinished. Verified keep_char(c, KEEP_BOTH) is char-for-char equivalent to sanitize_output's inline control check, then collapsed sanitize_output to: strip_escapes(strip_orphan_mouse_reports(s), KEEP_BOTH). Removed the ~70-line duplicate machine + the now-genuinely-dead strip_controls_compact (its 'future migration' took a different path). One escape stripper => the two guards can't drift. #3 (perm): the require_absolute_path + check_perm_path_resolve preamble (the Audit-H12 symlink-swap canonicalize→check invariant) was hand-rolled in 6 path tools. Hoisted to tools::require_and_resolve; read/write/edit/ read_minified/edit_minified/lsp now call it. apply_patch left as-is (its require + check are intentionally separated across validation vs apply). 2455 default / 2549 all-features tests pass (the existing ansi + sanitize + perm suites lock behavior); clean under -D warnings --all-features. Co-authored-by: Yogthos <yogthos@gmail.com>

Idea #3 from the Elastic agent-memory writeup. Each turn, auto-search long-term memory on the verbatim user message and inject the hits as a supplemental context note, so the agent sees relevant stored memory it wouldn't think to search for. Strictly supplemental: the block is pushed to the model-facing context ONLY — never to persisted history (new_messages) or the frozen <project_memory> snapshot — so it can't churn the prefix cache. A byte-identical-snapshot test guards that. Injected as a USER message (like the few-shot exemplar block), not system: the Codex/Responses path hoists system transcript items into the cached instructions, which would both drop the block and churn the prefix. Entries already inlined in the frozen snapshot are excluded, so pre-recall surfaces only the breadcrumb-tier memory the agent can't already see — no double-injection. Opt-in via memory.verbatim_pre_recall (default off), gated by a process-global set at build time (mirrors the existing MEMORIES_DIRTY flag, avoids threading a bool through every LoopConfig literal). The search is off-loaded to spawn_blocking since the hybrid provider may do a network round-trip; the memory_provider gate also keeps forked review/curator runners from pre-recalling. Documented in docs/config.md. bd: dirge-0gxb Co-authored-by: Yogthos <yogthos@gmail.com>

* memory: let a user see and edit what dirge remembers `/memory` had one subcommand, `reload`. There was no way to see what dirge had remembered about you — the store is SQLite with an FTS index, so the options were to ask the agent to call the `memory` tool and hope, or open sqlite3 and risk desyncing the index against the content. Memory is injected verbatim into the system prompt of every session in the project, and under global scope every project, so "you cannot read it" is a real gap. `/memory` now lists the store. `/memory edit` opens it in $EDITOR: reword a line to reword the memory, delete a block to forget it, add a block to record something new. Each entry is anchored on its id, rendered short (`[n7x4bhbp]`) and resolved by prefix. That is the load-bearing part. A memory row carries far more than its text — uid lineage, created_at, use_count, confidence, the supersession audit chain, and the procedural success/failure counters that the post-session expectation pass exists to move. Matching edited text back by content, or applying an edit as delete-then-recreate, silently resets all of it. With the id we UPDATE in place through the same path `replace_entry` already used, so it survives. `replace_entry`'s body is split into `apply_replacement` so the substring and by-id paths share one definition of what replacing means, rather than growing a second one that drifts. Deleting a block tombstones rather than destroys, so `restore` still works — removing a line in an editor should not be more destructive than the tool's own removal. An unparseable document aborts the whole edit intact; aborting the editor (`:cq`) changes nothing. $EDITOR handling is extracted from `Input::open_in_external_editor` into `ui::external_editor` and shared: the O_EXCL temp file, the /dev/tty fd juggling and the git-style argv are each easy to get subtly wrong twice. `/edit` should be retested by hand. Verified end to end against the built binary. Two bugs found by running it, both now covered by tests: a new entry written as `[identity] ...` — which the document's own header invites — was read as an unknown id; and because the document always echoes the kind back, every rewording looked like a re-classification and reset the outcome counters. A rewording now preserves uid, use_count, confidence and success_count while changing the text. * gate the /memory edit path to unix for the windows build render/parse/apply and the *_by_uid store methods are only reached from /memory edit, which needs $EDITOR (edit_text is unix-only), so windows flagged them dead. summarize stays cross-platform — the /memory listing works everywhere. also drops a needless borrow clippy flagged. * gate the SqliteMemoryStore import to unix too apply() was its only consumer left on windows after the previous commit. --------- Co-authored-by: Wayne <wayne@grange.la> Co-authored-by: Yogthos <yogthos@gmail.com>

Yogthos added 3 commits May 18, 2026 20:38

fmt: cargo fmt --all

f15b177

yogthos merged commit 82ffe88 into main May 19, 2026
1 check passed

yogthos mentioned this pull request May 21, 2026

fix(F3): align compress cut-point to user-message boundary #78

Merged

This was referenced May 29, 2026

feat(agent-loop): compaction circuit breaker (IMPROVEMENTS_PLAN #1) #220

Merged

feat(agent-loop): compaction report enrichment (#5) #224

Merged

feat(agent-loop): aggressive prune tier at 60% context (#3) #225

Merged

allen-munsch referenced this pull request in allen-munsch/dirge Jun 3, 2026

Merge pull request dirge-code#225 from dirge-code/feat/improve-2-aggr…

9d66791

…essive-prune-tier feat(agent-loop): aggressive prune tier at 60% context (#3)

yogthos mentioned this pull request Jun 6, 2026

Feature/microvm sandbox using libkrun #372

Merged

yogthos mentioned this pull request Jun 18, 2026

memory: verbatim pre-recall (supplemental, snapshot-safe) #440

Merged

Navigation Menu

Uh oh!

Per-turn read/grep/list cache with mutation invalidation - #3

Per-turn read/grep/list cache with mutation invalidation#3
yogthos merged 3 commits into
mainfrom
feature/concurrent-tools

yogthos commented May 19, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

yogthos commented May 19, 2026

Summary

Test plan

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant