Skip to content

chore(sessions): Implemented var and var policy primitive types + code ergonomics - #273

Merged
evanspearman merged 1 commit into
mainfrom
evan/vars
Jun 1, 2026
Merged

chore(sessions): Implemented var and var policy primitive types + code ergonomics#273
evanspearman merged 1 commit into
mainfrom
evan/vars

Conversation

@evanspearman

@evanspearman evanspearman commented Jun 1, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Member
  • Dropped the provenance stuff from primitives, that stuff should be injected later
  • Added implementation for variables
    • They can be either: specified, inherited, or inherited with a default (inherited will error if it is not defined)
    • Two versions: strict and lenient: This is because POSIX is strict about variable names but Linux is more lenient. In most cases we want the POSIX checking as this helps produce better error messages for certain types of typos, but we still need the lenient escape hatch
    • The different shape is to make it serialize, deserialize to more ergonomic toml
  • Bunch of other tweaks to other primitives for code ergonomic reasons (mostly builder pattern stuff)
  • Added a primitive for user policies since we have policies for vars and patches now

Summary by CodeRabbit

Release Notes

  • New Features
    • Sessions now support package installation alongside existing features
    • Added optional descriptions to lifecycle hooks and session loadouts
    • Introduced flexible variable handling with strict and lenient variable types
    • New policy system for controlling environment variable and file patch configurations
    • Enhanced builder-style APIs for constructing and modifying session configurations

@coderabbitai

coderabbitai Bot commented Jun 1, 2026 •
edited
Loading

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e228c4c2-10e1-4e85-9b8c-72e8f9feed1f

📥 Commits

Reviewing files that changed from the base of the PR and between cc168e9 and a13dac6.

📒 Files selected for processing (6)
  • crates/sessions/src/lib.rs
  • crates/sessions/src/lifecyclehook.rs
  • crates/sessions/src/loadout.rs
  • crates/sessions/src/patches.rs
  • crates/sessions/src/policy.rs
  • crates/sessions/src/vars.rs
📝 Walkthrough

Walkthrough

This PR establishes a comprehensive session configuration system for the sessions crate by introducing foundational primitives for environment variables and patches, refactoring patch provenance handling, enhancing lifecycle hooks, and composing these elements into loadout and policy layers with builder APIs throughout.

Changes

Session Configuration System Primitives and Composition

Layer / File(s) Summary
Variable system primitives and VarsPolicy
crates/sessions/src/vars.rs		 Introduces comprehensive variable subsystem: error types for validation, StrictVarName/LenientVarName with POSIX/kernel-legal constraints, VarValue (Inherit/InheritWithDefault/Specified), LenientVarEntry for wire encoding, unified VarName enum, VarNameGlobs with compiled globset patterns, and VarsPolicy with allow/deny/ignore rule sets and builder methods.
Patch system refactor and PatchPolicy builder
crates/sessions/src/patches.rs		 Removes PatchOrigin enum and origin plumbing; makes Patch/Patches serde-enabled (transparent wrapper); reduces Patch::new to accept only source/dest; adds FileSet::try_with_base and try_with_pattern builder methods; replaces PatchPolicy::new constructor with builder API (empty, with_allow/deny/ignore, try_with_allow/deny/ignore); updates policy docs for origin-free design; extends tests for builders and error propagation.
Lifecycle hook constructors and description field
crates/sessions/src/lifecyclehook.rs		 Adds HookScript::inline(...) and HookScript::try_external(...) constructors; adds optional description: Option<String> field to LifecycleHook and LifecycleHookBuilder with serde omission when unset; switches builder to use default() instead of explicit new(); includes tests for external script validation and TOML round-tripping.
Loadout configuration model
crates/sessions/src/loadout.rs		 Rebuilds Loadout struct with strongly-typed fields: description, packages, strict vars (BTreeMap<StrictVarName, VarValue>), vars_lenient (Vec<LenientVarEntry>), patches, and lifecycle_hooks; implements immutable builder API (empty, with_package, with_var, with_var_lenient, with_patch, with_lifecycle_hook); adds all_vars() iterator unifying strict and lenient variables in order; comprehensive tests validate assembly, ordering, and round-tripping.
UserPolicy composing VarsPolicy and PatchPolicy
crates/sessions/src/policy.rs		 Introduces UserPolicy struct composing VarsPolicy and PatchPolicy; implements serde with conditional field omission when policies match defaults; provides empty() constructor, domain setters (with_vars, with_patches), and accessors (vars(), patches()); tests verify TOML round-tripping, selective section deserialization, and default-skipping on serialization.
Session module surface export
crates/sessions/src/lib.rs		 Exposes new policy module in crate root public API.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

  • gominimal/minimal#248: This PR builds upon and further refactors the sessions primitives (vars, patches, lifecyclehook, and module surface) introduced by #248.

Suggested reviewers

  • norrietaylor
  • twitchyliquid64
  • 0chroma

Poem

🐰 A rabbit hops through config lands so grand,
Where variables dance and patches expand,
With strict names and lenient too,
Policies compose what sessions can do,
Origin-free patches, a cleaner review! 🌱

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the primary changes: introduction of variable and policy primitive types with improved ergonomics across the sessions crate.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

Comment @coderabbitai help to get the list of available commands and usage tips.

twitchyliquid64
twitchyliquid64 reviewed Jun 1, 2026
View reviewed changes
Comment thread crates/sessions/src/loadout.rs
//! ]
//!
//! [vars]
//! EDITOR = "hx"

@twitchyliquid64 twitchyliquid64 Jun 1, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some of these are kinda special (EDITOR, TERM, PAGER etc). Do we want to elevate them to their own thing in the config? Do we want to be able to set a default (separate to the default def in this file) if not set?

@evanspearman evanspearman Jun 1, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think that's necessary. Home-manager just treats them like normal environment variables, so I think doing the same is fine for us. Typically applications use fallbacks if these are not set. I don't think treating these like other environment variables precludes us from setting our own defaults either though.

@evanspearman
evanspearman merged commit 325d99c into main Jun 1, 2026
8 checks passed
@evanspearman
evanspearman deleted the evan/vars branch June 1, 2026 17:17
@coderabbitai coderabbitai Bot mentioned this pull request Jun 8, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@twitchyliquid64 twitchyliquid64 twitchyliquid64 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@evanspearman @twitchyliquid64