Skip to main content

ADR-015: Async Action Execution — Decouple Action Execution from Event Loop

Status​

Proposed (Revision 4 — after 4 rounds of LLM Council Review, reasoning tier)

Context​

Problem Statement​

The daemon's main event loop (EngineManager::run()) uses a single tokio::select! loop that processes input events, IPC commands, and timer ticks sequentially. When a mapping matches, action execution happens inline within process_device_event() — the same async task that reads from the input channel. This means the event loop is blocked for the entire duration of action execution.

For simple actions (single keystroke, volume change), this is imperceptible (<1ms). But for compound actions — particularly Action::Sequence with many steps — the blocking duration is significant:

  • Action::Sequence loops through sub-actions with thread::sleep(Duration::from_millis(50)) between each step (action_executor.rs:290-298)
  • Action::Delay(ms) uses thread::sleep directly (action_executor.rs:301-303)
  • Action::SendMidi performs synchronous MIDI I/O per message

Example: A mapping that plays a 25-note tune via SendMidi actions in a Sequence blocks the event loop for ~1.25 seconds (25 x 50ms inter-action delay). During this time:

  1. New MIDI events queue up in the device_event_rx channel (the MIDI device thread continues capturing independently)
  2. No events are emitted to the monitor broadcast channel (no push_monitor_event calls)
  3. The GUI event stream freezes — no new events appear
  4. Once the sequence completes, all queued events are processed in rapid succession and arrive at the GUI as a burst

This creates a poor user experience where the event stream appears to "hang" during long action sequences, contradicting the real-time feedback goals of ADR-014.

Root Cause Analysis​

Main Event Loop (single async task)
────────────────────────────────────────────────────────────
tokio::select! {
    device_event = device_event_rx.recv() => {
        process_device_event()           // <-- blocks here
            |-- event_processor.process()     // fast (~1us)
            |-- rules.match_event()           // fast (~65ns)
            |-- push_monitor_event(matched)   // fast
            |-- executor.execute(action)      // *** BLOCKING ***
            |   +-- Sequence [
            |       action_1 -> sleep(50ms)    // thread::sleep!
            |       action_2 -> sleep(50ms)
            |       ...
            |       action_25 -> sleep(50ms)   // total: ~1.25s
            |   ]
            |-- push_monitor_event(fired)     // only after execute returns
            +-- return                        // loop resumes
    }
    command = command_rx.recv() => { ... }    // also blocked
    _ = tick_interval.tick() => { ... }       // also blocked
}

The ActionExecutor::execute() method is synchronous (fn execute(&mut self, ...) -> DispatchResult) and uses thread::sleep for delays, which blocks the entire tokio runtime thread.

Impact Assessment​

ScenarioBlocking DurationUser Impact
Single keystroke<1msImperceptible
App launch~50msImperceptible
Shell commandVariable (unbounded)Potential hang
5-action sequence~250msNoticeable pause
25-action sequence (tune)~1.25sEvent stream freezes visibly
Sequence with Delay(500)500ms+ per delaySignificant freeze
Nested sequencesMultiplicativeExtended freeze

Constraints​

  1. Action ordering within a sequence must be preserved — actions in a Sequence must execute in order with their specified inter-action delays
  2. ModeChange propagation — Sequence currently short-circuits on ModeChangeRequested and propagates it to the caller; this must still work
  3. ActionExecutor is &mut self — it holds mutable state (Enigo instance, MIDI output connections) and is behind a Mutex
  4. Thread-safety — Enigo (keyboard/mouse simulation) is not Send + Sync; it must stay on the thread that created it
  5. Latency measurement — ADR-014 MappingFiredPayload.latency_us measures end-to-end time including execution; spawning changes what "latency" means
  6. Config hot-reload — The executor owns mutable state (MIDI connections, Enigo) that must stay current with config changes. However, action plans are immutable once dispatched (see D10) — config changes do NOT affect in-flight or queued actions
  7. Head-of-line blocking — Serialized execution means one long action delays all subsequent actions (acknowledged tradeoff; see D9)

Decision​

D1: Dedicated Executor Thread (Not spawn_blocking)​

Spawn a single, long-lived std::thread at startup that owns the ActionExecutor exclusively. This thread runs for the process lifetime and is explicitly joined on shutdown.

Why std::thread and not tokio::task::spawn_blocking: spawn_blocking is designed for short-lived blocking work units, not permanent service threads. Using it for a permanent actor loop holds a blocking-pool thread hostage and makes shutdown/cancellation harder to control. A dedicated std::thread with an explicit stop signal and cancellation-aware waiting is architecturally cleaner.

The executor thread reads from two channels using a priority-select pattern (see D2).

Rationale: This solves the Enigo thread-affinity constraint (it stays on one thread for the process lifetime), avoids mutex contention (the executor is owned, not shared), and naturally serializes actions (no concurrent execution that could cause keyboard/mouse conflicts).

