feat: enhance server generation with project name support#15
Merged
Bechma merged 1 commit intocyberfabric:mainfrom Mar 23, 2026
Merged
feat: enhance server generation with project name support#15Bechma merged 1 commit intocyberfabric:mainfrom
Bechma merged 1 commit intocyberfabric:mainfrom
Conversation
Contributor
📝 WalkthroughWalkthroughThe PR adds per-project generated directories and a CLI Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/cli/src/common.rs`:
- Around line 284-286: generated_project_dir currently produces collisions by
using only the config file stem; update it so default project names are
collision-resistant by incorporating more of the config path (e.g., include the
config parent directory or a short hash of the absolute config path) into the
returned PathBuf, and add a check where the generated directory already exists
to detect whether the existing dir was created from a different config and
return/fail with an explicit error instead of silently overwriting; apply the
same change/guard in the related code paths mentioned around the 324-345 range
that derive project names from file_stem().
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: dca44ede-f194-49f1-8a8e-fc01b4623822
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
crates/cli/src/build/mod.rscrates/cli/src/common.rscrates/cli/src/run/mod.rscrates/cli/src/run/run_loop.rs
Comment on lines
+284
to
+286
| pub fn generated_project_dir(path: &Path, project_name: &str) -> PathBuf { | ||
| PathBuf::from(path).join(BASE_PATH).join(project_name) | ||
| } |
Contributor
There was a problem hiding this comment.
Default project names can silently alias distinct configs.
Line 333 derives the default from file_stem() only. configs/admin.yml and examples/admin.yaml both resolve to admin, so Line 285 sends both commands to the same .cyberfabric/admin directory and the later build/run rewrites the earlier config's generated Cargo.toml and src/main.rs. That breaks the multi-server workflow this PR is trying to enable. Please make the default collision-resistant, or fail when the target directory is already associated with a different config.
Also applies to: 324-345
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/cli/src/common.rs` around lines 284 - 286, generated_project_dir
currently produces collisions by using only the config file stem; update it so
default project names are collision-resistant by incorporating more of the
config path (e.g., include the config parent directory or a short hash of the
absolute config path) into the returned PathBuf, and add a check where the
generated directory already exists to detect whether the existing dir was
created from a different config and return/fail with an explicit error instead
of silently overwriting; apply the same change/guard in the related code paths
mentioned around the 324-345 range that derive project names from file_stem().
Collaborator
Author
There was a problem hiding this comment.
that's fine. This is why we have the flag to override the default behaviour
Signed-off-by: Bechma <19294519+Bechma@users.noreply.github.com>
Bechma
force-pushed
the
add-multiple-servers
branch
from
5f61e2a to
7cac347
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
crates/cli/src/common.rs (1)
324-345: Consider adding a test for invalid name rejection.The validation logic correctly handles both explicit overrides and derived names, with helpful error context when the config stem fails validation. However, the test suite only covers happy paths.
🧪 Suggested additional test case
#[test] fn generated_project_name_rejects_invalid_override() { let result = resolve_generated_project_name(Path::new("/tmp/quickstart.yml"), Some("invalid name!")); assert!(result.is_err(), "should reject names with invalid characters"); } #[test] fn generated_project_name_rejects_invalid_config_stem() { let result = resolve_generated_project_name(Path::new("/tmp/invalid name!.yml"), None); assert!(result.is_err(), "should reject config stems with invalid characters"); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/cli/src/common.rs` around lines 324 - 345, Add unit tests to exercise resolve_generated_project_name's error paths: one where override_name is provided but invalid, and one where the config file stem derived from config_path is invalid. Use Path::new(...) to construct test paths and call resolve_generated_project_name; assert the Result is Err for both cases. Reference the function resolve_generated_project_name and the validate_name behavior so the tests fail when invalid names (e.g., containing spaces or punctuation) are allowed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@crates/cli/src/common.rs`:
- Around line 324-345: Add unit tests to exercise
resolve_generated_project_name's error paths: one where override_name is
provided but invalid, and one where the config file stem derived from
config_path is invalid. Use Path::new(...) to construct test paths and call
resolve_generated_project_name; assert the Result is Err for both cases.
Reference the function resolve_generated_project_name and the validate_name
behavior so the tests fail when invalid names (e.g., containing spaces or
punctuation) are allowed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3ded522b-c2cf-416a-a3a9-62708cea6869
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
AGENTS.mdCargo.tomlSKILLS.mdcrates/cli/src/build/mod.rscrates/cli/src/common.rscrates/cli/src/run/mod.rscrates/cli/src/run/run_loop.rs
✅ Files skipped from review due to trivial changes (2)
- AGENTS.md
- Cargo.toml
🚧 Files skipped from review as they are similar to previous changes (2)
- crates/cli/src/run/mod.rs
- crates/cli/src/build/mod.rs
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
1 participant
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.
Summary by CodeRabbit
New Features
Bug Fixes
--cleannow removes artifacts for the selected generated project onlyDocumentation