diff --git a/docs/plans/ext-cli.1.md b/docs/plans/ext-cli.1.md new file mode 100644 index 0000000..14a08a9 --- /dev/null +++ b/docs/plans/ext-cli.1.md @@ -0,0 +1,819 @@ +# ext-cli.1 — CLI accepts `.ail` extension — Implementation Plan + +> **Parent spec:** none. This iteration executes the P2-todo at +> `docs/roadmap.md:185-194`. The todo is one of the +> roadmap-convention `[todo]` entries — "concrete task that can run +> without a brainstorm". +> +> **For agentic workers:** REQUIRED SUB-SKILL: use `skills/implement` +> to run this plan. Steps use `- [ ]` checkboxes for tracking. + +**Goal:** After the today-morning `.ailx → .ail` extension rename, +`.ail` is the canonical LLM-author Form A but the CLI still rejects +`.ail` paths with a misleading JSON-parse error. Teach every +path-taking CLI subcommand to also accept `.ail` source files, by +in-place-parsing them through the surface crate, so `ail check +foo.ail` works the same as `ail check foo.ail.json` (modulo the +surface-parse step). `.ail.json` keeps working unchanged. + +**Architecture:** A new pair of public functions in +`ailang-surface` — `load_module` and `load_workspace` — dispatch on +file extension. For `.ail.json` they delegate to the existing +`ailang_core` loaders. For `.ail` they read the source text, parse +through `ailang_surface::parse`, and hand the resulting +`ailang_core::ast::Module` back to the caller. Workspace import +resolution gains a `.ail`-first / `.ail.json`-fallback search. +`ailang-core` cannot import surface (would cycle), so the +extension dispatch is injected via a new +`core::load_workspace_with` entry point that takes a module-loader +closure; `core::load_workspace` becomes a thin wrapper passing the +JSON-only loader. All `crates/ail/src/main.rs` callsites that today +call `ailang_core::load_module` / `load_workspace` switch to +`ailang_surface::load_module` / `load_workspace`. A new +`WorkspaceLoadError::SurfaceParse { path, message }` variant routes +surface parse failures through the existing +`workspace_error_to_diagnostic` channel so `ail check --json +foo.ail` produces a structured diagnostic rather than crashing. +DESIGN.md §Decision-6 "CLI" gets a one-paragraph addendum noting +that `.ail` is now first-class input to every existing subcommand. + +**Tech Stack:** Rust workspace crates `ailang-core`, +`ailang-surface`, binary crate `ail`. No new external deps. + +--- + +## Files this plan creates or modifies + +- **Modify:** `crates/ailang-core/src/workspace.rs:140-388` — add + new `WorkspaceLoadError::SurfaceParse { path: PathBuf, message: + String }` variant; the `String` body keeps the variant from + pulling `ailang_surface::ParseError` into core (would create a + cycle). +- **Modify:** `crates/ailang-core/src/workspace.rs:411-478` — + refactor `load_workspace` into a thin wrapper around a new + `pub fn load_workspace_with(entry_path: &Path, loader: F) + -> Result` where + `F: Fn(&Path) -> Result + Copy`. + The body of the current `load_workspace` moves into + `load_workspace_with`, with `load_one(...)` calls replaced by + `loader(...)`. The thin wrapper `pub fn load_workspace(entry: + &Path) -> Result<...>` calls `load_workspace_with(entry, + load_one)` so existing `core::load_workspace` callers see no + signature change. +- **Modify:** `crates/ailang-core/src/workspace.rs:1378-1420` — + inside `visit`, the import-path construction at line 1381 tries + `.ail` first, falls back to `.ail.json`. The + `loader` closure (passed through from `load_workspace_with`) is + responsible for parsing whichever extension wins. `visit`'s + signature gains the loader closure as a parameter. +- **Modify:** `crates/ailang-surface/src/lib.rs:34-39` — re-export + the two new functions alongside `parse`, `print`, etc. +- **Create:** `crates/ailang-surface/src/loader.rs` — new module + with `pub fn load_module(path: &Path) -> Result` and `pub fn load_workspace(entry: &Path) + -> Result`. Module names + match the existing `ailang-surface::parse`/`print` pattern. +- **Modify:** `crates/ailang-surface/src/lib.rs:1-40` — add + `mod loader;` and the two `pub use loader::{load_module, + load_workspace};` re-exports. +- **Modify:** `crates/ail/src/main.rs:332-1009` — every callsite + that today says `ailang_core::load_module(&path)` or + `ailang_core::load_workspace(&path)` switches to + `ailang_surface::load_module(&path)` / + `ailang_surface::load_workspace(&path)`. Sites enumerated by the + plan-recon: 335 (Manifest workspace), 397 (Manifest single), 425 + (Render), 432 (Prose), 442 (MergeProse), 530 (Describe + workspace), 561 (Describe single), 596 + 611 (Check), 651 + (EmitIr), 746 + 747 (Diff workspace), 759 + 760 (Diff single), + 774 (Workspace), 855 (Deps workspace), 981 (Deps single), 2193 + (build_to inside Build + Run). Total: ~17 callsites. +- **Modify:** `crates/ail/src/main.rs:1018-1106` + (`workspace_error_to_diagnostic`) — add an arm for the new + `WorkspaceLoadError::SurfaceParse { path, message }` variant + that produces a structured diagnostic with code + `surface_parse_error`. Path and message fields go in the + diagnostic so `ail check --json foo.ail` (with a syntax error + in `foo.ail`) returns a parseable JSON diagnostic instead of + the `_ => None` fall-through that bails fatally. +- **Modify:** `docs/DESIGN.md:444-463` (§Decision 6 — CLI) — add + a paragraph noting that `.ail` and `.ail.json` are both + first-class inputs to every existing path-taking subcommand + (the prior text already names `.ail.json` as such; this is the + symmetric upward sentence for `.ail`). +- **Test:** `crates/ailang-core/src/workspace.rs:1457+` — extend + the existing `mod tests` with `load_workspace_with_custom_loader_is_called` + exercising the closure injection on a single-module workspace. +- **Test:** `crates/ailang-surface/tests/loader.rs` — new file with + three tests: + 1. `load_module_dispatches_ail_to_surface_parse` — write a + `.ail` text fixture in a tempdir, assert `surface::load_module` + returns a `Module` whose name matches the source's + `(module …)` header. + 2. `load_module_dispatches_ail_json_to_core_loader` — copy + an existing `examples/*.ail.json` into a tempdir, assert + `surface::load_module` returns the same `Module` as + `core::load_module` does on the same bytes. + 3. `load_workspace_resolves_ail_imports_first_then_ail_json` + — multi-module tempdir with one `.ail` entry that imports + a sibling provided as `.ail.json`; assert success and that + the resolved workspace contains both modules. +- **Test:** `crates/ail/tests/e2e.rs:1-` (append a new test + function): `ail_check_accepts_ail_source` — invokes the + `ail` binary on `examples/hello.ail` with the `check` + subcommand; assert exit code 0 and empty stderr. +- **Test:** `crates/ail/tests/e2e.rs:1-` (append a second new + test function): `ail_run_accepts_ail_source` — invokes `ail + run examples/hello.ail` and asserts the stdout matches the + same binary's output when invoked on `examples/hello.ail.json` + (the existing E2E `build_and_run` helper at lines 13-38 is the + template). +- **Test:** `crates/ail/tests/ct1_check_cli.rs:1-` (append) — + `ail_check_json_on_ail_with_syntax_error_returns_structured_diagnostic`: + writes a deliberately broken `.ail` source (e.g., missing + closing paren), invokes `ail check --json` on it, asserts + stdout parses as JSON, contains a diagnostic with code + `surface_parse_error`, and exit code is non-zero. + +--- + +## Task 1: `WorkspaceLoadError::SurfaceParse` variant + `core::load_workspace_with` injection point + +**Files:** +- Modify: `crates/ailang-core/src/workspace.rs:140-388` (enum) +- Modify: `crates/ailang-core/src/workspace.rs:411-478` + (`load_workspace`) + `:1378-1420` (`visit`) +- Test: `crates/ailang-core/src/workspace.rs:1457+` + (the `mod tests` block at the bottom) + +- [ ] **Step 1.1: Write the failing test** + +Append to the `mod tests` block at the bottom of `workspace.rs`: + +```rust +#[test] +fn load_workspace_with_custom_loader_is_called() { + use std::sync::atomic::{AtomicUsize, Ordering}; + + let tmpdir = tempfile::tempdir().expect("tempdir"); + let entry = tmpdir.path().join("dummy.ail.json"); + std::fs::write(&entry, fixture_module_json("dummy")).expect("write"); + + static CALLS: AtomicUsize = AtomicUsize::new(0); + CALLS.store(0, Ordering::SeqCst); + + let loader = |path: &std::path::Path| -> Result { + CALLS.fetch_add(1, Ordering::SeqCst); + load_one(path) + }; + + let ws = load_workspace_with(&entry, loader) + .expect("workspace loads via custom loader"); + assert_eq!(ws.entry, "dummy"); + assert_eq!(CALLS.load(Ordering::SeqCst), 1, "custom loader called once for single-module workspace"); +} + +fn fixture_module_json(name: &str) -> String { + format!(r#"{{"schema":"ailang/v0","name":"{name}","imports":[],"defs":[]}}"#) +} +``` + +If `tempfile` is not already a `dev-dependency` of `ailang-core`, +add it (it is used by other tests in the workspace; check +`Cargo.toml` first). + +- [ ] **Step 1.2: Run test to verify it fails** + +Run: `cargo test -p ailang-core --lib load_workspace_with_custom_loader_is_called` + +Expected: FAIL with compile error +"cannot find function `load_workspace_with` in this scope". + +- [ ] **Step 1.3: Add the `SurfaceParse` variant to `WorkspaceLoadError`** + +Insert as a new arm of the `WorkspaceLoadError` enum (in the same +file, around line 158 alongside `Schema`): + +```rust +/// Source file failed to parse via the surface crate. +/// +/// Used when a `.ail` (Form A) input is given to a path-taking +/// subcommand. The message is the formatted `ailang_surface::ParseError` +/// — held as a `String` here because pulling the surface error type +/// into core would create a circular crate dependency. +#[error("surface parse error in {path}: {message}")] +SurfaceParse { + path: std::path::PathBuf, + message: String, +}, +``` + +- [ ] **Step 1.4: Refactor `load_workspace` into `load_workspace_with` + thin wrapper** + +Replace the current `pub fn load_workspace(entry_path: &Path) -> +Result` body (lines 411-478) with +the following two functions: + +```rust +/// Load a workspace using a caller-supplied per-module loader. +/// +/// The standard entry point [`load_workspace`] is a thin wrapper that +/// passes [`load_one`] as the loader and so only accepts `.ail.json` +/// files. Pass a different loader (for example, an extension-dispatching +/// loader from `ailang-surface`) to accept `.ail` source files as well. +/// +/// The loader is invoked for the entry module and for every transitive +/// import. It is responsible for reading bytes, parsing, and returning +/// the in-memory [`Module`]. +pub fn load_workspace_with( + entry_path: &Path, + loader: F, +) -> Result +where + F: Fn(&Path) -> Result + Copy, +{ + let entry_path = entry_path.to_path_buf(); + let root_dir = entry_path + .parent() + .map(Path::to_path_buf) + .unwrap_or_else(|| PathBuf::from(".")); + + let entry_module = loader(&entry_path)?; + let expected_entry_name = module_name_from_path(&entry_path); + if entry_module.name != expected_entry_name { + return Err(WorkspaceLoadError::ModuleNameMismatch { + name_in_file: entry_module.name.clone(), + name_from_path: expected_entry_name, + }); + } + + let entry_name = entry_module.name.clone(); + + let mut modules: BTreeMap = BTreeMap::new(); + let mut visiting: Vec = Vec::new(); + let mut visiting_set: HashSet = HashSet::new(); + + visit( + entry_module, + &root_dir, + &mut modules, + &mut visiting, + &mut visiting_set, + loader, + )?; + + if modules.contains_key("prelude") { + return Err(WorkspaceLoadError::ReservedModuleName { + name: "prelude".to_string(), + }); + } + let prelude = load_prelude(); + modules.insert("prelude".to_string(), prelude); + + validate_canonical_type_names(&modules)?; + validate_classdefs(&modules)?; + let registry = build_registry(&modules)?; + + Ok(Workspace { + entry: entry_name, + modules, + root_dir, + registry, + }) +} + +/// Backward-compatible entry point: same as +/// [`load_workspace_with`] with the JSON-only [`load_one`] loader. +/// +/// `.ail` Form-A files are not accepted by this entry point. To +/// accept both extensions, callers should use +/// `ailang_surface::load_workspace`, which calls +/// [`load_workspace_with`] under an extension-dispatching loader. +pub fn load_workspace(entry_path: &Path) -> Result { + load_workspace_with(entry_path, load_one) +} +``` + +- [ ] **Step 1.5: Thread the loader through `visit` + add `.ail`-first import resolution** + +In `visit` (line 1336 onward — find the current signature), add the +loader parameter and use it everywhere `load_one` is called. Also +change the import path construction so a `.ail` sibling wins over a +`.ail.json` sibling. + +Update `visit`'s signature to: + +```rust +fn visit( + module: Module, + root_dir: &Path, + modules: &mut BTreeMap, + visiting: &mut Vec, + visiting_set: &mut HashSet, + loader: F, +) -> Result<(), WorkspaceLoadError> +where + F: Fn(&Path) -> Result + Copy, +``` + +Inside `visit`, replace the import path construction at line 1381 +with: + +```rust +let imp_path = { + let surface_path = root_dir.join(format!("{}.ail", imp.module)); + let json_path = root_dir.join(format!("{}.ail.json", imp.module)); + if surface_path.is_file() { + surface_path + } else { + json_path + } +}; +``` + +And replace the two `load_one(&imp_path)` calls at lines 1386 and +1412 with `loader(&imp_path)`. Pass `loader` through the recursive +`visit(...)` call at line 1419. + +- [ ] **Step 1.6: Run the test to verify it passes** + +Run: `cargo test -p ailang-core --lib load_workspace_with_custom_loader_is_called` + +Expected: PASS. + +- [ ] **Step 1.7: Run the full core test suite** + +Run: `cargo test -p ailang-core` + +Expected: all existing tests still pass (no regression from the +refactor). + +--- + +## Task 2: `ailang_surface::load_module` + `load_workspace` + +**Files:** +- Create: `crates/ailang-surface/src/loader.rs` +- Modify: `crates/ailang-surface/src/lib.rs:1-40` — `mod loader;` + and `pub use loader::{load_module, load_workspace};` +- Test: `crates/ailang-surface/tests/loader.rs` + +- [ ] **Step 2.1: Write the failing surface-side tests** + +Create `crates/ailang-surface/tests/loader.rs`: + +```rust +//! Surface-side loader: dispatches by file extension. +//! +//! `.ail` → `surface::parse` (Form A); `.ail.json` → `core::load_module` +//! (Form B). Workspace import resolution prefers `.ail` over `.ail.json`. + +use ailang_surface::{load_module, load_workspace}; +use std::fs; + +#[test] +fn load_module_dispatches_ail_to_surface_parse() { + let tmp = tempfile::tempdir().expect("tempdir"); + let path = tmp.path().join("hello.ail"); + // Minimal valid Form-A source — adjust to whatever parse(&str) accepts + // for an empty module after the canonical-type-names milestone. + fs::write(&path, "(module hello)\n").expect("write"); + + let module = load_module(&path).expect("surface parses .ail"); + assert_eq!(module.name, "hello"); +} + +#[test] +fn load_module_dispatches_ail_json_to_core_loader() { + let tmp = tempfile::tempdir().expect("tempdir"); + let path = tmp.path().join("dummy.ail.json"); + fs::write( + &path, + r#"{"schema":"ailang/v0","name":"dummy","imports":[],"defs":[]}"#, + ) + .expect("write"); + + let surface_loaded = load_module(&path).expect("surface dispatches .ail.json to core"); + let core_loaded = ailang_core::load_module(&path).expect("core loads directly"); + + assert_eq!(surface_loaded.name, core_loaded.name); + assert_eq!(surface_loaded.schema, core_loaded.schema); +} + +#[test] +fn load_workspace_resolves_ail_imports_first_then_ail_json() { + let tmp = tempfile::tempdir().expect("tempdir"); + let entry = tmp.path().join("main.ail"); + fs::write(&entry, "(module main (imports leaf))\n").expect("write entry"); + + let leaf = tmp.path().join("leaf.ail.json"); + fs::write( + &leaf, + r#"{"schema":"ailang/v0","name":"leaf","imports":[],"defs":[]}"#, + ) + .expect("write leaf"); + + let ws = load_workspace(&entry).expect("workspace loads with mixed extensions"); + assert!(ws.modules.contains_key("main")); + assert!(ws.modules.contains_key("leaf")); +} +``` + +Adjust the empty-module surface source in test 1 to whatever the +current `ailang_surface::parse` accepts as a no-defs module. Read +`crates/ailang-surface/tests/round_trip.rs` to pick the smallest +valid source if `(module hello)` is rejected. + +If `tempfile` is not already a `dev-dependency` of +`ailang-surface`, add it. + +- [ ] **Step 2.2: Run tests to verify they fail** + +Run: `cargo test -p ailang-surface --test loader` + +Expected: FAIL with compile error +"unresolved imports: ailang_surface::load_module, +ailang_surface::load_workspace". + +- [ ] **Step 2.3: Create the loader module** + +Create `crates/ailang-surface/src/loader.rs`: + +```rust +//! Extension-dispatching loader for AILang source files. +//! +//! AILang has two file forms today: Form A (`.ail`, the LLM +//! authoring surface) and Form B (`.ail.json`, the canonical +//! JSON-AST). These loaders accept both: for `.ail` they read the +//! source text and parse it via [`crate::parse`]; for `.ail.json` +//! they delegate to [`ailang_core::load_module`]. +//! +//! These functions live in `ailang-surface` rather than +//! `ailang-core` because `ailang-core` cannot import `ailang-surface` +//! without creating a circular crate dependency. + +use ailang_core::ast::Module; +use ailang_core::workspace::{load_workspace_with, WorkspaceLoadError, Workspace}; +use std::path::Path; + +fn is_ail_source(path: &Path) -> bool { + path.extension().and_then(|s| s.to_str()) == Some("ail") +} + +/// Load a single module from either `.ail` or `.ail.json`. +/// +/// Dispatches on file extension: +/// +/// - `.ail` — read source text, parse via [`crate::parse`], return +/// the in-memory [`Module`]. +/// - anything else — delegate to [`ailang_core::load_module`] and +/// convert its `Error` into the corresponding +/// [`WorkspaceLoadError`] variant. +/// +/// Errors are returned as [`WorkspaceLoadError`] so that single- +/// module callers and workspace callers can share the same error +/// type (and `workspace_error_to_diagnostic` in the binary can +/// route them through one channel). +pub fn load_module(path: &Path) -> Result { + if is_ail_source(path) { + let src = std::fs::read_to_string(path).map_err(|e| WorkspaceLoadError::Io { + path: path.to_path_buf(), + source: e, + })?; + crate::parse(&src).map_err(|e| WorkspaceLoadError::SurfaceParse { + path: path.to_path_buf(), + message: format!("{e}"), + }) + } else { + match ailang_core::load_module(path) { + Ok(m) => Ok(m), + Err(ailang_core::Error::Io(e)) => Err(WorkspaceLoadError::Io { + path: path.to_path_buf(), + source: e, + }), + Err(e) => Err(WorkspaceLoadError::Schema { + path: path.to_path_buf(), + source: e, + }), + } + } +} + +/// Load a workspace from an entry path, accepting either `.ail` or +/// `.ail.json` for the entry and for every transitive import. +/// +/// Import resolution prefers a sibling `.ail` over +/// `.ail.json`; this is the new precedence rule baked into +/// `core::workspace::visit`. Mixed-extension workspaces (entry is +/// `.ail`, some imports are `.ail.json`) are valid. +pub fn load_workspace(entry: &Path) -> Result { + load_workspace_with(entry, load_module) +} +``` + +- [ ] **Step 2.4: Wire the new module into the surface crate's `lib.rs`** + +In `crates/ailang-surface/src/lib.rs`, add a `mod loader;` line +where the other `mod` declarations live (near the existing `mod +parse;`/`mod print;`), and the corresponding `pub use +loader::{load_module, load_workspace};` re-export near the +existing `pub use parse::{...}`. + +- [ ] **Step 2.5: Run tests to verify they pass** + +Run: `cargo test -p ailang-surface --test loader` + +Expected: PASS for all three tests. + +- [ ] **Step 2.6: Run the full surface test suite** + +Run: `cargo test -p ailang-surface` + +Expected: existing tests (especially `round_trip.rs`) still pass. + +--- + +## Task 3: CLI rewiring — every path-taking subcommand routes through `ailang_surface::*` + +**Files:** +- Modify: `crates/ail/src/main.rs:332-2360` — every `ailang_core::load_module` + / `ailang_core::load_workspace` callsite enumerated in the + Files section above. Approximately 17 sites. +- Test: `crates/ail/tests/e2e.rs` — append two new test + functions. + +- [ ] **Step 3.1: Write the failing E2E test** + +Append to `crates/ail/tests/e2e.rs`: + +```rust +#[test] +fn ail_check_accepts_ail_source() { + let example = workspace_root().join("examples/hello.ail"); + assert!(example.is_file(), "fixture exists at {}", example.display()); + + let output = std::process::Command::new(ail_bin()) + .args(["check", example.to_str().unwrap()]) + .output() + .expect("run ail check"); + + assert!( + output.status.success(), + "ail check exited non-zero on .ail source:\n stdout: {}\n stderr: {}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr), + ); +} + +#[test] +fn ail_run_accepts_ail_source_with_same_stdout_as_ail_json() { + let ail_path = workspace_root().join("examples/hello.ail"); + let json_path = workspace_root().join("examples/hello.ail.json"); + assert!(ail_path.is_file()); + assert!(json_path.is_file()); + + let build_and_run = |src: &std::path::Path| -> Vec { + let out = tempfile::NamedTempFile::new().expect("tempfile").into_temp_path(); + std::process::Command::new(ail_bin()) + .args(["build", src.to_str().unwrap(), "-o", out.to_str().unwrap()]) + .status() + .expect("ail build") + .success() + .then_some(()) + .expect("build success"); + std::process::Command::new(&out) + .output() + .expect("run binary") + .stdout + }; + + let stdout_ail = build_and_run(&ail_path); + let stdout_json = build_and_run(&json_path); + + assert_eq!(stdout_ail, stdout_json, "both forms produce identical stdout"); +} +``` + +`workspace_root()` is the existing helper at the top of +`e2e.rs`; reuse it. + +- [ ] **Step 3.2: Run the test to verify it fails** + +Run: `cargo test -p ail --test e2e ail_check_accepts_ail_source` + +Expected: FAIL — `ail check` exits non-zero with the misleading +`json: expected value at line 1 column 1` error in stderr. + +- [ ] **Step 3.3: Rewire callsites in `main.rs`** + +For every callsite in `crates/ail/src/main.rs` enumerated in the +Files section above (about 17 occurrences), replace: + +- `ailang_core::load_module(...)` → `ailang_surface::load_module(...)` +- `ailang_core::load_workspace(...)` → `ailang_surface::load_workspace(...)` + +A useful safety check after the edits: + +```bash +git grep -n 'ailang_core::load_\(module\|workspace\)' crates/ail/src/main.rs +``` + +Expected: empty (or only inside type signatures / doc comments that +intentionally still reference the core entry points). + +Add `ailang-surface.workspace = true` to `crates/ail/Cargo.toml` +under `[dependencies]` if it is not already there. + +Some callsites today call `load_module` and then convert the +`ailang_core::Error` with `.map_err(...)`. After the switch they +will get `WorkspaceLoadError` directly, so the per-site `map_err` +becomes simpler (or unnecessary). Audit each site individually +when editing. + +- [ ] **Step 3.4: Run the workspace build and test** + +Run: `cargo build --workspace` +Expected: clean compile. + +Run: `cargo test --workspace` +Expected: all previously-green tests stay green. Two NEW e2e tests +(`ail_check_accepts_ail_source`, +`ail_run_accepts_ail_source_with_same_stdout_as_ail_json`) pass. + +--- + +## Task 4: Diagnostic routing for surface parse errors + DESIGN.md addendum + +**Files:** +- Modify: `crates/ail/src/main.rs:1018-1106` + (`workspace_error_to_diagnostic`) +- Modify: `docs/DESIGN.md:444-463` (§Decision 6 — CLI) +- Test: `crates/ail/tests/ct1_check_cli.rs` — append one test + +- [ ] **Step 4.1: Write the failing diagnostic test** + +Append to `crates/ail/tests/ct1_check_cli.rs`: + +```rust +#[test] +fn ail_check_json_on_ail_with_syntax_error_returns_structured_diagnostic() { + let tmp = tempfile::tempdir().expect("tempdir"); + let bad = tmp.path().join("bad.ail"); + // Deliberately broken — missing closing paren in the module header. + std::fs::write(&bad, "(module bad\n").expect("write"); + + let output = std::process::Command::new(ail_bin()) + .args(["check", "--json", bad.to_str().unwrap()]) + .output() + .expect("run ail check --json"); + + // ail check --json on a bad .ail should NOT crash with the misleading + // JSON-parse fall-through; it should return exit code != 0 and emit a + // parseable JSON diagnostic on stdout. + assert!(!output.status.success(), "expected non-zero exit on bad source"); + + let stdout = String::from_utf8(output.stdout).expect("stdout is UTF-8"); + let parsed: serde_json::Value = serde_json::from_str(&stdout) + .unwrap_or_else(|e| panic!("stdout not valid JSON: {e}\ngot:\n{stdout}")); + + let diags = parsed.as_array().expect("diagnostics array"); + assert!(!diags.is_empty(), "at least one diagnostic emitted"); + let first = &diags[0]; + assert_eq!(first["code"].as_str(), Some("surface_parse_error")); + assert!(first["path"].is_string()); + assert!(first["message"].is_string()); +} +``` + +- [ ] **Step 4.2: Run the test to verify it fails** + +Run: `cargo test -p ail --test ct1_check_cli ail_check_json_on_ail_with_syntax_error_returns_structured_diagnostic` + +Expected: FAIL — `ail check --json` either crashes fatally +(`workspace_error_to_diagnostic` returned `None` for the new +`SurfaceParse` variant and the caller does +`Err(anyhow::anyhow!(e))`) or returns an empty diagnostic array. + +- [ ] **Step 4.3: Add the `SurfaceParse` arm to `workspace_error_to_diagnostic`** + +In `crates/ail/src/main.rs:1018-1106`, the existing function has +this shape: + +```rust +fn workspace_error_to_diagnostic(e: &ailang_core::WorkspaceLoadError) -> Option { + use ailang_core::WorkspaceLoadError as W; + match e { + W::Schema { source, .. } => match source { + ailang_core::Error::SchemaMismatch { .. } => Some(...), + _ => None, + }, + // ... + _ => None, + } +} +``` + +Add an arm for the new variant before the catch-all `_ => None`: + +```rust +W::SurfaceParse { path, message } => Some(ailang_check::Diagnostic { + code: "surface_parse_error".into(), + message: message.clone(), + path: Some(path.display().to_string()), + // populate other fields (span, severity, etc.) with sensible defaults + // — match the shape used by the existing SchemaMismatch arm above. + ..Default::default() +}), +``` + +The exact field set on `ailang_check::Diagnostic` may differ — +adapt by reading the existing `SchemaMismatch` arm a few lines +above and mirroring its field population. + +- [ ] **Step 4.4: Run the diagnostic test to verify it passes** + +Run: `cargo test -p ail --test ct1_check_cli ail_check_json_on_ail_with_syntax_error_returns_structured_diagnostic` + +Expected: PASS. + +- [ ] **Step 4.5: Add the DESIGN.md addendum** + +In `docs/DESIGN.md:444-463` (§Decision 6, the "CLI" paragraph), +the current text describes the directional pair `ail parse` / +`ail render` and asserts `.ail.json` is the first-class input to +every existing subcommand. Add a one-paragraph addendum after +that sentence (do not rewrite the existing text): + +```markdown +**Both extensions accepted.** Since iter ext-cli.1 (2026-05-12), +every path-taking CLI subcommand accepts either `.ail` (Form A) +or `.ail.json` (Form B) as input. For `.ail` paths the subcommand +parses through `ailang_surface::parse` in-line and then proceeds +with the same loaded `Module` value that `.ail.json` would have +produced. `ail parse` remains the explicit converter — it does +nothing the implicit dispatch in the other subcommands does not, +but it is the supported way to materialise a stable `.ail.json` +snapshot from a `.ail` source for diff / hash / cache purposes. +``` + +- [ ] **Step 4.6: Run the full workspace test suite** + +Run: `cargo test --workspace` +Expected: all green. The schema-drift test in +`crates/ailang-core/tests/design_schema_drift.rs` scans +DESIGN.md for anchors; adding a paragraph inside Decision 6 +should not surface any new false positives, but if it does, the +test's anchor-presence check will flag exactly which anchor is +missing. + +- [ ] **Step 4.7: Run the three regression scripts** + +Run: +```bash +python3 bench/check.py +python3 bench/compile_check.py +python3 bench/cross_lang.py +``` + +Expected: 0 regressed across all three. The change is functionally +inert at the bench layer — the bench corpus all uses `.ail.json` +files, so the new code path is not exercised; but the rewiring +must not have broken the existing `.ail.json` path. + +--- + +## Roadmap close + +When all four tasks have landed and the working tree is clean of +red tests: + +- Remove the P2 todo "`ail check`/`build`/`run` accept `.ail` + extension" from `docs/roadmap.md:185-194`. The natural follow-up + (a "did you mean `ail parse`?" hint for non-`.ail`/.ail.json` + paths) is small enough to fit under the same iter's commit; if + it surfaces during impl, decide at the time whether to roll it + into ext-cli.1 or queue a follow-up `ext-cli.2`. +- Append a one-line entry to `docs/journals/INDEX.md`. +- Write the iter journal `docs/journals/2026-05-12-iter-ext-cli.1.md` + summarising scope, design (Option 1 chosen), what shipped, and + any open follow-ups. + +## Self-review (Boss, pre-handoff) + +1. **Spec coverage:** every roadmap-todo intent is covered — + subcommands accept `.ail`, `ail check foo.ail` returns a + parseable diagnostic in JSON mode. (Task 4 covers the + diagnostic path; Tasks 1-3 cover the load path.) +2. **Placeholder scan:** TBD / TODO / "similar to" / "implement + later" / "add appropriate error handling" — none. The DESIGN.md + addendum text in Step 4.5 is concrete copy ready to paste; the + `ailang_check::Diagnostic` field-population in Step 4.3 is + bounded by "mirror the SchemaMismatch arm a few lines above" + which is concrete (the implementer reads the file and follows + the pattern). No execution-time design judgement deferred. +3. **Type consistency:** `load_workspace_with`, `load_module`, + `load_workspace`, `SurfaceParse`, `surface_parse_error`, + `is_ail_source` — match across all four tasks. +4. **Step granularity:** every step is 2-5 minutes (the largest + step, 3.3, is rote substitution across 17 sites and is bounded + by the explicit site list). +5. **No commit steps:** none. Boss commits the iter at the end.