Skip to content

fix: a typo in flakemetry.yml must not stop the tests running - #279

Merged
AKogut merged 1 commit into
mainfrom
fix/config-never-blocks-ci
Aug 18, 2026
Merged

fix: a typo in flakemetry.yml must not stop the tests running#279
AKogut merged 1 commit into
mainfrom
fix/config-never-blocks-ci

Conversation

@AKogut

@AKogut AKogut commented Aug 18, 2026

Copy link
Copy Markdown
Owner

Two defects found while auditing the platform against its promises rather than reading its code.

A config typo failed the build

flakemetry run -- <command> with an invalid flakemetry.yml and no FLAKEMETRY_ENDPOINT set:

exit: 1
stdout: []                                    ← the suite never ran
stderr: file:///…/contracts/dist/index.js:86  ← a stack trace, not a diagnosis

The wrapper exists so Flakemetry cannot fail a build. Not running the tests at all is the worst available way to break that, and a typo in a config file is a plausible route to it.

It survived because endpoint resolves as --endpoint ?? env ?? config, so a set FLAKEMETRY_ENDPOINT short-circuits the config lookup entirely. With the variable set — which is how CI is usually wired, and how every test exercised it — the bug is invisible.

run now resolves the config without raising and warns:

exit: 0
stdout: [TESTS RAN]
stderr: flakemetry: ignoring the config — invalid flakemetry configuration:
          - flaky.threshold: Number must be less than or equal to 1

A failing suite still exits with its own code (verified: exit 33), so the other half of the contract is intact.

config and doctor still refuse — telling you the configuration is wrong is their job, and shrugging would defeat it.

The stack trace

The validation messages were already excellent. They were buried inside a Node crash naming a bundled chunk, which is the wrong thing to hand someone whose file has a typo:

flakemetry: invalid flakemetry configuration:
  - flaky.threshold: Number must be less than or equal to 1
  - flaky.minSamples: Number must be greater than or equal to 1
  - quarantine.enabled: Expected boolean, received string
  - (root): Unrecognized key(s) in object: 'nonsense_key'

The tracker settings did nothing

FLAKEMETRY_TRACKER_ENABLED, FLAKEMETRY_TRACKER_AFTER_DAYS and FLAKEMETRY_TRACKER_RECOVERY_DAYS appear in POLICY_FIELDS, in docker-compose.yml, in deploy/compose/docker-compose.yml, in .env.example and in the configuration reference.

projectPolicyEnvOverrides mapped eight variables and none of those three. An operator setting FLAKEMETRY_TRACKER_ENABLED=true got nothing, and the Policy page reported the source as default while an environment variable said otherwise.

Proved before fixing:

expected { value: false, source: 'default' } to deeply equal { value: true, source: 'env' }

The new test walks every field the effective policy exposes and fails when one has no environment tier, so the next policy field cannot arrive half-connected.

How these were found

Not by reading. By running the documented commands against a live stack and checking what the product claims about itself — the same pass also confirmed the scope matrix, the export, badges, the CLI, both ingestion paths, every dashboard page and the webhook hardening, none of which needed changes.

62/62 turbo tasks.

Found by auditing the promise rather than the code. `flakemetry run` with an
invalid config file and no FLAKEMETRY_ENDPOINT in the environment:

  exit: 1
  stdout: []                      ← the suite never ran
  stderr: file:///…/index.js:86   ← a stack trace, not a diagnosis

The wrapper exists so that Flakemetry can never fail a build. Refusing to start
the command at all is the worst available way to break that, and a config typo
is a likely way to get there. It only survived this long because the endpoint
is usually set in the environment, which short-circuits the config lookup.

`run` now resolves the config without raising and warns instead. Commands whose
job is to tell you the configuration is wrong — config, doctor — still refuse,
because shrugging would defeat their purpose.

Config errors also print as a diagnosis. The validation messages were already
good; they were buried in a Node crash naming a bundled chunk.

Separately: FLAKEMETRY_TRACKER_ENABLED, _AFTER_DAYS and _RECOVERY_DAYS were in
POLICY_FIELDS, in both compose files, in .env.example and in the configuration
reference — and mapped by nothing. Setting them did nothing and the dashboard
reported the source as "default". A test now walks every field the effective
policy exposes and fails if one has no environment tier.
@AKogut
AKogut merged commit a04f9f7 into main Aug 18, 2026
8 checks passed
@AKogut
AKogut deleted the fix/config-never-blocks-ci branch August 18, 2026 10:04
AKogut added a commit that referenced this pull request Aug 18, 2026
Found while looking for tasks small enough to hand a first-time
contributor for #78. This one is too misleading to leave lying around as
bait.

```
$ flakemetry --version
0.0.0            ← package published 0.2.1
```

`CLI_VERSION` was a hardcoded `'0.0.0'`. The version is the first thing
anyone is asked for in a bug report, and this one identified nothing —
including for the config bug fixed in #279, where `0.2.0` and `0.2.1`
behave very differently.

Baked in at build time rather than read at runtime: the published
package has no `package.json` beside the bundle to resolve from.

## The test runs the binary

It executes the built `dist/cli.js` rather than importing the constant,
because the constant cannot tell whether the `define` reached the bundle
— which is the only thing that matters. Proved by removing the define:

```
× reports the version the package actually publishes
AssertionError: expected '0.0.0-dev' to be '0.2.1'
```

The fallback is `0.0.0-dev` rather than `0.0.0`, so an unbuilt or
misbuilt binary says so instead of impersonating a release.

62/62 turbo tasks.
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.

1 participant