Skip to content

feat(stdlib): implement the os standard library#289

Merged
davydog187 merged 7 commits into
mainfrom
fix/runtime-type-errors
May 31, 2026
Merged

feat(stdlib): implement the os standard library#289
davydog187 merged 7 commits into
mainfrom
fix/runtime-type-errors

Conversation

@davydog187
Copy link
Copy Markdown
Contributor

@davydog187 davydog187 commented May 31, 2026

Goal

Implement a sandbox-safe Lua 5.3 os standard library so code that calls os.clock, os.time, os.date, etc. no longer raises a runtime type error on a nil os global.

Plan: .agents/plans/A43-os-library.md (renamed from A21a during workflow recovery). Adds the side-effect-free os standard library (os.time/os.date/os.clock/os.difftime + safe os.getenv stub).

Triage of the cluster (#259)

  • utf8.lua — already passes; no work needed.
  • math.lua — first runtime failure under suite config is load() being sandboxed at line 277; a load/sandbox concern, out of scope here.
  • coroutine.luacoroutine global is nil; the whole coroutine subsystem is unimplemented, too large for this PR.
  • all.lua — first runtime failure was attempt to call a nil value (field 'clock' on global 'os') at line 57. The os library did not exist. This is the single most tractable concrete fix, so it is what this PR ships.

Success criteria

  • New Lua.VM.Stdlib.Os module installed into the global env.
  • os.time, os.clock, os.difftime, os.date, os.getenv, os.setlocale, os.tmpname, os.exit implemented per Lua 5.3.
  • Regression unit tests under test/lua/vm/stdlib/os_test.exs.
  • all.lua skip narrowed to its new (post-os) failure with a precise reason.
  • mix test and mix test test/lua53_suite_test.exs --only lua53 both green.

Changes

  • lib/lua/vm/stdlib/os.ex — new os library (time/date/env helpers; filesystem & subprocess functions stay sandboxed by the existing high-level API list).
  • lib/lua/vm/stdlib.ex — install the os library alongside the others.
  • test/lua/vm/stdlib/os_test.exs — regression coverage, including that os.getenv stays sandboxed by default.
  • test/lua53_skips.exs — narrow all.lua from :all to two ranges (137..208, 211..263): the harness body that drives every other suite file via dofile/loadfile/string.dump/coroutine.wrap and the post-run io.open summary, none supported in the sandbox.

Verification

mix format
mix compile --warnings-as-errors   # clean
mix test                           # 2047 passed, 21 skipped (was 2037 passed, 22 skipped)
mix test test/lua53_suite_test.exs --only lua53   # 15 passed, 14 skipped (was 14 passed, 15 skipped)

lua53 suite delta: all.lua flips from a fully-skipped file to a passing file (lines 1-136 and 209-210 now run clean).

Out of scope

  • coroutine.lua and the coroutine subsystem.
  • math.lua load() sandbox behaviour.
  • Filesystem / subprocess os functions beyond sandbox-safe stubs.
  • Making all.lua fully pass — it is the suite harness and loads other suite files via mechanisms the sandbox does not support.

Closes #280

Note: re-pointed from #259 to #280 during workflow recovery — this PR is the os stdlib implementation (issue #280). It also advances the #259 runtime-type-error cluster (math.lua seeds via os.time).

Add a sandbox-safe `os` library so Lua code that calls os.clock,
os.time, os.date and friends no longer raises a runtime type error on
a nil `os` global.

Functions: os.time (current epoch or from a date table), os.clock,
os.difftime, os.date (strftime-style formats plus *t / !*t broken-down
tables, ! selecting UTC), os.getenv, os.setlocale (no-op reporting the
C locale), os.tmpname, and os.exit. The high-level Lua API already
listed os.getenv/os.exit/os.tmpname/os.execute/os.remove/os.rename in
its default sandbox, so those stay blocked unless explicitly allowed.

This unblocks the os section of the all.lua suite harness; the
remaining harness body (dofile/loadfile/string.dump/coroutine.wrap and
io.open post-run summary) stays skipped via narrowed line ranges.

Plan: A21a
Closes #259
@davydog187
Copy link
Copy Markdown
Contributor Author

Review: feat(stdlib): implement the os standard library

Solid, well-scoped PR. Conventions are clean: subsystem scope (feat(stdlib)), the plan-only commit correctly uses the chore(A21a) exception, no Co-Authored-By trailers, no plan-id leaks into source/tests, and the body has Closes #280. Full suite is green locally (2047 passed / 21 skipped; lua53 15 passed / 14 skipped). The two-range all.lua skip keeps the do/end at lines 63/209 balanced, as claimed.

A few things to address:

Major — os.clock() can return a negative number on the first call

local a = os.clock()   --> -3.75e-7
print(a >= 0)          --> false

os_clock/2 computes :erlang.monotonic_time(:nanosecond) - boot_offset(). On the very first call, boot_offset() lazily reads the monotonic clock and stores it — but BEAM evaluates the left operand (monotonic_time) before boot_offset(), so the stored boot value is a few ns later than the minuend, yielding a small negative result. Lua 5.3 §6.9 specifies os.clock as an approximation of CPU time used by the program; it must never be negative. Fix by capturing the boot offset eagerly (e.g. seed :persistent_term in install/1, or read boot_offset() into a bound variable before the second monotonic read). Note also this measures wall time, not CPU time as the spec (and #280) describe — acceptable for a sandbox, but worth a doc note.

Minor — the os.clock test does not pin the bug it should

test "os.clock returns a number" asserts only is_number(c), which passes even with the negative value above. Tighten to assert c >= 0 so the regression is actually pinned.

Minor — plan frontmatter issue: 259 disagrees with Closes #280

.agents/plans/A21a-os-library.md has issue: 259 while the PR closes #280. The Closes requirement is satisfied, but the plan frontmatter should match the issue this PR actually closes (or document the split explicitly).

Nit — dead _utc? parameter in to_datetime/2

strip_utc_flag/1 computes utc?, but to_datetime(time, _utc?) ignores it and always builds a UTC DateTime. Result: os.date("%H", t) and os.date("!%H", t) are identical. Defensible in a tz-less sandbox, but the flag plumbing is then dead — either drop the parameter or add a comment that local == UTC here.

Nothing blocking once the negative-clock issue and its test are addressed.

os.clock computed monotonic_time - boot_offset(), but BEAM evaluates the
left operand first while boot_offset() lazily seeds the origin a few ns
later, producing a small negative value on the first call. Read the
offset before the current time and clamp the elapsed value at zero.

Pin the non-negative contract in the os.clock regression test, document
that local time equals UTC in the timezone-less sandbox, and align the
plan frontmatter issue with the issue this PR closes.

Closes #280
Comment thread lib/lua/vm/stdlib/os.ex
Comment on lines +157 to +158
name = Path.join(System.tmp_dir() || "/tmp", "lua_#{:erlang.unique_integer([:positive])}")
{[name], state}
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noting that in the future we don't want this as we will rely on VFS for this behavior. Let's leave a comment here to notate future improvement. We also want a VFS tracking ticket for milestone 1.0.0

The os standard library was misfiled under plan id A21a during an
interrupted batch; its true scope is issue #280. Rename to its own id A43
to avoid colliding with A21a-integer-divide-by-zero (#259).
@davydog187
Copy link
Copy Markdown
Contributor Author

All review points are resolved on the branch:

  • Major — os.clock negative on first call: os_clock/2 now binds offset = boot_offset() before reading monotonic_time and clamps with max(0, elapsed_ns), so it can never be negative (os.ex:52-57).
  • Minor — test didn't pin the bug: the test now asserts c >= 0 and is renamed "os.clock returns a non-negative number" (os_test.exs:10-14).
  • Minor — plan frontmatter issue mismatch: the plan was renamed A21a → A43-os-library.md with issue: 280, matching Closes #280.
  • Nit — dead _utc? parameter: to_datetime/2 now carries a comment that the sandbox has no tz database, so local == UTC and the flag is intentionally ignored (os.ex:196-197).

Branch is mergeable/clean against current main.

os.tmpname currently resolves against the host filesystem via
System.tmp_dir/0. Once the VFS layer lands it should resolve against the
sandboxed virtual filesystem so the VM never touches host paths.
@davydog187
Copy link
Copy Markdown
Contributor Author

Addressed the os.tmpname note (1685b4b). Added a FUTURE: comment on os_tmpname/2 flagging that it currently resolves against the host filesystem via System.tmp_dir/0 and should resolve against the sandboxed VFS once that layer lands, so the VM never touches host paths.

On the VFS tracking ticket for milestone 1.0.0: I have not created it (that is outside what I can action here) — flagging so it can be opened separately.

The four earlier review points (negative os.clock, the c >= 0 test, the plan issue: 280 frontmatter, and the dead _utc? parameter) were already resolved on the branch in prior commits; verified unchanged. Validation on the new commit: mix format --check-formatted clean, mix compile --warnings-as-errors clean, full suite 2047 passed / 21 skipped, lua53 15 passed / 14 skipped.

@davydog187 davydog187 merged commit 2f63f48 into main May 31, 2026
5 checks passed
@davydog187 davydog187 deleted the fix/runtime-type-errors branch May 31, 2026 12:27
davydog187 added a commit that referenced this pull request Jun 1, 2026
Merge main and rebuild the constructs.lua triage on top of it. The os
stdlib (#289) and debug.getinfo name resolution (#290) both landed, so
the debug.getinfo block (line 226), the os.time assignment (line 237),
and the GLOB1 concat (line 248) all pass now and no longer need skipping.

Empirically re-triaged constructs.lua against the current tree: the only
remaining failures are the short-circuit harness (284..299, level=4
combination explosion exceeds the test timeout) and the checkload block
(302..311, load() error messages do not contain the expected
'expected'/'too long' substrings). Replace the single 232..313 entry
with these two narrowed, disjoint ranges.

Drops the duplicate A43-os-stdlib.md plan (the os work shipped via
A43-os-library.md / #289) and the debug.getinfo name pinning test
(superseded by the tests #290 shipped on main).

Plan: A26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

os.time / os.date / os.clock / os.difftime missing from stdlib

1 participant