# balancer-lite-lua — Lessons Learned

Three recurring footguns caught during the routing/wg/ctl wiring (commit `daf240f`).
All three were *silent*: code parsed, tests passed, but end-to-end boot was broken.
**Per-module unit tests alone don't catch any of them** — they only surface when you
try to actually use the module from another module or boot the daemon.

---

## 1. `gsub` patterns treat `\0` as zero-width match (Lua 5.1)

**Symptom:** JSON output looked like `{"kind":"\u0000s\u0000w\u0000i\u0000t\u0000c\u0000h\u0000",...}` —
the `\u0000` text was injected between every character of every string field.

**Cause:** In Lua 5.1, the pattern character `\0` matches a *zero-width* position
between every character (it's the "match anywhere" anchor, like `\b` in some regex
flavors). So `s:gsub('\0', 'X')` on `"abc"` returns `"XaXbXcX"`, not `"abc"`.

**Same trap applies to:** `\0` as a character class, and possibly other `\d`-style
shortcuts. The null byte (0x00) is treated as the special "any position" anchor.

**Fix:** Don't use `\0` in patterns. To match a literal null byte, use `string.find(s,
'\0', i, true)` with the `plain=true` flag (which disables pattern matching), then
rebuild the string manually. Example from `src/balancerlite/json.lua`:

```lua
-- BAD: silent zero-width match
v:gsub('\0', '\\u0000')

-- GOOD: explicit find + manual rebuild
local out, i = {}, 1
while true do
  local p = string.find(v, '\0', i, true)
  if not p then out[#out+1] = v:sub(i); break end
  out[#out+1] = v:sub(i, p-1) .. '\\u0000'
  i = p + 1
end
table.concat(out)
```

**Reproduction:**
```lua
lua5.1 -e "print(('abc'):gsub('\0', 'X'))"
-- Output: XaXbXcX  4   (NOT 'abc')
```

---

## 2. `s and get_opt(...) or default` silently coerces `false` → `default`

**Symptom:** UCI option `option icmp_enabled '0'` was loaded back as `true` (the
default), so the smoke test's "all probes disabled" config actually ran all probes.

**Cause:** Lua's `and`/`or` short-circuit semantics: `A and B or C` evaluates to `B`
when `A` is truthy, else `C`. When `B` itself is `false` (a valid `bool` opt value),
the expression returns `C` instead — so an explicit `false` becomes the default.

**Fix:** Never use `X and Y or Z` when `Y` can be falsy. Use an explicit helper:

```lua
-- BAD: silently breaks for false / 0 / ""
icmp_enabled = sh and get_opt(sh, "icmp_enabled", true, "bool") or true
-- When sh is truthy AND get_opt returns false, expression returns the right-side true

-- GOOD: explicit branch
local function opt(s, k, default, conv)
  if not s then return default end
  return get_opt(s, k, default, conv)
end
icmp_enabled = opt(sh, "icmp_enabled", true, "bool")
```

**Rule of thumb:** `A and B or C` is safe only when `B` is guaranteed truthy
(string, table, non-zero number, or `true`). For all other types use an explicit `if`.

**Reproduction:**
```lua
lua5.1 -e "local sh = {_options={enabled='0'}}
local get_opt = function() return false end
print(sh and get_opt() or true)"
-- Output: true  (the default, NOT the configured false)
```

---

## 3. Module `new()` defined as local but accessed via the module table

**Symptom:** `lua5.1 src/balancerlite/main.lua --dry-run` crashed with
`attempt to call field 'new' (a nil value)` at the first `mod.new(...)` call.

**Cause:** Two of the existing modules had `local function new(cfg)` instead of
`function mod.new(cfg)`. The local function existed but was invisible to the
return table — only `mod.foo` style functions were exposed.

```lua
-- src/balancerlite/probes.lua  (BEFORE)
local function new(cfg) ... end
function probes.all(...) ... end
return probes          -- probes.new is nil!

-- AFTER
local function new(cfg) ... end
function probes.all(...) ... end
probes.new = new
probes.all = probes.all  -- self-redundant but makes exports explicit
return probes
```

**Fix:** Either (a) define `new` as `function mod.new(cfg)` like the other exported
functions, or (b) explicitly assign all exports before `return mod`. Option (a) is
shorter; option (b) gives you a single "public API" section at the bottom.

**Rule:** When defining a module, every function intended to be called from outside
must be reachable via the return table. `local function foo` is invisible to callers
unless you re-bind it: `mod.foo = foo` or `function mod.foo(...)`.

---

## General lesson

End-to-end boot/parse/run is the only test that catches cross-module wiring bugs.
Per-module unit tests catch logic bugs inside the module; lint catches syntax;
smoke tests catch everything else. **Always do an end-to-end run before committing.**

The wiring work that exposed all three bugs here was: "run the actual daemon in
dry-run mode after adding the new modules." That single command — `lua5.1 main.lua
--dry-run` — found three silent failures in ~10 seconds. Cheaper than any amount
of code review.

---

## 2026-09-04 — `docker cp` into a missing container path creates a nested dir

**Symptom:** `make test` green on testbed, but the container was running a
stale copy of the fixture: `ls /testbed/tests` showed both the files
*and* a nested `tests/` subdir inside it. The runner executed
`/testbed/tests/...` which resolved to the stale top-level files from a
previous run, while my updated files sat in `/testbed/tests/tests/`.

**Root cause:** `docker cp <src> container:/testbed/tests` — when
`/testbed/tests` doesn't exist, docker copies the *source dir itself*
into the destination name, i.e. `.../tests` becomes `/testbed/tests/`
only if the target exists; otherwise you get the classic "dir copied as
child" ambiguity (same as tar's behavior with missing targets). First
copy of a run created the right layout; a later `rm -rf /testbed/tests`
+ re-copy produced `/testbed/tests` as a *file-like* copy of the source
directory, and a third re-copy then nested `tests` inside it.

**Fix / rule:** Before `docker cp` of a directory into a container,
`docker exec <c> sh -c 'mkdir -p <dest>'` first (so the target exists
and the copy lands *inside* it), or `rm -rf` then `mkdir -p` in the
same `sh -c`. Verify with `md5sum` after the copy — a 2-second check
that caught the stale-fixture bug that made a "PASS" run validate the
wrong code.

**Corollary:** Testbed runners that re-derive filenames with patterns
(`f:match("tests/(%w+)%.lua")`) silently break on names with
underscores/hyphens (`probes_config.lua` → nil → `attempt to
concatenate a nil value`). Match `([%w_]+)` and assert the match is
non-nil before building the output path.

**Also (same day):** the `shellspec/openwrt` 19.07 image ships **no
`openssl`** — `sha256.lua` (openssl-CLI based) returned nil silently
and its unit test's `os.execute` exit code still read 0, so the runner
said "PASS" while 3 vectors actually failed. Two lessons: (1) record
image package state in the testbed notes (`opkg list-installed`);
install `openssl-util` once: `opkg install openssl-util`. (2) A test
suite whose pass/fail is decided by printed text AND exit code must
make the *runner* assert on the printed summary line, not just the
exit code — `FAIL:` in output with `exit=0` is a runner bug, not a
test pass.