Skip to content

refactor: simplify path handling in build and run commands #27

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Bechma merged 2 commits into from
Apr 7, 2026
Merged

refactor: simplify path handling in build and run commands #27

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b5439b4
refactor: simplify path handling in build and run commands
Bechma Apr 7, 2026
5ffc540
refactor: streamline module discovery by removing workspace path depe…
Bechma Apr 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
354 changes: 160 additions & 194 deletions Cargo.lock
View file
Open in desktop

Large diffs are not rendered by default.

8 changes: 4 additions & 4 deletions Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,17 +17,17 @@ module-parser = { path = "crates/module-parser" }
anyhow = { version = "1.0" }
clap = { version = "4.5", features = ["derive"] }

cargo-generate = { version = "0.23.7" }
cargo-generate = { version = "0.23.8" }
cargo_metadata = { version = "0.23.1" }

liquid = { version = "~0.26" } # align with cargo-generate

serde = { version = "1.0", features = ["derive"] }
serde-saphyr = { version = "0.0.22" }
serde-saphyr = { version = "0.0.23" }
serde_json = { version = "1.0" }

toml = { version = "1.0.7", features = ["serde"] }
toml_edit = "0.25.5"
toml = { version = "1.1.2", features = ["serde"] }
toml_edit = "0.25.10"

notify = { version = "8.2", features = ["serde"] }
reqwest = { version = "0.13", features = ["json"] }
Expand Down
68 changes: 42 additions & 26 deletions SKILLS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,11 @@ cyberfabric

## Shared Argument Patterns

- **[`-p, --path <PATH>`]** Workspace root path. Defaults to `.` in most commands that work on a workspace.
- **[`-c, --config <PATH>`]** Config file path. This is required anywhere `PathConfigArgs` is used because there is no
default.
- **[`-p, --path <PATH>`]** Optional workspace path. When provided to `config ...`, `build`, and `run` commands, the CLI
immediately changes the current working directory to this directory. Relative config paths and generated project
locations then resolve from that directory. When omitted, the current working directory is left unchanged.
- **[`-c, --config <PATH>`]** Config file path. This is required for `config ...`, `build`, and `run` commands because
there is no default.
- **[`--name <NAME>`]** For `build` and `run`, overrides the generated server project and binary name that would
otherwise default to the config filename stem.
- **[`-v, --verbose`]** Usually enables more logging or richer output.
Expand Down Expand Up @@ -205,7 +207,7 @@ cyberfabric config mod list -c <CONFIG> [-p <PATH>] [--system] [--verbose] [--re
Arguments:

- **[`-c, --config <CONFIG>`]** Required config file path
- **[`-p, --path <PATH>`]** Workspace root, defaults to `.`
- **[`-p, --path <PATH>`]** Optional workspace directory
- **[`-s, --system`]** Also print built-in system registry modules
- **[`-v, --verbose`]** Print full metadata
- **[`--registry <REGISTRY>`]** Registry used only for verbose system lookups, defaults to `crates.io`
Expand All @@ -214,6 +216,8 @@ Behavior:

- **[discovers local modules]** Scans the workspace for module crates
- **[loads configured modules]** Reads enabled modules from the config file
- **[path activation]** If `-p/--path` is provided, the CLI first changes the current working directory there before
resolving `-c/--config`
- **[marks enabled locals]** Shows when a workspace module is enabled in config
- **[shows missing locals]** Shows when a configured module is not present in the workspace
- **[optional crates.io fetch]** If both `--system` and `--verbose` are used, the CLI fetches registry metadata and
Expand Down Expand Up @@ -266,7 +270,7 @@ Arguments:

- **[`<module>`]** Module name in the config
- **[`-c, --config <CONFIG>`]** Required config file path
- **[`-p, --path <PATH>`]** Workspace root, defaults to `.`
- **[`-p, --path <PATH>`]** Optional workspace directory
- **[`--package <NAME>`]** Override metadata package name
- **[`--module-version <VER>`]** Override metadata version
- **[`--default-features <BOOL>`]** Persist Cargo `default_features`
Expand All @@ -276,6 +280,8 @@ Arguments:
Behavior:

- **[upsert]** Creates or updates `modules.<module>`
- **[path activation]** If `-p/--path` is provided, Clap changes the current working directory while parsing that value,
before `-c/--config` is resolved
- **[local-first discovery]** Tries to discover module metadata from the workspace
- **[remote module support]** If the module is not local, you must provide both `--package` and `--module-version`
- **[portable metadata]** Local filesystem paths are intentionally not persisted into config metadata
Expand Down Expand Up @@ -313,6 +319,8 @@ cyberfabric config mod rm -c <CONFIG> [-p <PATH>] <module>

Behavior:

- **[path activation]** If `-p/--path` is provided, Clap changes the current working directory while parsing that value,
before `-c/--config` is resolved
- **[strict removal]** Fails if the module is not present in config

Example:
Expand Down Expand Up @@ -353,6 +361,8 @@ Shared DB flags:

Rules:

- **[path activation]** If `-p/--path` is provided, each subcommand changes the current working directory while Clap is
parsing that value, before `-c/--config` is resolved
- **[payload required]** `add` and `edit` require at least one DB-related field
- **[module must exist]** `add` requires the module already exist in config and recommends `config mod add` first
- **[edit requires existing DB config]** `edit` fails if no module DB config exists yet
Expand Down Expand Up @@ -396,6 +406,8 @@ cyberfabric config db rm -c <CONFIG> [-p <PATH>] <name>

Behavior:

- **[path activation]** If `-p/--path` is provided, each subcommand changes the current working directory while Clap is
parsing that value, before `-c/--config` is resolved
- **[server name]** Stored under `database.servers.<name>`
- **[add is upsert]** `add` creates or patches an existing server entry
- **[edit is strict]** `edit` requires the server to already exist
Expand Down Expand Up @@ -545,13 +557,13 @@ Generate a server project under `.cyberfabric/<name>/` and run it.
Synopsis:

```bash
cyberfabric run -c <CONFIG> [-p <PATH>] [--name <NAME>] [--watch] [--otel] [--release] [--clean]
cargo cyberfabric run -c <CONFIG> [-p <PATH>] [--name <NAME>] [--watch] [--otel] [--release] [--clean]
```

Arguments:

- **[`-c, --config <CONFIG>`]** Required config file path
- **[`-p, --path <PATH>`]** Workspace root, defaults to `.`
- **[`-p, --path <PATH>`]** Optional workspace directory
- **[`--name <NAME>`]** Override the generated server project and binary name; defaults to the config filename stem
- **[`-w, --watch`]** Re-run when watched inputs change
- **[`--otel`]** Pass Cargo feature `otel`
Expand All @@ -562,6 +574,8 @@ Behavior:

- **[name resolution]** Uses the config filename stem by default, so `config/quickstart.yml` generates under
`.cyberfabric/quickstart/`; `--name` overrides that default
- **[path activation]** If `-p/--path` is provided, Clap changes the current working directory while parsing that value,
before `-c/--config` is resolved and `.cyberfabric/<name>/` is generated
- **[generates server structure]** Writes `.cyberfabric/<name>/Cargo.toml`, `.cyberfabric/<name>/.cargo/config.toml`,
and `.cyberfabric/<name>/src/main.rs`
- **[loads config dependencies]** Builds dependencies from the config and local module metadata
Expand All @@ -572,19 +586,19 @@ Behavior:
Examples:

```bash
cyberfabric run -p /tmp/cf-demo -c /tmp/cf-demo/config/quickstart.yml
cargo cyberfabric run -p /tmp/cf-demo -c /tmp/cf-demo/config/quickstart.yml
```

```bash
cyberfabric run -p /tmp/cf-demo -c /tmp/cf-demo/config/quickstart.yml --watch
cargo cyberfabric run -p /tmp/cf-demo -c /tmp/cf-demo/config/quickstart.yml --watch
```

```bash
cyberfabric run -p /tmp/cf-demo -c /tmp/cf-demo/config/quickstart.yml --otel --release --clean
cargo cyberfabric run -p /tmp/cf-demo -c /tmp/cf-demo/config/quickstart.yml --otel --release --clean
```

```bash
cyberfabric run -p /tmp/cf-demo -c /tmp/cf-demo/config/quickstart.yml --name demo-server
cargo cyberfabric run -p /tmp/cf-demo -c /tmp/cf-demo/config/quickstart.yml --name demo-server
```

### `build`
Expand All @@ -600,7 +614,7 @@ cyberfabric build -c <CONFIG> [-p <PATH>] [--name <NAME>] [--otel] [--release] [
Arguments:

- **[`-c, --config <CONFIG>`]** Required config file path
- **[`-p, --path <PATH>`]** Workspace root, defaults to `.`
- **[`-p, --path <PATH>`]** Optional workspace directory
- **[`--name <NAME>`]** Override the generated server project and binary name; defaults to the config filename stem
- **[`--otel`]** Pass Cargo feature `otel`
- **[`-r, --release`]** Use release mode
Expand All @@ -611,6 +625,8 @@ Behavior:
- **[generates before build]** Recreates the generated server project before invoking Cargo
- **[name resolution]** Uses the config filename stem by default, so `config/quickstart.yml` builds from
`.cyberfabric/quickstart/`; `--name` overrides that default
- **[path activation]** If `-p/--path` is provided, Clap changes the current working directory while parsing that value,
before `-c/--config` is resolved and `.cyberfabric/<name>/` is generated
- **[builds inside `.cyberfabric/<name>`]** Executes `cargo build` in the generated directory

Examples:
Expand Down Expand Up @@ -679,7 +695,7 @@ Current status:
cyberfabric mod init /tmp/cf-demo
cyberfabric mod add background-worker -p /tmp/cf-demo
cyberfabric config mod add background-worker -p /tmp/cf-demo -c /tmp/cf-demo/config/quickstart.yml
cyberfabric run -p /tmp/cf-demo -c /tmp/cf-demo/config/quickstart.yml
cargo cyberfabric run -p /tmp/cf-demo -c /tmp/cf-demo/config/quickstart.yml
```

### Add a module and wire a shared DB server
Expand All @@ -689,7 +705,7 @@ cyberfabric mod add api-db-handler -p /tmp/cf-demo
cyberfabric config db add primary -p /tmp/cf-demo -c /tmp/cf-demo/config/quickstart.yml --engine postgres --host localhost --port 5432 --user app --password '${DB_PASSWORD}' --dbname appdb
cyberfabric config mod add api-db-handler -p /tmp/cf-demo -c /tmp/cf-demo/config/quickstart.yml
cyberfabric config mod db add api-db-handler -p /tmp/cf-demo -c /tmp/cf-demo/config/quickstart.yml --server primary
cyberfabric run -p /tmp/cf-demo -c /tmp/cf-demo/config/quickstart.yml --watch
cargo cyberfabric run -p /tmp/cf-demo -c /tmp/cf-demo/config/quickstart.yml --watch
```

### Inspect source for a dependency
Expand All @@ -712,21 +728,21 @@ cyberfabric docs --verbose tokio::sync

```bash
cyberfabric mod init <path>
cyberfabric mod add <background-worker|api-db-handler|rest-gateway> -p <workspace>
cyberfabric mod add <background-worker|api-db-handler|rest-gateway> [-p <workspace>]

cyberfabric config mod list -p <workspace> -c <config>
cyberfabric config mod add <module> -p <workspace> -c <config>
cyberfabric config mod rm <module> -p <workspace> -c <config>
cyberfabric config mod db add <module> -p <workspace> -c <config> ...
cyberfabric config mod db edit <module> -p <workspace> -c <config> ...
cyberfabric config mod db rm <module> -p <workspace> -c <config>
cyberfabric config mod list [-p <workspace>] -c <config>
cyberfabric config mod add <module> [-p <workspace>] -c <config>
cyberfabric config mod rm <module> [-p <workspace>] -c <config>
cyberfabric config mod db add <module> [-p <workspace>] -c <config> ...
cyberfabric config mod db edit <module> [-p <workspace>] -c <config> ...
cyberfabric config mod db rm <module> [-p <workspace>] -c <config>

cyberfabric config db add <name> -p <workspace> -c <config> ...
cyberfabric config db edit <name> -p <workspace> -c <config> ...
cyberfabric config db rm <name> -p <workspace> -c <config>
cyberfabric config db add <name> [-p <workspace>] -c <config> ...
cyberfabric config db edit <name> [-p <workspace>] -c <config> ...
cyberfabric config db rm <name> [-p <workspace>] -c <config>

cyberfabric docs [-p <path>] [--version <version>] [--clean] [<query>]
cyberfabric tools --all
cyberfabric run -p <workspace> -c <config> [--name <name>] [--watch]
cyberfabric build -p <workspace> -c <config> [--name <name>]
cyberfabric run [-p <workspace>] -c <config> [--name <name>] [--watch]
cyberfabric build [-p <workspace>] -c <config> [--name <name>]
```
9 changes: 4 additions & 5 deletions crates/cli/src/build/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,12 @@ pub struct BuildArgs {

impl BuildArgs {
pub fn run(&self) -> anyhow::Result<()> {
let (path, config_path, project_name) =
self.build_run_args.resolve_workspace_and_config()?;
let (config_path, project_name) = self.build_run_args.resolve_config_and_name()?;

let dependencies = common::get_config(&path, &config_path)?.create_dependencies()?;
common::generate_server_structure(&path, &project_name, &config_path, &dependencies)?;
let dependencies = common::get_config(&config_path)?.create_dependencies()?;
common::generate_server_structure(&project_name, &config_path, &dependencies)?;

let cargo_dir = common::generated_project_dir(&path, &project_name);
let cargo_dir = common::generated_project_dir(&project_name)?;
let status = common::cargo_command(
"build",
&cargo_dir,
Expand Down
65 changes: 35 additions & 30 deletions crates/cli/src/common.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use module_parser::{
get_dependencies, get_module_name_from_crate,
};
use std::collections::{BTreeSet, HashMap};
use std::env;
use std::fmt::{self, Display};
use std::fs;
use std::path::{Path, PathBuf};
Expand All @@ -15,25 +16,36 @@ use std::sync::LazyLock;
#[derive(Args)]
pub struct PathConfigArgs {
/// Path to the module workspace root
#[arg(short = 'p', long, default_value = ".")]
pub path: PathBuf,
#[arg(short = 'p', long, value_parser = parse_and_chdir)]
pub path: Option<PathBuf>,
Comment on lines +19 to +20
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Apr 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

PathConfigArgs::path no longer affects config resolution.

Line 40 now canonicalizes only self.config, so any in-process caller that constructs PathConfigArgs { path, config } directly resolves a relative config against the ambient CWD instead of path. Also, if -p was relative, the PathBuf returned from parse_and_chdir() is stale as soon as Line 33 changes the CWD.

💡 One way to keep the struct self-consistent 
 fn parse_and_chdir(s: &str) -> Result<PathBuf, String> {
-    let path = PathBuf::from(s);
+    let path = PathBuf::from(s)
+        .canonicalize()
+        .map_err(|e| format!("failed to access {s}: {e}"))?;

     if !path.is_dir() {
         return Err(format!("not a directory: {}", path.display()));
     }

     env::set_current_dir(&path)
         .map_err(|e| format!("failed to change directory to {}: {e}", path.display()))?;

     Ok(path)
 }

 impl PathConfigArgs {
     pub fn resolve_config(&self) -> anyhow::Result<PathBuf> {
-        self.config
+        let config_path = if self.config.is_absolute() {
+            self.config.clone()
+        } else if let Some(path) = &self.path {
+            path.join(&self.config)
+        } else {
+            workspace_root()?.join(&self.config)
+        };
+
+        config_path
             .canonicalize()
             .context("can't canonicalize config")
     }
 }

Also applies to: 26-37, 40-44

🤖 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 19 - 20, PathConfigArgs::path
currently becomes inconsistent because parse_and_chdir() returns a PathBuf but
later code canonicalizes only self.config (so relative config paths are resolved
against the ambient CWD after parse_and_chdir() has already changed it), and a
relative -p value becomes stale after the chdir; fix by making parse_and_chdir()
return both a canonical/absolute path (or canonicalize its return) and ensure
the code that canonicalizes config uses the path from PathConfigArgs::path as
the base (i.e., canonicalize config relative to self.path before any chdir), or
alternatively move the chdir until after both self.path and self.config are
canonicalized; update references to parse_and_chdir, PathConfigArgs::path, and
the block that canonicalizes self.config so the struct remains self-consistent
and relative paths are resolved against the intended directory.

/// Path to the config file
#[arg(short = 'c', long)]
pub config: PathBuf,
}

fn parse_and_chdir(s: &str) -> Result<PathBuf, String> {
let path = PathBuf::from(s);

if !path.is_dir() {
return Err(format!("not a directory: {}", path.display()));
}

env::set_current_dir(&path)
.map_err(|e| format!("failed to change directory to {}: {e}", path.display()))?;

Ok(path)
}

impl PathConfigArgs {
pub fn resolve_config(&self) -> anyhow::Result<PathBuf> {
self.config
.canonicalize()
.context("can't canonicalize config")
}
}

pub fn resolve_path(&self) -> anyhow::Result<PathBuf> {
self.path
.canonicalize()
.context("can't canonicalize workspace path")
}
pub fn workspace_root() -> anyhow::Result<PathBuf> {
env::current_dir().context("can't determine current working directory")
}

#[derive(Args)]
Expand Down Expand Up @@ -76,15 +88,14 @@ impl Display for Registry {
}

impl BuildRunArgs {
pub fn resolve_workspace_and_config(&self) -> anyhow::Result<(PathBuf, PathBuf, String)> {
let path = self.path_config.resolve_path()?;
pub fn resolve_config_and_name(&self) -> anyhow::Result<(PathBuf, String)> {
let config_path = self.path_config.resolve_config()?;
let project_name = resolve_generated_project_name(&config_path, self.name.as_deref())?;
if self.clean {
remove_from_file_structure(&path, &project_name, "Cargo.lock")?;
remove_from_file_structure(&project_name, "Cargo.lock")?;
}

Ok((path, config_path, project_name))
Ok((config_path, project_name))
}
}

Expand Down Expand Up @@ -150,9 +161,9 @@ pub fn cargo_command(subcommand: &str, path: &Path, otel: bool, release: bool) -
cmd
}

pub fn get_config(path: &Path, config_path: &Path) -> anyhow::Result<Config> {
pub fn get_config(config_path: &Path) -> anyhow::Result<Config> {
let mut config = get_config_from_path(config_path)?;
let mut members = get_module_name_from_crate(&path.to_path_buf())?;
let mut members = get_module_name_from_crate()?;

config.modules.iter_mut().for_each(|module| {
if let Some(module_metadata) = members.remove(module.0.as_str()) {
Expand Down Expand Up @@ -214,8 +225,9 @@ static CARGO_DEPS: LazyLock<HashMap<String, String>> = LazyLock::new(|| {
res
});

fn create_required_deps(path: &Path) -> anyhow::Result<CargoTomlDependencies> {
let mut deps = get_dependencies(path, &CARGO_DEPS)?;
fn create_required_deps() -> anyhow::Result<CargoTomlDependencies> {
let workspace_path = workspace_root()?;
let mut deps = get_dependencies(&workspace_path, &CARGO_DEPS)?;
if let Some(modkit) = deps.get_mut("modkit") {
modkit.features.insert("bootstrap".to_owned());
} else {
Expand Down Expand Up @@ -244,13 +256,12 @@ fn create_required_deps(path: &Path) -> anyhow::Result<CargoTomlDependencies> {
}

pub fn generate_server_structure(
path: &Path,
project_name: &str,
config_path: &Path,
current_dependencies: &CargoTomlDependencies,
) -> anyhow::Result<()> {
let mut dependencies = current_dependencies.clone();
dependencies.extend(create_required_deps(path)?);
dependencies.extend(create_required_deps()?);
let cargo_toml = CargoToml {
package: Package {
name: project_name.to_owned(),
Expand All @@ -266,10 +277,9 @@ pub fn generate_server_structure(
.build()?
.parse(CARGO_SERVER_MAIN)?;

create_file_structure(path, project_name, "Cargo.toml", &cargo_toml_str)?;
create_file_structure(path, project_name, ".cargo/config.toml", CARGO_CONFIG_TOML)?;
create_file_structure(project_name, "Cargo.toml", &cargo_toml_str)?;
create_file_structure(project_name, ".cargo/config.toml", CARGO_CONFIG_TOML)?;
create_file_structure(
path,
project_name,
"src/main.rs",
&main_template.render(&prepare_cargo_server_main(
Expand All @@ -281,18 +291,17 @@ pub fn generate_server_structure(
Ok(())
}

pub fn generated_project_dir(path: &Path, project_name: &str) -> PathBuf {
PathBuf::from(path).join(BASE_PATH).join(project_name)
pub fn generated_project_dir(project_name: &str) -> anyhow::Result<PathBuf> {
Ok(workspace_root()?.join(BASE_PATH).join(project_name))
}

fn create_file_structure(
path: &Path,
project_name: &str,
relative_path: &str,
contents: &str,
) -> anyhow::Result<()> {
use std::io::Write;
let path = generated_project_dir(path, project_name).join(relative_path);
let path = generated_project_dir(project_name)?.join(relative_path);
fs::create_dir_all(
path.parent().context(
"this should be unreachable, the parent for the file structure always exists",
Expand All @@ -309,12 +318,8 @@ fn create_file_structure(
.context("can't write to file")
}

fn remove_from_file_structure(
path: &Path,
project_name: &str,
relative_path: &str,
) -> anyhow::Result<()> {
let path = generated_project_dir(path, project_name).join(relative_path);
fn remove_from_file_structure(project_name: &str, relative_path: &str) -> anyhow::Result<()> {
let path = generated_project_dir(project_name)?.join(relative_path);
if path.exists() {
fs::remove_file(path).context("can't remove file")?;
}
Expand Down
Loading