flan/docs/REVIEW-production-readiness.md

# 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~~ —
  fixed 2026-09-18. Two corrections to the entry as written. The hook bounds and
  arithmetic park through is `flan_break_hook`, not `flan_exit_hook` — that second one is
  normal termination, the one `main` reaches when it ends. And the six could not be routed
  through it as it stands: `flan_break_hook`'s contract is that the loop may answer by
  aiming a transfer channel, and these six are called by emitted code that falls off the
  end with no channel in the call at all, so a restart chosen against one would be accepted
  and silently dropped. So there is a second hook, `flan_trap_hook`, and a break that
  refuses the resume with a reason rather than a process that exits before anyone can ask
  a question. All six park, for two reasons rather than one. Four are guards that fire
  *before* the operation they guard (`if (!a)` and the capability test both precede
  `a->proc(...)`), so nothing is half done. The other two — `flan_transfer_fail` and
  `flan_restart_unarmed` — fire mid-transfer, with this frame's defers possibly half run,
  and park only to be *looked at*: stopping on a torn unwind is strictly more than exiting
  before anyone can ask what tore it. Standalone is unchanged: the
  hook is null in a program that did not import the agent, and the trap still exits 134
  with the same sentence.
- **Unchecked `malloc` in `flan_argv`** (`flan_rt.c:162`) — the one in the file.
- ~~**`v->gen` stale-slice word is maintained and never consulted**~~ — deleted
  2026-09-18 with the second round of the repeal; the header is five words now.
- **`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.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.