Skip to content

Unified state-driven rendering: single paint per event + UiState model (#387) - #388

Merged
yogthos merged 12 commits into
mainfrom
refactor/unified-rendering-387
Jun 5, 2026
Merged

Unified state-driven rendering: single paint per event + UiState model (#387)#388
yogthos merged 12 commits into
mainfrom
refactor/unified-rendering-387

Conversation

@yogthos

@yogthos yogthos commented Jun 5, 2026

Copy link
Copy Markdown
Collaborator

Closes #387.

Reworks the interactive TUI to a model-driven architecture: UiState is the single source of truth and the UI is rendered as a pure effect of the model changing, replacing ~85 ad-hoc inline paint sites and the 8-arg StatusLine construction duplicated ~48× (the merge-conflict surface the issue flagged).

Commits

  1. UiState data model — the source of truth (34 event-loop state fields grouped by concern).
  2. Renderer single-paint mechanismneeds_paint dirty flag, flush(), set_bottom(), and a dirty-on-change cache_bottom() extracted from draw_bottom.
  3. State migration — the 34 scattered locals in run_interactive become one ui: UiState; make_run_ctx! borrows ui.*; select! disjoint-field borrows compile cleanly.
  4. Render as a single effectrender_frame! builds the StatusLine once from the model and paints exactly once per event at the loop top (the trailing continues restart the loop, so no continue surgery); modal sub-loops flush after their own draws. Token-stream 60 fps coalescing and the spinner cadence are preserved (dirty-on-change + the timeout arm's request_repaint).
  5. Collapse the redundant inline sites — 46 main-arm draw_bottomrequest_repaint(), 6 modal trios→render_frame!(), 3 standalone render_viewportrequest_repaint(). StatusLine::render now appears exactly once in mod.rs.

Result

The chat scrollback buffer intentionally stays in Renderer (it's the painted output, appended as the effect of message/token/tool transitions); UiState holds the logical state that drives what's rendered.

Yogthos added 12 commits June 5, 2026 13:59
First step of the unified-rendering refactor (#387): define the event
loop's data model as a single source of truth before making the UI a
derived effect of it. UiState groups the ~36 mutable state locals
(run lifecycle, streaming buffers, tool-chamber state, loop/plan, chats/
subagents, interjection queue, pickers) currently threaded individually
through the event loop and run_handlers. Additive + allow(dead_code) until
the loop migration + render() effect land in the following commits.
Additive paint infrastructure for the model-driven render effect, reviewable
in isolation (no call-site changes yet):
- needs_paint dirty flag + request_repaint() to set it.
- flush(): the one tui_redraw per event, iff dirty (no-op otherwise, which
  preserves token-stream coalescing).
- Extracted cache_bottom() (bottom-area state derivation, no paint) from
  draw_bottom; draw_bottom = cache_bottom + immediate paint (legacy path),
  set_bottom = cache_bottom + mark dirty (the model-driven path).

Behavior unchanged; existing paint methods still paint inline. Next commits
wire render(&UiState) at the loop bottom + collapse the ~85 inline sites.
422 ui + 67 renderer tests green.
Phase 2 of #387 — track UI state as a data structure. Replace the ~34
scattered mutable locals in run_interactive (run lifecycle, streaming
buffers, tool-chamber state, loop/plan, chats/subagents, interjection queue,
pickers) with a single `let mut ui = state::UiState::new()`; all references
now read `ui.<field>`, and make_run_ctx! borrows `&mut ui.<field>`.
Behavior-preserving (still paints inline for now). The tokio::select! arms
borrow disjoint ui fields (ui.agent_rx, ui.plan_phase) without conflict.

Net -41 lines in mod.rs. 422 ui + 67 renderer + 99 input tests green;
all-features build clean. Next commit wires render(&ui) as the single-paint
effect and collapses the inline paint sites.
Make the paint a pure effect of UiState changing — one paint per event:
- render_frame! builds the StatusLine ONCE from the model + session/perm/
  stores, sets the bottom area, and flushes; called at the event-loop top
  (the trailing continues restart the loop, so it runs after every handler
  — no continue surgery) and after each modal sub-loop's draw.
- Mutators defer: write_line/write/render_viewport mark needs_paint instead
  of painting inline.
- cache_bottom is dirty-on-change, so the per-iteration render is a no-op
  when nothing changed — preserving 60fps token coalescing; the 200ms
  timeout arm request_repaint()s to animate the spinner.
- draw_bottom now defers (alias of set_bottom); modal sub-loops flush()
  after their draw to paint immediately (they don't reach the loop-top
  render).

Single-paint-per-event achieved. The ~80 inline draw_bottom/render_viewport
calls in the main-loop arms are now redundant (they defer; the loop-top
render is authoritative) and will be collapsed next. 422 ui + 67 renderer +
99 input tests green; all-features clean.
Finish #387's issue-#2 goal: remove the duplicated rendering call sites now
that the loop-top render_frame! effect is authoritative.
- 46 main-arm draw_bottom(<8-arg StatusLine>) calls -> renderer.request_repaint()
  (mark dirty; the loop-top render builds the status + paints). This deletes
  every duplicated 8-arg StatusLine construction — the merge-conflict surface
  the issue flagged. StatusLine::render now appears exactly ONCE in mod.rs.
- 6 modal render_viewport+draw_bottom+flush trios -> render_frame!() (build
  status once + paint).
- 3 standalone render_viewport()? -> request_repaint().
Net -303 lines in mod.rs. Behavior preserved: 422 ui + 67 renderer + 99
input tests green; full --all-features suite green; all-features build clean.
Regression from the single-paint refactor: modal sub-loops (question /
permission / dialog) rebuild content via replace_from each keystroke, but
replace_from didn't mark the frame dirty, so render_frame! skipped the paint
and the questionnaire looked frozen / uninteractable.

Audit of every visible-state mutator on Renderer; mark needs_paint at the
sources so no paint can be missed (the model can never get 'stuck'):
- buffer primitives: push_buffer_line (at-bottom) + replace_from.
- scrolling: scroll_line_up/down, scroll_page_up/down, scroll_to_top.
- chat tabs: add_chat, switch_chat (covers next/prev), remove_chat.
- overlays: set_alert_overlay, clear_alert_overlay, set_rewind_overlay.
- selection: clear_selection + selection::handle marks dirty on any
  Repaint/RepaintAndCopied outcome (fixes drag-select inside modals).
- avatar (on change), panel_modified_scroll (on change).
Panel data setters stay ambient (refreshed every loop-top; marking them
dirty would force a paint per iteration and defeat coalescing).

422 ui + 67 renderer + 11 selection tests green.
Make the main tokio::select! biased with user_rx first, so when a keystroke
and an agent event are both ready the keystroke wins. Keeps the UI
responsive to interactive events under a heavy agent stream — 'user
interactive events take priority' from the issue discussion. 422 ui tests
green.
First modal converted to the unified input mechanism. Adds InputMode +
ModalKind to UiState and a dispatch_modal! router in the single user_rx
arm. The plan_rx arm now hands its reply channel to InputMode::PlanSwitch
instead of spinning a nested blocking read loop; the y/n decision and
agent rebuild run in the dispatcher when the keystroke arrives, so the
event loop stays live (Ctrl+C, selection, resize) throughout the prompt.

Keys are swallowed while a modal owns input; non-key events fall through
to the normal handlers.
The question tool's triple-nested blocking loop (questions ->
option-select -> custom-text entry) is replaced by InputMode::Question +
a QuestionState the dispatcher walks one keystroke at a time. Option/stem/
custom-entry rendering is extracted into render_question_{stem,options}
and render_custom_entry so it runs both at setup and mid-walk. The event
loop now stays live throughout the questionnaire — this is the modal that
froze post-#387, and it can no longer block input.
…387)

harness/confirm and harness/select now render their prompt and hand the
reply Sender to InputMode::Dialog{Confirm,Select}; the dispatcher resolves
the keystroke and replies. Removes the nested blocking select! loops and
their deferred-event replay workaround (the unified loop swallows stray
keys and lets resize/scroll fall through, so events are no longer lost or
leaked into the compose box).
The last and most intricate blocking loop is gone. The ask_rx arm still
runs the parallel-prompt coalescing fast-path and the chamber-close /
alert-overlay setup, then hands the request to InputMode::Permission; the
dispatcher reads y/a/n/Esc (and Ctrl+C/D = deny) and runs all the
post-decision work (reply, avatar reset, cascade-deny drain + interject,
allowlist save, chamber reopen).

Also gate the four modal-triggering arms (ask/question/dialog/plan) on
!input_mode.is_modal() so a sibling request from a parallel tool batch
can't overwrite an in-flight modal's reply channel — it stays queued in
its channel until the active modal resolves, preserving the former serial
(blocking-loop) semantics and the allow-always coalescing.
@yogthos
yogthos merged commit 5b3c2b0 into main Jun 5, 2026
10 checks passed
@yogthos
yogthos deleted the refactor/unified-rendering-387 branch June 5, 2026 22:01
allen-munsch added a commit to allen-munsch/dirge that referenced this pull request Jun 7, 2026
… timeout, locking, temp dirs

Phase 1 — Hard Blockers:
- Rebase to restore dirge-code#388 InputMode state machine (dispatch_modal/render_frame)
- Delete dead src/sandbox/backend.rs (trait never wired in)
- SSH host-key verification: compare guest ed25519 host key after handshake

Phase 2 — High Severity:
- Document file-tool sandbox gap in SECURITY.md + startup warning
- Wire bash timeout: timeout<N> prefix + tokio::time::timeout around spawn_blocking
- OCI blob size cap: stream chunked responses with running counter
- OCI whiteout handling: process .wh.<name> and .wh..wh..opq after layer extract
- OCI tar safety: --no-absolute-filenames, reject .. path traversal

Phase 3 — Medium Severity:
- Pipe runner stderr (was Stdio::null(), now Stdio::piped())
- Rootfs cache: lock file + atomic rename via staging directory
- mkdtemp: replace PID-based temp dirs with UUID-based names
- Document krun_set_port_map 127.0.0.1 bind scope
- Surface try_lock errors on config setters (return Result)

Phase 4 — Polish:
- Add sandbox-microvm to CI build matrix
- Tighten snapshot name validation to allowlist [a-zA-Z0-9._-]+
- Document ephemeral port TOCTOU (acceptable risk)
allen-munsch added a commit to allen-munsch/dirge that referenced this pull request Jun 7, 2026
… timeout, locking, temp dirs

Phase 1 — Hard Blockers:
- Rebase to restore dirge-code#388 InputMode state machine (dispatch_modal/render_frame)
- Delete dead src/sandbox/backend.rs (trait never wired in)
- SSH host-key verification: compare guest ed25519 host key after handshake

Phase 2 — High Severity:
- Document file-tool sandbox gap in SECURITY.md + startup warning
- Wire bash timeout: timeout<N> prefix + tokio::time::timeout around spawn_blocking
- OCI blob size cap: stream chunked responses with running counter
- OCI whiteout handling: process .wh.<name> and .wh..wh..opq after layer extract
- OCI tar safety: --no-absolute-filenames, reject .. path traversal

Phase 3 — Medium Severity:
- Pipe runner stderr (was Stdio::null(), now Stdio::piped())
- Rootfs cache: lock file + atomic rename via staging directory
- mkdtemp: replace PID-based temp dirs with UUID-based names
- Document krun_set_port_map 127.0.0.1 bind scope
- Surface try_lock errors on config setters (return Result)

Phase 4 — Polish:
- Add sandbox-microvm to CI build matrix
- Tighten snapshot name validation to allowlist [a-zA-Z0-9._-]+
- Document ephemeral port TOCTOU (acceptable risk)
yogthos pushed a commit that referenced this pull request Jun 8, 2026
* feat: microvm sandbox via libkrun

Hardware-isolated sandbox backend using libkrun microVMs:

- Runner binary (src/bin/dirge-microvm-runner.rs) boots KVM guest
- SSH-based command execution via ssh2 crate + ephemeral ed25519 keys
- virtio-fs workspace mirroring at /workspace
- OCI image support: buildah for local images, pure-Rust puller for remote
- Rootfs caching with CoW-optimized per-session clones
- /sandbox slash commands: attach (PTY relay), snapshot, reboot
- PTY relay for interactive SSH sessions with scheduler isolation
- Config keys: sandbox.mode/image/cpus/memory_mib
- Three built-in images: debian, alpine, dev (Rust toolchain)
- Permission popup rendering fix (last_paint throttle + render_frame!)
- Windows/Unix cfg-gating for all sandbox-specific code paths
- CI: all 10 build variants pass (including Windows cross-build)
- Docs: docs/microvm/* (8 files, ~47 KB)

* fix(sandbox): review remediation — host-key verification, OCI safety, timeout, locking, temp dirs

Phase 1 — Hard Blockers:
- Rebase to restore #388 InputMode state machine (dispatch_modal/render_frame)
- Delete dead src/sandbox/backend.rs (trait never wired in)
- SSH host-key verification: compare guest ed25519 host key after handshake

Phase 2 — High Severity:
- Document file-tool sandbox gap in SECURITY.md + startup warning
- Wire bash timeout: timeout<N> prefix + tokio::time::timeout around spawn_blocking
- OCI blob size cap: stream chunked responses with running counter
- OCI whiteout handling: process .wh.<name> and .wh..wh..opq after layer extract
- OCI tar safety: --no-absolute-filenames, reject .. path traversal

Phase 3 — Medium Severity:
- Pipe runner stderr (was Stdio::null(), now Stdio::piped())
- Rootfs cache: lock file + atomic rename via staging directory
- mkdtemp: replace PID-based temp dirs with UUID-based names
- Document krun_set_port_map 127.0.0.1 bind scope
- Surface try_lock errors on config setters (return Result)

Phase 4 — Polish:
- Add sandbox-microvm to CI build matrix
- Tighten snapshot name validation to allowlist [a-zA-Z0-9._-]+
- Document ephemeral port TOCTOU (acceptable risk)

* fix: add timeout + stderr tests, fix host-key wire format comparison

Phase 2.2: Add timeout_kills_long_running_command integration test
- Boots microVM, runs sleep 300 with 2s timeout, verifies prompt return

Phase 1.3 bugfix: session.host_key() returns SSH wire-format blob (51 bytes
for ed25519), not raw key. Add extract_ed25519_raw_key() to parse wire
format and compare raw keys correctly. Add 4 unit tests.

Phase 3.1: Add runner_stderr_captured_on_crash test
- Spawns runner with garbage JSON, verifies stderr is captured

* chore: cargo fmt

* docs(microvm): sync architecture, security, and config docs with remediation changes

- Replace dead backend.rs/MicrovmBackend references with Sandbox::exec dispatch
- Document host-key verification step in ssh_exec and SSH handshake
- Document dual-layer command timeout (guest-side timeout + tokio::time::timeout)
- Document rootfs cache advisory lock and atomic staging → base rename
- Document OCI streaming byte counter cap for chunked-encoded responses
- Correct runner line count (~200 → 109)
- Update cache directory layout to include .lock and .staging/
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

rendering refactor ideas

1 participant