From 4e6d1899074a75006ab9a60117d2ca54618803f8 Mon Sep 17 00:00:00 2001 From: Alex Oliveira <4482374+aroff@users.noreply.github.com> Date: Thu, 23 Jul 2026 13:46:48 +0000 Subject: [PATCH] refactor(http): resolve registry repos from AppState.project_file_path, not process cwd --- .../src/http/handlers/registry.rs | 22 +++++------ .../tests/http_handler_route_tests.rs | 39 +++++-------------- 2 files changed, 19 insertions(+), 42 deletions(-) diff --git a/crates/fastskill-core/src/http/handlers/registry.rs b/crates/fastskill-core/src/http/handlers/registry.rs index d485f2f..1df1f16 100644 --- a/crates/fastskill-core/src/http/handlers/registry.rs +++ b/crates/fastskill-core/src/http/handlers/registry.rs @@ -11,12 +11,10 @@ use axum::{ }; use std::collections::HashSet; -fn get_repository_manager(_service: &crate::core::service::FastSkillService) -> RepositoryManager { - let current_dir = std::env::current_dir().unwrap_or_else(|_| std::path::PathBuf::from(".")); - let project_file = crate::core::project::resolve_project_file(¤t_dir); - if project_file.found { - let project_path = project_file.path; - if let Ok(project) = crate::core::manifest::SkillProjectToml::load_from_file(&project_path) +fn get_repository_manager(project_file_path: &std::path::Path) -> RepositoryManager { + if project_file_path.exists() { + if let Ok(project) = + crate::core::manifest::SkillProjectToml::load_from_file(project_file_path) { if let Some(tool) = project.tool { if let Some(fastskill_config) = tool.fastskill { @@ -155,7 +153,7 @@ pub async fn list_sources( State(state): State, ) -> HttpResult>>> { // Get repository manager (supports all formats) - let repo_manager = get_repository_manager(&state.service); + let repo_manager = get_repository_manager(&state.project_file_path); let repos = repo_manager.list_repositories(); @@ -200,7 +198,7 @@ pub async fn list_sources( pub async fn list_all_skills( State(state): State, ) -> HttpResult>> { - let repo_manager = get_repository_manager(&state.service); + let repo_manager = get_repository_manager(&state.project_file_path); let (sources_manager, _sources_tmp) = get_sources_manager_from_repos(&repo_manager) .await .map_err(|e| { @@ -287,7 +285,7 @@ pub async fn list_source_skills( Path(source_name): Path, State(state): State, ) -> HttpResult>> { - let repo_manager = get_repository_manager(&state.service); + let repo_manager = get_repository_manager(&state.project_file_path); let (sources_manager, _sources_tmp) = get_sources_manager_from_repos(&repo_manager) .await .map_err(|e| { @@ -353,7 +351,7 @@ pub async fn get_marketplace( Path(source_name): Path, State(state): State, ) -> HttpResult>> { - let repo_manager = get_repository_manager(&state.service); + let repo_manager = get_repository_manager(&state.project_file_path); let (sources_manager, _sources_tmp) = get_sources_manager_from_repos(&repo_manager) .await .map_err(|e| { @@ -387,7 +385,7 @@ pub async fn get_marketplace( pub async fn refresh_sources( State(state): State, ) -> HttpResult>> { - let repo_manager = get_repository_manager(&state.service); + let repo_manager = get_repository_manager(&state.project_file_path); let (sources_manager, _sources_tmp) = get_sources_manager_from_repos(&repo_manager) .await .map_err(|e| { @@ -429,7 +427,7 @@ pub async fn list_skill_versions( })) }; - let repo_manager = get_repository_manager(&state.service); + let repo_manager = get_repository_manager(&state.project_file_path); let (sources_manager, _sources_tmp) = match get_sources_manager_from_repos(&repo_manager).await { Ok(pair) => pair, diff --git a/crates/fastskill-core/tests/http_handler_route_tests.rs b/crates/fastskill-core/tests/http_handler_route_tests.rs index d134c7f..5fc2ec0 100644 --- a/crates/fastskill-core/tests/http_handler_route_tests.rs +++ b/crates/fastskill-core/tests/http_handler_route_tests.rs @@ -1175,17 +1175,12 @@ async fn registry_marketplace_unknown_is_404() { // ---- list_skill_versions (spec 003 v2 / Phase 4 version picker) ---- #[tokio::test] -#[allow(clippy::await_holding_lock)] async fn registry_skill_versions_empty_when_no_registry_configured() { - // No skill-project.toml with `[[tool.fastskill.repositories]]` reachable - // from cwd -> no sources -> empty `versions`, still 200 (never 404). - // `get_repository_manager` resolves off the process cwd, which is shared - // process-wide state; hold `DIR_MUTEX` so this can't race the other - // `registry_skill_versions_*` tests that `set_current_dir` into a temp - // project with repositories configured. - let _lock = fastskill_core::test_utils::DIR_MUTEX - .lock() - .unwrap_or_else(|e| e.into_inner()); + // No skill-project.toml with `[[tool.fastskill.repositories]]` at + // `state.project_file_path` -> no sources -> empty `versions`, still 200 + // (never 404). `fixture_with_skills` points `project_file_path` at a file + // that is never written, so `get_repository_manager` falls back to an + // empty manager. let f = fixture_with_skills(false).await; let (status, body) = do_get(f.state, "/registry/skills/widget/versions").await; assert_eq!(status, StatusCode::OK, "body: {body}"); @@ -1194,43 +1189,29 @@ async fn registry_skill_versions_empty_when_no_registry_configured() { } #[tokio::test] -#[allow(clippy::await_holding_lock)] async fn registry_skill_versions_unknown_id_is_empty_not_404() { - let _lock = fastskill_core::test_utils::DIR_MUTEX - .lock() - .unwrap_or_else(|e| e.into_inner()); - let original_dir = std::env::current_dir().ok(); - let _guard = fastskill_core::test_utils::DirGuard(original_dir); + let f = fixture_with_skills(false).await; - let project = TempDir::new().unwrap(); let repo_dir = TempDir::new().unwrap(); write_skill(repo_dir.path(), "known", "Known", "d"); fs::write( - project.path().join("skill-project.toml"), + &f.project_file_path, format!( "[dependencies]\n\n[[tool.fastskill.repositories]]\nname = \"localrepo\"\ntype = \"local\"\npath = \"{}\"\npriority = 0\n", repo_dir.path().display() ), ) .unwrap(); - std::env::set_current_dir(project.path()).unwrap(); - let f = fixture_with_skills(false).await; let (status, body) = do_get(f.state, "/registry/skills/does-not-exist/versions").await; assert_eq!(status, StatusCode::OK, "body: {body}"); assert!(body.contains("\"versions\":[]"), "body: {body}"); } #[tokio::test] -#[allow(clippy::await_holding_lock)] async fn registry_skill_versions_populated_and_sorted_descending() { - let _lock = fastskill_core::test_utils::DIR_MUTEX - .lock() - .unwrap_or_else(|e| e.into_inner()); - let original_dir = std::env::current_dir().ok(); - let _guard = fastskill_core::test_utils::DirGuard(original_dir); + let f = fixture_with_skills(false).await; - let project = TempDir::new().unwrap(); let repo_dir = TempDir::new().unwrap(); // Two versions of the same skill id, in separate subdirectories, discovered // by the local-source scanner (walks for any nested SKILL.md). @@ -1244,16 +1225,14 @@ async fn registry_skill_versions_populated_and_sorted_descending() { .unwrap(); } fs::write( - project.path().join("skill-project.toml"), + &f.project_file_path, format!( "[dependencies]\n\n[[tool.fastskill.repositories]]\nname = \"localrepo\"\ntype = \"local\"\npath = \"{}\"\npriority = 0\n", repo_dir.path().display() ), ) .unwrap(); - std::env::set_current_dir(project.path()).unwrap(); - let f = fixture_with_skills(false).await; let (status, body) = do_get(f.state, "/registry/skills/widget/versions").await; assert_eq!(status, StatusCode::OK, "body: {body}"); assert!(body.contains("\"id\":\"widget\""), "body: {body}");