ReviewOS

home-lang/home

Native dotenv: restore value expansion and same-file override boundaries

#510
Closed chris opened this 10 days ago ·
10 days ago

Follow-up to #66 and #506. Carrying real CLI configuration into native module/eval execution fixes the flag-related failures, but the unchanged packages/runtime/test/bun-corpus/cli/run/env.test.ts still has three independent parser failures:

  • .env value expansion: $FOO, ${FOO}, and ${NAME:-default} stay literal.
  • .env escaped dollar sign: the escaping backslash remains in the value.
  • .env special characters 1 (issue #2823): quoted dollar expansion/escaping stays literal.

The published pre-context binary 06a212514bfd7f8b5a17778543642f7aeb36598d reports 64 pass / 15 fail / 87 assertions. The context change reports 76 pass / 3 fail / 89 assertions, with 2 skips and 2 TODOs excluded in both. These same three failing names reproduce in the baseline; they were not introduced by #506.

In packages/runtime/src/dotenv/env_loader.zig, Parser._parse starts count at the existing map size but increments it for every new key. Its later expansion loop then skips the entire map. The selected Bun source at 4982b91e3702094330f3be3883354c52b8c01323 keeps the initial boundary fixed, expands newly added entries, and permits later definitions in the same file to replace earlier definitions while preserving inherited-environment precedence. Port the actual behavior and audit allocation ownership during replacements; do not simply expand all inherited environment values.

Verify the full unchanged env file plus native controls for same-file duplicates, inherited values, sequential expansion, escaping, repeated file loads and replacement lifetimes. Keep #506's module/eval flags and existing process-environment precedence checks green. No corpus expectations, workloads or deadlines have been changed.

Evidence: zig-out/cli-context-{baseline,build1}.json, cli-context-build1-env.stderr. This is an open parser parity gap, not a claim that the overall suite is complete.

10 days ago

The configuration checkpoint is now published in bfdb5f3188e6a6b8f0bea7070131144f5813c046 (#506). This issue's three parser failures remain reproduced on that exact binary: full unchanged env file 76 pass / 3 fail / 89 assertions, with 2 skips and 2 TODOs excluded. The same three names fail on parent 06a212514bfd7f8b5a17778543642f7aeb36598d; there are no newly failing env cases. This issue remains open; #66 tracks complete parity.

10 days ago

Published 7592a69bf4d268c5e50796e72ef86c8470026733 on codex/runtime-port, authored/committed as Chris Breuer with no coauthors.

The parser now preserves the initial map boundary, permits later definitions within the same file, and expands only new entries in insertion order. Inherited process/earlier-file values keep precedence and are not expanded again. Replacement allocation precedes map mutation; parser-owned same-file values are released safely, while borrowed preexisting values remain with their owners. Repeated overrides of an inherited slot are tracked explicitly so subsequent parser allocations are released. This retains Home's borrowed-map contract rather than claiming all environment/worker storage now has Rust's owning representation.

Complete unchanged env file: 79 pass / 0 fail / 89 assertions, previously 76 / 3 on published parent bfdb5f3188e6a6b8f0bea7070131144f5813c046. Two skips and two TODOs are excluded. The exact comparison resolves all three failing names with no newly failing names. no-envfile remains 9 pass / 36 assertions; upstream Node test-util-parse-env.js exits successfully. These files and its dotenv fixture are byte-identical to pin 4982b91e3702094330f3be3883354c52b8c01323.

Native package controls cover chained/default/escaped expansion, duplicate definitions, inherited literals/empty values, explicit/repeated env files, module/run/eval paths and 512 non-expanding util.parseEnv calls with GC. The parent binary fails the new control. The final binary also passes the expanded package/eval checks with VM destruction enabled.

Both new Zig allocator tests pass, including every allocation-failure point. Enabling the full unit gate required correcting obsolete spawn-test .unwrap() calls without removing assertions and linking the test-root N-API registration symbol to the existing native implementation. The full substrate result is 1,818 pass / 19 skip / 1 fail. The separate stream failure and a preexisting native unsized-read discrepancy are tracked in #512; the unit suite is not fully green.

The final binary passes 54 runtime regression commands, including all 23 native files, full spawn (126 pass / 5,484 assertions; 5 skips excluded) and full child-process (32 pass / 72 assertions; 1 skip and 1 TODO excluded). The original 1,000-iteration spawn workload remains unchanged. Full 291-case bun-run stays at 259 pass / 32 fail / 381 assertions, with no new failing names (#508).

Native build and cached build pass 17/17 steps; focused Pickier, Zig formatting and whitespace checks pass. Final binary SHA256: e69ddfa682af5566ed63bfa617dc9dcf11c37d1555c2944ff92af503d5abff8a. Evidence: zig-out/dotenv-{baseline,current,comparison,final-regressions,destruction-final,corpus-integrity,final-binary}.json, dotenv-unit-build3.log, and dotenv-stream-unit-controls.json.

Keeping this issue open for branch integration/platform verification. Darwin execution does not establish Linux/Windows parity, and #66 remains incomplete.

10 days ago

The published implementation is now on main in 7592a69bf, so the pending-integration condition is resolved.

Reverified the complete unchanged pinned cli/run/env.test.ts on current native Home: 79 pass / 2 platform skips / 2 TODO / 0 fail / 89 assertions. The formerly failing value expansion, escaped-dollar, and special-character cases all pass, together with precedence, multiple env files, and large/boundary buffer controls. Skips and TODOs are not counted as completed parity.

Closing this implemented parser item; remaining CLI and full-suite scope stays under #66.

Sign in to comment on this issue.