Important: ActionExecutor (including Enigo) must be constructed inside the dedicated thread's closure, not on the main thread and moved. Enigo::new() may bind to the calling thread's event loop on some platforms.

D2: Separate Control Plane from Data Plane​

The executor receives work on one action channel and uses separate signal primitives for lifecycle control:

  1. Shutdown signal (shutdown: Arc<AtomicBool>): Set by the event loop, checked by the executor between actions and between sequence steps. The signal itself is zero-latency (no channel overhead), but the executor observes it with up to 50ms delay when idle (bounded by recv_timeout), or immediately between sequence steps.

  2. Config updates (config_watch_rx: watch::Receiver<ExecutorConfig>): A tokio::sync::watch channel (latest-wins semantics). The executor calls config_watch_rx.borrow_and_update() between actions to pick up the latest config. Bursts of reloads coalesce naturally — only the most recent version is seen.

  3. Action channel (action_rx): crossbeam_channel::Receiver<ActionRequest> — bounded (capacity 32), best-effort. Uses crossbeam_channel instead of std::sync::mpsc because it supports select! with the shutdown signal for responsive cancellation without polling.

// Executor thread main loop
loop {
    // Check shutdown before every action
    if shutdown.load(Ordering::Relaxed) {
        // Synthesize mapping_cancelled for queued requests (D13)
        drain_and_cancel(&action_rx, &completion_tx);
        return;
    }

    // Check for config updates (latest-wins, no queue)
    if config_watch_rx.has_changed().unwrap_or(false) {
        executor.apply_config(config_watch_rx.borrow_and_update().clone());
    }

    // Process one action (blocks until available or timeout)
    match action_rx.recv_timeout(Duration::from_millis(50)) {
        Ok(request) => {
            let result = executor.execute_interruptible(
                request.action, request.context, &shutdown
            );
            let _ = completion_tx.send(ActionCompletion { ... });
        }
        Err(RecvTimeoutError::Timeout) => continue,
        Err(RecvTimeoutError::Disconnected) => return,
    }
}

Why not multiplex control onto the action channel: Lifecycle messages (shutdown, config) on the same bounded, lossy channel as actions could be dropped under load via try_send. Shutdown and config updates must be reliable. Using separate signal primitives (AtomicBool, watch) ensures they are instantaneous and never lost.

Why recv_timeout(50ms) instead of blocking recv: The executor needs to check shutdown and config_watch periodically even when no actions are queued. The 50ms timeout bounds the worst-case response time for shutdown/config without busy-spinning. An alternative using crossbeam_channel::select! with a shutdown channel could eliminate polling entirely, but adds dependency complexity for marginal benefit.

D3: Non-Blocking Action Dispatch with try_send​

The event loop dispatches actions to the executor using try_send on the bounded action channel. The event loop never blocks on dispatch.

// In process_device_event() — NEVER awaits
match action_tx.try_send(request) {
    Ok(()) => { /* dispatched */ }
    Err(TrySendError::Full(dropped)) => {
        warn!("Action queue full — dropping action");
        push_monitor_event(mapping_dropped_event(dropped.invocation_id));
    }
    Err(TrySendError::Disconnected(dropped)) => {
        error!("Executor thread died — emitting mapping_dropped");
        push_monitor_event(mapping_dropped_event(dropped.invocation_id));
        // D13 invariant: every matched invocation gets a terminal event
    }
}

Executor death handling: When the executor thread dies (panic, OS-level failure), try_send returns Disconnected. The event loop emits mapping_dropped for the current invocation (preserving the D13 lifecycle invariant). For any in-flight invocations that were already dispatched but not yet completed, the event loop's handle_executor_death() method (triggered by recv() returning None in D4) emits mapping_cancelled for each pending invocation ID. The daemon transitions to a degraded state and logs a critical error. Automatic executor respawn is explicitly deferred — the daemon operates without action execution until restarted.

Overload policy: When the queue is full, the action is dropped and a mapping_dropped monitor event is emitted (carrying the invocation_id) so the GUI can display it. Dropping is preferable to blocking the event loop, which would reintroduce the original problem.

Channel capacity: 32 slots. At one action per mapping match, this accommodates rapid-fire scenarios without overflow. If overflow occurs, it indicates a pathological input pattern or a hung action.

D4: Completion Channel (Unbounded, Event-Loop-Drained)​

The executor sends completions back via tokio::sync::mpsc::unbounded_channel. The event loop drains completions with priority using biased select:

