runner: turn-boundary detection (TurnStart/TurnEnd events) — P0 by yogthos · Pull Request #37 · dirge-code/dirge

yogthos · 2026-05-20T15:53:36Z

Summary

Preparatory work for P3 (granular streaming hook events) and P4
(session tree branch-aware accounting).

Rig's multi_turn stream is a flat sequence of assistant content +
tool results + a final response. This PR adds a pure state machine
(`TurnTracker`) that detects per-turn boundaries inside that stream
and emits them as `AgentEvent::TurnStart { index }` /
`AgentEvent::TurnEnd { index }` events on the existing channel.

One "turn" = one LLM call + the tool calls it dispatched + the
tool results returning. Pure-text response = 1 turn. Run with two
cycles of tool calls = 2 turns.

Implementation

`TurnTracker` is a 3-state machine (`Idle` / `InTurn` /
`AwaitingNext`). The lazy `AwaitingNext` state collapses N
parallel tool results into a single turn boundary regardless of
how many tool results arrive before the next assistant message,
without needing lookahead.
`run_stream` instantiates one TurnTracker and emits the returned
`Boundary`s through the existing event_tx before each
assistant-content event and at every termination path (Done,
Error, Interjected, unexpected stream close).
UI currently handles both new events with a no-op match arm.
P3 will wire them into plugin `on-turn-start` / `on-turn-end` hooks.

Test plan

8 new unit tests in `mod turn_tracker_tests`:

`pure_text_emits_one_turn_around_content`
`single_tool_call_produces_two_turns`
`multiple_tool_results_collapse_into_one_turn_boundary`
`alternating_text_and_tools_advances_turn_index`
`empty_stream_emits_no_boundaries`
`stream_end_during_tool_dispatch_still_closes_turn`
`tool_result_without_open_turn_is_a_noop`
`tracker_can_be_reused_across_stream_restarts`
`cargo test --features plugin` — 510 pass, 0 fail (was 502).
`cargo test` (no plugin) — 460 pass + 12 pre-existing fails (unchanged).
`cargo build` clean both flavors.

Refs dirge-e17.

Preparatory work for P3 (granular streaming hook events) and P4 (session tree branch-aware accounting). Rig's multi_turn stream is a flat sequence of assistant content + tool results + a final response; this commit introduces a pure state machine that detects per-turn boundaries and emits them on the event channel. A "turn" = one LLM call + the tool calls it dispatched + the tool results returning. Pure-text response = 1 turn. Run with two cycles of tool calls = 2 turns. Implementation: * `TurnTracker` (src/agent/runner.rs) is a 3-state state machine — Idle, InTurn, AwaitingNext — that consumes observation calls (`observe_assistant_content`, `observe_tool_result`, `observe_stream_end`) and returns `Vec<Boundary>` describing the TurnStart/TurnEnd events to emit. * The InTurn→AwaitingNext→InTurn transition collapses N parallel tool results into a single turn boundary, no matter how many tool results arrive between assistant messages. * Lazy emission: TurnEnd fires either when the next assistant content begins (closing the previous turn) or on stream end. This avoids needing lookahead to know whether more tool results are still coming. * `run_stream` instantiates one TurnTracker and forwards Boundary events through the existing `event_tx` before each assistant-content event and at stream termination (Done, Error, Interjected, or unexpected stream close). New events: * `AgentEvent::TurnStart { index: u32 }` — fires before the first assistant content of each turn. * `AgentEvent::TurnEnd { index: u32 }` — fires after the last event of each turn (either before the next TurnStart, or before Done / Error / Interjected). The UI loop currently handles both with a no-op match arm; P3 will wire them into plugin `on-turn-start` / `on-turn-end` hooks. Tests: 8 new unit tests on `TurnTracker` in `mod turn_tracker_tests` — pure-text, single tool call, multi-result collapsing, alternating turns, empty stream, mid-dispatch end, lone tool result, restart reuse. Total: 510 pass with plugin feature (was 502); 460 pass without (was 452); 12 pre-existing plugin-test fails unchanged. Refs dirge-e17.

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

…e#37) Preparatory work for P3 (granular streaming hook events) and P4 (session tree branch-aware accounting). Rig's multi_turn stream is a flat sequence of assistant content + tool results + a final response; this commit introduces a pure state machine that detects per-turn boundaries and emits them on the event channel. A "turn" = one LLM call + the tool calls it dispatched + the tool results returning. Pure-text response = 1 turn. Run with two cycles of tool calls = 2 turns. Implementation: * `TurnTracker` (src/agent/runner.rs) is a 3-state state machine — Idle, InTurn, AwaitingNext — that consumes observation calls (`observe_assistant_content`, `observe_tool_result`, `observe_stream_end`) and returns `Vec<Boundary>` describing the TurnStart/TurnEnd events to emit. * The InTurn→AwaitingNext→InTurn transition collapses N parallel tool results into a single turn boundary, no matter how many tool results arrive between assistant messages. * Lazy emission: TurnEnd fires either when the next assistant content begins (closing the previous turn) or on stream end. This avoids needing lookahead to know whether more tool results are still coming. * `run_stream` instantiates one TurnTracker and forwards Boundary events through the existing `event_tx` before each assistant-content event and at stream termination (Done, Error, Interjected, or unexpected stream close). New events: * `AgentEvent::TurnStart { index: u32 }` — fires before the first assistant content of each turn. * `AgentEvent::TurnEnd { index: u32 }` — fires after the last event of each turn (either before the next TurnStart, or before Done / Error / Interjected). The UI loop currently handles both with a no-op match arm; P3 will wire them into plugin `on-turn-start` / `on-turn-end` hooks. Tests: 8 new unit tests on `TurnTracker` in `mod turn_tracker_tests` — pure-text, single tool call, multi-result collapsing, alternating turns, empty stream, mid-dispatch end, lone tool result, restart reuse. Total: 510 pass with plugin feature (was 502); 460 pass without (was 452); 12 pre-existing plugin-test fails unchanged. Refs dirge-e17. 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 mentioned this pull request May 20, 2026

plugin: granular streaming hooks — P3 (on-turn-start/end + on-message-update) #38

Closed

3 tasks

yogthos force-pushed the feat/runner-turn-events branch from 4505898 to 4f8ed7d Compare May 20, 2026 16:44

yogthos merged commit 5b965ab into main May 20, 2026
1 check passed

yogthos deleted the feat/runner-turn-events branch May 20, 2026 16:45

Navigation Menu

Uh oh!

runner: turn-boundary detection (TurnStart/TurnEnd events) — P0 - #37

runner: turn-boundary detection (TurnStart/TurnEnd events) — P0#37
yogthos merged 1 commit into
mainfrom
feat/runner-turn-events

yogthos commented May 20, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

yogthos commented May 20, 2026

Summary

Implementation

Test plan

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant