cleanup(plugin): L-R1, L-R2, M-R1, M-R2 — review-2 polish

Four small fixes from the second review pass:

L-R1 (dead binding): JanetLoopTool::execute captured `name_owned`
from `self.name.clone()` and then `let _ = name_owned;` at the end.
Leftover from H2 work — probably for a debug log that got removed.
Dropped.

L-R2 (cfg salad): the CustomMessage arm in ui/mod.rs had four
`#[cfg]` gates around a single body (let-with-plugin, let-without-
plugin, if-with-plugin, drop-without-plugin). Replace with two
separate arms — one gated under cfg(plugin) doing the resolve, one
gated under cfg(not(plugin)) as a no-op (variant exists
unconditionally in event.rs, so the match must cover it, but no
producer exists without the feature so the arm is unreachable).

M-R1 (dead filter): JanetLoopTool::execute filtered the progress
drain results by tcid. Single-threaded Janet plus the emit-tool-
progress slot guard means every drained entry already belongs to
THIS handler invocation — the filter was dead-code defensive that
masked the invariant. Removed; the comment now documents the
single-threaded reasoning.

M-R2 (double-lock): resolve_custom_message_render took the PM
mutex twice (once for list_message_renderers, once for the invoke).
Folded into a single acquisition. Holding across the invoke is
safe — the Janet worker is single-threaded so nothing else can run
on it while we wait — and removes the lock-release-lock dance.

Tests: no new coverage; the existing 16 extension tests +
end-to-end smoke continue to pass. 1078 / 873 unchanged.

Closes dirge-5km.

Author: Yogthos

.beads/issues.jsonl

@@ -59,8 +59,8 @@
 {"_type":"issue","id":"dirge-ny0","title":"Phase 3: right-side info panel (cwd, MCP, LSP, todos, modified files)","description":"Carve right ~32 cols (auto-hide when terminal narrower than ~100). Sources: cwd from env, MCP from McpClientManager.handles, LSP via new public accessor on LspManager, todos from TODO_LIST mutex, modified files via new shared Arc\u003cMutex\u003cIndexSet\u003cPathBuf\u003e\u003e\u003e populated by Write/Edit/ApplyPatch tools. New /panel on|off toggle. Default on when wide enough.","status":"closed","priority":2,"issue_type":"feature","owner":"yogthos@gmail.com","created_at":"2026-05-20T03:40:08Z","created_by":"Yogthos","updated_at":"2026-05-20T04:21:53Z","started_at":"2026-05-20T04:11:20Z","closed_at":"2026-05-20T04:21:53Z","close_reason":"Closed","dependency_count":0,"dependent_count":1,"comment_count":0}
 {"_type":"issue","id":"dirge-r2u","title":"Phase 2A: queue user input while agent is running","description":"Remove 'agent is busy' guard at src/ui/mod.rs:663 and :720 for plain text. Push to VecDeque\u003cString\u003e interjection_queue. Show queue count + dim preview above input. Esc/Ctrl-X drops most recent. Ctrl-C still aborts. Slash commands stay gated to current allow-list. On AgentEvent::Done, drain queue and run as next turn.","status":"closed","priority":2,"issue_type":"feature","owner":"yogthos@gmail.com","created_at":"2026-05-20T03:40:07Z","created_by":"Yogthos","updated_at":"2026-05-20T04:04:50Z","started_at":"2026-05-20T03:58:39Z","closed_at":"2026-05-20T04:04:50Z","close_reason":"Closed","dependency_count":0,"dependent_count":1,"comment_count":0}
 {"_type":"issue","id":"dirge-sxt","title":"Phase 1: soft-wrap input box instead of horizontal scroll","description":"Replace horizontal scroll logic in src/ui/renderer.rs::draw_bottom (lines ~500-660) with display-column wrap. Drop input_scroll_offset; compute visual rows from logical lines wrapped to visible_width; keep MAX_INPUT_VISIBLE_LINES cap with vertical scroll keeping cursor visible. Move or guard the token counter so it doesn't collide with wrapped text. Add a unit test for cursor (logical -\u003e visual) mapping.","status":"closed","priority":2,"issue_type":"feature","owner":"yogthos@gmail.com","created_at":"2026-05-20T03:40:06Z","created_by":"Yogthos","updated_at":"2026-05-20T03:58:35Z","started_at":"2026-05-20T03:52:14Z","closed_at":"2026-05-20T03:58:35Z","close_reason":"Closed","dependency_count":0,"dependent_count":1,"comment_count":0}