// Main event loop — completions are ALWAYS drained first
loop {
    tokio::select! {
        biased;  // deterministic priority ordering

        // Priority 1: drain ALL pending completions before anything else
        completion_opt = action_completion_rx.recv() => {
            let Some(completion) = completion_opt else {
                // Executor thread died — transition to degraded state
                warn!("Executor thread terminated unexpectedly");
                self.handle_executor_death().await;
                continue;
            };
            self.handle_action_completion(completion).await;
            // Continue loop to drain more completions before device events
        }

        // Priority 2: device events (only when no completions pending)
        device_event = device_event_rx.recv() => { ... }

        // Priority 3: IPC commands
        command = command_rx.recv() => { ... }

        // Priority 4: timer ticks
        _ = tick_interval.tick() => { ... }
    }
}

Why biased with completions first: Without priority, tokio::select! uses pseudo-random branch selection for fairness. This means a pending completion (including ModeChangeRequested) could be delayed indefinitely while device events continue to be processed under the old mode. Using biased ensures completions are always drained before new device events are matched. This is critical for ModeChange correctness — the mode change is applied as soon as the event loop iterates, not after an arbitrary number of device events.

Why the None case is handled explicitly: When the executor thread dies (panic, OS kill), recv() returns None. Without the else branch, tokio::select! would disable this branch and silently lose all completion tracking. The handler emits mapping_cancelled for any pending invocations and logs a critical error.

Why unbounded: The completion channel flows from executor to event loop. Completions arrive at most as fast as actions complete (rate-limited by execution time — minimum ~1ms per action). The event loop is the sole consumer and drains promptly via the priority mechanism above. Making this channel bounded would create a deadlock risk: if the executor blocks on a full completion channel, it stops processing actions, and the event loop may be waiting for actions to complete. However, the "always drains promptly" assumption depends on handle_action_completion() being fast — it must not perform blocking I/O or heavy computation. If future changes add heavyweight completion processing, this should be revisited.

D5: Invocation ID for Event Correlation​

Each action dispatch is assigned a unique invocation_id: u64 (monotonic counter). This ID is carried in mapping_matched, mapping_fired, and mapping_dropped monitor events, allowing the GUI and downstream consumers to reliably pair them even when events interleave:

Timeline with async execution:
  mapping_matched(invocation=1, trigger=Note36)
  mapping_matched(invocation=2, trigger=Note48)   // arrives during action 1
  mapping_fired(invocation=1, result=ok)           // action 1 completes
  mapping_fired(invocation=2, result=ok)           // action 2 completes

Why needed: Under synchronous execution, matched and fired events always appeared consecutively. With async execution, they interleave. The invocation_id is the only reliable way to correlate them. Note: this is a change in event ordering semantics — downstream consumers that assume consecutive matched/fired pairs must be updated to use correlation IDs.

The ActionCompletion struct:

struct ActionCompletion {
    invocation_id: u64,
    dispatch_result: DispatchResult,
    action_type: String,
    action_summary: String,
    processing_start: Instant,
    completed_at: Instant,        // captured by executor immediately after execute()
    execution_time_us: u64,       // executor-measured, excludes channel/scheduling delay
    device_id: DeviceId,
    mode_name: String,
    mapping_label: Option<String>,
    trigger_info: FiredTriggerInfo,
}

D6: Emit mapping_matched Immediately, mapping_fired on Completion​

Split the current single emission point into two:

  1. mapping_matched — emitted immediately when the rule matches, before action execution begins. Includes invocation_id for correlation.
  2. mapping_fired — emitted when the action completes, via the completion handler. Includes the same invocation_id, result, and latency.

Note on ordering: This changes the event model. Under synchronous execution, matched and fired appeared as an atomic pair. Under async execution, they can be separated by arbitrary other events. Consumers (GUI, CLI monitor, audit log) must be updated to handle interleaving. This is a breaking change in the monitor event stream semantics.

D7: ModeChange — Deliberate Semantic Change​

ModeChange is the most important semantic concern with async execution. Under the current synchronous model, while an action sequence runs, no new events are processed — they queue. Under async execution, new events continue to be matched against the current mode while the action runs in the background.

This means: if a Sequence contains Delay(500) -> ModeChange("Layer2"), and the user presses another key 100ms after the sequence starts:

  • Old behavior: the key is effectively queued and processed after the mode change completes
  • New behavior: the key is matched immediately against the old mode

Decision: This is a deliberate semantic change, documented as a known behavioral difference:

  1. The new behavior is more responsive: users expect their inputs to be processed immediately, not silently delayed behind a running sequence. The old behavior was an unintentional side effect of blocking, not a designed feature.
  2. ModeChange is applied atomically on completion: When a ModeChangeRequested completion arrives, the mode is updated via the existing ArcSwap atomic update. New events arriving after the completion is processed match against the new mode.
  3. Deterministic completion priority: The event loop uses biased select with completions first (D4). When a ModeChangeRequested completion is ready, it is always processed before any pending device events. The race window is reduced to events that arrive during the current handle_action_completion() call — a window of <100μs. This is acceptable because:
    • Human input timing has millisecond-scale jitter anyway
    • The alternative (pausing event processing) would reintroduce blocking
  4. Future option — BarrierAction: If exact synchronous-barrier semantics are needed, a future BarrierAction type could pause event-loop processing until the action completes. This is explicitly deferred — no current user workflow requires it.

D8: MIDI Output Recursion Guard via Message Fingerprinting​

When SendMidi or MidiForward actions output MIDI to a port, those messages must not be re-ingested by the input listener and trigger further mappings. Two levels of protection exist:

Existing protection (virtual ports): ADR-009 D21 auto-excludes the daemon's own virtual output ports from input scanning via set_exclude_port_names().

New protection (external echo/thru): Use message fingerprinting with a short TTL.

Fingerprint design:

  • Fingerprint content: Full raw MIDI bytes (3 bytes for most messages) + output port name. This is more precise than message-type-only matching and avoids false positives from the user playing the same note.
  • Storage: Bounded ring buffer, 64 entries, with timestamps. No heap allocation after initialization.
  • On outgoing MIDI: Insert fingerprint with current timestamp.
  • On incoming MIDI: Check if the raw bytes + source port match any entry within a configurable TTL window (default: 100ms, configurable per-port in [event_console]).
    • Match: Skip rule matching, but still emit to the monitor stream with source: "echo" provenance. The GUI displays these events visually annotated (e.g., dimmed or with an echo indicator).
    • No match: Process normally as user input.
  • Port matching: The output port name is compared against the input port name. For external MIDI thru (output="Synth In", input="Synth Out"), the user must configure a thru_pairs mapping in config. Without this, cross-port echo suppression is opt-in only.
  • Metrics: A counter tracks suppressed events per port for diagnostic visibility.

Why not an execution-context flag (see A5): A per-device flag set for the entire action duration would suppress all legitimate user input from the same device during long sequences — worse than the original problem.

Known limitation: False positives remain possible when the user plays the exact same note with the exact same velocity within the TTL window. This is rare in practice and acceptable; the TTL is configurable if a specific setup needs tuning.

D9: Head-of-Line Blocking — Acknowledged Tradeoff​

Serializing all actions through a single executor means one long-running shell command or MIDI sequence can delay all subsequent actions. This is an accepted tradeoff, documented as a first-class architectural choice:

  • Enigo (keyboard/mouse) is inherently single-threaded — concurrent keystrokes would interleave and produce garbled output
  • MIDI output ports are single-writer — concurrent sends would corrupt messages
  • The alternative (parallel execution) introduces far more complexity for minimal real-world benefit
  • Mitigation: Shell commands should use timeout or have bounded execution time. The executor emits a mapping_stalled event if an action exceeds a configurable threshold (default: 5 seconds).

If this becomes a problem in practice, a future enhancement could partition actions by type (e.g., MIDI output on one thread, keyboard on another). This is explicitly deferred.

D10: Config Hot-Reload for Executor State​

The executor owns long-lived mutable state and actions may sit queued before execution. The config reload policy is:

  1. ActionRequest.action is a fully resolved, immutable execution plan: The Action enum (already used by the current synchronous path) is a self-contained data structure — it carries all the data needed for execution (key names, app paths, MIDI bytes, delay durations, etc.). It does not reference config by pointer or index. This means a queued action always executes exactly as it was resolved at match time, regardless of subsequent config changes.
  2. A single Sequence never spans config generations: Because the entire Action tree is resolved at match time and carried immutably in the ActionRequest, config changes cannot affect in-flight or queued actions. There is no partial re-resolution.
  3. Executor-level state updates via watch channel: Executor-owned mutable state (e.g., MIDI output connections, Enigo instance) is updated via a tokio::sync::watch channel rather than queued control messages. The watch channel is "latest wins" — bursts of reloads coalesce naturally, and no stale intermediate versions accumulate. The executor checks the watch receiver between actions (never mid-sequence).
  4. Queued actions are not invalidated on reload: Actions already in the queue were matched under the old config and execute as-is. This is consistent with the current behavior where in-flight actions are not interrupted by hot-reload.

D11: Graceful Shutdown with Bounded Latency​

The executor thread supports clean shutdown via the shutdown: Arc<AtomicBool> signal (D2):

  1. Signal: The event loop sets shutdown.store(true, Ordering::Relaxed). This is instantaneous — no channel, no risk of being dropped under load.
  2. Detection points: The executor checks shutdown at three granularities:
    • Before each action (recv_timeout loop in D2)
    • Between sequence steps (execute_interruptible() checks after each sub-action)
    • During delays (sleep is replaced with a loop of 50ms intervals, each checking shutdown)
  3. execute_interruptible(action, context, &shutdown): A new method that wraps execute() with shutdown-awareness. Cancellation semantics vary by action type:
    • Delay(ms): Interruptible — sleeps in 50ms increments, checks shutdown between each. Cancellation latency: ≤50ms.
    • Sequence: Interruptible between steps — checks shutdown after each sub-action. The current sub-action completes atomically, then the sequence aborts. Cancellation latency: ≤(current sub-action duration + 50ms).
    • Keystroke/Text/MouseClick: Atomic — completes in <1ms, not interruptible. Cancellation latency: <1ms.
    • SendMidi/MidiForward: Atomic — single MIDI write, completes in <1ms. Not interruptible.
    • Shell: Non-cancellable by flag — blocking wait() on child process cannot observe AtomicBool. Instead, shell commands are spawned in a new process group. On shutdown, the process group is killed (SIGTERM, then SIGKILL after 2 seconds via a watchdog timer spawned alongside the command). The executor thread does not block indefinitely — it uses wait_timeout(5s) instead of wait().
    • Launch: Atomic — fire-and-forget Command::spawn(), returns immediately. Not interruptible.
    • On shutdown detection, returns DispatchResult::Cancelled.
  4. Terminal events on shutdown: Before exiting, the executor drains all queued ActionRequests and emits mapping_cancelled for each (D13). For the currently executing action (if interrupted mid-sequence), a mapping_fired with result: "cancelled" is emitted with partial execution noted.
  5. Join timeout: The main thread joins the executor thread with a 5-second timeout. If the thread does not exit (e.g., a truly hung system call that escaped process-group kill), the process exits anyway — the OS reclaims resources.

Maximum shutdown latency: 5 seconds (join timeout). Typical shutdown latency: <100ms (one sleep interval + current atomic action completion).

D12: Latency Measurement​

With async execution, latency has two distinct measurements:

  1. execution_time_us (new): Measured by the executor thread — Instant::now() before execute_interruptible(), elapsed after it returns. This is the pure action execution time, excluding channel transit and event-loop scheduling. Carried in ActionCompletion.execution_time_us.

  2. latency_us (existing): End-to-end from event receipt (processing_start captured in process_device_event()) to completion observation (completed_at from ActionCompletion). This includes queue wait time and channel transit, reflecting the true user-perceived delay.

Both are reported in MappingFiredPayload:

  • execution_time_us: how long the action took to execute (useful for action performance tuning)
  • latency_us: total time from MIDI event to completion (useful for responsiveness monitoring)

Under normal load, the difference is ~microseconds. Under queue pressure, the gap reveals queue depth.

D13: Terminal Event Lifecycle — Every Match Has Exactly One Outcome​

Every mapping_matched event must be followed by exactly one terminal event carrying the same invocation_id. The terminal event types are:

Terminal EventMeaning
mapping_firedAction executed (successfully or with error)
mapping_droppedAction never started — queue was full at dispatch time (D3)
mapping_cancelledAction was queued or in-flight but abandoned due to shutdown (D11)

Invariant: For every invocation_id emitted in a mapping_matched, exactly one of {fired, dropped, cancelled} will eventually be emitted with the same ID. This is a hard guarantee — consumers (GUI, audit log, metrics) can rely on it for bookkeeping, timeout detection, and UI state cleanup.

Lifecycle state machine per invocation:

matched ──dispatch──► queued ──execute──► fired
   │                    │
   │ (queue full)       │ (shutdown)
   ▼                    ▼
 dropped            cancelled

Shutdown behavior: When the executor receives a shutdown signal:

  1. The currently executing action (if any) completes its current atomic step, then aborts the remainder of the sequence. A mapping_fired is emitted with result: "cancelled" and the partial execution noted.
  2. All queued ActionRequests are drained. For each, a mapping_cancelled event is emitted with the request's invocation_id.
  3. Only after all terminal events are emitted does the executor thread signal completion and exit.

Why mapping_cancelled is distinct from mapping_dropped: dropped means the action was rejected at dispatch time (never entered the queue). cancelled means the action was accepted into the queue but never executed (or partially executed) due to shutdown. This distinction matters for diagnostics — dropped indicates overload, cancelled indicates shutdown timing.

D14: Mode-Change Completion Lag Bounds​

When a ModeChangeRequested result arrives via the completion channel, the mode is updated atomically via ArcSwap. The lag between the executor completing the action and the event loop applying the mode change depends on:

  1. Completion channel transit: ~microseconds (unbounded, never contended)
  2. Event loop biased select: Completions have highest priority in the select! loop (D4). When a completion is ready, it is always chosen before device events, IPC commands, or timer ticks. No starvation of completions is possible.
  3. Worst-case lag: If the event loop is mid-way through processing a device event when the completion arrives, the completion waits for that single processing cycle to finish — at most ~1ms for complex events (rule matching + monitor emission + try_send), typically <100μs.

