From 5a877fa151e6f26c665fd10c94d2f1371bd35121 Mon Sep 17 00:00:00 2001 From: strider Date: Mon, 13 Jul 2026 21:33:24 -0400 Subject: [PATCH] fix(hook): report real injection count in remote deployments (#42) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `hook status` reported `Injection count: 0` even when injection was firing, because of two independent client-side telemetry bugs: Bug A — the remote inject path (taken whenever BOBBIN_SERVER is set) emitted metrics and stored the injection server-side but never advanced hook_state.json. Only the local path incremented the counter, so the client-side count froze at its last local value. Fix: after a successful remote injection, load / increment / save hook_state, mirroring the local path. Bug B — `hook status` used `args.path` verbatim as the bobbin root, so from a sub-directory it read a non-existent hook_state.json and defaulted to 0, while injection recorded to the true root. Fix: resolve the root by walking up to the first ancestor with `.bobbin/config.toml`, the same resolver the inject path uses. Stray `.bobbin/` dirs (config-less) are walked past. Adds a regression test for the resolver's stray-dir behavior. Fixes #42 Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01VsuPDxSUjMYqHvQyPvSAPW --- src/cli/hook.rs | 43 ++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 42 insertions(+), 1 deletion(-) diff --git a/src/cli/hook.rs b/src/cli/hook.rs index 48b8408..f2b01b1 100644 --- a/src/cli/hook.rs +++ b/src/cli/hook.rs @@ -765,10 +765,17 @@ async fn run_uninstall(args: UninstallArgs, output: OutputConfig) -> Result<()> } async fn run_status(args: StatusArgs, output: OutputConfig) -> Result<()> { - let repo_root = args + let start = args .path .canonicalize() .with_context(|| format!("Invalid path: {}", args.path.display()))?; + // Resolve the bobbin root by walking up to the first ancestor holding + // `.bobbin/config.toml`, matching the resolver the inject path uses + // (bobbin #42, Bug B). Using `args.path` verbatim made the reported + // injection count depend on CWD: from a sub-dir the status read a + // non-existent hook_state.json and defaulted to 0, while injection recorded + // to the true root. Fall back to `start` when not inside a bobbin tree. + let repo_root = find_bobbin_root(&start).unwrap_or(start); let config = Config::load(&Config::config_path(&repo_root)).unwrap_or_default(); @@ -1510,6 +1517,16 @@ async fn inject_context_remote( Some(&out), ).await; + // Mirror the local inject path: advance the client-side counter so + // `hook status` reflects remote injections too (bobbin #42, Bug A). + // Without this, hook_state.json is only ever written by the local + // path, which is never taken when BOBBIN_SERVER is set — freezing + // the reported injection count at its last local value. + let mut state = load_hook_state(&repo_root); + state.injection_count += 1; + state.last_injection_time = chrono::Utc::now().to_rfc3339(); + save_hook_state(&repo_root, &state); + Ok(()) } Err(_) => { @@ -7141,6 +7158,30 @@ mod tests { assert_eq!(loaded.injection_count, 3); } + #[test] + fn test_find_bobbin_root_skips_stray_bobbin_dir() { + // Bug B (bobbin #42): the resolver must require .bobbin/config.toml so + // `hook status` and the inject path agree on the root. A stray .bobbin/ + // dir (e.g. one holding only tags.toml) between the CWD and the real + // root must be walked past, not treated as the root — otherwise status + // reads a hook_state.json that doesn't exist there and reports 0. + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path().canonicalize().unwrap(); + + // Real root: has .bobbin/config.toml + std::fs::create_dir_all(root.join(".bobbin")).unwrap(); + std::fs::write(root.join(".bobbin").join("config.toml"), "").unwrap(); + + // Stray sub-dir: has a .bobbin/ but no config.toml + let stray = root.join("sub"); + std::fs::create_dir_all(stray.join(".bobbin")).unwrap(); + std::fs::write(stray.join(".bobbin").join("tags.toml"), "").unwrap(); + + // Resolving from the stray dir must land on the real root. + assert_eq!(find_bobbin_root(&stray), Some(root.clone())); + assert_eq!(find_bobbin_root(&root), Some(root)); + } + #[test] fn test_compute_session_id_deterministic() { let bundle = ContextBundle {