feat: move config defaults into the JSON schema

[utkuozdemir]

Since omissis/go-jsonschema#521 is merged and released in v0.23.0, we can now use the goJSONSchema.pointer field to force pointer generation on leaf fields regardless of whether they have a default value. This removes the blocker that previously prevented us from declaring defaults in the schema.

All static default values are now defined in schema.json and read at runtime by walking the schema and extracting them. The hardcoded Default() function is replaced with a schema-driven one. Only three dynamic defaults that depend on runtime state (local IP, machine API endpoint, resource logger types) remain in Go code.

Additionally, the SQLite metrics config was incorrectly generated as a nillable (pointer) intermediate struct. It is now non-nillable, matching all other intermediate structs. Missing CLI flag bindings for the SQLite metrics fields are also added.

A new unit test validates the structural invariant of the config types: all leaf fields must be nillable (to distinguish unset from zero value during config layer merging), and all intermediate struct fields must be non-nillable (for safe navigation without nil checks).

[claude]

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

[utkuozdemir]

@claude review

[Copilot]

Pull request overview

This PR migrates Omni’s static configuration defaults out of Go and into internal/pkg/config/schema.json, leveraging goJSONSchema.pointer support in github.com/atombender/go-jsonschema v0.23.0 to keep leaf fields pointer/nillable even when defaults exist. It also fixes the SQLite metrics config shape to be non-nillable (as an intermediate struct) and adds CLI bindings + a unit test to prevent future regressions in config struct nillability.

Changes:

• Move static defaults into schema.json and generate defaults at runtime by walking the embedded schema.
• Make storage.sqlite.metrics a non-pointer intermediate struct and add missing CLI flag bindings for its leaf fields.
• Add a unit test enforcing: leaf fields are nillable; intermediate structs are non-nillable.

Reviewed changes

Copilot reviewed 8 out of 9 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
internal/pkg/config/types.generated.go	Regenerated config types; ensures leaf fields remain nillable and SQLite.Metrics becomes non-pointer.
internal/pkg/config/schema.json	Adds static defaults + goJSONSchema.pointer where needed; marks SQLite.metrics required.
internal/pkg/config/defaults.go	Introduces schema-walking default extraction to build a Params defaults object.
internal/pkg/config/config.go	Replaces hardcoded defaults with schema-driven defaults + a few dynamic runtime defaults.
internal/pkg/config/config_test.go	Adds structural invariant test for config struct nillability.
internal/pkg/config/accessors.generated.go	Removes obsolete SQLite.GetMetrics/SetMetrics accessors after Metrics becomes non-pointer.
cmd/omni/cmd/cmd.go	Adds CLI flag bindings for SQLite metrics refresh interval/timeout.
go.mod / go.sum	Bumps github.com/atombender/go-jsonschema to v0.23.0.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[claude]

The schema-walking defaults extraction in defaults.go is clever and well-structured, but this PR changes how config defaults are fundamentally loaded at runtime — a human should verify the new defaultsFromSchema() logic handles all edge cases correctly, especially since there are no tests that explicitly compare the new schema-extracted defaults against the old hardcoded values.

Extended reasoning...

Overview

This PR moves all static config defaults from a large hardcoded Default() function in config.go into schema.json, and extracts them at runtime via a new schema-walking function in defaults.go. It also upgrades go-jsonschema to v0.23.0 for the new goJSONSchema.pointer field, fixes SQLite.Metrics from a pointer to a non-pointer intermediate struct, adds missing CLI flag bindings, and adds a struct nillability invariant test.

Security Risks

No security-sensitive code is touched. The changes are confined to configuration loading infrastructure, with no auth, crypto, or permissions logic involved.

Level of Scrutiny

This warrants careful human review. The new defaultsFromSchema() function in defaults.go implements custom JSON schema walking with $ref resolution and a YAML marshal/unmarshal roundtrip to populate Params. This is the core mechanism that now drives all config defaults. While the logic appears sound, there is no test that explicitly validates the new schema-extracted defaults against the old hardcoded values — making silent regressions possible if a default is misexpressed in the schema.

Other Factors

• The structural nillability test (TestConfigStructNillability) is a genuine improvement and correctly enforces the config type invariants.
• The Init() function improvement (propagating error from loadDefault() instead of using the panicking Default()) is strictly better.
• The omitzero tag additions in generated types appear intentional for Go 1.26+ JSON compatibility.
• The CLI flag bindings and SQLite.Metrics non-pointer fix are clearly correct.
• Given this is a core infrastructure change touching config defaults for the entire system, a human reviewer should validate the completeness and correctness of the schema default entries versus the original Go code.

[Unix4ever]

Do we have tests for the defaults?

[utkuozdemir]

Do we have tests for the defaults?

I wrote one to make sure that we didn't regress on any of the defaults. It passed.
But then, I thought, having such test would beat the purpose of "changing only a single place to add a default" idea. We would need to add the default to the schema, and then go and add the same value into the unit test as well.

That's why I removed that test afterwards. I can bring it back. We can keep the existing values tested at least, even if we don't add the new ones' defaults. Wdyt?

[Unix4ever]

Do we have tests for the defaults?

I wrote one to make sure that we didn't regress on any of the defaults. It passed. But then, I thought, having such test would beat the purpose of "changing only a single place to add a default" idea. We would need to add the default to the schema, and then go and add the same value into the unit test as well.

That's why I removed that test afterwards. I can bring it back. We can keep the existing values tested at least, even if we don't add the new ones' defaults. Wdyt?

I thought it could be enough to test different cases we have for defaults: with refs, without refs. No need to cover every default value.

[utkuozdemir]

Do we have tests for the defaults?

I wrote one to make sure that we didn't regress on any of the defaults. It passed. But then, I thought, having such test would beat the purpose of "changing only a single place to add a default" idea. We would need to add the default to the schema, and then go and add the same value into the unit test as well.
That's why I removed that test afterwards. I can bring it back. We can keep the existing values tested at least, even if we don't add the new ones' defaults. Wdyt?

I thought it could be enough to test different cases we have for defaults: with refs, without refs. No need to cover every default value.

I actually brought back that test - it covers a lot of cases (all defaults, with and without refs), and I don't think we will ever gonna change any defaults, needing to change two places in the code is not really an issue (even if were, no big deal).

[utkuozdemir]

/m