From a39e511cc4587b226118fe45a40b75ed8f84c688 Mon Sep 17 00:00:00 2001
From: "J. Champagne" <jchampagne@maplegoosestudio.ca>
Date: Sat, 14 Mar 2026 01:42:50 -0400
Subject: [PATCH] doc: Add lesson learned about dyn async traits.

---
 docs/lessons/dyn-safe-hook-traits.md | 64 ++++++++++++++++++++++++++++
 1 file changed, 64 insertions(+)
 create mode 100644 docs/lessons/dyn-safe-hook-traits.md

diff --git a/docs/lessons/dyn-safe-hook-traits.md b/docs/lessons/dyn-safe-hook-traits.md
new file mode 100644
index 0000000..e30a01b
--- /dev/null
+++ b/docs/lessons/dyn-safe-hook-traits.md
@@ -0,0 +1,64 @@
+# Lesson: async fn in traits vs dyn-compatibility
+
+## The problem
+
+The `HookHandler` trait needs to be stored as
+`Arc<dyn HookHandler>` so the menu engine can hold a
+type-erased handler without knowing whether it's an exec
+hook, a handler hook, or a composite. But `async fn` in
+traits (even with edition 2024's native support) makes the
+trait not dyn-compatible. The compiler can't build a vtable
+for an async method because the return type (an opaque
+`impl Future`) varies per implementation.
+
+## What we tried
+
+Official documentation of edition 2024 supports async fn
+in traits natively, async-trait is NOT needed." That's true
+for static dispatch (generics), but wrong for `dyn` dispatch.
+
+## The fix
+
+Made `handle()` a regular sync `fn`. The trait is
+deliberately synchronous:
+
+```rust
+pub trait HookHandler: Send + Sync {
+    fn handle(&self, event: HookEvent)
+        -> Result<Vec<HookResponse>, PiklError>;
+}
+```
+
+This works because handlers don't actually need to `await`
+anything at the call site:
+
+- **Exec hooks** spawn a subprocess via `tokio::spawn` and
+  return immediately. The process runs in the background.
+- **Handler hooks** write to an internal channel via
+  `try_send` (non-blocking) and return. A background task
+  handles the actual I/O.
+
+Both return `Ok(vec![])` instantly. Responses from handler
+hooks flow back through the action channel asynchronously.
+
+## Takeaway
+
+`async fn in trait` (edition 2024) is for generic/static
+dispatch. If you need `dyn Trait`, you still need either:
+1. A sync method (if the work can be spawned internally)
+2. A boxed future return type
+3. The `async-trait` crate
+
+Option 1 is the simplest when the handler doesn't need to
+return data from async work. If responses are needed, they
+go through a separate channel rather than the return value.
+
+## Related: tokio test-util for debounce tests
+
+Tests that verify debounce timing need `tokio::time::pause`
+and `tokio::time::advance`, which require the `test-util`
+feature on tokio in dev-dependencies. Use
+`#[tokio::test(start_paused = true)]` and
+`tokio::time::sleep()` (which auto-advances in paused mode)
+rather than manual `pause()` + `advance()` + `yield_now()`
+chains, which are fragile with spawned tasks.