From 835149028381a4286b0c3593d5e931f615cb511d Mon Sep 17 00:00:00 2001 From: A Tobey Date: Thu, 27 Aug 2026 08:49:34 -0400 Subject: [PATCH 1/5] Add the random builtin kaish has no $RANDOM, and the arithmetic rewrite that follows this change refuses $((RANDOM % 100)) with an error naming a builtin as the fix. That builtin has to exist first, so this adds it: random [--min N] [--max N] prints one integer chosen from min..=max, both inclusive (defaults 0 and 32767, matching bash's $RANDOM range). The output is typed, not stringified. The schema declares with_typed_substitution() and the result carries Value::Int, so $(random --max 6) binds a number and x=$(random --max 6); echo $((x + 1)) works without a cast. --json emits the bare number, not a string-wrapped envelope. Sampling reads 8 bytes from getrandom and maps them onto the range with Lemire's method (widen the multiply into 128 bits, reject the low slice that would otherwise skew one bucket) instead of draw % width, which is measurably biased whenever the range doesn't evenly divide 2^64. The width itself is carried in u128 so the full i64 span (--min i64::MIN --max i64::MAX) doesn't overflow computing it. A getrandom failure is a hard error, not a fallback value - a script that trusted random for a coin flip must never get a predictable substitute silently. Bounds and shape are enforced before any draw happens, each a curated exit-2 error naming the value and the fix: --min greater than --max, a non-integer bound (left to clap's own parse error), and a positional argument (random takes none - the fix always spells out the equivalent --max flag). random registers alphabetically in the builtin table between pwd and read. Co-Authored-By: Claude Fable 5 --- crates/kaish-kernel/src/tools/builtin/mod.rs | 2 + .../kaish-kernel/src/tools/builtin/random.rs | 275 ++++++++++++++++++ .../tests/random_builtin_tests.rs | 145 +++++++++ 3 files changed, 422 insertions(+) create mode 100644 crates/kaish-kernel/src/tools/builtin/random.rs create mode 100644 crates/kaish-kernel/tests/random_builtin_tests.rs diff --git a/crates/kaish-kernel/src/tools/builtin/mod.rs b/crates/kaish-kernel/src/tools/builtin/mod.rs index 85fd4ec3..73fbb26d 100644 --- a/crates/kaish-kernel/src/tools/builtin/mod.rs +++ b/crates/kaish-kernel/src/tools/builtin/mod.rs @@ -70,6 +70,7 @@ mod plan; mod printf; mod push; mod pwd; +mod random; mod read; mod readlink; mod realpath; @@ -325,6 +326,7 @@ pub fn register_builtins(registry: &mut ToolRegistry) { #[cfg(all(target_os = "linux", feature = "host"))] registry.register(kaish_tools_host::Ps); registry.register(pwd::Pwd); + registry.register(random::Random); registry.register(read::Read); registry.register(readlink::Readlink); registry.register(realpath::Realpath); diff --git a/crates/kaish-kernel/src/tools/builtin/random.rs b/crates/kaish-kernel/src/tools/builtin/random.rs new file mode 100644 index 00000000..7e037376 --- /dev/null +++ b/crates/kaish-kernel/src/tools/builtin/random.rs @@ -0,0 +1,275 @@ +//! random — Print one random integer from `--min` to `--max`, inclusive. +//! +//! kaish has no `$RANDOM`; this builtin is the typed replacement. `$(random +//! --max 100)` binds a `number`, not a string, so `$((x + 1))` works on the +//! result directly. +//! +//! # Examples +//! +//! ```kaish +//! random # 0..=32767, like bash's $RANDOM +//! random --max 6 # roll a die: 0..=6 +//! random --min -5 --max 5 # negative bounds are fine +//! x=$(random --max 6); echo $((x + 1)) +//! ``` + +use async_trait::async_trait; +use clap::{CommandFactory, Parser}; + +use crate::ast::Value; +use crate::interpreter::{value_to_string, ExecResult}; +use crate::tools::{schema_from_clap, ExecContext, GlobalFlags, Tool, ToolArgs, ToolCtx, ToolSchema}; + +/// Random tool: print one random integer, uniformly, from a range. +pub struct Random; + +/// `$RANDOM`-compatible default: bash's range is 0..=32767. +const DEFAULT_MIN: i64 = 0; +const DEFAULT_MAX: i64 = 32767; + +/// clap-derived argv layer for random. +#[derive(Parser, Debug)] +#[command( + name = "random", + about = "Print one random integer from --min to --max, inclusive. `$(random --max 100)` replaces bash's `$RANDOM`." +)] +struct RandomArgs { + /// Lowest value that can be returned, inclusive. Default 0. + #[arg(long = "min")] + min: Option, + + /// Highest value that can be returned, inclusive. Default 32767. + #[arg(long = "max")] + max: Option, + + #[command(flatten)] + global: GlobalFlags, +} + +#[async_trait] +impl Tool for Random { + fn name(&self) -> &str { + "random" + } + + fn schema(&self) -> ToolSchema { + schema_from_clap( + &RandomArgs::command(), + "random", + "Print one random integer from --min to --max, inclusive. `$(random --max 100)` replaces bash's `$RANDOM`.", + [ + ("Default range (like $RANDOM)", "random"), + ("Roll a die", "random --max 6"), + ("Capture and use", "x=$(random --max 6); echo $((x + 1))"), + ], + ) + .with_typed_substitution() + } + + async fn execute(&self, args: ToolArgs, ctx: &mut dyn ToolCtx) -> ExecResult { + let Some(ctx) = ctx.as_any_mut().downcast_mut::() else { + return ExecResult::failure(1, "internal error: kernel builtin requires ExecContext"); + }; + + // random takes no positional argument. Check before argv/clap ever + // sees one, so the curated error names the value the caller gave + // instead of clap's generic "unexpected argument". + if let Some(v) = args.positional.first() { + let value = value_to_string(v); + return ExecResult::failure( + 2, + format!("random: takes no positional argument; write `--max {value}`"), + ); + } + + let argv = match args.to_argv() { + Ok(v) => v, + Err(e) => return ExecResult::failure(2, format!("random: {e}")), + }; + let parsed = match RandomArgs::try_parse_from( + std::iter::once("random".to_string()).chain(argv), + ) { + Ok(p) => p, + Err(e) => return ExecResult::failure(2, format!("random: {e}")), + }; + parsed.global.apply(ctx); + + let min = parsed.min.unwrap_or(DEFAULT_MIN); + let max = parsed.max.unwrap_or(DEFAULT_MAX); + + if min > max { + return ExecResult::failure( + 2, + format!( + "random: --min {min} is greater than --max {max}; swap them or widen the range" + ), + ); + } + + let value = match draw_random(min, max) { + Ok(v) => v, + Err(e) => { + return ExecResult::failure( + 1, + format!("random: could not obtain system entropy: {e}"), + ); + } + }; + + ExecResult::success_with_data(format!("{value}\n"), Value::Int(value)) + } +} + +/// Draw one integer in `min..=max`, inclusive, uniformly at random from the +/// OS CSPRNG. +/// +/// No fallback: a `getrandom` failure is a hard error, not a predictable +/// substitute — a `random` call a script trusted for a coin flip must never +/// silently return a fixed or guessable value. +fn draw_random(min: i64, max: i64) -> Result { + loop { + let mut entropy = [0u8; 8]; + getrandom::fill(&mut entropy)?; + let draw = u64::from_le_bytes(entropy); + if let Some(value) = map_draw_to_range(draw, min, max) { + return Ok(value); + } + // Rejected below to avoid bias — redraw. + } +} + +/// Map one 64-bit draw onto `min..=max` (inclusive) via Lemire's method: +/// widen the multiply into 128 bits, then reject the low slice that would +/// otherwise make one bucket slightly more likely than the rest. This is +/// what `draw % width` gets wrong whenever `width` doesn't evenly divide +/// 2^64 — the plain modulo skews toward the low end of the range. +/// +/// `None` means the draw must be discarded and redrawn; the caller loops. +/// Pure and deterministic: the same `draw`, `min`, `max` always agree, so it +/// never itself touches the CSPRNG. +fn map_draw_to_range(draw: u64, min: i64, max: i64) -> Option { + debug_assert!(min <= max); + + // Width as u128: `max - min + 1` can be exactly 2^64 (the full i64 + // span), which does not fit a u64. + let width: u128 = (max as i128 - min as i128) as u128 + 1; + + if width > u64::MAX as u128 { + // min == i64::MIN, max == i64::MAX: every draw is already a unique, + // uniform point in the range. No scaling, no bias, no rejection. + return Some((min as i128 + draw as i128) as i64); + } + let width = width as u64; + if width == 1 { + return Some(min); + } + + let product = (draw as u128) * (width as u128); + let hi = (product >> 64) as u64; + let lo = product as u64; + + // Draws below this threshold would land in a short final bucket, + // making it less likely than the rest — reject instead of accepting + // that skew. + let threshold = width.wrapping_neg() % width; + if lo < threshold { + return None; + } + + Some((min as i128 + hi as i128) as i64) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::vfs::{MemoryFs, VfsRouter}; + use std::sync::Arc; + + fn make_ctx() -> ExecContext { + let mut vfs = VfsRouter::new(); + vfs.mount("/", MemoryFs::new()); + ExecContext::new(Arc::new(vfs)) + } + + #[test] + fn map_draw_is_deterministic() { + let a = map_draw_to_range(0x1234_5678_9abc_def0, -100, 100); + let b = map_draw_to_range(0x1234_5678_9abc_def0, -100, 100); + assert_eq!(a, b); + } + + #[test] + fn map_draw_min_equals_max_ignores_the_draw() { + assert_eq!(map_draw_to_range(0, 7, 7), Some(7)); + assert_eq!(map_draw_to_range(u64::MAX, 7, 7), Some(7)); + } + + #[test] + fn map_draw_stays_in_bounds() { + for draw in [0u64, 1, u64::MAX / 2, u64::MAX - 1, u64::MAX] { + for (min, max) in [(0i64, 6i64), (-5, 5), (i64::MIN, -1), (0, i64::MAX)] { + if let Some(v) = map_draw_to_range(draw, min, max) { + assert!(v >= min && v <= max, "{v} outside {min}..={max}"); + } + } + } + } + + #[test] + fn map_draw_full_i64_span_never_panics_and_never_rejects() { + for draw in [0u64, 1, u64::MAX / 2, u64::MAX - 1, u64::MAX] { + let v = map_draw_to_range(draw, i64::MIN, i64::MAX); + assert!(v.is_some(), "the full span must never reject a draw"); + } + } + + #[test] + fn map_draw_extreme_spans_do_not_panic() { + let spans = [ + (i64::MIN, i64::MIN), + (i64::MAX, i64::MAX), + (i64::MIN, i64::MAX), + (i64::MIN, i64::MIN + 1), + (i64::MAX - 1, i64::MAX), + ]; + for (min, max) in spans { + for draw in [0u64, u64::MAX] { + let _ = map_draw_to_range(draw, min, max); + } + } + } + + #[tokio::test] + async fn execute_defaults_are_random_range() { + let mut ctx = make_ctx(); + let result = Random.execute(ToolArgs::new(), &mut ctx).await; + assert!(result.ok()); + let n: i64 = result.text_out().trim().parse().expect("integer output"); + assert!((DEFAULT_MIN..=DEFAULT_MAX).contains(&n)); + } + + #[tokio::test] + async fn execute_rejects_min_greater_than_max() { + let mut ctx = make_ctx(); + let mut args = ToolArgs::new(); + args.named.insert("min".to_string(), Value::Int(10)); + args.named.insert("max".to_string(), Value::Int(5)); + + let result = Random.execute(args, &mut ctx).await; + assert!(!result.ok()); + assert_eq!(result.code, 2); + assert!(result.err.contains("--min 10 is greater than --max 5")); + } + + #[tokio::test] + async fn execute_rejects_positional_argument() { + let mut ctx = make_ctx(); + let mut args = ToolArgs::new(); + args.positional.push(Value::Int(100)); + + let result = Random.execute(args, &mut ctx).await; + assert!(!result.ok()); + assert_eq!(result.code, 2); + assert!(result.err.contains("--max 100")); + } +} diff --git a/crates/kaish-kernel/tests/random_builtin_tests.rs b/crates/kaish-kernel/tests/random_builtin_tests.rs new file mode 100644 index 00000000..05812f65 --- /dev/null +++ b/crates/kaish-kernel/tests/random_builtin_tests.rs @@ -0,0 +1,145 @@ +//! Kernel-routed tests for the `random` builtin. +//! +//! kaish has no `$RANDOM` variable; `random` is its typed replacement. These +//! tests drive real command strings through `kernel.execute()` so the full +//! pipeline runs (lex → parse → validate → dispatch → builtin → `--json`), +//! not just the bare `Random::execute` entry point. + +// Test-fixture code: unwrap/expect on known-good setup is the idiom here. +#![allow(clippy::unwrap_used, clippy::expect_used)] +#![cfg(feature = "localfs")] + +mod common; + +use common::{kernel_at, run}; +use std::collections::HashSet; +use tempfile::tempdir; + +/// 200 draws of `random --max 6` all land in `0..=6`, and the draws are not +/// all the same value — pins both the bound and that it's actually random. +#[tokio::test] +async fn stays_in_bounds_and_varies() { + let dir = tempdir().unwrap(); + let kernel = kernel_at(dir.path()); + + let mut seen: HashSet = HashSet::new(); + for _ in 0..200 { + let (out, code) = run(&kernel, "random --max 6").await; + assert_eq!(code, 0, "random --max 6 should succeed: {out:?}"); + let n: i64 = out.parse().expect("random --max 6 prints an integer"); + assert!((0..=6).contains(&n), "{n} outside 0..=6"); + seen.insert(n); + } + assert!( + seen.len() >= 2, + "200 draws of random --max 6 produced only {:?} — looks non-random", + seen + ); +} + +/// `--min N --max N` has exactly one legal value. +#[tokio::test] +async fn min_equals_max_prints_that_value() { + let dir = tempdir().unwrap(); + let kernel = kernel_at(dir.path()); + let (out, code) = run(&kernel, "random --min 5 --max 5").await; + assert_eq!(code, 0); + assert_eq!(out, "5"); +} + +/// Negative bounds are legal and the draw stays inside them. +#[tokio::test] +async fn negative_range_stays_in_range() { + let dir = tempdir().unwrap(); + let kernel = kernel_at(dir.path()); + for _ in 0..50 { + let (out, code) = run(&kernel, "random --min -5 --max 5").await; + assert_eq!(code, 0); + let n: i64 = out.parse().expect("integer output"); + assert!((-5..=5).contains(&n), "{n} outside -5..=5"); + } +} + +/// `$(random)` binds a typed number, not a string — `typeof` must say so. +#[tokio::test] +async fn command_substitution_is_typed_as_number() { + let dir = tempdir().unwrap(); + let kernel = kernel_at(dir.path()); + let (out, code) = run(&kernel, "typeof $(random)").await; + assert_eq!(code, 0, "typeof $(random) should succeed: {out:?}"); + assert_eq!(out, "number"); +} + +/// The typed capture is usable directly in arithmetic. +#[tokio::test] +async fn captured_value_is_arithmetic_ready() { + let dir = tempdir().unwrap(); + let kernel = kernel_at(dir.path()); + let (out, code) = run(&kernel, "x=$(random --min 3 --max 3); echo $((x * 2))").await; + assert_eq!(code, 0, "arithmetic on captured random should succeed: {out:?}"); + assert_eq!(out, "6"); +} + +/// `--min` greater than `--max` is a curated exit-2 error naming both bounds. +#[tokio::test] +async fn min_greater_than_max_is_an_error() { + let dir = tempdir().unwrap(); + let kernel = kernel_at(dir.path()); + let result = kernel.execute("random --min 10 --max 5").await.unwrap(); + assert_eq!(result.code, 2, "got: {:?}", result.err); + assert!( + result.err.contains("--min 10 is greater than --max 5"), + "error should name both bounds: {:?}", + result.err + ); +} + +/// A positional argument is refused with a curated error naming the fix. +#[tokio::test] +async fn positional_argument_is_an_error() { + let dir = tempdir().unwrap(); + let kernel = kernel_at(dir.path()); + let result = kernel.execute("random 100").await.unwrap(); + assert_eq!(result.code, 2, "got: {:?}", result.err); + assert!( + result.err.contains("--max 100"), + "error should name the fix: {:?}", + result.err + ); +} + +/// A non-integer bound is a clap parse error, exit 2. +#[tokio::test] +async fn non_integer_bound_is_an_error() { + let dir = tempdir().unwrap(); + let kernel = kernel_at(dir.path()); + let result = kernel.execute("random --max abc").await.unwrap(); + assert_eq!(result.code, 2, "got: {:?}", result.err); +} + +/// `--json` output is a bare JSON number, not a string or an envelope. +#[tokio::test] +async fn json_output_is_a_number() { + let dir = tempdir().unwrap(); + let kernel = kernel_at(dir.path()); + let result = kernel.execute("random --max 6 --json").await.unwrap(); + assert!(result.ok(), "random --json should succeed: {:?}", result); + let parsed: serde_json::Value = + serde_json::from_str(result.text_out().trim()).expect("random --json is JSON"); + assert!(parsed.is_number(), "random --json should be a JSON number: {parsed}"); +} + +/// The full i64 span must not panic or overflow the range mapper. +#[tokio::test] +async fn full_i64_span_succeeds() { + let dir = tempdir().unwrap(); + let kernel = kernel_at(dir.path()); + let (out, code) = run( + &kernel, + "random --min -9223372036854775808 --max 9223372036854775807", + ) + .await; + assert_eq!(code, 0, "full i64 span should succeed: {out:?}"); + let n: i64 = out.parse().expect("integer output"); + assert!((i64::MIN..=i64::MAX).contains(&n)); +} From 24e81173fe6a2249cac62f5e35d2ec4d30c4466c Mon Sep 17 00:00:00 2001 From: A Tobey Date: Thu, 27 Aug 2026 08:49:38 -0400 Subject: [PATCH 2/5] Document the random builtin Add random to the README's System category table (alphabetical, between push and read) and a CHANGELOG Unreleased/Added bullet summarizing the contract and the unbiased-sampling method. Co-Authored-By: Claude Fable 5 --- CHANGELOG.md | 4 ++++ README.md | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 27810010..ece7c23a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,10 @@ breaking entries are marked **BREAKING**. discipline. `tools::DEFAULT_KILL_GRACE` is 2s. Only a struct literal changes. ### Added +- **`random` builtin**: `random [--min N] [--max N]` prints one typed random + integer, uniform via Lemire's method (no modulo bias). Default range + matches bash's `$RANDOM` (0-32767). + - **Wrapped commands** (`kaish_kernel::tools::wrapped`, `subprocess` feature): register an external program as a tool with a declared grammar. Verbs and flags are deny-by-default, refused with exit 2 before any spawn; the kernel renders diff --git a/README.md b/README.md index b260523a..16916add 100644 --- a/README.md +++ b/README.md @@ -244,7 +244,7 @@ an `awk` that never surprises. | **Text** | awk, base64, cut, diff, grep, head, sed, sort, split, tac, tail, tr, uniq, wc, xxd | | **Files** | basename, cat, cd, checksum, cmp, cp, dd, dirname, file, find, glob, ln, ls, mkdir, mktemp, mv, patch, pwd, readlink, realpath, rm, stat, tee, touch, tree, write | | **JSON** | fromjson, fromjsonl, jq, keys, tojson, tojsonl, typeof, values | -| **System** | alias, bg, date, echo, env, exec, export, fg, help, hostname, jobs, kill, plan, printf, ps, push, read, seq, set, sleep, spawn, timeout, tokens, uname, unalias, unset, wait, which | +| **System** | alias, bg, date, echo, env, exec, export, fg, help, hostname, jobs, kill, plan, printf, ps, push, random, read, seq, set, sleep, spawn, timeout, tokens, uname, unalias, unset, wait, which | | **Parallel** | scatter, gather | | **Meta** | `:`, assert, false, test, true | | **kaish-*** | kaish-ast, kaish-clear, kaish-ignore, kaish-last, kaish-mounts, kaish-output-limit, kaish-status, kaish-tools, kaish-trash, kaish-validate, kaish-vars, kaish-version, kaish-vfs | From 2f2373bd22798da56d273a90ede038d052ce5219 Mon Sep 17 00:00:00 2001 From: A Tobey Date: Thu, 27 Aug 2026 08:55:34 -0400 Subject: [PATCH 3/5] The JSON sweep covers random sweep_covers_every_registered_builtin failed with random missing from CASES. Add it between pwd and read, using a fixed --min 3 --max 3 range so the draw is deterministic and the case only pins the shape: --json on a typed Int emits a bare JSON number. Co-Authored-By: Claude Fable 5 --- crates/kaish-kernel/tests/json_sweep_tests.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/kaish-kernel/tests/json_sweep_tests.rs b/crates/kaish-kernel/tests/json_sweep_tests.rs index ec4389a0..e5712e9c 100644 --- a/crates/kaish-kernel/tests/json_sweep_tests.rs +++ b/crates/kaish-kernel/tests/json_sweep_tests.rs @@ -178,6 +178,8 @@ const CASES: &[Case] = &[ // push mutates in place and is silent on success, like unset. Case { name: "push", setup: &["xs=[a b]"], cmd: "push xs c --json", expect: Expect::Empty }, Case { name: "pwd", setup: &[], cmd: "pwd --json", expect: Expect::String }, + // Fixed min==max range so the draw is deterministic. + Case { name: "random", setup: &[], cmd: "random --min 3 --max 3 --json", expect: Expect::Number }, Case { name: "read", setup: &[], cmd: "echo hi | read X --json", expect: Expect::Empty }, Case { name: "readlink", setup: &["ln -s tmp/data.json link.json"], cmd: "readlink link.json --json", expect: Expect::String }, Case { name: "realpath", setup: &[], cmd: "realpath tmp/data.json --json", expect: Expect::String }, From 47b8e9555fdcc226c46fdc4ebee8e267c444c5a3 Mon Sep 17 00:00:00 2001 From: A Tobey Date: Thu, 27 Aug 2026 08:58:05 -0400 Subject: [PATCH 4/5] Cut the random comments to the house length random.rs's non-test comments ran 45 lines against 113 lines of code - narrative that belongs in the commit history, which already carries it (the previous commit explains why Lemire's method, why the u128 width, why entropy failure is fatal). Trimmed the module doc, the map_draw_to_range and draw_random doc comments, and the inline // notes down to what a reader needs at the point of the code - 20 comment lines against 112 lines of code. The published /// on --min, --max, and the about text are untouched. Also trims the CHANGELOG bullet: drop the algorithm name and the modulo-bias parenthetical, since that detail lives in the code and the commit, not the release notes. Co-Authored-By: Claude Fable 5 --- CHANGELOG.md | 5 +- .../kaish-kernel/src/tools/builtin/random.rs | 49 +++++-------------- 2 files changed, 14 insertions(+), 40 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ece7c23a..dca46dc6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,9 +16,8 @@ breaking entries are marked **BREAKING**. discipline. `tools::DEFAULT_KILL_GRACE` is 2s. Only a struct literal changes. ### Added -- **`random` builtin**: `random [--min N] [--max N]` prints one typed random - integer, uniform via Lemire's method (no modulo bias). Default range - matches bash's `$RANDOM` (0-32767). +- **`random` builtin** — `random [--min N] [--max N]` prints one uniformly + chosen integer, typed; the default range is bash's `$RANDOM` (0 to 32767). - **Wrapped commands** (`kaish_kernel::tools::wrapped`, `subprocess` feature): register an external program as a tool with a declared grammar. Verbs and flags diff --git a/crates/kaish-kernel/src/tools/builtin/random.rs b/crates/kaish-kernel/src/tools/builtin/random.rs index 7e037376..d37ec93e 100644 --- a/crates/kaish-kernel/src/tools/builtin/random.rs +++ b/crates/kaish-kernel/src/tools/builtin/random.rs @@ -1,16 +1,8 @@ -//! random — Print one random integer from `--min` to `--max`, inclusive. -//! -//! kaish has no `$RANDOM`; this builtin is the typed replacement. `$(random -//! --max 100)` binds a `number`, not a string, so `$((x + 1))` works on the -//! result directly. -//! -//! # Examples +//! random — print one integer from `--min` to `--max`, inclusive. +//! kaish has no `$RANDOM`; `$(random --max 100)` is the typed replacement. //! //! ```kaish -//! random # 0..=32767, like bash's $RANDOM -//! random --max 6 # roll a die: 0..=6 -//! random --min -5 --max 5 # negative bounds are fine -//! x=$(random --max 6); echo $((x + 1)) +//! random --max 6 # roll a die: 0..=6 //! ``` use async_trait::async_trait; @@ -71,9 +63,7 @@ impl Tool for Random { return ExecResult::failure(1, "internal error: kernel builtin requires ExecContext"); }; - // random takes no positional argument. Check before argv/clap ever - // sees one, so the curated error names the value the caller gave - // instead of clap's generic "unexpected argument". + // No positional args; curate the error before clap ever sees one. if let Some(v) = args.positional.first() { let value = value_to_string(v); return ExecResult::failure( @@ -120,12 +110,8 @@ impl Tool for Random { } } -/// Draw one integer in `min..=max`, inclusive, uniformly at random from the -/// OS CSPRNG. -/// -/// No fallback: a `getrandom` failure is a hard error, not a predictable -/// substitute — a `random` call a script trusted for a coin flip must never -/// silently return a fixed or guessable value. +/// Draws one integer in `min..=max` from the OS CSPRNG. No fallback: a +/// `getrandom` failure is a hard error, never a silently guessable value. fn draw_random(min: i64, max: i64) -> Result { loop { let mut entropy = [0u8; 8]; @@ -134,29 +120,20 @@ fn draw_random(min: i64, max: i64) -> Result { if let Some(value) = map_draw_to_range(draw, min, max) { return Ok(value); } - // Rejected below to avoid bias — redraw. } } -/// Map one 64-bit draw onto `min..=max` (inclusive) via Lemire's method: -/// widen the multiply into 128 bits, then reject the low slice that would -/// otherwise make one bucket slightly more likely than the rest. This is -/// what `draw % width` gets wrong whenever `width` doesn't evenly divide -/// 2^64 — the plain modulo skews toward the low end of the range. -/// -/// `None` means the draw must be discarded and redrawn; the caller loops. -/// Pure and deterministic: the same `draw`, `min`, `max` always agree, so it -/// never itself touches the CSPRNG. +/// Maps one 64-bit draw onto `min..=max`, inclusive, via Lemire's method: +/// widen the multiply to 128 bits and reject the low slice that would bias +/// a bucket. `None` means redraw. fn map_draw_to_range(draw: u64, min: i64, max: i64) -> Option { debug_assert!(min <= max); - // Width as u128: `max - min + 1` can be exactly 2^64 (the full i64 - // span), which does not fit a u64. + // u128: width can be exactly 2^64, which doesn't fit u64. let width: u128 = (max as i128 - min as i128) as u128 + 1; if width > u64::MAX as u128 { - // min == i64::MIN, max == i64::MAX: every draw is already a unique, - // uniform point in the range. No scaling, no bias, no rejection. + // Full i64 span: every draw already maps 1:1, no bias possible. return Some((min as i128 + draw as i128) as i64); } let width = width as u64; @@ -168,9 +145,7 @@ fn map_draw_to_range(draw: u64, min: i64, max: i64) -> Option { let hi = (product >> 64) as u64; let lo = product as u64; - // Draws below this threshold would land in a short final bucket, - // making it less likely than the rest — reject instead of accepting - // that skew. + // Below threshold: redraw to avoid skewing the low bucket. let threshold = width.wrapping_neg() % width; if lo < threshold { return None; From 9340c4de5a9f527730fb874fef2f6543f93abf29 Mon Sep 17 00:00:00 2001 From: A Tobey Date: Thu, 27 Aug 2026 09:16:22 -0400 Subject: [PATCH 5/5] The mapper test pins values instead of comparing a function to itself map_draw_is_deterministic called map_draw_to_range twice with the same draw and asserted the two results were equal - trivially true for any pure function, and the chosen draw landed on an accepted value, so no test pinned an actual mapped result or a rejection. An off-by-one or a flipped threshold comparison would have passed unnoticed. Replaced it with map_draw_accepts_and_maps_a_known_draw (draw 0x1234_5678_9abc_def0 over -100..=100 must map to exactly -86) and map_draw_rejects_a_known_biased_draw (draw 0 over 0..=6 must reject, since it falls under the width-7 threshold of 2). Both values were computed independently of the implementation before writing the assertions. Also pinned the full i64 span's boundary outputs (map_draw_full_i64_span_pins_boundary_values): draw 0 must map to i64::MIN and draw u64::MAX to i64::MAX, not just "some value in range" as the existing never_panics test already checked. Verified the new tests actually discriminate: flipping the threshold comparison from < to > failed both map_draw_accepts_and_maps_a_known_draw and map_draw_rejects_a_known_biased_draw before the fix was reverted. Co-Authored-By: Claude Fable 5 --- .../kaish-kernel/src/tools/builtin/random.rs | 20 +++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/crates/kaish-kernel/src/tools/builtin/random.rs b/crates/kaish-kernel/src/tools/builtin/random.rs index d37ec93e..bf67b377 100644 --- a/crates/kaish-kernel/src/tools/builtin/random.rs +++ b/crates/kaish-kernel/src/tools/builtin/random.rs @@ -167,10 +167,16 @@ mod tests { } #[test] - fn map_draw_is_deterministic() { - let a = map_draw_to_range(0x1234_5678_9abc_def0, -100, 100); - let b = map_draw_to_range(0x1234_5678_9abc_def0, -100, 100); - assert_eq!(a, b); + fn map_draw_accepts_and_maps_a_known_draw() { + // width 201, hi 14 (verified independently against the algorithm, + // not re-derived from this code): min + hi = -86. + assert_eq!(map_draw_to_range(0x1234_5678_9abc_def0, -100, 100), Some(-86)); + } + + #[test] + fn map_draw_rejects_a_known_biased_draw() { + // width 7, threshold 2: draw 0 has lo 0 < 2, so it must reject. + assert_eq!(map_draw_to_range(0, 0, 6), None); } #[test] @@ -198,6 +204,12 @@ mod tests { } } + #[test] + fn map_draw_full_i64_span_pins_boundary_values() { + assert_eq!(map_draw_to_range(0, i64::MIN, i64::MAX), Some(i64::MIN)); + assert_eq!(map_draw_to_range(u64::MAX, i64::MIN, i64::MAX), Some(i64::MAX)); + } + #[test] fn map_draw_extreme_spans_do_not_panic() { let spans = [