ReviewOS

also looking at this

stacks/uplink

feat(config): move Uplink's settings into config/uplink.ts

#42
Merged glennmichael123 wants to merge feat/config-in-config-dir into main
5 files +309 -71
10 days ago

Closes the config half of #40. The auto-import half is separate and comes next.

Uplink kept its own config shape outside config/, read by a hand-rolled UPLINK_* parser. The settings now live in config/uplink.ts, written the way every other Stacks config file is written, and the environment is applied by bunfig's own applyEnvVarsToConfig rather than by int() and str() helpers.

The part that matters: this is not the call #40 suggests

#40 proposes loadConfig({ name: 'uplink', defaultConfig, checkEnv: true }). That is the obvious call, and it would have silently broken every variable .env.example documents.

From bunfig's own source, dist/index.js:2303:

const configWithEnvVars = checkEnv && ... ? applyEnvVarsToConfig(name, defaultConfig2, ...) : defaultConfig2

The environment is applied to defaultConfig, and the config file is then merged over the result. So the precedence is file > env > defaults, and any key present in config/uplink.ts would ignore its UPLINK_* variable. Confirmed experimentally before deciding:

keyin config/uplink.ts?UPLINK_* setresult
pollMsyes551234, the file
workdirno/from/env/from/env
maxCharsno77

Uplink is configured through .env, and uplink:install writes one, so file-wins is the wrong way round here. This uses the exported applyEnvVarsToConfig directly: the file is the defaults, the environment wins, and it is bunfig's mechanism either way rather than a second hand-rolled one. A test pins it, because the failure mode is quiet rather than loud.

No variable was renamed

bunfig derives the name from the key, name.toUpperCase() plus k.replace(/([A-Z])/g, '_$1').toUpperCase(). All seventeen names it generates already match .env.example, so an existing .env keeps working untouched. UPLINK_MODEL, which predates the second engine, stays an explicit fallback.

It also coerces by the type of the default, which covers Uplink's shapes: UPLINK_POLL_MS to a number, and UPLINK_ALLOWED comma separated into an array.

What stayed in code

A config file cannot state these, so they are resolved after the merge: ~ expansion, handle normalization, and finding the agent CLIs. An empty claudeBin or codexBin means "find it on this Mac", which is the honest default given launchd hands agents a bare PATH.

The compiled app

Still framework-free. grep -rn "@stacksjs" app/Uplink/*.ts app/Desktop/*.ts returns one hit and it is a comment. It imports the config file directly rather than reading it from disk, which it could not do anyway from an arbitrary working directory.

One cost worth your call: bunfig's index is 311 KB and does not tree-shake, so the bundled entry goes from 101.90 KB to 400 KB. In a 35 MB DMG that is under 1%, and bunfig/browser is 8 KB but does not export the function. I took the size in exchange for one config mechanism. Say if you would rather not.

Verification

  • 45 tests pass, including 8 new ones covering the precedence trap, the derived names for every documented variable, handle normalization, the UPLINK_MODEL fallback, and that process.env is left alone.
  • bun build app/Desktop/launcher.ts bundles, 22 modules.
  • bun build --compile produces a binary, so bunfig's dynamic imports do not break the ship path.

🤖 Generated with Claude Code

5 changed files on the files tab, with 0 review threads.

Sign in to comment on this pull request.