Bounded lag: Mode-change application latency is bounded by the duration of whatever event-loop operation is currently in progress when the completion arrives. With action execution moved off the event loop, no operation blocks for more than ~1ms. Therefore, mode-change lag is <1ms under any realistic load.

Alternatives Considered​

A1: tokio::spawn (async task) Instead of Dedicated Thread​

Use tokio::spawn and convert thread::sleep to tokio::time::sleep.

Rejected because:

  • Requires making ActionExecutor Send + Sync, which conflicts with Enigo's thread-affinity
  • Would need to refactor all synchronous I/O (MIDI sends, shell commands) to async equivalents
  • Enigo::new() panics if called from the wrong thread on some platforms
  • Much larger refactoring scope for marginal benefit

A2: Per-Action spawn_blocking​

Spawn a new blocking task for each individual action, rather than maintaining a persistent executor task.

Rejected because:

  • ActionExecutor holds mutable state (Enigo instance, MIDI connections) that would need to be shared across tasks via Arc<Mutex<_>>
  • Creates Enigo thread-affinity issues — each spawn_blocking may run on a different thread
  • No guarantee of execution order for rapid-fire mappings
  • Higher overhead from repeated spawning

A3: Thread Pool with Work Stealing​

Maintain a pool of executor threads for parallel action execution.

Rejected because:

  • Keyboard/mouse simulation (Enigo) is inherently single-threaded — concurrent keystrokes would interleave
  • MIDI output ports are single-writer — concurrent sends would corrupt messages
  • Adds complexity without benefit since actions must serialize anyway
  • Over-engineered for the actual problem

A4: Event Loop Yielding​

Insert tokio::task::yield_now() between sequence steps instead of thread::sleep.

Rejected because:

  • Doesn't solve the fundamental issue — Enigo calls and MIDI I/O are still blocking
  • yield_now() only yields to other tasks on the same runtime, not to the select! loop
  • Would require converting execute() to async fn, which cascades through the entire API
  • Doesn't help with shell commands or other blocking operations

A5: Execution-Context Flag for MIDI Recursion Guard​

Set a per-device flag during action execution to suppress all input from that device.

Rejected because:

  • Suppresses legitimate user input for the entire duration of long action sequences
  • A 25-note tune (1.25s) would make the controller unresponsive — worse than the original problem
  • Device identity may not map cleanly from output port to echoed input source
  • Message fingerprinting (D8) is more precise with bounded overhead

Implementation Plan​

Phase 1: Executor Thread Infrastructure​

  1. Create ActionRequest and ActionCompletion structs with invocation_id
  2. Create signal primitives:
    • Shutdown: Arc<AtomicBool> (shared between event loop and executor)
    • Config: tokio::sync::watch::channel::<ExecutorConfig>() (latest-wins)
    • Actions: crossbeam_channel::bounded::<ActionRequest>(32)
    • Completions: tokio::sync::mpsc::unbounded_channel::<ActionCompletion>()
  3. Spawn dedicated std::thread in EngineManager::new() owning the ActionExecutor
  4. Implement executor loop: recv_timeout(50ms) on action channel, check AtomicBool and watch between iterations (D2)
  5. Add action_completion_rx branch to the main select! loop
  6. Remove action_executor: Mutex<ActionExecutor> from EngineManager
  7. Add monotonic invocation_counter: u64 to EngineManager

Phase 2: Decouple Execution​

  1. In process_device_event(), use try_send to dispatch ActionRequest — emit mapping_dropped with invocation_id on queue full
  2. Emit mapping_matched with invocation_id immediately at match time
  3. In handle_action_completion(), run handle_dispatch_result() + emit mapping_fired with same invocation_id
  4. Wire config reload to send new config via watch::Sender::send()
  5. Wire shutdown to set shutdown.store(true, Ordering::Relaxed) + join executor thread

Phase 3: Cancellation-Aware Execution​

  1. Implement execute_interruptible(action, context, &shutdown) — checks AtomicBool between sequence steps and during delays (50ms sleep intervals)
  2. Add shell command timeout and process-group kill on shutdown
  3. Add mapping_stalled event for actions exceeding 5-second threshold
  4. On shutdown: drain queued ActionRequests, emit mapping_cancelled for each (D13)

Phase 4: MIDI Recursion Guard​

  1. Implement fingerprint ring buffer (bounded, 64 entries, full raw MIDI bytes + port)
  2. In ActionExecutor::execute_send_midi() and MidiForward, record outgoing fingerprints
  3. In process_device_event(), check incoming events against fingerprint buffer before rule matching
  4. Tag matched echoes with source: "echo" provenance in monitor events
  5. Add thru_pairs config for cross-port echo suppression

