From 3c468f9ea2ca052da82a00690d48465d932f924f Mon Sep 17 00:00:00 2001 From: protagonista-design <64110002+usefulish@users.noreply.github.com> Date: Tue, 14 Jul 2026 15:05:04 -0400 Subject: [PATCH] fix: identify hooks by parse time index instead of memory address FolderHookId was the hook's memory address, which only works while the hook map is built once and never moved or mutated, and gives clones of the same hook different identities. Assign each hook a unique index during deserialization and use that instead. Co-Authored-By: Claude Fable 5 --- src/config.rs | 23 +++++++++++++++++++++++ src/hook.rs | 28 ++++++++++++++++++++++++---- 2 files changed, 47 insertions(+), 4 deletions(-) diff --git a/src/config.rs b/src/config.rs index ef32f3c..4d89487 100644 --- a/src/config.rs +++ b/src/config.rs @@ -72,11 +72,31 @@ impl Default for Config { /// Folder hooks configurations #[derive(Debug, serde::Deserialize)] +#[serde(from = "RawFolderConfig")] pub(crate) struct FolderConfig { /// Hooks array pub hooks: Vec, } +/// Folder hooks configurations as deserialized, before hook indexes are assigned +#[derive(serde::Deserialize)] +struct RawFolderConfig { + /// Hooks array + hooks: Vec, +} + +impl From for FolderConfig { + fn from(raw: RawFolderConfig) -> Self { + let mut hooks = raw.hooks; + // Assign hook indexes during deserialization so that every parsing path gets + // unique ones, they identify each hook at runtime + for (index, hook) in hooks.iter_mut().enumerate() { + hook.index = index; + } + Self { hooks } + } +} + /// Path string with ~ replaced, and canonicalized #[derive(Clone, Debug, Hash, PartialEq, Eq)] pub(crate) struct NormalizedPath(PathBuf); @@ -126,6 +146,9 @@ pub(crate) struct FolderHook { pub command: Vec, /// Allow concurrent runs for the same hook pub allow_concurrent: Option, + /// Unique hook index, assigned when parsing, used to identify the hook at runtime + #[serde(skip)] + pub index: usize, } /// Deserialize filter into a glob matcher to validate glob expression diff --git a/src/hook.rs b/src/hook.rs index adc32fa..e25b295 100644 --- a/src/hook.rs +++ b/src/hook.rs @@ -4,7 +4,6 @@ use std::{ collections::HashSet, path::{Path, PathBuf}, process::{Child, Command, Stdio}, - ptr, sync::{Arc, Mutex, mpsc}, time::Duration, }; @@ -12,14 +11,14 @@ use std::{ use crate::config; /// Unique identifier for a folder hook +#[cfg_attr(test, derive(Debug))] #[derive(Clone, Eq, Hash, PartialEq)] pub(crate) struct FolderHookId(usize); impl FolderHookId { - /// Create unique identifier for hook + /// Create unique identifier for hook, from its parse time index pub(crate) fn from_hook(hook: &config::FolderHook) -> Self { - let val = ptr::from_ref(hook) as usize; - Self(val) + Self(hook.index) } } @@ -111,6 +110,7 @@ mod tests { filter: None, command: command.iter().map(|a| (*a).to_owned()).collect(), allow_concurrent, + index: 0, } } @@ -221,4 +221,24 @@ mod tests { thread::sleep(Duration::from_millis(10)); } } + + /// Hook identity must be identical for clones of the same hook (the hook map + /// stores clones), and unique across different hooks + #[test] + fn hook_id_is_stable_and_unique() { + let hook0 = hook(&["true"], None); + let hook1 = config::FolderHook { + index: 1, + ..hook0.clone() + }; + + assert_eq!( + FolderHookId::from_hook(&hook0), + FolderHookId::from_hook(&hook0.clone()) + ); + assert_ne!( + FolderHookId::from_hook(&hook0), + FolderHookId::from_hook(&hook1) + ); + } }