What stands between this and daily use, ranked to be implemented from

docs/REVIEW-production-readiness.md

@@ -0,0 +1,207 @@
+# Production-readiness review — 2026-09-17
+
+Three review lanes (C runtime, compiler robustness, tooling/UX) plus a suite run on the
+merged tree (`e9d0b99`, 232 checks, 0 failures, `@x86` 104 MATCH / 0 DIFFER). This file is
+written to be implemented from: each item says what is wrong, where, and what the fix is.
+Items marked **(known)** are already recorded in NEXT.md/FIX.org and are listed so the
+ranking is complete, not because they are news.
+
+**The verdict in one paragraph.** The development loop is the most finished part of the
+project and the shipping loop is the least. A solo developer in Emacs on this machine can
+build a small raylib game today and the experience is genuinely good — the module system,
+the Emacs client's failure handling, the `import-c` header diffing, the diagnostics (a real
+span-and-notes system, ~365 located refusal sites, zero warning suppressions), and full
+LLVM/x86 backend parity are production-grade. Nearly every gap sits at the boundary where a
+second person, a second machine, or a shipped binary appears — plus three memory-safety
+holes in the runtime that nothing currently mentions.
+
+---
+
+## Tier 1 — correctness blockers
+
+These produce silent wrong values or memory corruption in programs that look fine.
+
+### 1.1 Every number→string conversion aliases one static buffer **(known, unenforced)**
+`runtime/flan_rt.c:217-218` — `flan_i64_to_bytes` / `flan_f64_to_bytes` / `flan_u64_to_bytes`
+all return `{scratch, len}` into one shared `static char scratch[64]`, and `emit.ml:2266-2270`
+never copies the bytes out. Holding two results — `(vec-push! v (str i))` in a loop, a
+`(str n)` stored in a struct — silently reads clobbered bytes. No crash, no diagnostic, so
+sanitizers never see it. NEXT.md's "Sharp edges" records it; the prelude's `append-i64!`
+family works around it; nothing *enforces* it.
+**Fix to decide, then do:** either make the runtime conversions allocate from
+`context/temp` (semantic change, kills the hazard everywhere), or have the checker type
+these results as a distinct short-lived slice that may not be stored or outlive the next
+conversion. The first is simpler and matches the Odin idiom already adopted for arenas.
+
+### 1.2 `cap * size` overflow at container growth
+`runtime/flan_rt.c:1336` (Vec), `:1574` (Pool, `cap*size + cap*sslot`). `flan_vec_reserve`
+(`:1374`) forwards arbitrary `n` and the `1<<40` clamp at `:1333` is bypassed when `want`
+exceeds it (`cap = want; break;`). A wrap to a small positive allocates a tiny block while
+`v->cap` stores the unwrapped value; the next push memcpys far past the block (`:1386`).
+Same family, lower reach: `flan_over_budget` (`:785`), `flan_map_block_size` (`:2080`).
+**Fix:** `__builtin_mul_overflow` (or `size > INT64_MAX/cap`) at every grow/reserve site;
+overflow reports as `StorageExhausted` like any other allocation failure. Small, mechanical.
+
+### 1.3 `Map` has no removal
+`runtime/flan_rt.c:1788`, `:2439` — deferred, no tombstones. A `Map` you cannot delete a
+key from is a daily-use gap, not an edge case (entity tables, caches).
+**Fix:** Robin Hood backward-shift deletion (no tombstones needed with the existing
+cache-line-run layout), a `map-remove!` builtin through check/emit/x86, tests on both
+backends. Medium-sized, self-contained.
+
+### 1.4 The dev allocation registry is read cross-thread with no synchronisation
+Writer: game thread inside every alloc/free (`runtime/flan_dev.c:1130`, `:1180`). Reader:
+the agent's listener thread (`vendor/agent/flan_agent.c:1081`, `:1146`) on a *running*
+program — the `reg` verb has no stopped-gate, unlike the frame chain. No seqlock, no
+atomics, and `flan_reg_compact` (`flan_dev.c:1099`) memsets and reinserts the whole table
+mid-scan. A torn `(type, typelen)` pair is an out-of-bounds read in the listener.
+Ranked Tier 1 because the dev loop is the project's priority.
+**Fix:** either gate the `reg` verb on stopped (matching the frame chain — smallest change),
+or give the registry the same per-slot seqlock the watch table already has
+(`flan_dev.c:380-870` is the template in the same file).
+
+### 1.5 `(addr (.field x))` on an `Option` — confirm, then fix
+FIX.org records `Tast.Addr (Tast.Pfield ...)` failing on both backends (working route:
+`Prim (AddrOf, [Field ...])`). `check.ml:4715` builds `Tast.Addr` from user-writable
+`(addr <place>)` and `check.ml:2968` builds `Pfield` from `(.field x)`, so the combination
+is expressible today; neither `emit.ml:1371-1375` nor `x86.ml:2070` has an Option arm.
+**Fix:** first write the failing program to confirm reachability from source; then either
+lower `Addr(Pfield)` through the AddrOf route in `check.ml`, or add the Option arm to both
+backends. If unreachable from source, add the refusal-by-name that the house rule requires.
+
+---
+
+## Tier 2 — the install and shipping story
+
+One coherent problem: the compiler runs only from its checkout, and its output runs only on
+this machine. This is the single largest thing between "the author's language" and
+"a language someone else can try."
+
+### 2.1 There is no install path
+`dune-project` is two stanzas — no `(package ...)`, so `dune install` cannot work; the
+documented way to run the compiler is `dune exec`. Worse, the README's suggested workaround
+(copy the binary onto PATH) silently breaks the flagship feature: the merged `flan dev`
+needs `flan.cmxa` + `flan.a` *beside the binary* (`lib/dev.ml:3198-3203`) and `ocamlfind`
+on PATH at runtime (`lib/dev.ml:2898`). The failure message names the escape hatches
+(`FLAN_LIBDIR`, `--two-process`) but neither README nor `emacs/MANUAL.md` mentions them.
+**Fix:** a `(package)` stanza with install rules that place `flan.cmxa`/`flan.a` where the
+binary's own lookup finds them; document `FLAN_LIBDIR`; make the README's install section
+truthful about what `flan dev` needs.
+
+### 2.2 A built game runs only on machines configured like this one
+No `-static` anywhere in `lib/build.ml`; `vendor/raylib/link` names `-l:libraylib.so.550`
+by exact soname (a Fedora-ism — no unversioned symlink). The binary is otherwise genuinely
+standalone (the C runtime is embedded in the compiler, `lib/dune:25-41` — right design).
+**Fix:** a `flan build --static` (or bundled-raylib) option, and per-target link lines that
+do not hardcode one distro's soname.
+
+### 2.3 The env-var surface is real and entirely undocumented
+`FLAN_CLANG`, `FLAN_EMCC`, `FLAN_LD`, `FLAN_LLC`, `FLAN_WASM_SYSROOT`, `FLAN_WASM_BUILTINS`,
+`FLAN_CACHE_DIR`, `FLAN_OCAMLFIND`, `FLAN_LIBDIR`, `FLAN_RAYLIB_WEB` — none in the README.
+Also undocumented: the live loop needs `llc`/`ld` at an LLVM version matching clang
+(`lib/build.ml:886-962`), and a mismatch breaks `C-c C-c` while `flan build` keeps working.
+**Fix:** an env-var table in the README, and a sentence about the llc/clang version coupling.
+
+---
+
+## Tier 3 — the standard library
+
+The containers, strings/UTF-8, sequences, random, and printing layers are decent. The gaps:
+
+- **No clock of any kind** — no `now`, no monotonic time, no `sleep`, not in prelude or
+  runtime. A raylib game gets time from raylib; a non-graphical tool cannot time anything.
+  Blocker for the "or tool" half of day-to-day use. A `time`/`sleep` builtin pair backed by
+  `clock_gettime` is small.
+- **Math is five f32 functions** (`lib/prelude.ml:624-677`: sqrt, sin, cos, atan2, pow).
+  No tan/asin/acos/log/exp/fmod/abs/hypot, no f64 variants, no PI constant.
+- **File IO is whole-file only** (`slurp`/`barf`). No streaming, stdin, directory listing,
+  metadata, delete/rename/mkdir.
+- **No env vars, no process spawn.** `argv` and `exit` are the whole OS surface.
+- **The prelude is an OCaml string literal** (`lib/prelude.ml:32`) — cannot be read as Flan,
+  extended, or replaced without rebuilding the compiler. Its own docstring promises a
+  `core:` package "at milestone 3" that does not exist. The `import-c` header-diffing is a
+  strong mitigation (users can bind libc and be told when they get it wrong), but the
+  promised `core:` migration is the structural fix.
+
+---
+
+## Tier 4 — robustness and polish (small, high-value)
+
+- **`Sys_error` uncaught in the CLI** — `flan check nosuch.flan` →
+  `Fatal error: exception Sys_error(...)`. The daemon already has the arm
+  (`lib/dev.ml:2613-2621`); copy it into `bin/main.ml:8-32`. One line. Add a `Not_found`
+  backstop arm at the same time — nothing reaches it today, but the failure would be a
+  message-less `Fatal error: exception Not_found`.
+- **Three tests leave `Fatal error: exception Flan.Loc.Error(_)` on stderr**
+  (test_acceptance, test_session, test_web; suite still passes). Some spawned compiler
+  process dies without going through the error printer — locate the spawn (grep the dune
+  log attribution), and either wrap it or assert on the formatted message instead.
+- **`abort()` in the dev runtime** — `flan_dev.c:47-50`, `:103`: reload-name-table
+  exhaustion, intern OOM, and "global changed size" kill the game instead of signalling.
+  Against the grain of everything else in the runtime; route through `flan_error`.
+- **Six trap paths bypass `flan_exit_hook`** and end a merged `flan dev` session
+  (`flan_restart_fail`, `flan_restart_args_fail`, `flan_restart_unarmed`,
+  `flan_transfer_fail`, `flan_null_alloc_fail`, `flan_free_all_fail` → `rt_die`). Bounds
+  and arithmetic already park via the break-loop hook; these should too.
+- **Unchecked `malloc` in `flan_argv`** (`flan_rt.c:162`) — the one in the file.
+- **`v->gen` stale-slice word is maintained and never consulted** (`flan_rt.c:1347`) —
+  either implement the check or delete the field before someone trusts it.
+- **`flan_slurp_into` conflates elements and bytes and skips `flan_vec_check`**
+  (`flan_rt.c:2746`) — safe only because check.ml pins slurp to `(Vec u8)`; a latent trap.
+- **`flan run` swallows build flags as program arguments** (`bin/main.ml:634-651`) —
+  `flan run game.flan --debug` hands `--debug` to the game. Filter or refuse by name like
+  every other subcommand. Also: no `-O` control anywhere (`Build.default` pins `-O2`;
+  `--debug` is the only route to `-O0`).
+- **`main` signature errors print `<unknown>:0:0`** (`check.ml:6033`, `:6038`) —
+  `env.locs` already holds the decl location; use the existing `find_opt` idiom.
+- **`flan_shim_cstr` accepts embedded NULs** that `flan_path_cstr` refuses
+  (`lib/shim.ml:310` vs `flan_rt.c:2639`) — pick one policy.
+- **No `-Wall -Wextra` on the runtime's C compile** (`lib/build.ml:827`, `:1069`).
+- **Package visibility** — everything in a package is public except `main`
+  (`lib/load.ml:25-31`); `rl/get-color-raw` is the recorded symptom.
+- **Emacs client**: 30s hard deadline with no retry on long builds (`flan-dev.el:155-170`);
+  `accept-process-output` loops can freeze Emacs up to 60s on a hung daemon (`:521`,
+  `:576-600`); no package headers, so not installable off MELPA or by path alone.
+- **No CI** — README states it openly and records two silent-failure incidents. `@checks`
+  exists; a workflow that runs `dune build @checks` on push is the whole job. Note `@x86`
+  parity is only under `@checks`, so routine `dune test` does not protect parity.
+
+## Documentation corrections (cheap, decision-relevant)
+
+- **`docs/DISCUSS.md:762` is stale and argues the opposite of the truth**: it lists the
+  condition/restart family under "no plan" for the x86 backend; `x86.ml:1587-1615` lowers
+  all of it and the survey shows 104/104 parity. Anyone reading it for a production
+  decision concludes wrongly.
+- README documents 4 of 11 subcommands — `import-c`/`generate-c`, the most valuable
+  undocumented feature, are missing (`README.md:105-119` vs `bin/main.ml`).
+- `lib/prelude.ml:8-10` promises the nonexistent `core:` package.
+- Root clutter: working artifacts (`MY-NOTES.org`, `plan.org`, committed binaries,
+  `sand.js`/`sand.wasm`, `old-ocaml/`) a newcomer must ignore.
+
+---
+
+## Deliberately out of scope — do not pick these up from this report
+
+- **`drop` / recursive teardown** — parked on `worktree-agent-a18e9e62485eaedb5` with
+  `docs/handoffs/HANDOFF-drop.md`; the arena route replaced it (FIX.org item 4, merged).
+- **JavaScript backend** — held (FIX.org item 6).
+- **wasm32/browser** — explicitly deprioritised; the ILP32 `(size_t)` truncations and the
+  emscripten gaps are recorded here but not queued.
+- **macOS/Windows portability** (`aligned_alloc`, `MSG_NOSIGNAL`, `__atomic_*` vs
+  `stdatomic.h`, `long ftell` 2 GiB cap) — real, recorded, not current-machine problems.
+- **Known and accepted**: seqlock memcpy formal-UB (correctly fenced, retry-bounded);
+  one-`.so`-leak-per-reload (cells hold module text addresses by design); the arena route's
+  compile-time→runtime-trap trade (stated in FIX.org); string-literal write-through
+  (waiting on provenance, plan.org decision #3).
+
+## Suggested order
+
+1. **Quick wins, one sitting**: Sys_error + Not_found arms; `flan_argv` malloc check;
+   `main`-signature locations; `flan run` flag filtering; DISCUSS.md:762 correction;
+   README subcommand + env-var tables.
+2. **Runtime correctness**: mul-overflow guards (1.2), registry sync (1.4),
+   scratch-buffer decision + fix (1.1), `map-remove!` (1.3), dev-runtime aborts → signals.
+3. **Confirm and fix** `Addr(Pfield)` on Option (1.5); locate the stderr `Loc.Error` fatals.
+4. **Install story** (2.1, 2.3), then **shipping** (2.2).
+5. **Stdlib**: clock first, then math, then IO/env — each is independent.
+6. **CI**: `dune build @checks` on push.