Skip to content

feat(modules.makeWrapper): allow null values in env and envDefault to remove entries#343

Merged
BirdeeHub merged 1 commit intoBirdeeHub:mainfrom
vinnymeller:null-env-omits
Mar 15, 2026
Merged

feat(modules.makeWrapper): allow null values in env and envDefault to remove entries#343
BirdeeHub merged 1 commit intoBirdeeHub:mainfrom
vinnymeller:null-env-omits

Conversation

@vinnymeller
Copy link
Contributor

basically due to the fact that claude code is vibecoded slop (but slop that work pays for):

  1. when new features are released, if DISABLE_TELEMETRY (defaults to "1" in the module here) is set, that gets in the way of them fetching some server-side config to actually enable it for a given user
  2. it doesn't check any actual VALUE of DISABLE_TELEMETRY. if i override DISABLE_TELEMETRY to equal "0", it still won't work ([BUG] slash command /btw shown in tips but returns 'Unknown skill' when feature flag isn't enabled. anthropics/claude-code#33403 (comment))

only option is to completely remove the DISABLE_TELEMETRY env var, which I dont believe is currently possible with the way its currently set up? or at least not that i could figure out

alternatively could just remove DISABLE_TELEMETRY from claude code's envDefault but idk I felt weird being the one to magically change telemetry from being off by default to on by default (if anyone other than me even uses that module)

this tries to basically make it just work the same as flags

++ mapAndLiftDal "appendFlag" (config.appendFlag or [ ]);
in
if sortResult then wlib.dag.unwrapSort "makeWrapper" unsorted else unsorted;
builtins.filter (v: v.data or null != null) (
Copy link
Owner

@BirdeeHub BirdeeHub Mar 15, 2026

Choose a reason for hiding this comment

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

Should it only filter it out of env and envDefault? You could check type to see

@BirdeeHub
Copy link
Owner

It seems like a reasonable idea, I had a few comments but we can do something like this yes.

@vinnymeller
Copy link
Contributor Author

think the answer to both is that i was trying to be clever, this should do i think

@BirdeeHub
Copy link
Owner

BirdeeHub commented Mar 15, 2026

It looks good to me, let me look it over a few more times because its in the core, but it should be fine like this.

And, honestly, I think none of the other ones can do null, so the first filter was probably technically fine, although idk, it seemed safer to only filter the things we wanted to filter.

But we would definitely still want to still have fixArgs handle it. So, yeah, thats back in there now, thank you thats good.

Edit: If it turns out someone actually needs the unfiltered list we can add a boolean option for it like sortResult, but at least for now, Im going to go with not doing that because I am not sure why they would.

@BirdeeHub BirdeeHub merged commit c53fd67 into BirdeeHub:main Mar 15, 2026
2 checks passed
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.

2 participants