From 15de1196284e27d11bb9c39d4156169f682829c9 Mon Sep 17 00:00:00 2001 From: Robin Date: Tue, 28 Apr 2026 12:33:23 +0200 Subject: [PATCH] fix(cli,build): init . sanitization + render-worker rebuild trigger MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes spike-1 issues #4 and #5. #4 — `akua init .` no longer writes `name = "."` to akua.toml. The CLI dispatcher now routes `.` / `./` / path-like / non-identifier args through a sanitizer that derives a valid package name from the canonicalized basename (lowercase, non-`[a-z0-9_-]` → `_`, strip leading hyphens). Bare-identifier args keep the original behavior. Path-like args get path target + sanitized basename name. #5 — touching files under `crates/akua-render-worker/src` or `crates/akua-core/src` now re-runs `crates/akua-cli/build.rs` (via additional `cargo:rerun-if-changed=` directives). The script also walks both trees for `.rs` files newer than the staged worker `.wasm` and emits a `cargo:warning=` naming the offender, so contributors see a clear "run task build:render-worker first" without having to remember the gotcha. Tests: - 5 unit tests in main.rs covering sanitize_package_name + derive_init_target_and_name across the 4 input shapes. - 2 integration tests in cli_integration.rs covering `init .` end-to-end (record correct name, sanitize Capitals.With.Dots). - All 22 prior cli_integration tests still green. CLAUDE.md gotcha note replaced with the new behavior. --- CLAUDE.md | 2 +- crates/akua-cli/build.rs | 74 +++++++++++++ crates/akua-cli/src/main.rs | 131 ++++++++++++++++++++--- crates/akua-cli/tests/cli_integration.rs | 44 ++++++++ crates/akua-core/src/lib.rs | 4 +- crates/akua-core/src/mod_file.rs | 2 +- 6 files changed, 238 insertions(+), 19 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 1f081c8f..23056cb1 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -88,7 +88,7 @@ That's it. akua does **not** specify `App`, `Environment`, `Cluster`, `Secret`, - `docs/cli.md` section, verb-count bump (grep for the current count across docs/README), 🚧 → ✅ - `CHANGELOG.md` entry; `task release:validate` still green -**Touching `eval_kcl` or anything called from it:** `cargo build` does NOT rebuild `akua-render-worker.cwasm`. Run `task build:render-worker --force` first. Symptom of forgetting: binary + unit tests green, but `akua render` reproduces old behavior. +**Touching `eval_kcl` or anything called from it:** `cargo build` doesn't rebuild `akua-render-worker.cwasm` (the worker is compiled separately to `wasm32-wasip1` by `task build:render-worker`). `crates/akua-cli/build.rs` watches `crates/akua-render-worker/src` and `crates/akua-core/src` and emits a `cargo:warning=` when sources are newer than the staged `.wasm` — heed it and run `task build:render-worker` before re-running `cargo build`. ## What we refuse diff --git a/crates/akua-cli/build.rs b/crates/akua-cli/build.rs index 4eb65312..fb1d24e4 100644 --- a/crates/akua-cli/build.rs +++ b/crates/akua-cli/build.rs @@ -34,6 +34,23 @@ fn main() { println!("cargo:rerun-if-changed={}", worker_wasm.display()); println!("cargo:rerun-if-changed=build.rs"); + // Watch the source trees that feed into akua-render-worker.wasm so + // touching `eval_kcl` (or anything else the worker links) re-runs + // this build script — cargo otherwise considers the artifact + // up-to-date and silently keeps the stale `.cwasm`. The script can't + // *rebuild* the worker (that's a separate cargo invocation against + // wasm32-wasip1, run by `task build:render-worker`), but at least + // the contributor sees the cargo:warning telling them to run it. + let root = workspace_root(); + println!( + "cargo:rerun-if-changed={}", + root.join("crates/akua-render-worker/src").display() + ); + println!( + "cargo:rerun-if-changed={}", + root.join("crates/akua-core/src").display() + ); + if !worker_wasm.exists() { println!( "cargo:warning=akua-render-worker.wasm not found at {} — run `task build:render-worker` first. Emitting empty sandbox module (runtime will surface E_SANDBOX_UNAVAILABLE).", @@ -44,6 +61,24 @@ fn main() { return; } + // Best-effort freshness check: if any source file under the watched + // trees is newer than the staged `.wasm`, warn loudly. The rerun-if- + // changed lines above force this build.rs to re-run on source edits; + // this loop turns "build.rs re-ran" into "you need to rebuild the + // worker." No-op when both trees and the wasm artifact are clean. + if let Some(stale) = source_newer_than( + &worker_wasm, + &[ + root.join("crates/akua-render-worker/src"), + root.join("crates/akua-core/src"), + ], + ) { + println!( + "cargo:warning=akua-render-worker.wasm is older than {} — run `task build:render-worker` to rebuild it before re-running cargo build.", + stale.display() + ); + } + let wasm = std::fs::read(&worker_wasm) .unwrap_or_else(|e| panic!("read worker wasm from {}: {e}", worker_wasm.display())); // Stage the source `.wasm` regardless; lib.rs picks one of the @@ -72,6 +107,45 @@ fn main() { } } +/// Walk each tree under `roots` and return the first `.rs` file with +/// an mtime newer than `target`'s — or `None` if none is. Best-effort: +/// any I/O error is treated as "can't tell," skipped silently. The +/// caller turns the result into a `cargo:warning=`. +fn source_newer_than(target: &std::path::Path, roots: &[PathBuf]) -> Option { + let target_mtime = std::fs::metadata(target).ok()?.modified().ok()?; + for root in roots { + if let Some(stale) = walk_for_newer_rs(root, target_mtime) { + return Some(stale); + } + } + None +} + +fn walk_for_newer_rs( + dir: &std::path::Path, + target_mtime: std::time::SystemTime, +) -> Option { + let entries = std::fs::read_dir(dir).ok()?; + for entry in entries.flatten() { + let path = entry.path(); + let ft = entry.file_type().ok()?; + if ft.is_dir() { + if let Some(found) = walk_for_newer_rs(&path, target_mtime) { + return Some(found); + } + } else if path.extension().and_then(|s| s.to_str()) == Some("rs") { + if let Ok(meta) = entry.metadata() { + if let Ok(mtime) = meta.modified() { + if mtime > target_mtime { + return Some(path); + } + } + } + } + } + None +} + /// akua-cli's build.rs runs with CWD = crates/akua-cli. The workspace /// root sits two parents up. fn workspace_root() -> PathBuf { diff --git a/crates/akua-cli/src/main.rs b/crates/akua-cli/src/main.rs index eabf8c33..0129ca1d 100644 --- a/crates/akua-cli/src/main.rs +++ b/crates/akua-cli/src/main.rs @@ -1231,21 +1231,7 @@ fn run_lint(args: &UniversalArgs, package: &std::path::Path) -> ExitCode { fn run_init(args: &UniversalArgs, name: Option<&str>, force: bool) -> ExitCode { let ctx = resolve_ctx(args); - - // When `name` is absent, scaffold into CWD and derive the package - // name from its basename. When provided, scaffold into `.//`. - let (target, pkg_name) = match name { - Some(n) => (PathBuf::from(n), n.to_string()), - None => { - let cwd = std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")); - let derived = cwd - .file_name() - .and_then(|s| s.to_str()) - .unwrap_or("") - .to_string(); - (cwd, derived) - } - }; + let (target, pkg_name) = derive_init_target_and_name(name); let verb_args = init_verb::InitArgs { target: &target, package_name: &pkg_name, @@ -1262,6 +1248,78 @@ fn resolve_ctx(args: &UniversalArgs) -> Context { Context::resolve(args, AgentContext::detect()) } +/// Decide where `akua init` writes and what `[package].name` it records. +/// +/// Four cases: +/// 1. No name → CWD + sanitized basename (`mkdir foo && cd foo && akua init`). +/// 2. `.` / `./` → CWD + sanitized basename (the broken case from #4). +/// 3. Bare valid identifier → `.//` + name as-is. +/// 4. Path-like or invalid identifier → use as path, sanitize basename +/// for the package name. +fn derive_init_target_and_name(name: Option<&str>) -> (PathBuf, String) { + match name { + Some(n) if n != "." && n != "./" && akua_core::is_valid_package_name(n) => { + // Bare identifier — original behavior. + (PathBuf::from(n), n.to_string()) + } + Some(n) if n == "." || n == "./" => { + // Scaffold into CWD; derive name from CWD basename. + init_target_from_cwd() + } + Some(n) => { + // Path-like name (e.g. `./foo`, `../bar`, `my-pkg/sub`) — + // use as the target path, derive name from the basename. + let target = PathBuf::from(n); + let basename = target + .file_name() + .and_then(|s| s.to_str()) + .unwrap_or("") + .to_string(); + let sanitized = sanitize_package_name(&basename); + (target, sanitized) + } + None => init_target_from_cwd(), + } +} + +fn init_target_from_cwd() -> (PathBuf, String) { + let cwd = std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")); + // Canonicalize so `.` resolves to a real basename instead of the + // empty string. Falls back to the relative path if canonicalize + // fails (CWD deleted underneath us, etc.) — `EmptyName` will + // surface from the verb in that pathological case. + let resolved = cwd.canonicalize().unwrap_or_else(|_| cwd.clone()); + let basename = resolved + .file_name() + .and_then(|s| s.to_str()) + .unwrap_or("") + .to_string(); + let sanitized = sanitize_package_name(&basename); + (resolved, sanitized) +} + +/// Coerce an arbitrary string into something `is_valid_package_name` +/// accepts. Lowercases ASCII, replaces non-`[a-z0-9_-]` with `_`, +/// strips leading hyphens. Returns empty if nothing usable remains — +/// the verb's existing `EmptyName` error covers that. +fn sanitize_package_name(raw: &str) -> String { + let mut out: String = raw + .chars() + .map(|c| { + let lc = c.to_ascii_lowercase(); + if lc.is_ascii_alphanumeric() || lc == '_' || lc == '-' { + lc + } else { + '_' + } + }) + .collect(); + while out.starts_with('-') { + out.remove(0); + } + out +} + fn run_whoami(args: &UniversalArgs) -> ExitCode { let ctx = resolve_ctx(args); let mut stdout = io::stdout().lock(); @@ -1346,6 +1404,49 @@ fn emit_io_error(ctx: &Context, err: &io::Error) -> ExitCode { mod tests { use super::*; + #[test] + fn sanitize_package_name_lowercases_and_replaces_dots_with_underscore() { + assert_eq!(sanitize_package_name("Foo.Bar"), "foo_bar"); + assert_eq!(sanitize_package_name("my pkg"), "my_pkg"); + assert_eq!(sanitize_package_name("HELLO"), "hello"); + } + + #[test] + fn sanitize_package_name_strips_leading_hyphens_and_keeps_internal() { + assert_eq!(sanitize_package_name("-leading-dash"), "leading-dash"); + assert_eq!(sanitize_package_name("---multi"), "multi"); + assert_eq!(sanitize_package_name("hello-world"), "hello-world"); + } + + #[test] + fn sanitize_package_name_returns_empty_for_pathological_input() { + // The init verb surfaces this as E_INIT_EMPTY_NAME — it's the + // single failure mode `derive_init_target_and_name` doesn't + // try to fix. + assert_eq!(sanitize_package_name(""), ""); + assert_eq!(sanitize_package_name("---"), ""); + } + + // The `Some(".")` → CWD-basename path mutates process-global CWD + // and would race other tests; covered end-to-end in the + // tests/cli_integration.rs subprocess harness instead. + + #[test] + fn derive_init_with_bare_identifier_keeps_legacy_shape() { + let (target, name) = derive_init_target_and_name(Some("my_pkg")); + assert_eq!(target, std::path::PathBuf::from("my_pkg")); + assert_eq!(name, "my_pkg"); + } + + #[test] + fn derive_init_with_path_arg_uses_path_target_and_sanitized_basename() { + // `akua init ./Some.Subdir` → target `./Some.Subdir`, name + // `some_subdir`. Path-like args are a separate case from `.`. + let (target, name) = derive_init_target_and_name(Some("./Some.Subdir")); + assert_eq!(target, std::path::PathBuf::from("./Some.Subdir")); + assert_eq!(name, "some_subdir"); + } + #[test] fn parses_whoami_with_universal_flags() { let cli = Cli::parse_from(["akua", "whoami", "--json"]); diff --git a/crates/akua-cli/tests/cli_integration.rs b/crates/akua-cli/tests/cli_integration.rs index 00f623d7..230a8e89 100644 --- a/crates/akua-cli/tests/cli_integration.rs +++ b/crates/akua-cli/tests/cli_integration.rs @@ -99,6 +99,50 @@ fn init_scaffolds_three_files_and_reports_them() { assert!(pkg.join("inputs.example.yaml").is_file()); } +#[test] +fn init_dot_uses_cwd_basename_not_literal_dot() { + // Regression for #4: `akua init .` from a directory must record + // the directory's basename in [package].name, not the literal `.`. + // Without the fix, render → check fails because `.` isn't a valid + // KCL identifier and the manifest parser rejects it. + let dir = tempdir(); + let pkg = dir.path().join("hello-app"); + std::fs::create_dir_all(&pkg).unwrap(); + + let out = run(&pkg, &["init", ".", "--json"]); + assert_exit(&out, 0); + + let parsed = stdout_json(&out); + assert_eq!(parsed["name"], "hello-app"); + + // Files should land in `pkg`, not in a `./.` subdirectory. + assert!(pkg.join("akua.toml").is_file()); + assert!(pkg.join("package.k").is_file()); + + // The scaffolded manifest must round-trip through the parser — + // the bug surfaced as `E_MANIFEST_PARSE` on the next render. + let toml = std::fs::read_to_string(pkg.join("akua.toml")).unwrap(); + assert!( + toml.contains("name = \"hello-app\""), + "akua.toml lacks expected name line:\n{toml}" + ); +} + +#[test] +fn init_dot_sanitizes_basename_with_dots_and_capitals() { + // Directory name is `My.Pkg.v2` — invalid as a package name. + // The CLI sanitizes it to `my_pkg_v2` rather than refusing. + let dir = tempdir(); + let pkg = dir.path().join("My.Pkg.v2"); + std::fs::create_dir_all(&pkg).unwrap(); + + let out = run(&pkg, &["init", ".", "--json"]); + assert_exit(&out, 0); + + let parsed = stdout_json(&out); + assert_eq!(parsed["name"], "my_pkg_v2"); +} + #[test] fn init_without_force_refuses_to_clobber() { let dir = tempdir(); diff --git a/crates/akua-core/src/lib.rs b/crates/akua-core/src/lib.rs index 2468b3d6..adb3b39a 100644 --- a/crates/akua-core/src/lib.rs +++ b/crates/akua-core/src/lib.rs @@ -118,8 +118,8 @@ pub use lock_file::{ AkuaLock, LockError, LockLoadError, LockedPackage, Replaced, CURRENT_VERSION as LOCK_VERSION, }; pub use mod_file::{ - AkuaManifest, DependencySource, ManifestError, ManifestLoadError, PackageSection, Replace, - WorkspaceSection, + is_valid_package_name, AkuaManifest, DependencySource, ManifestError, ManifestLoadError, + PackageSection, Replace, WorkspaceSection, }; #[cfg(feature = "engine-kcl")] pub use package_k::{ diff --git a/crates/akua-core/src/mod_file.rs b/crates/akua-core/src/mod_file.rs index d2a2d871..335e5468 100644 --- a/crates/akua-core/src/mod_file.rs +++ b/crates/akua-core/src/mod_file.rs @@ -311,7 +311,7 @@ impl Dependency { /// - must not start with `-` (registry ergonomics) /// /// Digit-prefixed names are permitted (e.g. `01-hello-webapp`). -fn is_valid_package_name(s: &str) -> bool { +pub fn is_valid_package_name(s: &str) -> bool { let mut chars = s.chars(); let Some(first) = chars.next() else { return false;