Skip to content

fix(start-sdk): drop undefined keys when writing an env file model - #3874

Merged
dr-bonez merged 1 commit into
masterfrom
fix/file-helper-env-undefined
Sep 2, 2026
Merged

fix(start-sdk): drop undefined keys when writing an env file model#3874
dr-bonez merged 1 commit into
masterfrom
fix/file-helper-env-undefined

Conversation

@MattDHill

@MattDHill MattDHill commented Aug 31, 2026

Copy link
Copy Markdown
Member

merge(effects, { KEY: undefined }) removes the key from a file model — except on FileHelper.env, which writes the literal KEY=undefined.

What's happening

fileMerge leaves the key present with an undefined value, and z.deepLoose(shape).parse keeps it, so whether it survives to disk has been up to each writer:

writer drops an undefined-valued key?
json yes — JSON.stringify omits it
yaml / toml / xml yes — their serializers omit it
ini yes — INI.stringify(filterUndefined(inData)), explicitly
env noObject.entries(inData).map(([k, v]) => \${k}=${v}`)` stringifies it to the word

Same shape, same file, merge(effects, { K: undefined }) on 2.0.9:

json  -> {"A":"keep"}                 removed
ini   -> A=keep                       removed
yaml  -> A: keep                      removed
toml  -> A = "keep"                   removed
xml   -> <root><A>keep</A></root>     removed
env   -> A=keep\nK=undefined          NOT removed

A key simply absent from the merge object is a correct no-op in all six.

The fix

Filter in the one place write and merge both render through, rather than per writer — the contract then holds for every format instead of five of six by accident, and merge's dirty-check compares the same bytes it goes on to write. The ini writer's own filterUndefined call becomes redundant and is removed.

Why it matters

FileHelper.env is rare in the fleet — three packages across both registries — which is why this held up unnoticed. One of them hits it: bitcoin-explorer-startos writes

BTCEXP_REDIS_URL: input.redis ? redisUrl : undefined

so turning Redis off in its Configure action writes BTCEXP_REDIS_URL=undefined. btc-rpc-explorer then gets a Redis URL of "undefined", while the shape's .catch(redisUrl) masks the bad value on read — so the package's own guard still reports Redis as enabled and the setting silently doesn't take. Filed as an issue on that repo.

Tests

lib/test/fileHelper.merge.test.ts covers both directions across the five text formats — a key merged as undefined is removed, a key the merge doesn't name is left alone. Verified it fails on the env case without the fix (1 failed, 9 passed) and passes with it.

Full suite on this branch: 9 passed, 117 tests.

dr-bonez
dr-bonez previously approved these changes Sep 2, 2026
`merge(effects, { KEY: undefined })` removes the key everywhere except
`FileHelper.env`, which rendered it through a template literal and wrote the
string `KEY=undefined`. `fileMerge` leaves the key present with an undefined
value and `z.deepLoose` keeps it, so whether it survives has been up to each
writer: JSON, YAML, TOML and the XML builder drop it natively and `ini` was
filtered explicitly, leaving env alone in serializing the word.

Filter in one place both `write` and `merge` render through, so the contract
holds for every format rather than five of six by accident, and merge's
dirty-check compares the same bytes it writes. The ini writer's own
`filterUndefined` call is now redundant.

Live instance of the bug: bitcoin-explorer-startos writes
`BTCEXP_REDIS_URL: input.redis ? redisUrl : undefined`, so turning Redis off
handed btc-rpc-explorer a URL of "undefined" while the shape's `.catch()`
reported the real URL back to the package, which then believed Redis was on.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@dr-bonez
dr-bonez merged commit 3947e51 into master Sep 2, 2026
11 checks passed
@dr-bonez
dr-bonez deleted the fix/file-helper-env-undefined branch September 2, 2026 21:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants