fix(metrics): FLOX_DISABLE_METRICS not honoured - #209
Merged
Conversation
devusb
approved these changes
Jun 24, 2026
dcarley
enabled auto-merge (rebase)
June 24, 2026 14:22
dcarley
disabled auto-merge
June 24, 2026 14:23
Running npm install in the flox dev shell (npm 11.6.2 / Node 24) re-resolved the dependency graph and recorded "peer": true metadata on three devDependencies that the older npm which last wrote the lockfile did not annotate. No package versions, additions, or integrity hashes changed, and the published action (bundled dist/index.js) is unaffected. Forge-Agent: forge-implement (03fbd38a) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The action's sole entry point is `run()` (`action.yml` declares only `main: dist/index.js`), and `run()` exports `FLOX_DISABLE_METRICS` at the top of every invocation — before the nix/package branch — so it already covers every install path. `getDownloadUrl()` is reached only on the package path, and always after `run()` has exported the same value, so its own `FLOX_DISABLE_METRICS` export was dead duplication left behind when the export was hoisted into `run()`. Remove that export so there is a single source of truth. No behavior change: `run()` still exports unconditionally here — the guard that fixes the workflow-env override bug lands in the following commit. `getDownloadUrl()` keeps its `RETRIES`/`PROXY` exports, which are specific to the package path, and its test is narrowed to the RETRIES assertion it still covers. Refs: ECO-104 Forge-Agent: forge-implement (03fbd38a) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Callers who set `FLOX_DISABLE_METRICS: "true"` at the workflow `env:` level were silently having their setting overwritten. The `disable-metrics` input defaulted to `"false"`, and `core.exportVariable` writes to `$GITHUB_ENV`, which the runner folds into the job's global environment — overwriting any prior value. This meant the obvious way to opt out of metrics was broken. Guard the single `run()` export so it only fires when the caller actually passes the input. When `disable-metrics` is unset (the new default of `""`), the action leaves `FLOX_DISABLE_METRICS` untouched — at whatever value the caller already set, or absent, in which case the flox CLI applies its own default (disabled by its `#[serde(default)]` bool). `action.yml` default changed from `"false"` to `""` and the description expanded to explain the new semantics. `README.md` inputs table updated to match. `dist/index.js` rebuilt to match. Tests cover both branches of the `run()` guard: an unset input leaves `FLOX_DISABLE_METRICS` untouched (the regression), and an explicit value is still exported. 64 tests pass, statement/line coverage at 100%. Refs: ECO-104 Forge-Agent: implementation-worker (03fbd38a) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
dcarley
force-pushed
the
fix/eco-104-disable-metrics-default
branch
from
June 24, 2026 14:31
8cd1dfb to
8bbc0f0
Compare
Contributor
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
refactor(metrics): FLOX_DISABLE_METRICS dupe set
The action's sole entry point is
run()(action.ymldeclares onlymain: dist/index.js), andrun()exportsFLOX_DISABLE_METRICSat the topof every invocation — before the nix/package branch — so it already covers
every install path.
getDownloadUrl()is reached only on the package path,and always after
run()has exported the same value, so its ownFLOX_DISABLE_METRICSexport was dead duplication left behind when theexport was hoisted into
run().Remove that export so there is a single source of truth. No behavior change:
run()still exports unconditionally here — the guard that fixes theworkflow-env override bug lands in the following commit.
getDownloadUrl()keeps its
RETRIES/PROXYexports, which are specific to the package path,and its test is narrowed to the RETRIES assertion it still covers.
fix(metrics): FLOX_DISABLE_METRICS conditional
Callers who set
FLOX_DISABLE_METRICS: "true"at the workflowenv:levelwere silently having their setting overwritten. The
disable-metricsinputdefaulted to
"false", andcore.exportVariablewrites to$GITHUB_ENV,which the runner folds into the job's global environment — overwriting any
prior value. This meant the obvious way to opt out of metrics was broken.
Guard the single
run()export so it only fires when the caller actuallypasses the input. When
disable-metricsis unset (the new default of""),the action leaves
FLOX_DISABLE_METRICSuntouched — at whatever value thecaller already set, or absent, in which case the flox CLI applies its own
default (disabled/false by its
#[serde(default)]bool).action.ymldefault changed from"false"to""and the descriptionexpanded to explain the new semantics.
README.mdinputs table updated tomatch.
dist/index.jsrebuilt to match.Tests cover both branches of the
run()guard: an unset input leavesFLOX_DISABLE_METRICSuntouched (the regression), and an explicit value isstill exported. 64 tests pass, statement/line coverage at 100%.
--
I'll cut a release and bump our usage once this is merged.