eeymoo/claw-code
1
0
Fork 0
mirror of https://github.com/instructkr/claw-code.git synced 2026-04-13 19:44:53 +08:00
Code Issues Packages Projects Releases Wiki Activity

Keep poisoned test locks from cascading across unrelated regressions

Browse Source 
The repo-local backlog was effectively exhausted, so this sweep promoted the
newly observed test-lock poisoning pain point into ROADMAP #74 and fixed it in
place. Test-only env/cwd lock acquisition now recovers poisoned mutexes in the
remaining strict call sites, and each affected surface has a regression that
proves a panic no longer permanently poisons later tests.

Constraint: Keep the fix test-only and avoid widening runtime behavior changes
Rejected: Refactor shared helper signatures across broader call paths | unnecessary churn beyond the remaining strict test sites
Confidence: high
Scope-risk: narrow
Reversibility: clean
Directive: These guards only recover the mutex; tests that mutate env or cwd still must restore process-global state explicitly
Tested: cargo fmt --all --check
Tested: cargo clippy --workspace --all-targets -- -D warnings
Tested: cargo test --workspace
Tested: Architect review (APPROVE)
Not-tested: Additional fault-injection around partially restored env/cwd state after panic
Related: ROADMAP #74
This commit is contained in:
Yeachan-Heo
2026-04-12 13:52:41 +00:00
parent 6b4bb4ac26
commit f91d156f85
5 changed files with 112 additions and 38 deletions
Download Patch File Download Diff File Expand all files Collapse all files

20
rust/crates/commands/src/lib.rs
View File

@@ -4152,6 +4152,24 @@ mod tests {
LOCK.get_or_init(|| Mutex::new(()))
}
fn env_guard() -> std::sync::MutexGuard<'static, ()> {
env_lock()
.lock()
.unwrap_or_else(std::sync::PoisonError::into_inner)
}
#[test]
fn env_guard_recovers_after_poisoning() {
let poisoned = std::thread::spawn(|| {
let _guard = env_guard();
panic!("poison env lock");
})
.join();
assert!(poisoned.is_err(), "poisoning thread should panic");
let _guard = env_guard();
}
fn restore_env_var(key: &str, original: Option<OsString>) {
match original {
Some(value) => std::env::set_var(key, value),
@@ -5214,7 +5232,7 @@ mod tests {
#[test]
fn discovers_omc_skills_from_project_and_user_compatibility_roots() {
let _guard = env_lock().lock().expect("env lock");
let _guard = env_guard();
let workspace = temp_dir("skills-omc-workspace");
let user_home = temp_dir("skills-omc-home");
let claude_config_dir = temp_dir("skills-omc-claude-config");

70
rust/crates/plugins/src/lib.rs
View File

@@ -2294,6 +2294,12 @@ fn env_lock() -> &'static std::sync::Mutex<()> {
mod tests {
use super::*;
fn env_guard() -> std::sync::MutexGuard<'static, ()> {
env_lock()
.lock()
.unwrap_or_else(std::sync::PoisonError::into_inner)
}
fn temp_dir(label: &str) -> PathBuf {
let nanos = std::time::SystemTime::now()
.duration_since(std::time::UNIX_EPOCH)
@@ -2302,6 +2308,18 @@ mod tests {
std::env::temp_dir().join(format!("plugins-{label}-{nanos}"))
}
#[test]
fn env_guard_recovers_after_poisoning() {
let poisoned = std::thread::spawn(|| {
let _guard = env_guard();
panic!("poison env lock");
})
.join();
assert!(poisoned.is_err(), "poisoning thread should panic");
let _guard = env_guard();
}
fn write_file(path: &Path, contents: &str) {
if let Some(parent) = path.parent() {
fs::create_dir_all(parent).expect("parent dir");
@@ -2485,7 +2503,7 @@ mod tests {
#[test]
fn load_plugin_from_directory_validates_required_fields() {
let _guard = env_lock().lock().expect("env lock");
let _guard = env_guard();
let root = temp_dir("manifest-required");
write_file(
root.join(MANIFEST_FILE_NAME).as_path(),
@@ -2500,7 +2518,7 @@ mod tests {
#[test]
fn load_plugin_from_directory_reads_root_manifest_and_validates_entries() {
let _guard = env_lock().lock().expect("env lock");
let _guard = env_guard();
let root = temp_dir("manifest-root");
write_loader_plugin(&root);
@@ -2530,7 +2548,7 @@ mod tests {
#[test]
fn load_plugin_from_directory_supports_packaged_manifest_path() {
let _guard = env_lock().lock().expect("env lock");
let _guard = env_guard();
let root = temp_dir("manifest-packaged");
write_external_plugin(&root, "packaged-demo", "1.0.0");
@@ -2544,7 +2562,7 @@ mod tests {
#[test]
fn load_plugin_from_directory_defaults_optional_fields() {
let _guard = env_lock().lock().expect("env lock");
let _guard = env_guard();
let root = temp_dir("manifest-defaults");
write_file(
root.join(MANIFEST_FILE_NAME).as_path(),
@@ -2566,7 +2584,7 @@ mod tests {
#[test]
fn load_plugin_from_directory_rejects_duplicate_permissions_and_commands() {
let _guard = env_lock().lock().expect("env lock");
let _guard = env_guard();
let root = temp_dir("manifest-duplicates");
write_file(
root.join("commands").join("sync.sh").as_path(),
@@ -2862,7 +2880,7 @@ mod tests {
#[test]
fn discovers_builtin_and_bundled_plugins() {
let _guard = env_lock().lock().expect("env lock");
let _guard = env_guard();
let manager = PluginManager::new(PluginManagerConfig::new(temp_dir("discover")));
let plugins = manager.list_plugins().expect("plugins should list");
assert!(plugins
@@ -2875,7 +2893,7 @@ mod tests {
#[test]
fn installs_enables_updates_and_uninstalls_external_plugins() {
let _guard = env_lock().lock().expect("env lock");
let _guard = env_guard();
let config_home = temp_dir("home");
let source_root = temp_dir("source");
write_external_plugin(&source_root, "demo", "1.0.0");
@@ -2924,7 +2942,7 @@ mod tests {
#[test]
fn auto_installs_bundled_plugins_into_the_registry() {
let _guard = env_lock().lock().expect("env lock");
let _guard = env_guard();
let config_home = temp_dir("bundled-home");
let bundled_root = temp_dir("bundled-root");
write_bundled_plugin(&bundled_root.join("starter"), "starter", "0.1.0", false);
@@ -2956,7 +2974,7 @@ mod tests {
#[test]
fn default_bundled_root_loads_repo_bundles_as_installed_plugins() {
let _guard = env_lock().lock().expect("env lock");
let _guard = env_guard();
let config_home = temp_dir("default-bundled-home");
let manager = PluginManager::new(PluginManagerConfig::new(&config_home));
@@ -2975,7 +2993,7 @@ mod tests {
#[test]
fn bundled_sync_prunes_removed_bundled_registry_entries() {
let _guard = env_lock().lock().expect("env lock");
let _guard = env_guard();
let config_home = temp_dir("bundled-prune-home");
let bundled_root = temp_dir("bundled-prune-root");
let stale_install_path = config_home
@@ -3039,7 +3057,7 @@ mod tests {
#[test]
fn installed_plugin_discovery_keeps_registry_entries_outside_install_root() {
let _guard = env_lock().lock().expect("env lock");
let _guard = env_guard();
let config_home = temp_dir("registry-fallback-home");
let bundled_root = temp_dir("registry-fallback-bundled");
let install_root = config_home.join("plugins").join("installed");
@@ -3094,7 +3112,7 @@ mod tests {
#[test]
fn installed_plugin_discovery_prunes_stale_registry_entries() {
let _guard = env_lock().lock().expect("env lock");
let _guard = env_guard();
let config_home = temp_dir("registry-prune-home");
let bundled_root = temp_dir("registry-prune-bundled");
let install_root = config_home.join("plugins").join("installed");
@@ -3140,7 +3158,7 @@ mod tests {
#[test]
fn persists_bundled_plugin_enable_state_across_reloads() {
let _guard = env_lock().lock().expect("env lock");
let _guard = env_guard();
let config_home = temp_dir("bundled-state-home");
let bundled_root = temp_dir("bundled-state-root");
write_bundled_plugin(&bundled_root.join("starter"), "starter", "0.1.0", false);
@@ -3174,7 +3192,7 @@ mod tests {
#[test]
fn persists_bundled_plugin_disable_state_across_reloads() {
let _guard = env_lock().lock().expect("env lock");
let _guard = env_guard();
let config_home = temp_dir("bundled-disabled-home");
let bundled_root = temp_dir("bundled-disabled-root");
write_bundled_plugin(&bundled_root.join("starter"), "starter", "0.1.0", true);
@@ -3208,7 +3226,7 @@ mod tests {
#[test]
fn validates_plugin_source_before_install() {
let _guard = env_lock().lock().expect("env lock");
let _guard = env_guard();
let config_home = temp_dir("validate-home");
let source_root = temp_dir("validate-source");
write_external_plugin(&source_root, "validator", "1.0.0");
@@ -3223,7 +3241,7 @@ mod tests {
#[test]
fn plugin_registry_tracks_enabled_state_and_lookup() {
let _guard = env_lock().lock().expect("env lock");
let _guard = env_guard();
let config_home = temp_dir("registry-home");
let source_root = temp_dir("registry-source");
write_external_plugin(&source_root, "registry-demo", "1.0.0");
@@ -3251,7 +3269,7 @@ mod tests {
#[test]
fn plugin_registry_report_collects_load_failures_without_dropping_valid_plugins() {
let _guard = env_lock().lock().expect("env lock");
let _guard = env_guard();
// given
let config_home = temp_dir("report-home");
let external_root = temp_dir("report-external");
@@ -3296,7 +3314,7 @@ mod tests {
#[test]
fn installed_plugin_registry_report_collects_load_failures_from_install_root() {
let _guard = env_lock().lock().expect("env lock");
let _guard = env_guard();
// given
let config_home = temp_dir("installed-report-home");
let bundled_root = temp_dir("installed-report-bundled");
@@ -3327,7 +3345,7 @@ mod tests {
#[test]
fn rejects_plugin_sources_with_missing_hook_paths() {
let _guard = env_lock().lock().expect("env lock");
let _guard = env_guard();
// given
let config_home = temp_dir("broken-home");
let source_root = temp_dir("broken-source");
@@ -3355,7 +3373,7 @@ mod tests {
#[test]
fn rejects_plugin_sources_with_missing_failure_hook_paths() {
let _guard = env_lock().lock().expect("env lock");
let _guard = env_guard();
// given
let config_home = temp_dir("broken-failure-home");
let source_root = temp_dir("broken-failure-source");
@@ -3383,7 +3401,7 @@ mod tests {
#[test]
fn plugin_registry_runs_initialize_and_shutdown_for_enabled_plugins() {
let _guard = env_lock().lock().expect("env lock");
let _guard = env_guard();
let config_home = temp_dir("lifecycle-home");
let source_root = temp_dir("lifecycle-source");
let _ = write_lifecycle_plugin(&source_root, "lifecycle-demo", "1.0.0");
@@ -3407,7 +3425,7 @@ mod tests {
#[test]
fn aggregates_and_executes_plugin_tools() {
let _guard = env_lock().lock().expect("env lock");
let _guard = env_guard();
let config_home = temp_dir("tool-home");
let source_root = temp_dir("tool-source");
write_tool_plugin(&source_root, "tool-demo", "1.0.0");
@@ -3436,7 +3454,7 @@ mod tests {
#[test]
fn list_installed_plugins_scans_install_root_without_registry_entries() {
let _guard = env_lock().lock().expect("env lock");
let _guard = env_guard();
let config_home = temp_dir("installed-scan-home");
let bundled_root = temp_dir("installed-scan-bundled");
let install_root = config_home.join("plugins").join("installed");
@@ -3468,7 +3486,7 @@ mod tests {
#[test]
fn list_installed_plugins_scans_packaged_manifests_in_install_root() {
let _guard = env_lock().lock().expect("env lock");
let _guard = env_guard();
let config_home = temp_dir("installed-packaged-scan-home");
let bundled_root = temp_dir("installed-packaged-scan-bundled");
let install_root = config_home.join("plugins").join("installed");
@@ -3502,7 +3520,7 @@ mod tests {
/// host `~/.claw/plugins/` from bleeding into test runs.
#[test]
fn claw_config_home_isolation_prevents_host_plugin_leakage() {
let _guard = env_lock().lock().expect("env lock");
let _guard = env_guard();
// Create a temp directory to act as our isolated CLAW_CONFIG_HOME
let config_home = temp_dir("isolated-home");
@@ -3556,7 +3574,7 @@ mod tests {
use std::sync::Arc;
use std::thread;
let _guard = env_lock().lock().expect("env lock");
let _guard = env_guard();
// Shared base directory for all threads
let base_dir = temp_dir("parallel-base");

24
rust/crates/rusty-claude-cli/src/main.rs
View File

@@ -10534,7 +10534,7 @@ UU conflicted.rs",
#[test]
fn managed_sessions_default_to_jsonl_and_resolve_legacy_json() {
let _guard = cwd_lock().lock().expect("cwd lock");
let _guard = cwd_guard();
let workspace = temp_workspace("session-resolution");
std::fs::create_dir_all(&workspace).expect("workspace should create");
let previous = std::env::current_dir().expect("cwd");
@@ -10573,7 +10573,7 @@ UU conflicted.rs",
#[test]
fn latest_session_alias_resolves_most_recent_managed_session() {
let _guard = cwd_lock().lock().expect("cwd lock");
let _guard = cwd_guard();
let workspace = temp_workspace("latest-session-alias");
std::fs::create_dir_all(&workspace).expect("workspace should create");
let previous = std::env::current_dir().expect("cwd");
@@ -10606,7 +10606,7 @@ UU conflicted.rs",
#[test]
fn load_session_reference_rejects_workspace_mismatch() {
let _guard = cwd_lock().lock().expect("cwd lock");
let _guard = cwd_guard();
let workspace_a = temp_workspace("session-mismatch-a");
let workspace_b = temp_workspace("session-mismatch-b");
std::fs::create_dir_all(&workspace_a).expect("workspace a should create");
@@ -10680,6 +10680,24 @@ UU conflicted.rs",
LOCK.get_or_init(|| Mutex::new(()))
}
fn cwd_guard() -> MutexGuard<'static, ()> {
cwd_lock()
.lock()
.unwrap_or_else(std::sync::PoisonError::into_inner)
}
#[test]
fn cwd_guard_recovers_after_poisoning() {
let poisoned = std::thread::spawn(|| {
let _guard = cwd_guard();
panic!("poison cwd lock");
})
.join();
assert!(poisoned.is_err(), "poisoning thread should panic");
let _guard = cwd_guard();
}
fn temp_workspace(label: &str) -> PathBuf {
let nanos = std::time::SystemTime::now()
.duration_since(std::time::UNIX_EPOCH)

32
rust/crates/tools/src/lib.rs
View File

@@ -6047,6 +6047,24 @@ mod tests {
LOCK.get_or_init(|| Mutex::new(()))
}
fn env_guard() -> std::sync::MutexGuard<'static, ()> {
env_lock()
.lock()
.unwrap_or_else(std::sync::PoisonError::into_inner)
}
#[test]
fn env_guard_recovers_after_poisoning() {
let poisoned = std::thread::spawn(|| {
let _guard = env_guard();
panic!("poison env lock");
})
.join();
assert!(poisoned.is_err(), "poisoning thread should panic");
let _guard = env_guard();
}
fn temp_path(name: &str) -> PathBuf {
let unique = std::time::SystemTime::now()
.duration_since(std::time::UNIX_EPOCH)
@@ -7167,7 +7185,7 @@ mod tests {
#[test]
fn skill_loads_local_skill_prompt() {
let _guard = env_lock().lock().expect("env lock should acquire");
let _guard = env_guard();
let home = temp_path("skills-home");
let skill_dir = home.join(".agents").join("skills").join("help");
fs::create_dir_all(&skill_dir).expect("skill dir should exist");
@@ -7224,7 +7242,7 @@ mod tests {
#[test]
fn skill_resolves_project_local_skills_and_legacy_commands() {
let _guard = env_lock().lock().expect("env lock should acquire");
let _guard = env_guard();
let root = temp_path("project-skills");
let skill_dir = root.join(".claw").join("skills").join("plan");
let command_dir = root.join(".claw").join("commands");
@@ -7268,7 +7286,7 @@ mod tests {
#[test]
fn skill_loads_project_local_claude_skill_prompt() {
let _guard = env_lock().lock().expect("env lock should acquire");
let _guard = env_guard();
let root = temp_path("project-skills");
let home = root.join("home");
let workspace = root.join("workspace");
@@ -7319,7 +7337,7 @@ mod tests {
#[test]
fn skill_loads_project_local_omc_and_agents_skill_prompts() {
let _guard = env_lock().lock().expect("env lock should acquire");
let _guard = env_guard();
let root = temp_path("project-omc-skills");
let home = root.join("home");
let workspace = root.join("workspace");
@@ -7389,7 +7407,7 @@ mod tests {
#[test]
fn skill_loads_learned_skill_from_claude_config_dir() {
let _guard = env_lock().lock().expect("env lock should acquire");
let _guard = env_guard();
let root = temp_path("claude-config-learned-skill");
let home = root.join("home");
let claude_config_dir = root.join("claude-config");
@@ -7444,7 +7462,7 @@ mod tests {
#[test]
fn skill_loads_direct_skill_and_legacy_command_from_claude_config_dir() {
let _guard = env_lock().lock().expect("env lock should acquire");
let _guard = env_guard();
let root = temp_path("claude-config-direct-skill");
let home = root.join("home");
let claude_config_dir = root.join("claude-config");
@@ -7516,7 +7534,7 @@ mod tests {
#[test]
fn skill_loads_project_local_legacy_command_markdown() {
let _guard = env_lock().lock().expect("env lock should acquire");
let _guard = env_guard();
let root = temp_path("project-legacy-command");
let home = root.join("home");
let workspace = root.join("workspace");