feat(phase 5): /allow CRUD slash command (list, add, remove, clear) - #73
Merged
Conversation
Phase 5 of the plan. No clean equivalent in opencode or pi — both
manage allowlist entries only via the interactive permission
prompt's "allow always" answer. dirge adds explicit CRUD so users
can inspect, manually add, drop one, or clear all entries without
editing the session JSON.
## Commands
- `/allow` or `/allow list` — show numbered entries
- `/allow add <tool> <pattern>` — add a (tool, pattern) entry,
e.g. `/allow add bash 'cargo *'`
- `/allow remove <idx>` — drop one entry by 0-based index from
the `list` output
- `/allow clear` — drop all entries
The `add` form preserves pattern strings containing spaces by
re-deriving the args from the raw `text` slice (rather than
relying on the SmallVec `parts[2]` which would get truncated at
the first whitespace). Same approach `/cd` uses for paths.
## State sync
Both surfaces stay aligned:
- `PermissionChecker::session_allowlist` (in-memory, used by
`check` / `check_path`) — updated via the existing
`add_session_allowlist` and the two new
`remove_session_allowlist_at(idx)` + `clear_session_allowlist`.
- `Session::permission_allowlist` (persisted to JSON, restored
on resume via `load_session_allowlist`) — mirrored
manually in the slash handler so save/load round-trips show
the user-edited list.
Without the session-side mirror, a `/allow add foo bar` would
stay in memory for this run but vanish on `-c` resume. Dedup at
the session level too so `save()` writes a clean list.
## Tests
3 new permission tests, written failing first:
- `remove_session_allowlist_at_returns_removed_entry`:
remove(1) on [bash:cargo*, bash:git*, read:/tmp/*] yields
Some(("bash","git *")) and the surviving entries shift down.
- `remove_session_allowlist_at_out_of_range_returns_none`:
remove(99) returns None without panicking; existing entries
intact.
- `clear_session_allowlist_empties_the_list`: clear() drops
all entries.
The slash handler itself is not unit-tested (requires the full
UI loop fixtures); manual smoke test:
```
$ dirge --restrictive
> bash 'echo hi'
> (a) allow always
> /allow list → [0] bash echo *
> /allow add read /tmp/*
> /allow list → [0] bash echo * [1] read /tmp/*
> /allow remove 0
> /allow list → [0] read /tmp/*
> /allow clear
> /allow list → empty
```
## Docs
`/help` text gets four new lines covering each subcommand.
## Test plan
- [x] `cargo test --features plugin` -> 649 pass (was 646).
- [x] `cargo build --all-features` -> compiles.
- [x] `cargo build --no-default-features` -> compiles.
## Plan status
| Phase | PR | Done |
|---|---|---|
| 1 | #69 | first-wins block + docs |
| 2 | #70 | sibling-branch pruning + notification |
| 3 | #71 | structured tool-call persistence |
| 4 | #72 | branch summary metadata in /tree |
| 5 | this | /allow CRUD |
| 6 | — | skipped per user request (cost tracking) |
5 of 6 phases complete; Phase 6 (cost tracking) skipped.
yogthos
added a commit
that referenced
this pull request
May 21, 2026
… centering, perm mode + README docs (#104) Five verified bugs from the third 6-agent audit pass. ## Bug 1 — grep walks dotfiles (security, F2 carryover) `src/agent/tools/grep.rs:129` set `.hidden(false)` on the ignore walker. Same security issue F2 fixed for find_files / glob / list_dir — `.env`, `.git/` internals, etc. could be matched by a generic regex search and surfaced into LLM context. Added `include_hidden: bool` to `GrepArgs` (defaults false, matching the F2 pattern). Schema documents the flag. Cache key includes `:hidden=<bool>` so the same pattern with different hidden flags doesn't collide. ## Bug 2 — grep invalid include glob silently fell back to `src/agent/tools/grep.rs:121` used `Regex::new(&pattern).unwrap_or_else(|_| Regex::new(".*").unwrap())` — a malformed include like `"[a-z("` silently became match-everything. User's include filter appeared to do nothing. Now surfaces the compile error via `ToolError::Msg` so the LLM sees "Invalid include glob '...': <err>. Use forms like "*.rs" or "*.{ts,tsx}".". ## Bug 3 — default_permission_mode typo silently → Standard `src/main.rs::resolve_mode` matched `default_permission_mode` against "yolo"/"accept"/ "restrictive" and fell through to `SecurityMode::Standard` for anything else. A typo like `"restritctive"` silently ran the agent in standard mode while the user thought they had configured restrictive. Now warns to stderr naming the unknown value + valid options: `warning: unknown default_permission_mode "restritctive" in config; using standard. Valid values: yolo, accept, restrictive, standard.`. Also accepts "standard" explicitly (previously silent default). ## Bug 4 — chamber_row_centered padding off by 2 + char-count vs display-width `src/ui/mod.rs::chamber_row_centered` had TWO stacked bugs: (1) Used `content.chars().count()` instead of display width. The NO-OUTPUT chamber starts with `⚠` (2 cells / 1 char), so centering was off by 1 cell. (2) `pad = inner - (len + 2)` left the row `inner + 2` cells total — but `chamber_row` and `chamber_bottom` produce `inner + 4` cells. The right border was 2 cells to the LEFT of the chamber's / . PR #93's visible chamber-right-border misalignment was this. Fixed to `pad = inner - len` (using display width). Row now matches `inner + 4` cells exactly, so the right border lines up with the chamber's top/bottom corners. ## Bug 5 — README docs gaps for shipped features PR #102 (custom theme JSON) and PR #73 (`/allow` CRUD) were both shipped but never mentioned in README: - Added `/allow <list|add|remove|clear>` row to the slash table. - Added a paragraph under "UI theme" pointing at `docs/THEMES.md` for custom theme JSON. ## Tests 1 new test: - `chamber_row_centered_handles_wide_emoji`: asserts row is exactly `inner + 4` cells wide with a leading-emoji message (regression guard for both bugs). 716 pass (was 715). All build profiles + fmt clean. ## Other audit findings — verified false positives or deferred The 6-agent audit produced ~60 candidate findings. Most were: - Speculative (panic-safety, atomic-ordering, dead-code claims) - Already-documented design choices (subagent isolation, permission lock-while-ask) - Feature requests (hot-reload plugins, harness/register-tool, MCP resources/prompts, etc.) Real bugs above are the ones I could reproduce or verify by reading the actual code. The rest go on the ROADMAP candidates list (or stay as is). Co-authored-by: Yogthos <yogthos@gmail.com>
allen-munsch
pushed a commit
to allen-munsch/dirge
that referenced
this pull request
Jun 3, 2026
…irge-code#73) Phase 5 of the plan. No clean equivalent in opencode or pi — both manage allowlist entries only via the interactive permission prompt's "allow always" answer. dirge adds explicit CRUD so users can inspect, manually add, drop one, or clear all entries without editing the session JSON. ## Commands - `/allow` or `/allow list` — show numbered entries - `/allow add <tool> <pattern>` — add a (tool, pattern) entry, e.g. `/allow add bash 'cargo *'` - `/allow remove <idx>` — drop one entry by 0-based index from the `list` output - `/allow clear` — drop all entries The `add` form preserves pattern strings containing spaces by re-deriving the args from the raw `text` slice (rather than relying on the SmallVec `parts[2]` which would get truncated at the first whitespace). Same approach `/cd` uses for paths. ## State sync Both surfaces stay aligned: - `PermissionChecker::session_allowlist` (in-memory, used by `check` / `check_path`) — updated via the existing `add_session_allowlist` and the two new `remove_session_allowlist_at(idx)` + `clear_session_allowlist`. - `Session::permission_allowlist` (persisted to JSON, restored on resume via `load_session_allowlist`) — mirrored manually in the slash handler so save/load round-trips show the user-edited list. Without the session-side mirror, a `/allow add foo bar` would stay in memory for this run but vanish on `-c` resume. Dedup at the session level too so `save()` writes a clean list. ## Tests 3 new permission tests, written failing first: - `remove_session_allowlist_at_returns_removed_entry`: remove(1) on [bash:cargo*, bash:git*, read:/tmp/*] yields Some(("bash","git *")) and the surviving entries shift down. - `remove_session_allowlist_at_out_of_range_returns_none`: remove(99) returns None without panicking; existing entries intact. - `clear_session_allowlist_empties_the_list`: clear() drops all entries. The slash handler itself is not unit-tested (requires the full UI loop fixtures); manual smoke test: ``` $ dirge --restrictive > bash 'echo hi' > (a) allow always > /allow list → [0] bash echo * > /allow add read /tmp/* > /allow list → [0] bash echo * [1] read /tmp/* > /allow remove 0 > /allow list → [0] read /tmp/* > /allow clear > /allow list → empty ``` ## Docs `/help` text gets four new lines covering each subcommand. ## Test plan - [x] `cargo test --features plugin` -> 649 pass (was 646). - [x] `cargo build --all-features` -> compiles. - [x] `cargo build --no-default-features` -> compiles. ## Plan status | Phase | PR | Done | |---|---|---| | 1 | dirge-code#69 | first-wins block + docs | | 2 | dirge-code#70 | sibling-branch pruning + notification | | 3 | dirge-code#71 | structured tool-call persistence | | 4 | dirge-code#72 | branch summary metadata in /tree | | 5 | this | /allow CRUD | | 6 | — | skipped per user request (cost tracking) | 5 of 6 phases complete; Phase 6 (cost tracking) skipped. Co-authored-by: Yogthos <yogthos@gmail.com>
allen-munsch
pushed a commit
to allen-munsch/dirge
that referenced
this pull request
Jun 3, 2026
… centering, perm mode + README docs (dirge-code#104) Five verified bugs from the third 6-agent audit pass. ## Bug 1 — grep walks dotfiles (security, F2 carryover) `src/agent/tools/grep.rs:129` set `.hidden(false)` on the ignore walker. Same security issue F2 fixed for find_files / glob / list_dir — `.env`, `.git/` internals, etc. could be matched by a generic regex search and surfaced into LLM context. Added `include_hidden: bool` to `GrepArgs` (defaults false, matching the F2 pattern). Schema documents the flag. Cache key includes `:hidden=<bool>` so the same pattern with different hidden flags doesn't collide. ## Bug 2 — grep invalid include glob silently fell back to `src/agent/tools/grep.rs:121` used `Regex::new(&pattern).unwrap_or_else(|_| Regex::new(".*").unwrap())` — a malformed include like `"[a-z("` silently became match-everything. User's include filter appeared to do nothing. Now surfaces the compile error via `ToolError::Msg` so the LLM sees "Invalid include glob '...': <err>. Use forms like "*.rs" or "*.{ts,tsx}".". ## Bug 3 — default_permission_mode typo silently → Standard `src/main.rs::resolve_mode` matched `default_permission_mode` against "yolo"/"accept"/ "restrictive" and fell through to `SecurityMode::Standard` for anything else. A typo like `"restritctive"` silently ran the agent in standard mode while the user thought they had configured restrictive. Now warns to stderr naming the unknown value + valid options: `warning: unknown default_permission_mode "restritctive" in config; using standard. Valid values: yolo, accept, restrictive, standard.`. Also accepts "standard" explicitly (previously silent default). ## Bug 4 — chamber_row_centered padding off by 2 + char-count vs display-width `src/ui/mod.rs::chamber_row_centered` had TWO stacked bugs: (1) Used `content.chars().count()` instead of display width. The NO-OUTPUT chamber starts with `⚠` (2 cells / 1 char), so centering was off by 1 cell. (2) `pad = inner - (len + 2)` left the row `inner + 2` cells total — but `chamber_row` and `chamber_bottom` produce `inner + 4` cells. The right border was 2 cells to the LEFT of the chamber's / . PR dirge-code#93's visible chamber-right-border misalignment was this. Fixed to `pad = inner - len` (using display width). Row now matches `inner + 4` cells exactly, so the right border lines up with the chamber's top/bottom corners. ## Bug 5 — README docs gaps for shipped features PR dirge-code#102 (custom theme JSON) and PR dirge-code#73 (`/allow` CRUD) were both shipped but never mentioned in README: - Added `/allow <list|add|remove|clear>` row to the slash table. - Added a paragraph under "UI theme" pointing at `docs/THEMES.md` for custom theme JSON. ## Tests 1 new test: - `chamber_row_centered_handles_wide_emoji`: asserts row is exactly `inner + 4` cells wide with a leading-emoji message (regression guard for both bugs). 716 pass (was 715). All build profiles + fmt clean. ## Other audit findings — verified false positives or deferred The 6-agent audit produced ~60 candidate findings. Most were: - Speculative (panic-safety, atomic-ordering, dead-code claims) - Already-documented design choices (subagent isolation, permission lock-while-ask) - Feature requests (hot-reload plugins, harness/register-tool, MCP resources/prompts, etc.) Real bugs above are the ones I could reproduce or verify by reading the actual code. The rest go on the ROADMAP candidates list (or stay as is). Co-authored-by: Yogthos <yogthos@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Phase 5 of the plan. New
/allowsubcommands let users inspect, edit, and clear the session permission allowlist without touching the JSON. Mirrors changes into both the in-memory checker and the persisted session list. 3 new tests, 649 pass.