feat(server): rfc0020 green pt2 — YAML schema + scalar-value substitution walk by jensholdgaard · Pull Request #325 · jensholdgaard/ourios

jensholdgaard · 2026-07-01T17:33:01Z

What

RFC 0020 green slice 2 — the YAML configuration-file schema and its
environment-substitution walk, in a new ourios_server::config::file
module. Builds on slice 1 (config::env_subst, the WG-conformant ${env:…}
resolver, #322).

parse(yaml, lookup):

parses the file into a serde_yaml::Value node tree;
substitutes ${env:…} references in scalar values only (mapping keys
left verbatim — §3.3 rule 4); a substituted value is a scalar String that
is never re-parsed into YAML structure (rule 5 — the security boundary);
deserialises the substituted tree into a strict FileConfig
(deny_unknown_fields) whose top-level grouping (storage / receiver /
querier / compaction) echoes the chart's values.yaml (§3.4).

Type-after-substitution (§3.3 rule 7)

Resolved at the typed boundary, not by re-tagging the node tree.
serde_yaml's Value doesn't preserve a scalar's quoting style, so a literal
"re-interpret the substituted scalar by YAML's type rules" pass can't tell a
quoted string from a bare one and would coerce "01" → integer. Instead every
leaf is carried as its string form (a bare 3600 and a substituted
${env:W}→3600 both become "3600"), and the final type is resolved when
that string flows through the existing build_* validators — the single
validation path (§3.1), the same one the env values take. Observationally
identical for the bounded schema; a quoted scalar can never be corrupted into a
number. This is documented in the module header.

Scenarios

Closes RFC0020.2 — substitution semantics through the file, un-ignored and
pinned end-to-end via config::file::parse against the WG-conformant
env_subst resolver: ${env:NAME}/${NAME}, :-default on unset/empty,
undefined→empty, $$ escape, key-position verbatim, non-recursive /
no-structure-injection, type-after-substitution.
RFC0020.5 (partial) — the malformed-${…}-reference and unknown-key arms
land here (whole-file error, no partial resolution). Its value-validation arm,
plus RFC0020.1/.3/.4, follow with the --config CLI wiring (slice 3); the
RFC stays red until then.

Invariants (CLAUDE.md §3 / hazards)

Secret hygiene (RFC 0019 §3.4 / RFC 0020 §3.5). FileConfigError names
only structural locators — a YAML key path or a non-conforming ${…}
reference — never a resolved value, so it's safe to log with a secret in a
sibling scalar. Credentials remain env-only references in the schema; nothing
in this slice reads or renders a credential value. The dedicated
RFC0020.6 secret-hygiene test lands in slice 4.
No schema/format change, no data-path change; this is the server startup
layer only (§2.3). serde_yaml is already vetted in the dep tree
(ourios-querier pins the same =0.9.34) → no new cargo-deny surface.

Verification

cargo fmt --all --check ✅
cargo clippy --all-targets --all-features -- -D warnings ✅ (workspace-wide)
cargo test -p ourios-server --all-features ✅ — 10 config::file unit tests
- RFC0020.2 scenario green; the other RFC0020 stubs stay #[ignore]d.

🤖 Generated with Claude Code

Summary by CodeRabbit

New Features
- Added support for loading YAML-based server configuration with environment variable substitution and validation.
- Empty configuration files now resolve to safe defaults.
Bug Fixes
- Improved handling of malformed substitution references and invalid configuration shapes with clear errors.
- Ensured substitutions apply only to values, preventing unintended changes to keys or injected structure.
Tests
- Added end-to-end coverage for defaults, escaping, type handling, and error cases.

…tion walk Add `config::file`: the RFC 0020 configuration-file schema (`FileConfig`) and the walk that applies `${env:…}` substitution to the file. `parse` reads the YAML into a node tree, substitutes references in scalar **values** only (mapping keys left verbatim — §3.3 rule 4), then deserialises the substituted tree into a strict schema (`deny_unknown_fields`). A substituted value is a scalar `String` that is never re-parsed into YAML structure (rule 5), so it can neither rewrite keys nor inject a mapping/sequence. Type-after-substitution (§3.3 rule 7) is resolved at the typed boundary, not by re-tagging the node tree: `serde_yaml`'s `Value` does not preserve a scalar's quoting style, so a re-tag pass cannot distinguish a quoted string from a bare scalar and would coerce `"01"` to an integer. Instead every leaf is carried as its string form (a literal `3600` and a substituted `${env:W}`→`3600` both become `"3600"`) and the final type is resolved when that string flows through the existing `build_*` validators — the single validation path (§3.1), wired in the next slice. Observationally identical for the bounded schema, and a quoted scalar can never be corrupted into a number. Closes RFC0020.2 (substitution semantics through the file, pinned end-to-end via the un-ignored §5 scenario against the WG-conformant `env_subst` resolver). The malformed-reference and unknown-key arms of RFC0020.5 land here; its value-validation arm and RFC0020.1/.3/.4 follow with the `--config` wiring. `serde_yaml` is already vetted in the dep tree (ourios-querier pins the same `=0.9.34`), so this adds no new cargo-deny surface. Verified: `cargo fmt --all --check`, `cargo clippy --all-targets --all-features -D warnings`, `cargo test -p ourios-server --all-features` (config::file unit tests + RFC0020.2 scenario green). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

coderabbitai · 2026-07-01T17:33:47Z

Warning

Review limit reached

@jensholdgaard, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 28 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: cf5653fa-ea5a-49ad-9a8a-45205a97005b

📥 Commits

Reviewing files that changed from the base of the PR and between 004132d and 0798000.

📒 Files selected for processing (4)

crates/ourios-server/Cargo.toml
crates/ourios-server/src/config.rs
crates/ourios-server/src/config/file.rs
crates/ourios-server/tests/rfc0020_config_file.rs

📝 Walkthrough

Walkthrough

Adds a pinned serde_yaml dependency, introduces a new config::file module implementing RFC 0020 YAML configuration parsing with scalar-only ${env:...} substitution and typed deserialization, exports the module from config.rs with updated docs, and replaces an ignored test stub with an active end-to-end test.

Changes

RFC 0020 config file parsing

Layer / File(s)	Summary
Dependency and module wiring `crates/ourios-server/Cargo.toml`, `crates/ourios-server/src/config.rs`	Adds a pinned `serde_yaml` dependency with default features disabled and exports a new `pub mod file;` from `config.rs`, alongside updated module documentation.
Error types and config schema `crates/ourios-server/src/config/file.rs`	Introduces `FileConfigError` (substitution vs. schema failure variants with `Display`/`Error`) and the `FileConfig` data model with nested section structs (`StorageSection`, `S3Section`, `LocalSection`, `ReceiverSection`, `QuerierSection`, `CompactionSection`) using `deny_unknown_fields`.
Parse entry point and substitution logic `crates/ourios-server/src/config/file.rs`	Implements `parse()` to YAML-parse input, default an empty document to `FileConfig::default()`, apply `substitute_tree` for scalar-only `${env:...}` resolution via `env_subst::resolve`, and deserialize through a `Scalar` visitor that normalizes native scalars to strings.
Unit and integration tests `crates/ourios-server/src/config/file.rs`, `crates/ourios-server/tests/rfc0020_config_file.rs`	Adds unit tests covering substitution, escaping, scalar conversion, key/structure safety, and error classification; replaces the ignored RFC0020.2 stub with an active end-to-end test.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant parse
  participant substitute_tree
  participant EnvSubstResolve
  participant ScalarDeserializer

  Caller->>parse: parse(yaml, lookup)
  parse->>parse: yaml-parse into Value
  parse->>substitute_tree: walk tree for scalars
  substitute_tree->>EnvSubstResolve: resolve(${env:...})
  EnvSubstResolve-->>substitute_tree: substituted string
  parse->>ScalarDeserializer: deserialize substituted tree
  ScalarDeserializer-->>parse: FileConfig
  parse-->>Caller: Result<FileConfig, FileConfigError>

Possibly related PRs

jensholdgaard/ourios#321: Builds on the same RFC0020.2 test scaffolding replaced by this PR's active end-to-end test.
jensholdgaard/ourios#322: The new parser's substitution logic calls the env_subst::resolve API implemented in that PR.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly matches the PR’s main change: adding RFC0020 YAML schema and scalar-value substitution support.
Description check	✅ Passed	The description is detailed and covers the summary, RFC context, behavior, and verification, though it is not formatted exactly like the template.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch rfc0020-green-pt2-yaml-schema

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

Copilot

Pull request overview

Introduces RFC 0020 “green slice 2” for ourios-server by adding a YAML config-file schema plus a scalar-only ${env:…} substitution walk, exposed via a new ourios_server::config::file::parse entry point.

Changes:

Added config::file module to parse YAML into a serde_yaml::Value tree, apply env-substitution to scalar values only, then deserialize into a strict FileConfig.
Added unit tests in config::file covering scalar substitution rules, key-position verbatim behavior, and “no structure injection”.
Un-ignored and implemented RFC0020.2 end-to-end scenario test to pin config::file::parse to the WG-conformant env_subst resolver.

Reviewed changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
crates/ourios-server/src/config/file.rs	New YAML schema + substitution walk + unit tests for RFC0020.2/0020.5 partial behavior.
crates/ourios-server/tests/rfc0020_config_file.rs	Implements RFC0020.2 scenario test (no longer ignored).
crates/ourios-server/src/config.rs	Exposes new `config::file` module and updates module-level docs.
crates/ourios-server/Cargo.toml	Adds `serde_yaml` dependency (pinned) for config-file parsing.
Cargo.lock	Records `serde_yaml` as a dependency of `ourios-server`.

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

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/ourios-server/src/config/file.rs`:
- Around line 40-58: Validate schema on the raw YAML tree before running
substitution, because FileConfigError::Schema currently formats
serde_yaml::Error after `${env:...}` expansion and can leak resolved secret
values in mismatched-shape errors. Update the config loading path around the
FileConfigError and the file parsing/substitution flow to either reject
structural mismatches before substitution or scrub schema error messages before
they reach Display. Add a regression test that exercises the config loader and
asserts on err.to_string() for a case like `storage: ${env:SECRET}` to ensure no
secret value appears.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 5837f45e-7279-4f4b-9417-87f8cef573e4

📥 Commits

Reviewing files that changed from the base of the PR and between d0d6b22 and 004132d.

⛔ Files ignored due to path filters (1)

Cargo.lock is excluded by !**/*.lock

📒 Files selected for processing (4)

crates/ourios-server/Cargo.toml
crates/ourios-server/src/config.rs
crates/ourios-server/src/config/file.rs
crates/ourios-server/tests/rfc0020_config_file.rs

…it targets `cargo doc -D rustdoc::broken-intra-doc-links` failed: `[`file`]` is ambiguous (`file` module vs the `file!` macro). Disambiguate with `mod@file`. Also drop the now-redundant explicit `(super::env_subst…)` targets on the two `env_subst` links (the `use super::env_subst` import already resolves the bare labels), which were emitting `rustdoc::redundant_explicit_links` warnings. Verified: `RUSTDOCFLAGS="-D rustdoc::broken-intra-doc-links" cargo doc --workspace --no-deps --all-features` clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 4 out of 5 changed files in this pull request and generated 3 comments.

… hygiene Address the CodeRabbit + Copilot review of #325: - **Schema errors could echo a resolved secret (CodeRabbit).** `from_value` ran *after* substitution, so `storage: ${env:SECRET}` (a scalar where a section is expected) produced `invalid type: string "<secret>", expected struct`. Fix by construction: validate the schema on the **raw** (pre-substitution) tree, then substitute the typed scalar leaves in place. `serde` never sees a substituted value, so a shape / unknown-key error names the file's own text (`${env:SECRET}` — the reference, not the value). Replaces the `Value`-tree `substitute_tree` walk with per-section `substitute` methods over the `Option<String>` leaves; the scalar-only / non-recursive / no-structure-injection guarantees now hold structurally (keys are field names). Regression test asserts the error omits the resolved value and names the reference. - **`S3Section` derived `Debug` over credentials (Copilot).** Add a manual redacting `Debug` impl (presence only), mirroring `ourios_parquet::S3Config`; regression test asserts the access key / secret never render. - **`FileConfigError` → `#[non_exhaustive]` (Copilot)** — matches the public error-enum convention (`ourios_miner::tokenize::TokenizeError`); keeps adding the `--config` slice's I/O / validation variants non-breaking. - **Test comment accuracy (Copilot).** `${env:MISSING:-}` exercised the empty `:-default` path, not "undefined, no default"; switch to `${env:MISSING}` in both the unit and integration tests so the assertion matches its comment and covers the no-default branch. Verified: `cargo fmt --all --check`, `cargo clippy -p ourios-server --all-targets --all-features -D warnings`, `cargo test -p ourios-server --all-features`, `RUSTDOCFLAGS="-D rustdoc::broken-intra-doc-links" cargo doc`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 4 out of 5 changed files in this pull request and generated 3 comments.

…locations Address Copilot's re-review of #325: - **Preserve source locations on schema errors.** `parse` deserialized via an intermediate `serde_yaml::Value` + `from_value`, which drops line/column from schema/shape/unknown-field errors. Since substitution now runs on the typed scalar leaves (not the node tree), the `Value` intermediate is no longer needed: deserialize straight from the text via `Option<FileConfig>` (a null / empty document → `None` → all-default config). Errors keep their location, and validation still runs on the raw pre-substitution text — so it never echoes a resolved secret. Drops the now-unused `serde_yaml::Value` import. - **Fix the stale `serde_yaml` dep comment** — it still described the old substitute-then-deserialize order; corrected to deserialize-then-substitute and noted the order is security-relevant. Verified: `cargo test -p ourios-server --all-features`, `cargo clippy --all-targets --all-features -D warnings`, `cargo fmt --all --check`, `RUSTDOCFLAGS="-D rustdoc::broken-intra-doc-links" cargo doc`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 4 out of 5 changed files in this pull request and generated no new comments.

jensholdgaard requested a review from Copilot July 1, 2026 17:33

Copilot started reviewing on behalf of jensholdgaard July 1, 2026 17:33 View session

Copilot AI reviewed Jul 1, 2026

View reviewed changes

Comment thread crates/ourios-server/src/config/file.rs

Comment thread crates/ourios-server/src/config/file.rs

Comment thread crates/ourios-server/tests/rfc0020_config_file.rs

coderabbitai Bot reviewed Jul 1, 2026

View reviewed changes

Comment thread crates/ourios-server/src/config/file.rs

jensholdgaard requested a review from Copilot July 1, 2026 17:42

Copilot started reviewing on behalf of jensholdgaard July 1, 2026 17:43 View session

Copilot AI reviewed Jul 1, 2026

View reviewed changes

Comment thread crates/ourios-server/src/config/file.rs

Comment thread crates/ourios-server/src/config/file.rs Outdated

Comment thread crates/ourios-server/tests/rfc0020_config_file.rs Outdated

jensholdgaard requested a review from Copilot July 1, 2026 17:56

Copilot started reviewing on behalf of jensholdgaard July 1, 2026 17:56 View session

Copilot AI reviewed Jul 1, 2026

View reviewed changes

Comment thread crates/ourios-server/src/config/file.rs Outdated

Comment thread crates/ourios-server/Cargo.toml Outdated

Comment thread crates/ourios-server/src/config/file.rs

jensholdgaard requested a review from Copilot July 1, 2026 18:06

Copilot started reviewing on behalf of jensholdgaard July 1, 2026 18:06 View session

Copilot AI reviewed Jul 1, 2026

View reviewed changes

jensholdgaard merged commit c527361 into main Jul 1, 2026
21 checks passed

jensholdgaard deleted the rfc0020-green-pt2-yaml-schema branch July 1, 2026 18:12

This was referenced Jul 1, 2026

feat(server): rfc0020 green pt3 — --config CLI wiring + file→ServerConfig map #326

Merged

feat(helm): configure via a mounted RFC 0020 config file (--config) #328

Merged

coderabbitai Bot mentioned this pull request Jul 6, 2026

feat(server): rfc 0026 green a — the token store (RFC0026.1) #390

Merged

coderabbitai Bot mentioned this pull request Jul 26, 2026

feat(server): typed promoted-attribute config entries (RFC 0042 slice 2) #650

Merged

Navigation Menu

feat(server): rfc0020 green pt2 — YAML schema + scalar-value substitution walk - #325

feat(server): rfc0020 green pt2 — YAML schema + scalar-value substitution walk#325
jensholdgaard merged 4 commits into
mainfrom
rfc0020-green-pt2-yaml-schema

jensholdgaard commented Jul 1, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jul 1, 2026 •

edited

Loading

Review limit reached

Walkthrough

Changes

Sequence Diagram(s)

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

jensholdgaard commented Jul 1, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

Type-after-substitution (§3.3 rule 7)

Scenarios

Invariants (CLAUDE.md §3 / hazards)

Verification

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jul 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review limit reached

Walkthrough

Changes

Sequence Diagram(s)

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jensholdgaard commented Jul 1, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jul 1, 2026 •

edited

Loading