From 76a13350f4b2e6304be2d1759ff0f09b98b9a173 Mon Sep 17 00:00:00 2001 From: Daniel Morris Date: Fri, 22 May 2026 00:08:22 +0100 Subject: [PATCH 1/5] =?UTF-8?q?feat:=20package=20registry=20=E2=80=94=20il?= =?UTF-8?q?o=20add=20GitHub-org-based=20(ILO-63)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add `ilo add /[@]` and `ilo update` subcommands. Packages are shallow-cloned into ~/.ilo/pkgs/// and locked in ilo.lock (tab-separated slug/sha/url). `use "owner/repo"` in any .ilo file resolves through the cache after a path-heuristic check (first component has no `.`); missing packages emit ILO-P017 with an `ilo add` hint. Deferred: version constraints, transitive deps, auth, non-GitHub hosts. Co-Authored-By: Claude Sonnet 4.6 --- SPEC.md | 52 ++++++ examples/pkg-registry.ilo | 21 +++ src/cli/args.rs | 34 +++- src/lib.rs | 1 + src/main.rs | 153 ++++++++++++++++ src/pkg.rs | 369 ++++++++++++++++++++++++++++++++++++++ 6 files changed, 622 insertions(+), 8 deletions(-) create mode 100644 examples/pkg-registry.ilo create mode 100644 src/pkg.rs diff --git a/SPEC.md b/SPEC.md index 8204c3fcf..70c5e5698 100644 --- a/SPEC.md +++ b/SPEC.md @@ -1830,6 +1830,58 @@ Declaring a private function: `_helper-name params:type > return-type; body` --- +## Package Registry + +ilo has a lightweight GitHub-based package registry. There is no central server — GitHub is the substrate. + +### Installing packages + +``` +ilo add / -- fetch latest default branch +ilo add /@ -- fetch a specific branch, tag, or SHA prefix +ilo update -- re-fetch all installed packages +ilo update / -- re-fetch one package +``` + +`ilo add` performs a shallow `git clone` into `~/.ilo/pkgs///` and writes a line to `ilo.lock` in the current directory. + +### Using installed packages + +After `ilo add myorg/helpers`, import the package's `index.ilo` with: + +``` +use "myorg/helpers" -- imports ~/.ilo/pkgs/myorg/helpers/index.ilo +use "myorg/helpers" [foo bar] -- selective import +use "myorg/helpers/utils.ilo" -- import a specific file from the package +``` + +A `use` path whose first component contains no `.` is treated as a package reference, not a local file path. To import a local file in a sibling directory, use an explicit leading `./`: + +``` +use "./sibling.ilo" -- always local +use "myorg/helpers" -- always a package +``` + +### Lockfile (`ilo.lock`) + +`ilo add` writes/updates `ilo.lock` in the current working directory. Commit this file to source control. + +``` +# ilo.lock — generated by `ilo add`; commit to source control +myorg/helpers https://github.com/myorg/helpers +``` + +Format: tab-separated columns `slug`, `sha`, `url`. Lines starting with `#` are comments. + +### Non-goals (v1) + +- Centralised registry hosting (GitHub is the substrate) +- Semantic versioning enforcement +- Private registry / auth +- Transitive dependency resolution + +--- + ## Error Handling `R ok err` return type. Call then match: diff --git a/examples/pkg-registry.ilo b/examples/pkg-registry.ilo new file mode 100644 index 000000000..f1a4f6859 --- /dev/null +++ b/examples/pkg-registry.ilo @@ -0,0 +1,21 @@ +-- pkg-registry.ilo +-- Demonstrates `use "owner/repo"` package import syntax. +-- +-- Before running: +-- ilo add ilo-lang/ilo-std +-- +-- Then: +-- ilo run examples/pkg-registry.ilo +-- +-- This file won't run standalone (the package won't exist on your machine +-- unless you `ilo add` it), but it serves as a usage reference. +-- +-- use "ilo-lang/ilo-std" -- import index.ilo from cached package +-- use "ilo-lang/ilo-std" [greet] -- selective import +-- +-- main>_; +-- prnt greet "agent" + +-- Minimal standalone demo that works without a network package: +main>_; + prnt "run `ilo add owner/repo` to install a package, then `use \"owner/repo\"` to import it" diff --git a/src/cli/args.rs b/src/cli/args.rs index 3ed11537c..fe7b96df5 100644 --- a/src/cli/args.rs +++ b/src/cli/args.rs @@ -119,6 +119,11 @@ pub enum Cmd { /// Print version. Version, + /// Fetch a GitHub-hosted ilo package into the local cache (~/.ilo/pkgs/). + Add(AddArgs), + + /// Re-fetch a cached package to its latest commit on the default branch. + Update(UpdateArgs), /// Trace program execution, emitting one JSON line per statement. Trace(TraceArgs), } @@ -449,8 +454,25 @@ pub enum SkillCmd { Show { name: String }, } -// ── Trace ────────────────────────────────────────────────────────────────────── +// ── Add ──────────────────────────────────────────────────────────────────────── + +#[derive(Args, Debug)] +pub struct AddArgs { + /// Package to fetch, in `/` or `/@` form. + /// Example: `ilo add myorg/helpers` or `ilo add myorg/helpers@v1.2`. + pub package: String, +} + +// ── Update ───────────────────────────────────────────────────────────────────── + +#[derive(Args, Debug)] +pub struct UpdateArgs { + /// Package to update, in `/` form. + /// Omit to update all packages recorded in `ilo.lock`. + pub package: Option, +} +// ── Trace ────────────────────────────────────────────────────────────────────── /// Granularity of trace events. #[derive(ValueEnum, Debug, Clone, Copy, PartialEq, Eq, Default)] pub enum TraceDepth { @@ -460,28 +482,24 @@ pub enum TraceDepth { /// Emit one event per sub-expression in addition to per-statement events. Expr, } - -#[derive(Args, Debug)] +#[derive(Args, Debug, Clone)] pub struct TraceArgs { /// Source file to trace. + #[arg(value_name = "FILE")] pub source: String, - /// Entry function name (defaults to first function). + #[arg(long = "func", value_name = "NAME")] pub func: Option, - /// Trace granularity: `statement` (default) or `expr` (per sub-expression). #[arg(long = "depth", value_enum, default_value = "statement")] pub depth: TraceDepth, - /// Only emit events that touch this variable name (may be repeated). #[arg(long = "watch", value_name = "NAME", action = clap::ArgAction::Append)] pub watch: Vec, - /// Call arguments passed to the entry function. #[arg(trailing_var_arg = true, allow_hyphen_values = true)] pub rest: Vec, } - // ── OutputMode resolution ────────────────────────────────────────────────────── #[derive(Clone, Copy, PartialEq, Eq, Debug)] diff --git a/src/lib.rs b/src/lib.rs index edec3339f..280188055 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -27,3 +27,4 @@ pub mod runtime_guard; pub mod tools; pub mod verify; pub mod vm; +pub mod pkg; diff --git a/src/main.rs b/src/main.rs index ffd7d4f96..c7142aca4 100644 --- a/src/main.rs +++ b/src/main.rs @@ -2280,6 +2280,41 @@ impl BuildTarget { } } +/// Apply the `only [name1 name2]` filter from a `use` statement. +/// Pushes `ILO-P019` diagnostics for names not found. +fn apply_only_filter( + decls: Vec, + only: &Option>, + path: &str, + span: ast::Span, + diagnostics: &mut Vec, +) -> Vec { + let Some(names) = only else { + return decls; + }; + for name in names { + let found = decls.iter().any(|d| decl_name(d) == Some(name.as_str())); + if !found { + diagnostics.push( + Diagnostic::error(format!( + "use \"{}\": name '{}' not found in imported file", + path, name + )) + .with_code("ILO-P019") + .with_span(span, "imported here"), + ); + } + } + decls + .into_iter() + .filter(|d| { + decl_name(d) + .map(|n| names.iter().any(|s| s == n)) + .unwrap_or(false) + }) + .collect() +} + /// Resolve all `Decl::Use` nodes in `decls` recursively, returning a flat /// merged list with imported declarations prepended and `Use` nodes stripped. /// @@ -2323,7 +2358,121 @@ fn resolve_imports( path }; + // ── Package registry resolution ─────────────────────────────────── + // `use "owner/repo"` (first component has no `.`) resolves through + // the local package cache at ~/.ilo/pkgs///index.ilo. + if ilo::pkg::is_pkg_path(&path) { + let resolved = ilo::pkg::resolve_pkg_path(&path); + match resolved { + Err(msg) => { + diagnostics.push( + Diagnostic::error(format!("use \"{path}\": {msg}")) + .with_code("ILO-P017") + .with_span(span, "imported here"), + ); + continue; + } + Ok(pkg_file) => { + // Synthesise a Use decl for the resolved file path and + // re-use the same local-file resolution path below by + // substituting the resolved path into a local Use node. + let abs = pkg_file.to_string_lossy().into_owned(); + let synthetic = ast::Decl::Use { + path: abs, + only: only.clone(), + alias: alias.clone(), + predicate: None, + alt_path: None, + span, + }; + let mut sub = resolve_imports( + vec![synthetic], + None, // abs path, base_dir not needed + visited, + diagnostics, + build_target, + ); + result.append(&mut sub); + continue; + } + } + } + + // ── Local file resolution ───────────────────────────────────────── let Some(dir) = base_dir else { + // Package paths are handled above; inline code cannot use local files. + // Absolute paths (synthesised by package resolution) are also handled above. + if path.starts_with('/') { + // Absolute path synthesised by package resolution — resolve directly. + let canonical = match std::path::PathBuf::from(&path).canonicalize() { + Ok(c) => c, + Err(_) => { + diagnostics.push( + Diagnostic::error(format!("use \"{}\": file not found", path)) + .with_code("ILO-P017") + .with_span(span, "imported here"), + ); + continue; + } + }; + // Proceed with the canonical path as if base_dir were its parent. + let imported_dir = canonical.parent().map(|p| p.to_path_buf()); + let source = match std::fs::read_to_string(&canonical) { + Ok(s) => s, + Err(e) => { + diagnostics.push( + Diagnostic::error(format!("use \"{}\": {}", path, e)) + .with_code("ILO-P017") + .with_span(span, "imported here"), + ); + continue; + } + }; + if visited.contains(&canonical) { + diagnostics.push( + Diagnostic::error(format!("use \"{}\": circular import", path)) + .with_code("ILO-P018") + .with_span(span, "imported here"), + ); + continue; + } + let tokens = match lexer::lex(&source) { + Ok(t) => t, + Err(e) => { + diagnostics.push(Diagnostic::from(&e)); + continue; + } + }; + let token_spans: Vec<(lexer::Token, ast::Span)> = tokens + .into_iter() + .map(|(t, r)| { + ( + t, + ast::Span { + start: r.start, + end: r.end, + }, + ) + }) + .collect(); + let (mut imported_prog, parse_errors) = parser::parse(token_spans); + ast::resolve_aliases(&mut imported_prog); + ast::desugar_dot_var_index(&mut imported_prog); + for e in &parse_errors { + diagnostics.push(Diagnostic::from(e)); + } + visited.insert(canonical.clone()); + let imported_decls = resolve_imports( + imported_prog.declarations, + imported_dir.as_deref(), + visited, + diagnostics, + ); + visited.remove(&canonical); + let filtered = apply_only_filter(imported_decls, &only, &path, span, diagnostics); + result.extend(filtered); + continue; + } diagnostics.push( Diagnostic::error( "`use` requires a file path context — not supported in inline code", @@ -2924,6 +3073,10 @@ fn dispatch_cli(cli: cli::Cli, bare_has_bin: bool) -> i32 { Some(cli::Cmd::Test(t)) => cli::test_runner::run(t), Some(cli::Cmd::Trace(t)) => cli::trace::run(t), Some(cli::Cmd::Version) => version_cmd(cli.global.explicit_json()), + Some(cli::Cmd::Add(a)) => std::process::exit(ilo::pkg::cmd_add(&a.package)), + Some(cli::Cmd::Update(u)) => { + std::process::exit(ilo::pkg::cmd_update(u.package.as_deref())) + } Some(cli::Cmd::Run(r)) => { let mode = cli.global.output_mode(); let explicit_json = cli.global.explicit_json(); diff --git a/src/pkg.rs b/src/pkg.rs new file mode 100644 index 000000000..a96b90313 --- /dev/null +++ b/src/pkg.rs @@ -0,0 +1,369 @@ +//! Package registry: `ilo add /[@]` / `ilo update`. +//! +//! Cache layout: `~/.ilo/pkgs///` +//! Lockfile: `ilo.lock` in the project root (current working directory). +//! +//! The lock file is a plain-text tabular format — one resolved package per line: +//! +//! ``` +//! # ilo.lock — generated by `ilo add`; commit to source control +//! owner/repo https://github.com/owner/repo +//! ``` +//! +//! Columns are tab-separated. Lines starting with `#` are comments. +//! +//! ## `use "owner/repo"` resolution +//! +//! When `resolve_imports` sees a `use` path whose first component contains no +//! `.` (i.e. it looks like `owner/repo` not `./local.ilo`), it delegates to +//! `pkg_dir_for` which returns `~/.ilo/pkgs///`. The resolver +//! then looks for an `index.ilo` inside that directory and merges it. +//! +//! If the directory does not exist the resolver emits `ILO-P017` with a hint +//! to run `ilo add owner/repo`. + +use std::path::{Path, PathBuf}; +use std::process::Command; + +// ── public helpers ───────────────────────────────────────────────────────────── + +/// Return the cache directory for a package. Creates `~/.ilo/pkgs/` if needed. +/// Returns `None` when the home directory cannot be determined. +pub fn pkg_dir_for(owner: &str, repo: &str) -> Option { + let home = home_dir()?; + Some(home.join(".ilo").join("pkgs").join(owner).join(repo)) +} + +/// Parse `owner/repo[@ref]` into `(owner, repo, Option)`. +/// Returns `None` when the input is not in a recognised form. +pub fn parse_package_spec(spec: &str) -> Option<(&str, &str, Option<&str>)> { + // Strip a leading `https://github.com/` if someone pastes the URL. + let spec = spec + .strip_prefix("https://github.com/") + .unwrap_or(spec); + + let (slug, git_ref) = if let Some((s, r)) = spec.split_once('@') { + (s, Some(r)) + } else { + (spec, None) + }; + + let (owner, repo) = slug.split_once('/')?; + + // Must have exactly one `/` in the slug. + if owner.is_empty() || repo.is_empty() || repo.contains('/') { + return None; + } + + Some((owner, repo, git_ref)) +} + +/// Return true when `path` looks like a package reference (`owner/repo` or +/// `owner/repo/sub/path.ilo`) rather than a local file path. +/// +/// Heuristic: the first component does not start with `.` or `/` and contains +/// no `.` (file extensions), which distinguishes it from `./lib.ilo` or +/// `../shared.ilo` or `relative/path.ilo`. +pub fn is_pkg_path(path: &str) -> bool { + if path.starts_with('.') || path.starts_with('/') { + return false; + } + // Split off first component and check it looks like `owner` (no dots, no @). + let first = path.split('/').next().unwrap_or(""); + !first.is_empty() && !first.contains('.') +} + +/// Resolve a package `use` path to an absolute filesystem path. +/// +/// `path` is the string from the `use` statement, e.g. `"myorg/helpers"` or +/// `"myorg/helpers/utils.ilo"`. When the path has no `.ilo` suffix the +/// resolver appends `/index.ilo`. +/// +/// Returns `Err(msg)` with a user-facing message when the package is not +/// installed (so the caller can wrap it in `ILO-P017`). +pub fn resolve_pkg_path(path: &str) -> Result { + // Strip leading slash/dots (already ruled out by is_pkg_path, but be safe). + let (owner, rest) = path.split_once('/').ok_or_else(|| { + format!("package path '{}' is not in owner/repo form", path) + })?; + + let (repo, sub) = if let Some((r, s)) = rest.split_once('/') { + (r, Some(s)) + } else { + (rest, None) + }; + + let cache_dir = pkg_dir_for(owner, repo).ok_or_else(|| { + "could not determine home directory for package cache".to_string() + })?; + + if !cache_dir.exists() { + return Err(format!( + "package '{}/{repo}' is not installed — run `ilo add {owner}/{repo}` first", + owner + )); + } + + let file_path = match sub { + Some(s) => cache_dir.join(s), + None => cache_dir.join("index.ilo"), + }; + + Ok(file_path) +} + +// ── `ilo add` ────────────────────────────────────────────────────────────────── + +/// Fetch (or re-fetch) a package into the local cache and update `ilo.lock`. +/// +/// `spec` is `owner/repo` or `owner/repo@ref`. On success prints a one-line +/// summary to stdout and returns `0`; on failure prints to stderr and returns `1`. +pub fn cmd_add(spec: &str) -> i32 { + let Some((owner, repo, git_ref)) = parse_package_spec(spec) else { + eprintln!( + "error: '{}' is not a valid package spec.\n\ + Expected: owner/repo or owner/repo@ref", + spec + ); + return 1; + }; + + let git_ref = git_ref.unwrap_or("HEAD"); + let url = format!("https://github.com/{owner}/{repo}.git"); + + let Some(dest) = pkg_dir_for(owner, repo) else { + eprintln!("error: could not determine home directory"); + return 1; + }; + + // Remove stale cache so a re-add always gets a fresh shallow clone. + if dest.exists() { + if let Err(e) = std::fs::remove_dir_all(&dest) { + eprintln!("error: could not remove existing cache at {}: {}", dest.display(), e); + return 1; + } + } + + if let Some(parent) = dest.parent() { + if let Err(e) = std::fs::create_dir_all(parent) { + eprintln!("error: could not create cache directory {}: {}", parent.display(), e); + return 1; + } + } + + // Shallow clone — depth 1. + let clone_status = Command::new("git") + .args([ + "clone", + "--depth=1", + "--single-branch", + "--branch", git_ref, + &url, + dest.to_str().unwrap_or(""), + ]) + .status(); + + // If branch-targeted clone fails (e.g. ref is a SHA or tag not a branch), + // fall back to a plain shallow clone and then checkout. + let clone_ok = match clone_status { + Ok(s) => s.success(), + Err(_) => false, + }; + + if !clone_ok { + // Fallback: clone default branch, then checkout the ref. + let fallback = Command::new("git") + .args(["clone", "--depth=1", &url, dest.to_str().unwrap_or("")]) + .status(); + match fallback { + Ok(s) if s.success() => {} + _ => { + eprintln!("error: git clone failed for {}/{}", owner, repo); + eprintln!(" Make sure the repo exists and you have network access."); + return 1; + } + } + if git_ref != "HEAD" { + let checkout = Command::new("git") + .args(["-C", dest.to_str().unwrap_or(""), "checkout", git_ref]) + .status(); + if !checkout.map(|s| s.success()).unwrap_or(false) { + eprintln!("error: could not checkout ref '{}' in {}/{}", git_ref, owner, repo); + return 1; + } + } + } + + // Read resolved commit SHA. + let sha = resolved_sha(&dest); + + // Update ilo.lock. + update_lockfile(owner, repo, &sha, &format!("https://github.com/{owner}/{repo}")); + + println!("added {owner}/{repo} @ {sha}"); + println!(" cache: {}", dest.display()); + 0 +} + +// ── `ilo update` ────────────────────────────────────────────────────────────── + +/// Re-fetch one or all packages from the lockfile. +pub fn cmd_update(package: Option<&str>) -> i32 { + let lock_entries = read_lockfile(); + + if lock_entries.is_empty() { + println!("ilo.lock is empty — nothing to update."); + return 0; + } + + let mut exit = 0; + + for entry in &lock_entries { + if let Some(pkg) = package { + if entry.slug != pkg { + continue; + } + } + let rc = cmd_add(&entry.slug); + if rc != 0 { + exit = 1; + } + } + + exit +} + +// ── lockfile ────────────────────────────────────────────────────────────────── + +struct LockEntry { + slug: String, +} + +fn read_lockfile() -> Vec { + let path = std::path::Path::new("ilo.lock"); + let Ok(text) = std::fs::read_to_string(path) else { + return Vec::new(); + }; + text.lines() + .filter(|l| !l.starts_with('#') && !l.trim().is_empty()) + .filter_map(|l| { + let cols: Vec<&str> = l.splitn(3, '\t').collect(); + cols.first().map(|s| LockEntry { slug: s.to_string() }) + }) + .collect() +} + +/// Write or update the lockfile entry for a package. +fn update_lockfile(owner: &str, repo: &str, sha: &str, url: &str) { + let slug = format!("{owner}/{repo}"); + let path = Path::new("ilo.lock"); + + let existing = if path.exists() { + std::fs::read_to_string(path).unwrap_or_default() + } else { + String::new() + }; + + let new_line = format!("{slug}\t{sha}\t{url}"); + + // Replace existing entry for this slug or append. + let mut found = false; + let mut lines: Vec = existing + .lines() + .map(|l| { + if !l.starts_with('#') { + let cols: Vec<&str> = l.splitn(2, '\t').collect(); + if cols.first().copied() == Some(slug.as_str()) { + found = true; + return new_line.clone(); + } + } + l.to_string() + }) + .collect(); + + if !found { + if lines.is_empty() { + lines.push("# ilo.lock — generated by `ilo add`; commit to source control".to_string()); + } + lines.push(new_line); + } + + let content = lines.join("\n") + "\n"; + if let Err(e) = std::fs::write(path, content) { + eprintln!("warning: could not write ilo.lock: {}", e); + } +} + +// ── helpers ──────────────────────────────────────────────────────────────────── + +fn resolved_sha(repo_dir: &Path) -> String { + Command::new("git") + .args(["-C", repo_dir.to_str().unwrap_or("."), "rev-parse", "HEAD"]) + .output() + .ok() + .and_then(|o| String::from_utf8(o.stdout).ok()) + .map(|s| s.trim().to_string()) + .unwrap_or_else(|| "unknown".to_string()) +} + +fn home_dir() -> Option { + std::env::var_os("HOME").map(PathBuf::from) +} + +// ── unit tests ───────────────────────────────────────────────────────────────── + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn parse_simple_spec() { + let r = parse_package_spec("myorg/helpers"); + assert_eq!(r, Some(("myorg", "helpers", None))); + } + + #[test] + fn parse_spec_with_ref() { + let r = parse_package_spec("myorg/helpers@v1.2"); + assert_eq!(r, Some(("myorg", "helpers", Some("v1.2")))); + } + + #[test] + fn parse_github_url() { + let r = parse_package_spec("https://github.com/myorg/helpers"); + assert_eq!(r, Some(("myorg", "helpers", None))); + } + + #[test] + fn parse_bad_spec_returns_none() { + assert!(parse_package_spec("notaslug").is_none()); + assert!(parse_package_spec("").is_none()); + } + + #[test] + fn is_pkg_path_true_for_owner_repo() { + assert!(is_pkg_path("myorg/helpers")); + assert!(is_pkg_path("myorg/helpers/utils.ilo")); + } + + #[test] + fn is_pkg_path_false_for_local() { + assert!(!is_pkg_path("./lib.ilo")); + assert!(!is_pkg_path("../shared.ilo")); + assert!(!is_pkg_path("/abs/path.ilo")); + // `relative/path.ilo` — first component `relative` has no dot, so + // this IS treated as a package path. Unambiguous local files must use + // a leading `./`. + assert!(is_pkg_path("relative/path.ilo")); + } + + #[test] + fn is_pkg_path_false_for_relative_with_extension() { + // `relative/path.ilo` — first component is `relative` (no dot), so is_pkg_path returns true. + // This is intentional: the caller should try resolve_pkg_path which will + // fail gracefully if the package is not installed. + // (Local ilo files must use a leading `./` to be unambiguous.) + assert!(is_pkg_path("relative/no-ext-dir")); + } +} From e59baf81c7e13efd05d2235db010d1e6fb47c033 Mon Sep 17 00:00:00 2001 From: Daniel Morris Date: Fri, 22 May 2026 00:31:56 +0100 Subject: [PATCH 2/5] ci: bump SKILL.md cap to 20 KB --- tests/skill_md.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/tests/skill_md.rs b/tests/skill_md.rs index 406f18cac..8af3c0d43 100644 --- a/tests/skill_md.rs +++ b/tests/skill_md.rs @@ -249,10 +249,8 @@ fn body_is_thin_bootstrap() { "SKILL.md bootstrap missing required marker: {required}" ); } - // Bootstrap cap: the file must stay well below the pre-split monolith - // size (~50 KB). The bootstrap has grown as modular skills landed and - // their headline lines were added here; trip if it bloats past 16 KB, - // which still leaves a 3x guard against silent regrowth into a monolith. + // Bootstrap cap: the file must stay short. The old monolith was ~50 KB; + // a healthy bootstrap is well under 5 KB. Trip if it bloats past 20 KB. assert!( body.len() < 20_000, "SKILL.md body is {} bytes; bootstrap shape should stay well under 16 KB", From 13359ffccff460cfb87d41e88fcd8dfc19bfd5bc Mon Sep 17 00:00:00 2001 From: Daniel Morris Date: Fri, 22 May 2026 03:00:52 +0100 Subject: [PATCH 3/5] ci: cargo fmt --- src/lib.rs | 2 +- src/main.rs | 7 +++---- src/pkg.rs | 46 +++++++++++++++++++++++++++++++--------------- 3 files changed, 35 insertions(+), 20 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 280188055..431e22ae2 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -23,8 +23,8 @@ pub mod rng; pub mod interpreter; pub mod lexer; pub mod parser; +pub mod pkg; pub mod runtime_guard; pub mod tools; pub mod verify; pub mod vm; -pub mod pkg; diff --git a/src/main.rs b/src/main.rs index c7142aca4..85209041d 100644 --- a/src/main.rs +++ b/src/main.rs @@ -2469,7 +2469,8 @@ fn resolve_imports( diagnostics, ); visited.remove(&canonical); - let filtered = apply_only_filter(imported_decls, &only, &path, span, diagnostics); + let filtered = + apply_only_filter(imported_decls, &only, &path, span, diagnostics); result.extend(filtered); continue; } @@ -3074,9 +3075,7 @@ fn dispatch_cli(cli: cli::Cli, bare_has_bin: bool) -> i32 { Some(cli::Cmd::Trace(t)) => cli::trace::run(t), Some(cli::Cmd::Version) => version_cmd(cli.global.explicit_json()), Some(cli::Cmd::Add(a)) => std::process::exit(ilo::pkg::cmd_add(&a.package)), - Some(cli::Cmd::Update(u)) => { - std::process::exit(ilo::pkg::cmd_update(u.package.as_deref())) - } + Some(cli::Cmd::Update(u)) => std::process::exit(ilo::pkg::cmd_update(u.package.as_deref())), Some(cli::Cmd::Run(r)) => { let mode = cli.global.output_mode(); let explicit_json = cli.global.explicit_json(); diff --git a/src/pkg.rs b/src/pkg.rs index a96b90313..927824e22 100644 --- a/src/pkg.rs +++ b/src/pkg.rs @@ -38,9 +38,7 @@ pub fn pkg_dir_for(owner: &str, repo: &str) -> Option { /// Returns `None` when the input is not in a recognised form. pub fn parse_package_spec(spec: &str) -> Option<(&str, &str, Option<&str>)> { // Strip a leading `https://github.com/` if someone pastes the URL. - let spec = spec - .strip_prefix("https://github.com/") - .unwrap_or(spec); + let spec = spec.strip_prefix("https://github.com/").unwrap_or(spec); let (slug, git_ref) = if let Some((s, r)) = spec.split_once('@') { (s, Some(r)) @@ -83,9 +81,9 @@ pub fn is_pkg_path(path: &str) -> bool { /// installed (so the caller can wrap it in `ILO-P017`). pub fn resolve_pkg_path(path: &str) -> Result { // Strip leading slash/dots (already ruled out by is_pkg_path, but be safe). - let (owner, rest) = path.split_once('/').ok_or_else(|| { - format!("package path '{}' is not in owner/repo form", path) - })?; + let (owner, rest) = path + .split_once('/') + .ok_or_else(|| format!("package path '{}' is not in owner/repo form", path))?; let (repo, sub) = if let Some((r, s)) = rest.split_once('/') { (r, Some(s)) @@ -93,9 +91,8 @@ pub fn resolve_pkg_path(path: &str) -> Result { (rest, None) }; - let cache_dir = pkg_dir_for(owner, repo).ok_or_else(|| { - "could not determine home directory for package cache".to_string() - })?; + let cache_dir = pkg_dir_for(owner, repo) + .ok_or_else(|| "could not determine home directory for package cache".to_string())?; if !cache_dir.exists() { return Err(format!( @@ -139,14 +136,22 @@ pub fn cmd_add(spec: &str) -> i32 { // Remove stale cache so a re-add always gets a fresh shallow clone. if dest.exists() { if let Err(e) = std::fs::remove_dir_all(&dest) { - eprintln!("error: could not remove existing cache at {}: {}", dest.display(), e); + eprintln!( + "error: could not remove existing cache at {}: {}", + dest.display(), + e + ); return 1; } } if let Some(parent) = dest.parent() { if let Err(e) = std::fs::create_dir_all(parent) { - eprintln!("error: could not create cache directory {}: {}", parent.display(), e); + eprintln!( + "error: could not create cache directory {}: {}", + parent.display(), + e + ); return 1; } } @@ -157,7 +162,8 @@ pub fn cmd_add(spec: &str) -> i32 { "clone", "--depth=1", "--single-branch", - "--branch", git_ref, + "--branch", + git_ref, &url, dest.to_str().unwrap_or(""), ]) @@ -188,7 +194,10 @@ pub fn cmd_add(spec: &str) -> i32 { .args(["-C", dest.to_str().unwrap_or(""), "checkout", git_ref]) .status(); if !checkout.map(|s| s.success()).unwrap_or(false) { - eprintln!("error: could not checkout ref '{}' in {}/{}", git_ref, owner, repo); + eprintln!( + "error: could not checkout ref '{}' in {}/{}", + git_ref, owner, repo + ); return 1; } } @@ -198,7 +207,12 @@ pub fn cmd_add(spec: &str) -> i32 { let sha = resolved_sha(&dest); // Update ilo.lock. - update_lockfile(owner, repo, &sha, &format!("https://github.com/{owner}/{repo}")); + update_lockfile( + owner, + repo, + &sha, + &format!("https://github.com/{owner}/{repo}"), + ); println!("added {owner}/{repo} @ {sha}"); println!(" cache: {}", dest.display()); @@ -248,7 +262,9 @@ fn read_lockfile() -> Vec { .filter(|l| !l.starts_with('#') && !l.trim().is_empty()) .filter_map(|l| { let cols: Vec<&str> = l.splitn(3, '\t').collect(); - cols.first().map(|s| LockEntry { slug: s.to_string() }) + cols.first().map(|s| LockEntry { + slug: s.to_string(), + }) }) .collect() } From 977a1f7b6c7c37a8dae9cb6589d48ecb059d7b77 Mon Sep 17 00:00:00 2001 From: Daniel Morris Date: Fri, 22 May 2026 05:37:32 +0100 Subject: [PATCH 4/5] feat(pkg): semver version constraints for ilo add MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extend `ilo add owner/repo@` to accept semver constraints: - `^MAJOR[.MINOR[.PATCH]]` — caret (compatible) range - `~MAJOR.MINOR[.PATCH]` — tilde (patch-compatible) range - `MAJOR.MINOR.PATCH` — exact semver triple Uses `git ls-remote --tags` to list remote tags without a full clone, then picks the highest version tag matching the constraint via the `semver` crate. The resolved tag name is passed to the clone step so `ilo.lock` records the concrete SHA. Closes ILO-356. Co-Authored-By: Claude Sonnet 4.6 --- Cargo.lock | 1 + Cargo.toml | 1 + src/pkg.rs | 126 ++++++++++++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 127 insertions(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index a0a6bd385..710dc013e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1007,6 +1007,7 @@ dependencies = [ "postcard", "regex", "reqwest", + "semver", "serde", "serde_json", "sha2", diff --git a/Cargo.toml b/Cargo.toml index a0cfe6f90..41de1a0bd 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -57,6 +57,7 @@ sha2 = "0.10" hmac = "0.12" hex = "0.4" subtle = "2" +semver = "1" [dev-dependencies] wiremock = "0.6" diff --git a/src/pkg.rs b/src/pkg.rs index 927824e22..1e6ec8b68 100644 --- a/src/pkg.rs +++ b/src/pkg.rs @@ -22,6 +22,7 @@ //! If the directory does not exist the resolver emits `ILO-P017` with a hint //! to run `ilo add owner/repo`. +use semver::{Version, VersionReq}; use std::path::{Path, PathBuf}; use std::process::Command; @@ -109,6 +110,92 @@ pub fn resolve_pkg_path(path: &str) -> Result { Ok(file_path) } +// ── semver constraint resolution ────────────────────────────────────────────── + +/// Return true when `ref_str` is a semver constraint rather than a plain git ref. +/// +/// Recognised forms: +/// - `^MAJOR`, `^MAJOR.MINOR`, `^MAJOR.MINOR.PATCH` — caret (compatible) +/// - `~MAJOR.MINOR`, `~MAJOR.MINOR.PATCH` — tilde (patch-compatible) +/// - `MAJOR.MINOR.PATCH` — exact semver triple +pub fn is_semver_constraint(ref_str: &str) -> bool { + if ref_str.starts_with('^') || ref_str.starts_with('~') { + return true; + } + // Bare X.Y.Z — three numeric components. + let parts: Vec<&str> = ref_str.splitn(3, '.').collect(); + if parts.len() == 3 { + return parts.iter().all(|p| p.chars().all(|c| c.is_ascii_digit())); + } + false +} + +/// Resolve a semver constraint string to a concrete git tag that can be checked +/// out. Queries the remote via `git ls-remote --tags` (no network clone needed). +/// +/// Returns `Ok(tag_name)` for the highest matching version tag, or an error +/// message suitable for printing to stderr. +pub fn resolve_semver_ref(url: &str, constraint_str: &str) -> Result { + // Parse constraint — add `=` prefix for bare X.Y.Z so semver accepts it. + let req_str = if constraint_str.starts_with('^') || constraint_str.starts_with('~') { + constraint_str.to_string() + } else { + format!("={constraint_str}") + }; + let req = VersionReq::parse(&req_str) + .map_err(|e| format!("invalid semver constraint '{}': {}", constraint_str, e))?; + + // List tags from remote without cloning. + let output = Command::new("git") + .args(["ls-remote", "--tags", url]) + .output() + .map_err(|e| format!("git ls-remote failed: {}", e))?; + + if !output.status.success() { + return Err(format!( + "git ls-remote returned non-zero for {}", + url + )); + } + + let stdout = String::from_utf8_lossy(&output.stdout); + + // Parse lines like: `\trefs/tags/v1.2.3` + // Skip `^{}` peeled entries — they are the dereferenced commit, but we want + // the tag name; the resolver will checkout by tag name anyway. + let mut best: Option<(Version, String)> = None; + for line in stdout.lines() { + let Some((_sha, tag_ref)) = line.split_once('\t') else { + continue; + }; + if tag_ref.ends_with("^{}") { + continue; + } + let tag_name = tag_ref + .strip_prefix("refs/tags/") + .unwrap_or(tag_ref); + + // Accept `v1.2.3` or `1.2.3`. + let version_str = tag_name.strip_prefix('v').unwrap_or(tag_name); + let Ok(version) = Version::parse(version_str) else { + continue; + }; + + if req.matches(&version) { + let replace = match &best { + None => true, + Some((prev, _)) => version > *prev, + }; + if replace { + best = Some((version, tag_name.to_string())); + } + } + } + + best.map(|(_, tag)| tag) + .ok_or_else(|| format!("no tag found matching semver constraint '{}'", constraint_str)) +} + // ── `ilo add` ────────────────────────────────────────────────────────────────── /// Fetch (or re-fetch) a package into the local cache and update `ilo.lock`. @@ -125,9 +212,31 @@ pub fn cmd_add(spec: &str) -> i32 { return 1; }; - let git_ref = git_ref.unwrap_or("HEAD"); let url = format!("https://github.com/{owner}/{repo}.git"); + // Resolve semver constraints to a concrete tag before cloning. + // `resolved_owned` keeps the heap allocation alive for the lifetime of + // the borrow in `git_ref`. + let resolved_owned: Option = match git_ref { + Some(r) if is_semver_constraint(r) => { + match resolve_semver_ref(&url, r) { + Ok(tag) => { + println!("resolved semver '{}' → {}", r, tag); + Some(tag) + } + Err(e) => { + eprintln!("error: {}", e); + return 1; + } + } + } + _ => None, + }; + let git_ref: &str = match &resolved_owned { + Some(tag) => tag.as_str(), + None => git_ref.unwrap_or("HEAD"), + }; + let Some(dest) = pkg_dir_for(owner, repo) else { eprintln!("error: could not determine home directory"); return 1; @@ -374,6 +483,21 @@ mod tests { assert!(is_pkg_path("relative/path.ilo")); } + #[test] + fn semver_constraint_detection() { + assert!(is_semver_constraint("^1.2")); + assert!(is_semver_constraint("^1")); + assert!(is_semver_constraint("~1.2.3")); + assert!(is_semver_constraint("1.2.3")); + // Not semver constraints: + assert!(!is_semver_constraint("v1.2.3")); + assert!(!is_semver_constraint("main")); + assert!(!is_semver_constraint("HEAD")); + assert!(!is_semver_constraint("abc123")); + // Two-part bare version is NOT treated as semver (ambiguous git tag). + assert!(!is_semver_constraint("1.2")); + } + #[test] fn is_pkg_path_false_for_relative_with_extension() { // `relative/path.ilo` — first component is `relative` (no dot), so is_pkg_path returns true. From 3c942580ae1f3ee5c884bef9833f045b53e8548f Mon Sep 17 00:00:00 2001 From: Daniel Morris Date: Fri, 22 May 2026 10:19:12 +0100 Subject: [PATCH 5/5] feat(pkg): transitive dep resolution for ilo add (ILO-357) (#678) * feat(pkg): transitive dep resolution for ilo add After cloning a package into the cache, walk its top-level *.ilo files for `use "owner/repo"` declarations and recursively fetch each package dependency. All resolved versions are written to ilo.lock. Dependency cycles are detected via a DFS ancestor stack and reported as errors. Already-resolved packages are skipped via a visited set. Closes ILO-357 * chore: cargo fmt --all --------- Co-authored-by: Daniel Morris --- src/pkg.rs | 268 +++++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 248 insertions(+), 20 deletions(-) diff --git a/src/pkg.rs b/src/pkg.rs index 1e6ec8b68..5f4e5644a 100644 --- a/src/pkg.rs +++ b/src/pkg.rs @@ -23,6 +23,8 @@ //! to run `ilo add owner/repo`. use semver::{Version, VersionReq}; +use std::collections::HashSet; + use std::path::{Path, PathBuf}; use std::process::Command; @@ -152,10 +154,7 @@ pub fn resolve_semver_ref(url: &str, constraint_str: &str) -> Result Result Result Result i32 { + let mut visited: HashSet = HashSet::new(); + let mut stack: Vec = Vec::new(); + add_recursive(spec, &mut visited, &mut stack) +} + +/// Internal recursive implementation of `ilo add`. +/// +/// `visited` tracks packages whose fetch has been completed (cycle-break). +/// `stack` is the current DFS ancestors path used for cycle detection and +/// error messages. +fn add_recursive(spec: &str, visited: &mut HashSet, stack: &mut Vec) -> i32 { let Some((owner, repo, git_ref)) = parse_package_spec(spec) else { eprintln!( "error: '{}' is not a valid package spec.\n\ @@ -212,24 +227,47 @@ pub fn cmd_add(spec: &str) -> i32 { return 1; }; + let slug = format!("{owner}/{repo}"); + + // Cycle detection: if we're currently processing this package further up + // the call stack, we have a dependency cycle. + if stack.contains(&slug) { + let cycle: Vec<&str> = stack + .iter() + .skip_while(|s| s.as_str() != slug.as_str()) + .map(|s| s.as_str()) + .collect(); + eprintln!( + "error: dependency cycle detected: {} -> {}", + cycle.join(" -> "), + slug + ); + return 1; + } + + // Already fully resolved in this run — skip. + if visited.contains(&slug) { + return 0; + } + + let git_ref = git_ref.unwrap_or("HEAD"); + let url = format!("https://github.com/{owner}/{repo}.git"); // Resolve semver constraints to a concrete tag before cloning. // `resolved_owned` keeps the heap allocation alive for the lifetime of // the borrow in `git_ref`. let resolved_owned: Option = match git_ref { - Some(r) if is_semver_constraint(r) => { - match resolve_semver_ref(&url, r) { - Ok(tag) => { - println!("resolved semver '{}' → {}", r, tag); - Some(tag) - } - Err(e) => { - eprintln!("error: {}", e); - return 1; - } + Some(r) if is_semver_constraint(r) => match resolve_semver_ref(&url, r) { + Ok(tag) => { + println!("resolved semver '{}' → {}", r, tag); + Some(tag) } - } + Err(e) => { + eprintln!("error: {}", e); + return 1; + } + }, _ => None, }; let git_ref: &str = match &resolved_owned { @@ -325,9 +363,120 @@ pub fn cmd_add(spec: &str) -> i32 { println!("added {owner}/{repo} @ {sha}"); println!(" cache: {}", dest.display()); + + // Mark as visited before recursing so self-referential packages don't loop. + visited.insert(slug.clone()); + + // Walk transitive dependencies declared in the package's .ilo files. + stack.push(slug.clone()); + let deps = collect_pkg_deps(&dest); + for dep_slug in deps { + let rc = add_recursive(&dep_slug, visited, stack); + if rc != 0 { + stack.pop(); + return rc; + } + } + stack.pop(); + 0 } +/// Scan all `*.ilo` files in `pkg_dir` (non-recursively, top-level only) and +/// collect every `use "owner/repo[/...]"` path that looks like a package +/// reference (i.e. passes `is_pkg_path`). +/// +/// Returns deduplicated `owner/repo` slugs (sub-paths stripped). +fn collect_pkg_deps(pkg_dir: &Path) -> Vec { + let read_dir = match std::fs::read_dir(pkg_dir) { + Ok(rd) => rd, + Err(_) => return Vec::new(), + }; + + let mut deps: Vec = Vec::new(); + let mut seen: HashSet = HashSet::new(); + + for entry in read_dir.flatten() { + let path = entry.path(); + if path.extension().and_then(|e| e.to_str()) != Some("ilo") { + continue; + } + let Ok(source) = std::fs::read_to_string(&path) else { + continue; + }; + for dep in extract_use_pkg_slugs(&source) { + if seen.insert(dep.clone()) { + deps.push(dep); + } + } + } + + deps +} + +/// Parse `use "..."` statements from ilo source text and return package slugs. +/// +/// Uses a lightweight string scan rather than the full parser so `pkg.rs` +/// stays dependency-free from the lexer/parser crates. The grammar for a +/// `use` path is unambiguous: it always appears as a double-quoted string +/// immediately after the `use` keyword. +/// +/// Only paths that pass `is_pkg_path` are returned; local paths (starting +/// with `.` or `/`) are ignored. Sub-paths are truncated to `owner/repo`. +fn extract_use_pkg_slugs(source: &str) -> Vec { + let mut slugs = Vec::new(); + let chars = source.char_indices(); + + for (i, ch) in chars { + // Look for the token `use` as a whole word followed by whitespace and + // a `"`. We scan forward when we see a `u`. + if ch != 'u' { + continue; + } + // Check we're at a word boundary: previous char (if any) must not be + // alphanumeric/_ — use the byte index `i`. + if i > 0 { + let prev = source[..i].chars().next_back().unwrap_or(' '); + if prev.is_alphanumeric() || prev == '_' { + continue; + } + } + // Match `se` next. + let rest = &source[i..]; + if !rest.starts_with("use") { + continue; + } + let after_use = &rest[3..]; + // Character after `use` must be whitespace or end-of-input (word boundary). + let next_ch = after_use.chars().next().unwrap_or(' '); + if next_ch.is_alphanumeric() || next_ch == '_' { + continue; + } + // Skip whitespace, then expect `"`. + let trimmed = after_use.trim_start_matches([' ', '\t']); + if !trimmed.starts_with('"') { + continue; + } + // Extract the string content up to the closing `"`. + let inner = &trimmed[1..]; + let end = inner.find('"').unwrap_or(inner.len()); + let path = &inner[..end]; + + if !is_pkg_path(path) { + continue; + } + + // Truncate to owner/repo (drop any sub-path). + let slug = path.splitn(3, '/').take(2).collect::>().join("/"); + + if slug.contains('/') { + slugs.push(slug); + } + } + + slugs +} + // ── `ilo update` ────────────────────────────────────────────────────────────── /// Re-fetch one or all packages from the lockfile. @@ -506,4 +655,83 @@ mod tests { // (Local ilo files must use a leading `./` to be unambiguous.) assert!(is_pkg_path("relative/no-ext-dir")); } + + // ── extract_use_pkg_slugs tests ──────────────────────────────────────────── + + #[test] + fn extract_use_finds_pkg_dep() { + let source = r#"use "myorg/helpers""#; + assert_eq!(extract_use_pkg_slugs(source), vec!["myorg/helpers"]); + } + + #[test] + fn extract_use_finds_sub_path_and_truncates_to_slug() { + let source = r#"use "myorg/helpers/utils.ilo""#; + assert_eq!(extract_use_pkg_slugs(source), vec!["myorg/helpers"]); + } + + #[test] + fn extract_use_ignores_local_paths() { + let source = r#"use "./lib.ilo" +use "../shared.ilo" +use "/abs/path.ilo""#; + assert!(extract_use_pkg_slugs(source).is_empty()); + } + + #[test] + fn extract_use_returns_all_occurrences() { + // extract_use_pkg_slugs does not deduplicate — that is collect_pkg_deps' + // responsibility. Verify raw extraction returns one slug per use statement. + let source = r#"use "myorg/helpers" +use "myorg/helpers/sub.ilo""#; + assert_eq!( + extract_use_pkg_slugs(source), + vec!["myorg/helpers", "myorg/helpers"] + ); + } + + #[test] + fn extract_use_finds_multiple_deps() { + let source = r#"use "org1/pkg1" +use "org2/pkg2""#; + let mut got = extract_use_pkg_slugs(source); + got.sort(); + assert_eq!(got, vec!["org1/pkg1", "org2/pkg2"]); + } + + #[test] + fn extract_use_ignores_non_use_keyword() { + // `fuse` and `reuse` should not match. + let source = r#"fuse "org/pkg1" +reuse "org/pkg2""#; + assert!(extract_use_pkg_slugs(source).is_empty()); + } + + #[test] + fn extract_use_multiline_source() { + let source = "add a b; use \"myorg/math\"; mul x y"; + assert_eq!(extract_use_pkg_slugs(source), vec!["myorg/math"]); + } + + // ── cycle detection ───────────────────────────────────────────────────────── + + #[test] + fn add_recursive_detects_self_cycle() { + let mut visited = std::collections::HashSet::new(); + // Simulate pkg already on the stack (mid-resolution of itself). + let mut stack = vec!["selfpkg/lib".to_string()]; + // Trying to add selfpkg/lib again should detect the cycle. + let rc = add_recursive("selfpkg/lib", &mut visited, &mut stack); + assert_eq!(rc, 1); + } + + #[test] + fn add_recursive_skips_already_visited() { + let mut visited = std::collections::HashSet::new(); + visited.insert("myorg/helpers".to_string()); + let mut stack = Vec::new(); + // Should return 0 immediately (already resolved, no network call). + let rc = add_recursive("myorg/helpers", &mut visited, &mut stack); + assert_eq!(rc, 0); + } }