-{"_type":"issue","id":"dirge-64e","title":"doc + buffer_pos_at fix: M-R3, H-R2, L-R3, L-R4","description":"Four doc-leaning items + one small code fix from review-2:\\n- M-R3: PLUGINS.md note that dirge's display=false is suppression-only (no plugin-observable transcript yet)\\n- H-R2: PLUGINS.md note that prepare-arguments blocks the executor synchronously; keep handlers light\\n- L-R3: buffer_pos_at uses entry.text.chars().count() which includes escape bytes. Switch to strip_ansi-aware count so column clamping reflects visible chars.\\n- L-R4: one-line code comment in JanetLoopTool::execute explaining post-handler batch replay semantics for emit-tool-progress","status":"open","priority":3,"issue_type":"task","owner":"yogthos@gmail.com","created_at":"2026-05-23T02:15:32Z","created_by":"Yogthos","updated_at":"2026-05-23T02:15:32Z","dependency_count":0,"dependent_count":0,"comment_count":0}
-{"_type":"issue","id":"dirge-5km","title":"cleanup batch: L-R1 dead binding, L-R2 cfg salad, M-R1 dead filter, M-R2 double-lock","description":"Four cosmetic fixes from review-2:\\n- L-R1: drop unused name_owned capture in JanetLoopTool::execute\\n- L-R2: simplify the four-#[cfg] block around CustomMessage arm in ui/mod.rs — gate the whole arm under cfg(feature=plugin) since AgentEvent::CustomMessage can't be produced without it\\n- M-R1: remove dead filter in drain_tool_progress call (filter for matching tcid only — invariant says all entries match, dead-code defensive)\\n- M-R2: fold resolve_custom_message_render to one PM acquisition instead of two","status":"in_progress","priority":3,"issue_type":"task","assignee":"Yogthos","owner":"yogthos@gmail.com","created_at":"2026-05-23T02:15:31Z","created_by":"Yogthos","updated_at":"2026-05-23T02:17:16Z","started_at":"2026-05-23T02:17:16Z","dependency_count":0,"dependent_count":0,"comment_count":0}
+{"_type":"issue","id":"dirge-64e","title":"doc + buffer_pos_at fix: M-R3, H-R2, L-R3, L-R4","description":"Four doc-leaning items + one small code fix from review-2:\\n- M-R3: PLUGINS.md note that dirge's display=false is suppression-only (no plugin-observable transcript yet)\\n- H-R2: PLUGINS.md note that prepare-arguments blocks the executor synchronously; keep handlers light\\n- L-R3: buffer_pos_at uses entry.text.chars().count() which includes escape bytes. Switch to strip_ansi-aware count so column clamping reflects visible chars.\\n- L-R4: one-line code comment in JanetLoopTool::execute explaining post-handler batch replay semantics for emit-tool-progress","status":"in_progress","priority":3,"issue_type":"task","assignee":"Yogthos","owner":"yogthos@gmail.com","created_at":"2026-05-23T02:15:32Z","created_by":"Yogthos","updated_at":"2026-05-23T02:19:30Z","started_at":"2026-05-23T02:19:30Z","dependency_count":0,"dependent_count":0,"comment_count":0}
+{"_type":"issue","id":"dirge-5km","title":"cleanup batch: L-R1 dead binding, L-R2 cfg salad, M-R1 dead filter, M-R2 double-lock","description":"Four cosmetic fixes from review-2:\\n- L-R1: drop unused name_owned capture in JanetLoopTool::execute\\n- L-R2: simplify the four-#[cfg] block around CustomMessage arm in ui/mod.rs — gate the whole arm under cfg(feature=plugin) since AgentEvent::CustomMessage can't be produced without it\\n- M-R1: remove dead filter in drain_tool_progress call (filter for matching tcid only — invariant says all entries match, dead-code defensive)\\n- M-R2: fold resolve_custom_message_render to one PM acquisition instead of two","status":"closed","priority":3,"issue_type":"task","assignee":"Yogthos","owner":"yogthos@gmail.com","created_at":"2026-05-23T02:15:31Z","created_by":"Yogthos","updated_at":"2026-05-23T02:19:28Z","started_at":"2026-05-23T02:17:16Z","closed_at":"2026-05-23T02:19:28Z","close_reason":"Closed","dependency_count":0,"dependent_count":0,"comment_count":0}
 {"_type":"issue","id":"dirge-3s1","title":"L*: phase-9 polish (key spec strictness, modifier matching, registration validation, unregister symmetry)","description":"Five low-severity items from phase-9 review:\\n\\nL1: parse_key_spec accepts 'f01' as F(1); s[1..].parse::\u003cu8\u003e() reads leading zeros. Add strict-digits check.\\n\\nL2: match_shortcut does exact modifier equality. ctrl-x binding doesn't match Ctrl+Shift+X. Documented behavior in PLUGINS.md so plugin authors know to bind shift- explicitly when wanted.\\n\\nL3: no unregister-* counterparts. Pi has unregisterProvider. Defer unless a plugin author asks; pi's symmetry is also partial.\\n\\nL4: smoke test depends on plugins/example_*.janet disk state. Convention matches existing test_plugin.janet; document as load-bearing.\\n\\nL5: harness/register-tool doesn't validate name charset. LLM tool-call mechanism may break on names with spaces or special chars. Add regex match [a-zA-Z0-9_-]+ in the Janet helper.","status":"closed","priority":3,"issue_type":"task","assignee":"Yogthos","owner":"yogthos@gmail.com","created_at":"2026-05-23T01:10:15Z","created_by":"Yogthos","updated_at":"2026-05-23T01:38:24Z","started_at":"2026-05-23T01:36:09Z","closed_at":"2026-05-23T01:38:24Z","close_reason":"Closed","dependencies":[{"issue_id":"dirge-3s1","depends_on_id":"dirge-0iy","type":"blocks","created_at":"2026-05-22T21:10:23Z","created_by":"Yogthos","metadata":"{}"}],"dependency_count":1,"dependent_count":0,"comment_count":0}
 {"_type":"issue","id":"dirge-hjz","title":"H2/M1/M2: handler arg expansion (tool_call_id, display flag, dynamic shortcut refresh)","description":"Three gap-fillers, batched because they share the harness-API expansion theme.\\n\\nH2: JanetLoopTool::execute discards tool_call_id and on_update. Pi forwards both (toolCallId for LLM correlation, onUpdate for streaming progress). Add tool-call-id as second handler arg; expose (harness/emit-tool-progress text) helper bridged via thread-local to on_update.\\n\\nM1: Pi CustomMessage has display: boolean (messages.ts:50) — when false the message is in transcript but not rendered. Dirge always renders. Add display to the wrapper (lands with C1's customType change).\\n\\nM2: plugin_shortcuts snapshotted once at UI startup. Pi rebuilds the shortcut map on demand (interactive-mode.ts:1625, 5324). Re-snapshot when a new registration happens (or on each keystroke — cost is one Janet eval).","status":"closed","priority":3,"issue_type":"feature","assignee":"Yogthos","owner":"yogthos@gmail.com","created_at":"2026-05-23T01:10:14Z","created_by":"Yogthos","updated_at":"2026-05-23T01:36:08Z","started_at":"2026-05-23T01:30:49Z","closed_at":"2026-05-23T01:36:08Z","close_reason":"Closed","dependencies":[{"issue_id":"dirge-hjz","depends_on_id":"dirge-0iy","type":"blocks","created_at":"2026-05-22T21:10:22Z","created_by":"Yogthos","metadata":"{}"}],"dependency_count":1,"dependent_count":0,"comment_count":0}
 {"_type":"issue","id":"dirge-btb","title":"9e: docs + integration test + plugin example for phase 9","description":"Update docs/plugins.md (or create if missing) covering the new ExtensionApi surface. Add an end-to-end integration test that loads a Janet plugin which registers all 5 things (tool, command, provider, shortcut, message-renderer) and verifies each path. Add an example plugin under examples/ or test fixtures.","status":"closed","priority":3,"issue_type":"task","assignee":"Yogthos","owner":"yogthos@gmail.com","created_at":"2026-05-23T00:19:25Z","created_by":"Yogthos","updated_at":"2026-05-23T00:48:59Z","started_at":"2026-05-23T00:43:45Z","closed_at":"2026-05-23T00:48:59Z","close_reason":"9b deferred separately; 9a/9c/9d complete with docs + examples + smoke test","dependencies":[{"issue_id":"dirge-btb","depends_on_id":"dirge-gfs","type":"blocks","created_at":"2026-05-22T20:19:32Z","created_by":"Yogthos","metadata":"{}"},{"issue_id":"dirge-btb","depends_on_id":"dirge-l95","type":"blocks","created_at":"2026-05-22T20:19:33Z","created_by":"Yogthos","metadata":"{}"},{"issue_id":"dirge-btb","depends_on_id":"dirge-sw8","type":"blocks","created_at":"2026-05-22T20:19:32Z","created_by":"Yogthos","metadata":"{}"}],"dependency_count":3,"dependent_count":0,"comment_count":0}

