fix: config validation and flag-override precedence

Problem

Three config bugs let a misconfigured run produce a near-empty dashboard with exit 0, or silently ignore what the operator asked for.

  1. window_days <= 0 collected roughly one day, silently. A 0 or missing window made the collector window start at midnight today, so the dashboard showed near-zero adoption while the process exited 0. Nothing flagged it.
  2. Integer parser overflow. The YAML scalar parser accumulated digits as n = n*10 + digit with no overflow guard, so a large integer such as window_days: 99999999999999999999 wrapped to a garbage or negative value.
  3. Flag overrides and config-file skip_pipelines did not take effect.
    • collect.go and run.go detected "was this flag set" by comparing the value to the default (if *windowDays != 90), so an explicit --window-days 90 was treated as unset and could not override a differing config-file value back to the default.
    • LoadFile could not propagate skip_pipelines: true from a config file: the bool zero value (false) was indistinguishable from "unset", so the merge dropped it. A test comment recorded this as a pre-existing gap.

Fix

  1. Added Config.Validate, called in both collect.go and run.go after the config file and flags are merged. It returns an error when collect.window_days <= 0. Unset still defaults to 90 via Defaults(), so the default path is unchanged.
  2. parseScalar now uses strconv.Atoi, so an oversized or invalid integer returns an error instead of wrapping. The error propagates through yamlToJSON with a line number.
  3. Precedence:
    • collect.go and run.go now detect explicitly provided flags with flag.FlagSet.Visit (matching the existing purge.go pattern) and apply only those over the config. An explicit --window-days 90 is honored.
    • LoadFile parses into a tri-state struct that uses a *bool for skip_pipelines. A non-nil pointer means the file set the value, so skip_pipelines: true (and false) propagates through the merge. The CLI layer applies an explicit flag after the merge, so a flag still wins over the file.

Validation

Toolchain: Go 1.24.2.

  • make check (go vet plus full test suite): passed.
  • go test -race ./...: passed.

New and converted assertions:

  • window_days <= 0 returns an error; unset defaults to 90 and passes; a valid value passes.
  • An oversized integer (99999999999999999999) returns an error at both parseScalar and yamlToJSON level, not a wrapped negative.
  • An explicit --window-days 90 is honored even when the config sets a different value (Visit-based detection).
  • A config-file skip_pipelines: true propagates; the previously commented "pre-existing gap" in TestLoadFile_YAML is now a real passing assertion.

Fails-on-old proof for skip_pipelines propagation: with the merge fix temporarily reverted, TestLoadFile_YAML failed with skip_pipelines: want true (from file), got false (and TestLoadFile_SkipPipelinesFalsePropagates failed for the false case). Restoring the tri-state merge made both pass.

Merge request reports

Loading