Phase 5: Testing & Verification​

  1. Unit test: verify event loop continues processing events during long sequences
  2. Integration test: 25-action sequence doesn't block event emission
  3. Test: ModeChange applied correctly via completion handler
  4. Test: MIDI echo fingerprinting suppresses re-ingestion but not user input
  5. Test: config hot-reload via watch channel — verified under action load
  6. Test: graceful shutdown mid-sequence — verify <100ms typical latency, D13 lifecycle invariant (all matched IDs get terminal events)
  7. Test: invocation_id correlation between matched/fired/dropped/cancelled events
  8. Test: try_send overflow emits mapping_dropped with correct invocation_id
  9. Test: AtomicBool shutdown is never lost under action queue pressure
  10. Test: shell command timeout and kill on shutdown
  11. Performance benchmark: measure overhead of channel-based dispatch vs inline

Consequences​

Positive​

  • Event stream remains responsive during long action sequences
  • ADR-014 feedback is immediate — mapping_matched appears instantly, mapping_fired follows on completion
  • No more blocking the async runtime with thread::sleep — eliminates a class of potential issues
  • Clean separation of concerns — event processing and action execution are independent
  • Reliable lifecycle management — shutdown and config updates are never dropped under load
  • Complete event lifecycle — every mapping_matched is guaranteed exactly one terminal event (D13)
  • MIDI echo protection without suppressing legitimate user input
  • Graceful shutdown with bounded latency (~100ms typical, 5s maximum)
  • Bounded mode-change lag — <2ms under realistic load (D14)

Negative​

  • More complex architecture — adds a thread, three channels, a completion handler, and a fingerprint buffer
  • ModeChange semantic change — events arriving during async action execution are processed under the current mode rather than queued (see D7). This is more responsive but differs from the old blocking behavior. Narrow nondeterminism at completion boundaries due to select! scheduling.
  • Head-of-line blocking — one long action delays all subsequent actions (accepted tradeoff, see D9)
  • Debugging is harder — action execution is on a different thread; stack traces span thread boundaries
  • Action drops on overload — if the queue fills, actions are dropped with a mapping_dropped event. This is recoverable but user-visible.
  • Event ordering changes — matched and fired events may interleave with other events. Consumers must use invocation_id for correlation, not positional ordering.

Neutral​

  • Action ordering is preserved — the dedicated executor thread processes actions FIFO
  • Latency measurement includes channel transit and queue wait — adds ~microseconds under normal load
  • Existing tests need updates — tests that assert synchronous execution or consecutive matched/fired ordering
  • Invocation IDs add a small per-event overhead but enable reliable event correlation

Council Review History​

Review 1 (2026-03-07, reasoning tier, verdict: FAIL, confidence: 0.49)​

Transcript: .council/logs/2026-03-07T23-42-34-f2adea2e

Critical issues identified (all 4 reviewers ranked unanimously — GPT-5.4-pro top):

  1. D1/D3 inconsistency: D1 described per-action spawn_blocking, D3 described a persistent executor, A2 rejected D1's model. Fixed: Consolidated into D1 as a single std::thread actor.
  2. Bidirectional channel deadlock: Bounded channels both ways + send().await. Fixed: try_send for actions, unbounded for completions.
  3. ModeChange semantics broken: Claimed "preserved" but events matched under old mode. Fixed: Acknowledged as deliberate semantic change in D7.
  4. MIDI recursion guard too coarse: Per-device flag suppresses user input. Fixed: Message fingerprinting in D8.
  5. Config hot-reload unresolved. Fixed: D10 defines config-at-match-time policy.
  6. Head-of-line blocking undocumented. Fixed: D9 explicitly documents tradeoff.
  7. spawn_blocking misuse. Fixed: D1 uses std::thread.
  8. Event correlation missing. Fixed: D5 adds invocation_id.

Review 2 (2026-03-07, reasoning tier, verdict: FAIL, confidence: 0.52)​

Transcript: .council/logs/2026-03-07T23-56-45-a2db55de

Critical issues identified (all 4 reviewers ranked unanimously — GPT-5.4-pro top):

  1. Control-plane/data-plane mixing: Config updates and shutdown shared the bounded lossy action channel — could be dropped under load. Fixed: D2 separates into reliable control channel + best-effort action channel with priority-select pattern.
  2. Shutdown not actually graceful: Blocking actions prevent shutdown and config updates. Fixed: D11 defines interruptible sleep loop (50ms intervals), shell command timeouts with process-group kill, and bounded shutdown latency (5s max).
  3. ModeChange nondeterminism at completion boundaries: select! scheduling determines which mode events match near completion. Fixed: D7 explicitly documents this narrow race window and explains why it's acceptable.
  4. MIDI fingerprint too coarse: Message-type + channel + note was insufficient. Fixed: D8 now uses full raw MIDI bytes + port name, with configurable thru_pairs for cross-port echo.
  5. "Preserves ordering" claim inaccurate: Event ordering changes materially with async execution. Fixed: D6 explicitly notes this as a breaking change in monitor event stream semantics.
  6. "Unbounded is safe" overstated. Fixed: D4 qualifies the assumption and notes when it should be revisited.
  7. Global queue unfairness: One noisy device can starve others. Acknowledged: This is a consequence of the serialized executor (D9). Per-device queues or priority classes are deferred as future work if needed.