src/plugin/extension.rs

@@ -83,18 +83,21 @@ pub fn resolve_custom_message_render(
         .to_string();
 
     let rendered: Option<String> = pm.and_then(|pm_arc| {
-        let handler = {
-            let mut mgr = pm_arc.lock().unwrap_or_else(|e| e.into_inner());
-            mgr.list_message_renderers()
-                .into_iter()
-                .find(|(t, _)| t == &custom_type)
-                .map(|(_, h)| h)
-        };
-        handler.and_then(|h| {
-            let payload_str = payload.to_string();
-            let mut mgr = pm_arc.lock().unwrap_or_else(|e| e.into_inner());
-            mgr.invoke_message_renderer(&h, &payload_str).ok().flatten()
-        })
+        // M-R2: single PM acquisition for both the registry lookup
+        // and the handler invoke. Holding across the invoke is safe
+        // — the Janet worker is single-threaded, so nothing else can
+        // run on it while we wait — and avoids the lock-release-lock
+        // dance the prior split incurred.
+        let mut mgr = pm_arc.lock().unwrap_or_else(|e| e.into_inner());
+        let handler = mgr
+            .list_message_renderers()
+            .into_iter()
+            .find(|(t, _)| t == &custom_type)
+            .map(|(_, h)| h)?;
+        let payload_str = payload.to_string();
+        mgr.invoke_message_renderer(&handler, &payload_str)
+            .ok()
+            .flatten()
     });
 
     let body = rendered.unwrap_or_else(|| {
@@ -384,7 +387,6 @@ impl LoopTool for JanetLoopTool {
         let pm = self.pm.clone();
         let handler = self.handler.clone();
         let tool_call_id_owned = tool_call_id.to_string();
-        let name_owned = self.name.clone();
         Box::pin(async move {
             // Cancellation pre-flight. The dispatcher (tools.rs)
             // races this whole future against `wait_for_cancel` so
@@ -422,14 +424,15 @@ impl LoopTool for JanetLoopTool {
                     }
                     let r = guard.invoke_plugin_tool(&handler, &args_json, &tcid_for_blocking)?;
                     // H2: drain any harness/emit-tool-progress entries
-                    // the handler pushed during execution. We do this
-                    // under the same lock so a subsequent plugin tool
-                    // can't race in between and steal entries.
-                    let prog = guard
-                        .drain_tool_progress()
-                        .into_iter()
-                        .filter(|(id, _)| id == &tcid_for_blocking)
-                        .collect::<Vec<_>>();
+                    // the handler pushed during execution. Single-
+                    // threaded Janet guarantees every queued entry
+                    // belongs to THIS handler invocation (the slot
+                    // guard in emit-tool-progress ignores calls made
+                    // outside an active tool), so no filtering is
+                    // needed. Drained under the same lock that ran
+                    // the handler so a subsequent plugin tool can't
+                    // observe a stale buffer.
+                    let prog = guard.drain_tool_progress();
                     Ok((r, prog))
                 },
             )
@@ -441,15 +444,14 @@ impl LoopTool for JanetLoopTool {
             // single-threaded; we couldn't interleave during execute).
             // Plugin authors using emit-tool-progress get the events
             // batched but in-order — same observable surface as a
-            // synchronous progress-emitting handler.
+            // synchronous progress-emitting handler (L-R4).
             for (_id, text) in progress_events {
                 on_update(&LoopToolResult {
                     content: vec![serde_json::json!({"type": "text", "text": text})],
                     details: Value::Null,
                     terminate: None,
                 });
             }
-            let _ = name_owned;
             Ok(LoopToolResult {
                 content: vec![serde_json::json!({"type": "text", "text": result})],
                 details: Value::Null,

src/ui/mod.rs

@@ -2759,6 +2759,7 @@ pub async fn run_interactive(
                             is_running = true;
                         }
                     }
+                    #[cfg(feature = "plugin")]
                     AgentEvent::CustomMessage { payload } => {
                         // Plugin-emitted custom message (P9d).
                         // Resolution lives in `plugin::extension`
@@ -2767,26 +2768,28 @@ pub async fn run_interactive(
                         // here just sanitizes + writes the line.
                         // `None` means `display=false` — the message
                         // stays in the transcript but no chat row.
-                        #[cfg(feature = "plugin")]
-                        let resolved = crate::plugin::extension::resolve_custom_message_render(
+                        // Arm gated under cfg(plugin) because the
+                        // variant can't be constructed without it
+                        // (bridge.rs emits it only for plugin-fed
+                        // LoopMessage::Custom).
+                        if let Some(r) = crate::plugin::extension::resolve_custom_message_render(
                             &payload,
                             plugin_manager,
-                        );
-                        #[cfg(not(feature = "plugin"))]
-                        let resolved: Option<()> = {
-                            let _ = &payload;
-                            None
-                        };
-                        #[cfg(feature = "plugin")]
-                        if let Some(r) = resolved {
+                        ) {
                             let safe = sanitize_output(&r.body);
                             renderer.write_line(
                                 &format!("[{}] {}", r.label, safe),
                                 theme::dim(),
                             )?;
                         }
-                        #[cfg(not(feature = "plugin"))]
-                        let _ = resolved;
+                    }
+                    #[cfg(not(feature = "plugin"))]
+                    AgentEvent::CustomMessage { payload } => {
+                        // No producer exists without the plugin
+                        // feature, so this arm is unreachable in
+                        // practice — but the variant is unconditional
+                        // in event.rs, so the match must handle it.
+                        let _ = payload;
                     }
                     AgentEvent::Interjected { partial_response, tokens } => {
                         was_reasoning = false;