fix(substrate): E3 — CLI contract compliance (exit codes + invoke Err)
Four audit findings on CLI contract violations per locked §Runtime schema:
- crit#7: invoke returned Ok with error payload — now returns
Err(InvokeError::NotImplemented) → exit 64
- crit#5: typed errors collapsed via anyhow::anyhow!("{e}") in kei-task —
replaced with CliError { code, msg } + classify_*_error helpers;
validation errors exit 2, storage errors exit 1 (spec-compliant)
- crit#8: lint.rs wikilink parser accepted [[[foo]] — strict parse_wikilink
from kei-atom-discovery used; emits finding for malformed entries
- crit#15: draft-07 detection was substring match — is_draft07_uri exact
match against canonical URIs only
Tests: 4/4 kei-runtime (was 2; +2 invoke exit-code tests) + 8/8 kei-task
(was 7; +1 empty-title exit-2 test) = 12/12 green.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
42fe08232e
commit
1bc6fbf4e3
6 changed files with 261 additions and 28 deletions
|
|
@ -15,14 +15,19 @@ pub enum InvokeError {
|
|||
AtomNotFound(String),
|
||||
InputParse(String),
|
||||
InputInvalid(String),
|
||||
NotImplemented { atom: String },
|
||||
}
|
||||
|
||||
impl std::fmt::Display for InvokeError {
|
||||
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
|
||||
match self {
|
||||
Self::AtomNotFound(id) => write!(f, "atom not found: {id}"),
|
||||
Self::InputParse(e) => write!(f, "input parse: {e}"),
|
||||
Self::InputInvalid(e) => write!(f, "input invalid: {e}"),
|
||||
Self::AtomNotFound(id) => write!(f, "no atom matching {id}"),
|
||||
Self::InputParse(e) => write!(f, "input rejected: {e}"),
|
||||
Self::InputInvalid(e) => write!(f, "input rejected: {e}"),
|
||||
Self::NotImplemented { atom } => write!(
|
||||
f,
|
||||
"invoke not yet wired for this atom ({atom}); use the underlying CLI directly"
|
||||
),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -32,24 +37,22 @@ impl std::error::Error for InvokeError {}
|
|||
/// MVP stub output — shape documents the boundary for downstream wire-up.
|
||||
#[derive(Debug, Serialize)]
|
||||
pub struct Output {
|
||||
pub error: String,
|
||||
pub atom: String,
|
||||
}
|
||||
|
||||
/// Invoke an atom by full ID with a JSON input string.
|
||||
///
|
||||
/// MVP contract: discover atom → parse input → validate against schema →
|
||||
/// return stub acknowledgement. Exec wire-up is a follow-up.
|
||||
/// return `NotImplemented` until Stream B wires the executor. The callers
|
||||
/// (main.rs) map `NotImplemented` to a dedicated non-zero exit code so CI
|
||||
/// scripts can distinguish "unimplemented" from "atom rejected input".
|
||||
pub fn invoke(root: &Path, atom_id: &str, input_json: &str) -> Result<Output, InvokeError> {
|
||||
let meta = find_atom(root, atom_id)?;
|
||||
let input: Value =
|
||||
serde_json::from_str(input_json).map_err(|e| InvokeError::InputParse(e.to_string()))?;
|
||||
validate_input(&meta.input_schema_path, &input)
|
||||
.map_err(|e| InvokeError::InputInvalid(e.to_string()))?;
|
||||
Ok(Output {
|
||||
error: "atom invocation not yet implemented — wire needs Stream B atom impls".to_string(),
|
||||
atom: atom_id.to_string(),
|
||||
})
|
||||
Err(InvokeError::NotImplemented { atom: atom_id.to_string() })
|
||||
}
|
||||
|
||||
fn find_atom(root: &Path, atom_id: &str) -> Result<AtomMeta, InvokeError> {
|
||||
|
|
|
|||
|
|
@ -149,18 +149,33 @@ fn check_draft07(schema_path: &Path, key: &str, errs: &mut Vec<String>) {
|
|||
return;
|
||||
};
|
||||
let draft = json.get("$schema").and_then(|v| v.as_str()).unwrap_or("");
|
||||
if !draft.contains("draft-07") {
|
||||
if !is_draft07_uri(draft) {
|
||||
errs.push(format!("{key} schema missing draft-07 $schema"));
|
||||
}
|
||||
}
|
||||
|
||||
/// Exact-match check for the draft-07 meta-schema URI.
|
||||
///
|
||||
/// Accepts the canonical URI with or without the trailing `#` fragment.
|
||||
/// A substring check (`contains("draft-07")`) would falsely match URIs like
|
||||
/// `http://example.com/draft-07-tutorial.html` — forbidden by §Validation.
|
||||
fn is_draft07_uri(uri: &str) -> bool {
|
||||
uri == "http://json-schema.org/draft-07/schema#"
|
||||
|| uri == "http://json-schema.org/draft-07/schema"
|
||||
}
|
||||
|
||||
fn check_related(fm: &YamlValue, known: &HashSet<String>, errs: &mut Vec<String>) {
|
||||
let Some(seq) = fm.get("related").and_then(|v| v.as_sequence()) else {
|
||||
return;
|
||||
};
|
||||
for entry in seq {
|
||||
let Some(link) = entry.as_str() else { continue };
|
||||
let inner = link.trim_start_matches("[[").trim_end_matches("]]");
|
||||
let Some(inner) = parse_wikilink(link) else {
|
||||
errs.push(format!(
|
||||
"related entry {link} is not a valid [[atom-id]] wikilink"
|
||||
));
|
||||
continue;
|
||||
};
|
||||
if inner.starts_with("rules/") {
|
||||
continue;
|
||||
}
|
||||
|
|
@ -169,3 +184,17 @@ fn check_related(fm: &YamlValue, known: &HashSet<String>, errs: &mut Vec<String>
|
|||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Strict `[[...]]` wikilink parse.
|
||||
///
|
||||
/// Returns the inner text only when the string starts with exactly `[[`
|
||||
/// and ends with exactly `]]`, with no extra brackets on either side
|
||||
/// and a non-empty body. Rejects malformed forms like `[[[foo]]`,
|
||||
/// `[[foo]]]`, `[[foo]`, `[foo]]`, and `[[]]`.
|
||||
fn parse_wikilink(raw: &str) -> Option<&str> {
|
||||
let inner = raw.strip_prefix("[[")?.strip_suffix("]]")?;
|
||||
if inner.is_empty() || inner.starts_with('[') || inner.ends_with(']') {
|
||||
return None;
|
||||
}
|
||||
Some(inner)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -4,10 +4,16 @@
|
|||
//! Default --root: `~/.claude/agents/_primitives/_rust`.
|
||||
|
||||
use clap::{Parser, Subcommand};
|
||||
use kei_runtime::invoke::InvokeError;
|
||||
use kei_runtime::{discover, invoke, lint};
|
||||
use std::path::PathBuf;
|
||||
use std::process::ExitCode;
|
||||
|
||||
/// Exit code returned when `invoke` lands on a not-yet-wired atom.
|
||||
/// Distinct from exit 2 (atom rejected input) so CI can branch.
|
||||
/// Chosen in the EX_USAGE range per `sysexits.h` convention.
|
||||
const EXIT_INVOKE_NOT_IMPLEMENTED: u8 = 64;
|
||||
|
||||
#[derive(Parser)]
|
||||
#[command(name = "kei-runtime", version, about = "Atom invocation runtime + schema linter")]
|
||||
struct Cli {
|
||||
|
|
@ -100,12 +106,25 @@ fn run_invoke(root: PathBuf, atom_id: String, input_arg: String) -> ExitCode {
|
|||
ExitCode::SUCCESS
|
||||
}
|
||||
Err(e) => {
|
||||
eprintln!("invoke: {e}");
|
||||
ExitCode::from(2)
|
||||
eprintln!("{e}");
|
||||
ExitCode::from(invoke_exit_code(&e))
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Map typed invoke errors to exit codes per locked §Runtime schema.
|
||||
///
|
||||
/// - `AtomNotFound | InputParse | InputInvalid` → 2 (atom error)
|
||||
/// - `NotImplemented` → 64 (CLI contract not yet honoured)
|
||||
fn invoke_exit_code(err: &InvokeError) -> u8 {
|
||||
match err {
|
||||
InvokeError::AtomNotFound(_)
|
||||
| InvokeError::InputParse(_)
|
||||
| InvokeError::InputInvalid(_) => 2,
|
||||
InvokeError::NotImplemented { .. } => EXIT_INVOKE_NOT_IMPLEMENTED,
|
||||
}
|
||||
}
|
||||
|
||||
fn load_input(arg: &str) -> Result<String, String> {
|
||||
if let Some(path) = arg.strip_prefix('@') {
|
||||
std::fs::read_to_string(path).map_err(|e| format!("read {path}: {e}"))
|
||||
|
|
|
|||
|
|
@ -0,0 +1,94 @@
|
|||
//! Integration test — `kei-runtime invoke` exit codes per §Runtime contract.
|
||||
//!
|
||||
//! - Unknown atom id → exit 2 (atom rejected)
|
||||
//! - Known atom, valid input → exit 64 (CLI contract not yet honoured / NotImplemented)
|
||||
//!
|
||||
//! The invoke binary is a stub (no executor wired yet). Previously it
|
||||
//! returned `Ok(...)` with an error payload → ExitCode::SUCCESS. Now every
|
||||
//! invocation path yields a typed `InvokeError`; main.rs maps variants to
|
||||
//! distinct exit codes so CI scripts can distinguish cases.
|
||||
|
||||
use std::fs;
|
||||
use std::path::Path;
|
||||
use std::process::Command;
|
||||
|
||||
const BIN: &str = env!("CARGO_BIN_EXE_kei-runtime");
|
||||
|
||||
fn write_atom(root: &Path, crate_name: &str, verb: &str) {
|
||||
let atoms = root.join(crate_name).join("atoms");
|
||||
let schemas = atoms.join("schemas");
|
||||
fs::create_dir_all(&schemas).unwrap();
|
||||
let input = format!("{verb}-input.json");
|
||||
let output = format!("{verb}-output.json");
|
||||
let input_schema = r#"{
|
||||
"$schema": "http://json-schema.org/draft-07/schema#",
|
||||
"type": "object",
|
||||
"properties": { "title": { "type": "string" } }
|
||||
}"#;
|
||||
let output_schema = r#"{
|
||||
"$schema": "http://json-schema.org/draft-07/schema#",
|
||||
"type": "object"
|
||||
}"#;
|
||||
fs::write(schemas.join(&input), input_schema).unwrap();
|
||||
fs::write(schemas.join(&output), output_schema).unwrap();
|
||||
let md = format!(
|
||||
r#"---
|
||||
atom: {crate_name}::{verb}
|
||||
kind: command
|
||||
version: "0.1.0"
|
||||
input:
|
||||
schema: schemas/{input}
|
||||
output:
|
||||
schema: schemas/{output}
|
||||
side_effects: []
|
||||
idempotent: true
|
||||
stability: stable
|
||||
---
|
||||
|
||||
# {crate_name}::{verb}
|
||||
"#,
|
||||
);
|
||||
fs::write(atoms.join(format!("{verb}.md")), md).unwrap();
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn invoke_atom_not_found_exits_2() {
|
||||
let tmp = tempfile::tempdir().unwrap();
|
||||
write_atom(tmp.path(), "kei-demo", "create");
|
||||
let out = Command::new(BIN)
|
||||
.arg("invoke")
|
||||
.arg("kei-demo::ghost")
|
||||
.arg("--input")
|
||||
.arg("{}")
|
||||
.arg("--root")
|
||||
.arg(tmp.path())
|
||||
.output()
|
||||
.expect("spawn kei-runtime");
|
||||
assert_eq!(out.status.code(), Some(2),
|
||||
"expected exit 2 on unknown atom; stderr: {}",
|
||||
String::from_utf8_lossy(&out.stderr));
|
||||
let stderr = String::from_utf8_lossy(&out.stderr);
|
||||
assert!(stderr.contains("no atom matching"),
|
||||
"expected 'no atom matching' in stderr: {stderr}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn invoke_not_implemented_exits_64() {
|
||||
let tmp = tempfile::tempdir().unwrap();
|
||||
write_atom(tmp.path(), "kei-demo", "create");
|
||||
let out = Command::new(BIN)
|
||||
.arg("invoke")
|
||||
.arg("kei-demo::create")
|
||||
.arg("--input")
|
||||
.arg(r#"{"title":"hello"}"#)
|
||||
.arg("--root")
|
||||
.arg(tmp.path())
|
||||
.output()
|
||||
.expect("spawn kei-runtime");
|
||||
assert_eq!(out.status.code(), Some(64),
|
||||
"expected exit 64 on NotImplemented; stderr: {}",
|
||||
String::from_utf8_lossy(&out.stderr));
|
||||
let stderr = String::from_utf8_lossy(&out.stderr);
|
||||
assert!(stderr.contains("invoke not yet wired"),
|
||||
"expected 'invoke not yet wired' in stderr: {stderr}");
|
||||
}
|
||||
|
|
@ -13,6 +13,31 @@ use kei_task::{Milestone, Store};
|
|||
use std::path::PathBuf;
|
||||
use std::process::ExitCode;
|
||||
|
||||
/// Typed error used by the CLI to carry both a message and an exit code.
|
||||
///
|
||||
/// Exit-code contract (§Runtime):
|
||||
/// - 2 — atom rejected input (validation / semantic error)
|
||||
/// - 1 — usage / IO / storage failure
|
||||
struct CliError {
|
||||
code: u8,
|
||||
msg: String,
|
||||
}
|
||||
|
||||
impl CliError {
|
||||
fn atom(msg: impl Into<String>) -> Self {
|
||||
Self { code: 2, msg: msg.into() }
|
||||
}
|
||||
fn io(msg: impl Into<String>) -> Self {
|
||||
Self { code: 1, msg: msg.into() }
|
||||
}
|
||||
}
|
||||
|
||||
impl From<anyhow::Error> for CliError {
|
||||
fn from(e: anyhow::Error) -> Self {
|
||||
Self::io(format!("{e:#}"))
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Parser)]
|
||||
#[command(name = "kei-task", version, about = "Task DAG CLI")]
|
||||
struct Cli {
|
||||
|
|
@ -41,13 +66,13 @@ fn db_path(cli_db: Option<PathBuf>) -> PathBuf {
|
|||
PathBuf::from(home).join(".claude/task/task.sqlite")
|
||||
}
|
||||
|
||||
fn run() -> anyhow::Result<()> {
|
||||
fn run() -> Result<(), CliError> {
|
||||
let cli = Cli::parse();
|
||||
let s = Store::open(&db_path(cli.db))?;
|
||||
dispatch(&s, cli.cmd)
|
||||
}
|
||||
|
||||
fn dispatch(s: &Store, cmd: Cmd) -> anyhow::Result<()> {
|
||||
fn dispatch(s: &Store, cmd: Cmd) -> Result<(), CliError> {
|
||||
match cmd {
|
||||
Cmd::Create { title, description, priority } =>
|
||||
cmd_create(s, title, description, priority),
|
||||
|
|
@ -63,16 +88,26 @@ fn dispatch(s: &Store, cmd: Cmd) -> anyhow::Result<()> {
|
|||
}
|
||||
}
|
||||
|
||||
fn cmd_create(s: &Store, title: String, description: String, priority: String) -> anyhow::Result<()> {
|
||||
fn cmd_create(s: &Store, title: String, description: String, priority: String) -> Result<(), CliError> {
|
||||
let out = atoms::create::run(s, atoms::create::Input {
|
||||
title, description, priority, milestone_id: None,
|
||||
}).map_err(|e| anyhow::anyhow!("{e}"))?;
|
||||
}).map_err(classify_create_error)?;
|
||||
println!("{}", out.id);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn cmd_update(s: &Store, id: i64, status: Option<String>, title: Option<String>) -> anyhow::Result<()> {
|
||||
let mut t = s.get_task(id)?.ok_or_else(|| anyhow::anyhow!("id {id} not found"))?;
|
||||
/// Per §Runtime contract: validation errors → exit 2, storage/IO → exit 1.
|
||||
fn classify_create_error(e: atoms::create::Error) -> CliError {
|
||||
match e {
|
||||
atoms::create::Error::InvalidTitle
|
||||
| atoms::create::Error::InvalidPriority(_) => CliError::atom(format!("{e}")),
|
||||
atoms::create::Error::StoreError(_) => CliError::io(format!("{e}")),
|
||||
}
|
||||
}
|
||||
|
||||
fn cmd_update(s: &Store, id: i64, status: Option<String>, title: Option<String>) -> Result<(), CliError> {
|
||||
let mut t = s.get_task(id)?
|
||||
.ok_or_else(|| CliError::atom(format!("TaskNotFound: id {id} not found")))?;
|
||||
if let Some(st) = status { t.status = st; }
|
||||
if let Some(ti) = title { t.title = ti; }
|
||||
s.update_task(&t)?;
|
||||
|
|
@ -80,45 +115,63 @@ fn cmd_update(s: &Store, id: i64, status: Option<String>, title: Option<String>)
|
|||
Ok(())
|
||||
}
|
||||
|
||||
fn cmd_add_dep(s: &Store, from_id: i64, to_id: i64, dep_type: String) -> anyhow::Result<()> {
|
||||
fn cmd_add_dep(s: &Store, from_id: i64, to_id: i64, dep_type: String) -> Result<(), CliError> {
|
||||
let dep_display = if dep_type.is_empty() { "blocks".to_string() } else { dep_type.clone() };
|
||||
atoms::add_dependency::run(s, atoms::add_dependency::Input {
|
||||
from: from_id, to: to_id, dep_type,
|
||||
}).map_err(|e| anyhow::anyhow!("{e}"))?;
|
||||
}).map_err(classify_add_dep_error)?;
|
||||
println!("dep: {} -> {} ({})", from_id, to_id, dep_display);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn cmd_graph(s: &Store) -> anyhow::Result<()> {
|
||||
/// Per §Runtime contract: semantic rejections → exit 2, storage/IO → exit 1.
|
||||
fn classify_add_dep_error(e: atoms::add_dependency::Error) -> CliError {
|
||||
match e {
|
||||
atoms::add_dependency::Error::SelfDependency
|
||||
| atoms::add_dependency::Error::InvalidDepType(_)
|
||||
| atoms::add_dependency::Error::CycleDetected => CliError::atom(format!("{e}")),
|
||||
atoms::add_dependency::Error::StoreError(_) => CliError::io(format!("{e}")),
|
||||
}
|
||||
}
|
||||
|
||||
fn cmd_graph(s: &Store) -> Result<(), CliError> {
|
||||
for e in list_edges(s)? {
|
||||
println!("{}\t-[{}]->\t{}", e.task_id, e.dep_type, e.depends_on);
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn cmd_chain(s: &Store, id: i64) -> anyhow::Result<()> {
|
||||
fn cmd_chain(s: &Store, id: i64) -> Result<(), CliError> {
|
||||
for did in dependency_chain(s, id)? { println!("{}", did); }
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn cmd_search(s: &Store, query: String, limit: i64) -> anyhow::Result<()> {
|
||||
fn cmd_search(s: &Store, query: String, limit: i64) -> Result<(), CliError> {
|
||||
let out = atoms::search::run(s, atoms::search::Input {
|
||||
query, limit: Some(limit),
|
||||
}).map_err(|e| anyhow::anyhow!("{e}"))?;
|
||||
}).map_err(classify_search_error)?;
|
||||
for t in out.results {
|
||||
println!("{}\t{}\t{}", t.id, t.status, t.title);
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn cmd_milestone(s: &Store, name: String, description: String) -> anyhow::Result<()> {
|
||||
/// Per §Runtime contract: query validation → exit 2, storage/IO → exit 1.
|
||||
fn classify_search_error(e: atoms::search::Error) -> CliError {
|
||||
match e {
|
||||
atoms::search::Error::InvalidQuery => CliError::atom(format!("{e}")),
|
||||
atoms::search::Error::StoreError(_) => CliError::io(format!("{e}")),
|
||||
}
|
||||
}
|
||||
|
||||
fn cmd_milestone(s: &Store, name: String, description: String) -> Result<(), CliError> {
|
||||
let id = create_milestone(s, &Milestone {
|
||||
name, description, ..Default::default() })?;
|
||||
println!("{}", id);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn cmd_link_milestone(s: &Store, task_id: i64, milestone_id: i64) -> anyhow::Result<()> {
|
||||
fn cmd_link_milestone(s: &Store, task_id: i64, milestone_id: i64) -> Result<(), CliError> {
|
||||
link_task_to_milestone(s, task_id, milestone_id)?;
|
||||
println!("linked {} -> milestone {}", task_id, milestone_id);
|
||||
Ok(())
|
||||
|
|
@ -127,6 +180,9 @@ fn cmd_link_milestone(s: &Store, task_id: i64, milestone_id: i64) -> anyhow::Res
|
|||
fn main() -> ExitCode {
|
||||
match run() {
|
||||
Ok(()) => ExitCode::SUCCESS,
|
||||
Err(e) => { eprintln!("kei-task: {e:#}"); ExitCode::from(1) }
|
||||
Err(CliError { code, msg }) => {
|
||||
eprintln!("{msg}");
|
||||
ExitCode::from(code)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
32
_primitives/_rust/kei-task/tests/exit_codes_smoke.rs
Normal file
32
_primitives/_rust/kei-task/tests/exit_codes_smoke.rs
Normal file
|
|
@ -0,0 +1,32 @@
|
|||
//! kei-task CLI exit-code smoke tests (§Runtime contract).
|
||||
//!
|
||||
//! Atom-layer errors (validation / semantic) → exit 2.
|
||||
//! Storage/IO errors → exit 1.
|
||||
//!
|
||||
//! `create --title ""` is the canonical validation-failure case: the
|
||||
//! atom's typed Error enum returns `InvalidTitle`, which main.rs maps
|
||||
//! to exit 2, NOT the old anyhow collapse at exit 1.
|
||||
|
||||
use std::process::Command;
|
||||
|
||||
const BIN: &str = env!("CARGO_BIN_EXE_kei-task");
|
||||
|
||||
#[test]
|
||||
fn create_empty_title_exits_2() {
|
||||
let tmp = tempfile::tempdir().unwrap();
|
||||
let db = tmp.path().join("task.sqlite");
|
||||
let out = Command::new(BIN)
|
||||
.arg("--db")
|
||||
.arg(&db)
|
||||
.arg("create")
|
||||
.arg("")
|
||||
.output()
|
||||
.expect("spawn kei-task");
|
||||
assert_eq!(out.status.code(), Some(2),
|
||||
"expected exit 2 on InvalidTitle; stdout: {}, stderr: {}",
|
||||
String::from_utf8_lossy(&out.stdout),
|
||||
String::from_utf8_lossy(&out.stderr));
|
||||
let stderr = String::from_utf8_lossy(&out.stderr);
|
||||
assert!(stderr.contains("InvalidTitle"),
|
||||
"expected 'InvalidTitle' in stderr: {stderr}");
|
||||
}
|
||||
Loading…
Reference in a new issue