Background task push notifications by yogthos · Pull Request #22 · dirge-code/dirge

yogthos · 2026-05-19T19:49:06Z

Summary

Implements push-style lifecycle notifications for background subagents, modeled on Claude Code. The parent agent no longer needs to poll task_status — when a backgrounded subagent finishes, the result arrives as a <system-reminder> at the start of the parent's next turn. The human user sees [task abc12345 completed] (green) or [task abc12345 failed: ...] (red) in the transcript as soon as the subagent finishes.

Subagents remain tool-less (no FS access); the only thing that changes is delivery semantics.

Phases (one commit each)

BackgroundStore notification queue + LRU cap. Replaces "get evicts on read" with: read-only get(), terminal-state notify() that queues a pending notification, drain_notifications() that pops the queue once. LRU cap of 32 keeps the store bounded.
TaskTool subagent uses notify. The spawned future writes via notify() instead of update(). Drops the Phase-1 backward-compat shim.
Drain + inject at turn boundary. BackgroundStore is hoisted to main::build_channels and threaded through build_agent / run_interactive. Each of the six spawn_runner sites in the UI loop prepends a <system-reminder> block via prepend_pending_notifications before sending the prompt to the LLM. The reminder is added only to the LLM-bound prompt — session.add_message still records the bare user message so re-replay doesn't re-deliver.
Prompt steering. task and task_status tool descriptions now tell the agent "don't poll; completion arrives automatically as <system-reminder> on your next turn." Adds two description-guard regression tests so the wording can't drift back.
UI lifecycle rendering. BackgroundStore carries an optional unbounded mpsc sink. notify() best-effort-sends a TaskNotification to it, drained by the UI's select! and rendered as a colored line in the user's scrollback.

Test coverage (29 store-level tests; 275 total)

Read-only get after completion (regression)
notify truncates Completed/Failed by chars (UTF-8 safe via emoji round-trip)
notify ignores Running state
Double-notify is idempotent on the queue
LRU evicts oldest at capacity; re-insert of existing id doesn't evict
notify on an evicted id is a no-op
drain_notifications returns once + leaves tasks in store for task_status
prepend_pending_notifications passthrough when None/empty
Drain consumes the queue (regression)
32-thread concurrent insert + notify
UI sink: receives completion + failure events
UI sink: event carries truncated payload (not the full original)
UI sink: Running doesn't emit; evicted id doesn't emit
UI sink: receiver-gone is silent (no panic; LLM-side drain still works)
Tool description guard: must mention <system-reminder> / automatically, must say do not poll

Code review fixes applied during the work

Phase 1: clarified doc comment from "LRU" → "FIFO by insertion (get doesn't bump)"
Phase 2: reverted a premature message change that promised the Phase-3 injection
Phase 3: noted slash.rs sub-rebuilds pass None for bg_store (consistent with question/plan); plan-switch path keeps the live store
Phase 5: explicitly drop the mutex guard before signalling the UI sink (no lock-across-send)

Known limitations

Mid-turn injection isn't possible — rig drives the full tool-call loop inside one agent.stream_chat(). Notifications arrive at the next spawn_runner boundary (next user turn / continuation). Matches Claude Code's behavior.
acp/mod.rs and the slash-command agent rebuilds get None for bg_store, so they lack the task tools. Pre-existing limitation for question_tx/plan_tx; consistent.
Adding bg_store brings several function signatures over the clippy too_many_arguments threshold. Pre-existing; flagged for a separate "bundle into Channels struct" refactor.

Test plan

cargo build clean
cargo test --bin dirge -- --skip plugin → 275/275 passing
cargo fmt --check clean
Manual: spawn a slow background task, do other work, observe the [task ... completed] line appearing mid-session; observe the <system-reminder> arriving on the next prompt

Replaces 'get evicts on read' with: - Read-only get(): completed tasks persist until LRU eviction (cap 32) - notify(id, state): records terminal state AND queues pending notification - drain_notifications(): pops the pending queue; tasks remain looked-up-able The pending queue is consumed by the UI at turn boundaries (Phase 3) to inject a <system-reminder> for completed background tasks, so the agent no longer needs to poll task_status in a loop. Phase-1 only changes the storage layer + tests. The existing TaskTool spawn path still calls store.update() — kept as a shim that delegates to notify(); removed in Phase 2. task_status remains read-only. 17 store-level tests (was 11), covering: read-only-after-completion, notify-truncates-by-chars (UTF-8 safe), running ignored by notify, double-notify idempotent on queue, LRU evicts oldest, re-insert doesn't evict, notify on evicted id is no-op, drain returns once + leaves tasks in store, 32-thread concurrent insert+notify. Adds indexmap as a direct dep.