Review 3 (2026-03-07, reasoning tier, verdict: UNCLEAR, confidence: 0.62)​

Transcript: .council/logs/2026-03-08T00-12-18-c3f1a9b7

Critical issues identified:

  1. Config generation consistency: D10 claimed actions could span config generations. Fixed: D10 rewritten to clarify ActionRequest.action is a fully resolved immutable plan — config changes cannot affect queued/in-flight actions.
  2. Control-plane blocks during execution: D2 used multiplexed channel with polling. Fixed: D2 rewritten to use separate primitives — AtomicBool for shutdown (instantaneous), watch for config (latest-wins), crossbeam_channel for actions.
  3. Terminal event lifecycle incomplete: No guarantee that every mapping_matched gets a corresponding terminal event. Fixed: Added D13 defining complete lifecycle invariant — every matched invocation gets exactly one of {fired, dropped, cancelled}.
  4. Mode-change lag understated: Claimed "instantaneous" but depends on event loop scheduling. Fixed: Added D14 with explicit lag bounds (<2ms under realistic load) and analysis of select! fairness.
  5. Head-of-line blocking just moved, not solved: Single executor thread still serializes all actions. Acknowledged: Already documented in D9 as accepted tradeoff. Added note about future per-type lanes.
  6. Shutdown hangs on monolithic actions: thread::sleep not interruptible. Fixed: D11 rewritten — execute_interruptible() with 50ms sleep intervals checking AtomicBool, shell command process-group kill, 5s join timeout.
  7. MIDI fingerprinting brittle for multi-byte messages: SysEx or complex messages. Acknowledged: D8 uses full raw bytes, which handles SysEx correctly. The TTL and ring buffer size are configurable. Added note about metrics for tuning.

Review 4 (2026-03-08, reasoning tier, verdict: FAIL, confidence: 0.52)​

Transcript: .council/logs/2026-03-08T00-34-50-27625537

Critical issues identified (all 4 reviewers ranked Response A [GPT-5.4-pro] top unanimously):

  1. Completion not prioritized in select!: Mode-change completions could be delayed indefinitely by device events. Fixed: D4 rewritten to use biased select with completions as highest priority. D7/D14 updated to reflect actual guarantees.
  2. Latency measurement includes channel/scheduling delay: latency_us inflated by queue wait and event-loop scheduling. Fixed: D5 ActionCompletion now carries completed_at: Instant and execution_time_us: u64 (executor-measured). D12 rewritten with two distinct measurements.
  3. Shutdown hangs on non-interruptible OS calls: Shell commands block wait(), blind to AtomicBool. Fixed: D11 rewritten with per-action-type cancellation semantics. Shell commands use wait_timeout(5s) + process-group SIGTERM/SIGKILL. Each action type's interruptibility explicitly documented.
  4. Dangling invocations on executor death: TrySendError::Disconnected didn't emit terminal event. Fixed: D3 now emits mapping_dropped on Disconnected. D4 handles recv() → None via handle_executor_death() which emits mapping_cancelled for all pending invocations.
  5. Config constraint contradiction: Constraint #6 said "mid-sequence" but executor only checks between actions. Fixed: Constraint #6 rewritten to clarify actions are immutable plans; config updates affect executor state (connections, Enigo) between actions only.
  6. "Instantaneous" wording misleading: Shutdown signal is zero-latency but observation is bounded by 50ms timeout. Fixed: D2 wording corrected.
  7. Enigo thread-affine construction: Must be created inside executor thread, not moved. Fixed: D1 now explicitly requires construction inside the thread closure.

References​

  • Issue #516: Decouple action execution from event loop (this ADR's tracking issue)
  • ADR-014: Mapping Feedback & Simulate (the feature that exposed this issue)
  • Issue #510: capture_actions not updated on config hot-reload
  • conductor-daemon/src/daemon/engine_manager.rs:637 — Main event loop (tokio::select!)
  • conductor-daemon/src/daemon/engine_manager.rs:3389-3393 — Inline action execution
  • conductor-daemon/src/action_executor.rs:270 — ActionExecutor::execute() (synchronous)
  • conductor-daemon/src/action_executor.rs:290-298 — Sequence with thread::sleep(50ms)
  • conductor-daemon/src/action_executor.rs:301-303 — Delay with thread::sleep