refactor(http): resolve registry repos from AppState.project_file_path, not process cwd#217
Merged
Merged
Conversation
…h, not process cwd
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes the Low-severity follow-up noted in #216.
Problem
The
/registry/*handlers resolved the project's[tool.fastskill].repositoriesconfig from process cwd (get_repository_managercalledstd::env::current_dir()→resolve_project_fileon every request), ignoring its_servicearg — unlikemanifest.rs/skills.rs, which useAppState.project_file_path. That inconsistency is why theregistry_skill_versions_*tests had to serialize on a globalDIR_MUTEXandset_current_dir.Change
get_repository_manager(project_file_path: &Path)now loads the manifest directly from the path, resolved once at serve startup intoAppState.project_file_path(viaload_project_config, which already walks up the tree and canonicalizes). All 6 call sites pass&state.project_file_path.AppState.project_file_pathis the same walked-up file the old cwd resolution would have found, so no change in which config is read — but request handling no longer depends on (or reads) process cwd, and stays correct even if the process cwd changes after startup.DIR_MUTEX/DirGuard/set_current_dirscaffolding in the threeregistry_skill_versions_*tests; they now write the repo config to the fixture's absoluteproject_file_path. The version tests are now parallel-safe.DIR_MUTEX/DirGuardremain used elsewhere, so their defs are untouched.Testing
cargo fmt+clippy -D warnings+ fullcargo test --workspacegreen; the three registry-version tests pass without cwd serialization.Reviewed by hand (single-file cohesive refactor — implemented by one sonnet agent, reviewed directly rather than by a review agent).
🤖 Generated with Claude Code