The spawned background subagent now calls BackgroundStore::notify on completion (Completed/Failed), which records terminal state AND queues a pending notification. Drops the Phase-1 update() backward-compat shim. Migrates 4 test_status tests from update() to notify(). User-visible behavior unchanged in this phase — notifications are queued but not yet drained. Phase 3 wires the drain into the UI turn boundary.

BackgroundStore is now created in main::build_channels and threaded through build_agent + run_interactive, mirroring the question_tx / plan_tx plumbing. Each spawn_runner site in the UI loop prepends a <system-reminder> block listing any background tasks that finished since the last turn, then sends the augmented prompt to the model. The reminder is added ONLY to the prompt sent to the LLM — the bare user message is what gets recorded in session history, so re-replay doesn't re-deliver the same notification. Six spawn_runner sites wrapped (shell-command rerun, worktree exit, loop start, user submit, plugin followup, loop iter). The plan-switch agent rebuild also passes the live bg_store through so the rebuilt agent's TaskTool still feeds the same notification queue. prepend_pending_notifications is a pure helper in background.rs with six tests: pass-through when None / nothing pending, format check, failed-task rendering, drain-consumes-once regression, FIFO ordering. Total: 266 tests passing.

Updates the task tool and task_status tool descriptions to reflect the new push-notification flow. The task tool's background=true field tells the agent that completion arrives automatically; the call's return message reinforces 'do NOT poll'. task_status now self-describes as a rarely-needed lookup ('you usually do NOT need this'). Adds two description-guard regression tests (task tool + task_status): re-introducing polling-style language fails CI.

BackgroundStore now carries an optional UI sink (unbounded mpsc) that notify() best-effort-sends a TaskNotification into. The UI loop drains the lifecycle receiver in its select! and prints: [task abc12345 completed] (green) [task abc12345 failed: <head ...>] (red) appearing as soon as the subagent finishes, regardless of whether the parent agent is mid-stream. The line uses the short id (first 8 UUID chars) for legibility; the LLM-side notification still carries the full task_id. main::build_channels now creates the unbounded channel and constructs the store via with_ui_sink(); the receiver flows to run_interactive alongside the other channels. Six new tests on the channel semantics: - completion + failure events delivered - payload is truncated in the event (not just in the stored state) - Running state does NOT emit an event - evicted ids produce no phantom event - dropping the receiver doesn't break notify (best-effort) - LLM-side pending queue still filled when UI receiver gone Total: 275 tests passing.

H1: sanitize lifecycle failure-head before rendering. Newlines/tabs collapse to spaces, ANSI/control chars are stripped, truncation uses char count so multi-byte content isn't split. 7 dedicated tests. H2: thread bg_store through slash.rs handler signatures + all 7 build_agent rebuild sites + 2 ui/mod.rs sub-rebuild sites. Slash commands no longer drop the task tools. M1: pending queue carries pre-snapshotted TaskNotifications instead of just ids. Task eviction between notify and drain can no longer lose the payload. drain_notifications becomes a one-liner. Regression test inserts a task, notifies, evicts via 32 fillers, drains — payload intact. M2: notify de-dups id.to_string() into a single binding. M3: doc-comment the <system-reminder> convention next to prepend_pending_notifications so future features pick the same wrapper. prepend is now pub(crate) (was unnecessarily pub). M5: notify_started fires a LifecycleEvent::Started so the UI prints [task abc12345 started] (yellow) symmetric with completed/failed. LifecycleEvent is now an enum (Started | Finished). 2 dedicated tests plus a UI handler update. M6: task_status wait=true now caps at 600s. Uses tokio::time::Instant so paused-time tests are deterministic. Returns a 'still running' message after timeout instead of looping forever. Test fast-forwards through 630s of virtual time and asserts the timeout path fires. Adds tokio test-util as a dev-dependency for paused-time tests. Total: 275 → 287 tests passing.

…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>

…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>

Yogthos added 6 commits May 19, 2026 14:03

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

yogthos deleted the feature/bg-notifications branch May 19, 2026 20:44

Navigation Menu

Uh oh!

Background task push notifications - #22

Background task push notifications#22
yogthos merged 6 commits into
mainfrom
feature/bg-notifications

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

Phases (one commit each)

Test coverage (29 store-level tests; 275 total)

Code review fixes applied during the work

Known limitations

Test plan

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant