Add RFC 0001 for field-level environment aliases#354
Open
lodyai[bot] wants to merge 4 commits into
Open
Conversation
Propose first-class field-level environment projections in ortho_config: a declarative `env(...)` field attribute binding a field to one or more environment-variable names, with deterministic intra-environment precedence (canonical name shadows aliases by default), empty-value handling, and secret-redaction metadata. Motivated by the vk (GITHUB_TOKEN fallback) and podbot (hand-written env-to-path table) use cases. The RFC keeps the orthodox precedence (defaults < files < environment < CLI) and the single-environment-layer merge model: aliases SELECT one value before the layer, never concatenate, so collection append strategies are unaffected. It specifies the macro grammar, internal resolver glue (not public API), the Secret<T> + secret-path-set redaction model, a backwards-compatible EnvMetadata IR extension (ir_version 1.1 -> 1.2), and vk/podbot migration. The "env var names as values" primitive (podbot env_allowlist) is deferred to RFC 0002. Grounded in prior art (pydantic AliasChoices, Viper BindEnv, clap env, config-rs ignore_empty, GitHub CLI GH_TOKEN/GITHUB_TOKEN ordering, 12-factor) and revised after a multi-perspective design review (recorded in section 13). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Added RFC 0001 documenting field-level environment-variable aliases for
Also updated WalkthroughAdd a new RFC for field-level environment aliases, link it from the contents page, update one execution plan’s PowerShell validation guidance and Milestone 2 scenario example, and ignore the ChangesRFC 0001: Field-level environment aliases
Nested command-tree behavioural fixtures
.gitignore maintenance
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 20✅ Passed checks (20 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![codescene-delta-analysis[bot]](https://avatars.githubusercontent.com/in/53921?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Extend RFC 0001's testing strategy to require property coverage for resolver precedence, no-whole-environment scanning, and collection merge safety. These are the ordering and state-transition invariants raised by review feedback, so the implementation plan now names the generated input ranges and the oracle for each property. Shorten two stale execplan lines that blocked the repository Markdown lint gate while validating this documentation change.
![codescene-access[bot]](https://avatars.githubusercontent.com/in/3755875?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Collaborator
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@docs/rfcs/0001-field-level-environment-aliases.md`:
- Around line 830-850: Rewrap the affected paragraphs in §6.2 so they respect
the 80-column Markdown wrapping rule and don’t split clauses awkwardly. In the
list item about runtime behavior, keep “compiles and loads identically” together
instead of leaving “and” dangling, and in the item about runtime glue types,
keep “are internal” with the `#[doc(hidden)]` parenthetical on adjacent lines.
Use the existing prose around the aliases/runtime-glue bullets as the anchor
when reflowing the text.
- Around line 896-909: The migration example uses .as_deref() on Secret<String>,
but Secret<T> in §5.5 does not currently specify any Deref-style access, so fix
the inconsistency by either adding an explicit accessor/Deref behavior to
Secret<T> or changing the example to use the intended secret-exposure method;
update the Secret<T> definition and the token access example together so they
match, using the unique symbols Secret<T> and the github_token example in the
migration section.
- Around line 1246-1284: The RFC still uses named footnotes in the footnote
definitions, which conflicts with the required GitHub-flavoured numeric style.
Update the footnote references and definitions in this section so they use
sequential numeric labels in order of first appearance, and renumber any later
citations consistently; use the existing footnote blocks like ghcli, viper,
figment, pydantic, configrs, clap, twelvefactor, and spring as the set to
convert.
- Around line 559-567: The object-merge example in the RFC uses prime notation
in the pseudocode, which is ambiguous and may be read as a literal key name.
Update the example in the merge semantics section near the
`merge_value`/`declarative/merge.rs` discussion to either replace `url'` with a
clearer placeholder like `url_new` or add a brief parenthetical clarifying that
the prime marks a replaced value, not a field name.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: efc03a1b-b69d-4b1e-bf9e-987c265fe8c5
📒 Files selected for processing (3)
docs/contents.mddocs/execplans/6-1-2-nested-command-tree-behavioural-fixtures.mddocs/rfcs/0001-field-level-environment-aliases.md
Clarify the field-level environment alias RFC after review. Keep the technical requirements wrapped cleanly, define an explicit `Secret<T>` accessor used by the `vk` migration example, replace ambiguous prime notation in the merge example, and convert prior-art footnotes to the numeric style required by the documentation guide.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Add `.memdb/` to the repository ignore list so local agent memory daemon state does not leave worktrees dirty after validation runs.
Collaborator
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This branch adds the repository's first Request for Comments,
RFC 0001: Field-level environment aliases,
and indexes it from the documentation contents file. The RFC proposes that
ortho_configgrow first-class, field-level environment projections: adeclarative
env(...)field attribute binding a configuration field to one ormore environment-variable names, with deterministic intra-environment
precedence (the prefixed canonical name shadows aliases by default),
empty-value handling, and secret-redaction metadata. It is motivated by two
consumers —
vk(aGITHUB_TOKENfallback resolved today by bespoke code) andpodbot(a hand-written environment-to-config-path table) — and lets bothdelete that hand-rolled machinery without the crate ever scanning the whole
process environment.
This is a documentation-only change carrying a proposal for review. The RFC
status is
Proposed; no runtime, derive-macro, or IR code changes in thisbranch. There is no associated roadmap task, issue, or execplan: the document
itself is the deliverable, and it is the artefact under review.
Review walkthrough
docs/rfcs/0001-field-level-environment-aliases.md
— the proposal itself. Sections 1–4 establish the problem, the current
single-environment-layer behaviour, and the goals and non-goals.
§5 Proposed design:
the precedence model and the central invariant that aliases select a single
value before the one environment layer and never concatenate (§5.4), the
macro attribute grammar (§5.2), the internal resolver glue (§5.3), the
Secret<T>plus secret-path-set redaction model (§5.5), and thebackwards-compatible
EnvMetadataIR extension with the honestir_versionbump (§5.6).§7 Compatibility and migration
for the before/after
vkandpodbotmigrations and the SemVer impact, and§9 Alternatives considered
and §10 Prior art
for the rejected options and the precedents they draw on.
§13 Design-review findings folded in,
which records the material findings from a multi-perspective review and where
each was addressed.
docs/contents.md
for the new "Requests for comments (RFCs)" section linking the document.
Scope notes
(
podbot'senv_allowlist, the proposedenv_value_names/EnvPassthrough)is deliberately deferred to a planned companion, RFC 0002. The conceptual
separation is settled in §5.7; the concrete design is out of scope here
because it shares no mechanism with aliasing beyond the secret vocabulary.
prior-art research, then stress-tested through a multi-perspective design
review before being revised; the panel's verdict was "revise", and the
material findings are folded into the current draft (see §13).
Validation
make markdownlint: 61 files linted, 0 errors.make nixie: all Mermaid diagrams validated successfully.make check-fmt: passed.make lint: passed.make typecheck: passed.RUST_TEST_THREADS=1 make test BUILD_JOBS=-j1: passed. The initialunbounded
make testrun hit rustc thread/process resource exhaustion inparallel BDD bridge builds, then passed when serialized.
References