Skip to content

fix: emit warning when config.LoadFromDir rejects invalid config - #226

Open
jflowers wants to merge 5 commits into
unbound-force:mainfrom
jflowers:opsx/config-load-warning
Open

fix: emit warning when config.LoadFromDir rejects invalid config#226
jflowers wants to merge 5 commits into
unbound-force:mainfrom
jflowers:opsx/config-load-warning

Conversation

@jflowers

Copy link
Copy Markdown
Collaborator

Summary

Fixes #155. config.LoadFromDir previously silently discarded validation
errors from config.Load and returned DefaultConfig() with no indication
to the user. This created a behavioral inconsistency: commands using
config.Load directly (gaze analyze, gaze quality) surfaced config
errors, while commands using LoadFromDir (gaze crap, gaze report)
silently substituted defaults — a CI gate integrity issue where threshold
gates could evaluate against incorrectly-computed scores.

The fix adds an io.Writer parameter to LoadFromDir. When config.Load
returns a validation or parse error, a warning is emitted before falling
back to defaults. File-not-found remains a silent fallback (the config
file is optional). Nil-safe: passing nil silently discards warnings.

Remediates Constitution Principle II (silent assumption) and Principle III
(no actionable guidance) violations confirmed by all five Divisor panel
agents during triage.

How to Test

# Run the full test suite
go test -race -count=1 -short ./...

# Run config-specific tests
go test -race -count=1 -run TestLoadFromDir ./internal/config/

# Verify with invalid config (manual)
echo 'contractual: 500' > /tmp/test-proj/.gaze.yaml
# Then run gaze crap against that directory — should see:
# warning: config "/tmp/test-proj/.gaze.yaml" rejected, using defaults: ...

Key acceptance scenarios from the spec:

  • Invalid threshold (contractual=500) -> warning emitted, defaults returned
  • Malformed YAML -> warning emitted, defaults returned
  • Missing config -> no warning, defaults returned
  • Valid config -> no warning, config values used
  • Nil writer -> no panic, defaults returned

How to Demo

  1. Create a .gaze.yaml with an invalid threshold: contractual: 500
  2. Run gaze crap ./... -- observe the warning on stderr
  3. Fix the threshold to a valid value (e.g., contractual: 80)
  4. Run gaze crap ./... again -- no warning

Key Files Changed

Production (5 files):

  • internal/config/config.go -- LoadFromDir gains stderr io.Writer param + warning logic
  • internal/aireport/runner_steps.go -- DI type update, 4 call sites pass stderr
  • internal/aireport/runner.go -- pipelineStepFuncs type updates for docscan/classify
  • internal/provider/goprovider/contract.go -- DI type update, 1 call site
  • cmd/gaze/main.go -- 3 call sites, 2 function signature changes

Tests (4 files):

  • internal/config/config_test.go -- 5 new tests, 3 updated for new signature
  • internal/aireport/pipeline_internal_test.go -- 4 fake function updates
  • internal/aireport/runner_steps_test.go -- 7 call site updates
  • internal/provider/goprovider/contract_internal_test.go -- 4 fake updates

Spec artifacts (4 files):

  • openspec/changes/config-load-warning/ -- proposal, design, specs, tasks

This PR was generated by /uf.finale (AI-assisted).

Add OpenSpec change artifacts for issue unbound-force#155: emit warning when
config.LoadFromDir silently rejects invalid config.

Artifacts:
- proposal.md: problem statement and IO Writer approach
- design.md: D1-D5 decisions, coverage strategy
- specs/config-load-warning.md: Given/When/Then scenarios
- tasks.md: 4 groups, 13 tasks, spec-review passed
Change LoadFromDir signature from func(moduleDir string) *GazeConfig
to func(moduleDir string, stderr io.Writer) *GazeConfig. When
config.Load returns a validation or parse error, LoadFromDir now
writes a warning to stderr before falling back to DefaultConfig().
File-not-found remains a silent fallback (handled by config.Load).

Updated call sites:
- internal/aireport/runner_steps.go: qualityPipelineDeps.loadConfig
  type, runDocscanStep, runClassifyStep
- internal/aireport/runner.go: pipelineStepFuncs.docscanStep and
  classifyStep types
- internal/provider/goprovider/contract.go:
  buildContractCoverageFuncDeps.loadConfig type
- cmd/gaze/main.go: initExternalSession, resolveBaselinePath,
  loadAndCompare

Added 5 new tests for LoadFromDir warning behavior. Updated all
existing test fakes across 4 test files.

Closes unbound-force#155
Consolidate TestLoadFromDir_MalformedYAML_EmitsWarning,
TestLoadFromDir_MissingFile_NoWarning, and
TestLoadFromDir_ValidConfig_NoWarning into the existing
TestLoadFromDir_InvalidYAML, TestLoadFromDir_MissingFile,
and TestLoadFromDir_ValidConfig — the originals already
contain the same assertions after their io.Writer updates.

Add missing .gaze.yaml path assertion to
TestLoadFromDir_InvalidYAML for warning format coverage.
- Add 3 Dewey learning files (io-writer DI cascade pattern,
  test duplication avoidance gotcha, config.Load error handling)
- Add PR feedback state

Assisted-by: claude-opus-4
Generated with AI assistance (claude-opus-4)
@jflowers jflowers self-assigned this Aug 17, 2026
@jflowers jflowers moved this to Ready for Review 👀 in Unbound Force Planning Aug 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Ready for Review 👀

Development

Successfully merging this pull request may close these issues.

bug: : emit warning when loadGazeConfigBestEffort silently rejects invalid config

2 participants