diff --git a/dev/config.example.toml b/dev/config.example.toml index e31e2814c..683549b1d 100644 --- a/dev/config.example.toml +++ b/dev/config.example.toml @@ -238,7 +238,7 @@ # # Entries are keyed by project identifier — `//` derived from the primary remote URL (no `.git` suffix), or the canonical repo path when there is no remote. Run `wt config show` inside the repo to see the identifier for the current project; it appears in the `PROJECT CONFIG` section as `Identifier: …`. # -# Scalar values (like `worktree-path`) replace the global value; everything else (hooks, aliases, etc.) appends, global first. +# Scalar values (like `worktree-path`) replace the global value; everything else (hooks, aliases, etc.) appends, global first. An entry outranks the global key of the same name, and both lose to a `WORKTRUNK_` env var or `--config-set` — see how the layers rank (https://worktrunk.dev/config/#precedence). # # [projects."github.com/user/repo"] # worktree-path = ".worktrees/{{ branch | sanitize }}" diff --git a/docs/content/config.md b/docs/content/config.md index 0e554bcee..b89780205 100644 --- a/docs/content/config.md +++ b/docs/content/config.md @@ -344,7 +344,7 @@ User config can include a `[projects]` table for project-specific settings — w Entries are keyed by project identifier — `//` derived from the primary remote URL (no `.git` suffix), or the canonical repo path when there is no remote. Run `wt config show` inside the repo to see the identifier for the current project; it appears in the `PROJECT CONFIG` section as `Identifier: …`. -Scalar values (like `worktree-path`) replace the global value; everything else (hooks, aliases, etc.) appends, global first. +Scalar values (like `worktree-path`) replace the global value; everything else (hooks, aliases, etc.) appends, global first. An entry outranks the global key of the same name, and both lose to a `WORKTRUNK_` env var or `--config-set` — see [how the layers rank](@/config.md#precedence). ```toml [projects."github.com/user/repo"] @@ -634,7 +634,7 @@ On first run without shell integration, Worktrunk offers to install it. On first ## Environment variables -All user config options can be overridden with environment variables using the `WORKTRUNK_` prefix. +All user config options can be overridden with environment variables using the `WORKTRUNK_` prefix, which [outrank config files](@/config.md#precedence). ### Naming convention @@ -675,12 +675,27 @@ Override the LLM command in CI to use a mock: ## Inline config overrides (`--config-set`) -`--config-set ` overrides any user config key for a single invocation, with higher priority than both config files and `WORKTRUNK_` env vars. The value is a TOML fragment, so arrays and tables work directly; the flag is global (works before or after the subcommand), repeatable, and a later `--config-set` replaces an earlier one for the same key. +`--config-set ` overrides any user config key for a single invocation, ahead of every other source. The value is a TOML fragment, so arrays and tables work directly; the flag is global (works before or after the subcommand), repeatable, and a later `--config-set` replaces an earlier one for the same key. {{ terminal(cmd="wt --config-set list.full=true list|||wt step copy-ignored --config-set 'step.copy-ignored.exclude=[__WT_QUOT__target__WT_QUOT__, __WT_QUOT__dist__WT_QUOT__]'") }} This composes with aliases — an alias body can invoke `wt --config-set … ` to render a named view without changing the saved config. +## Precedence + +Sources rank by how close they are to the invocation, which puts user config above system config. Within a config file, [an entry keyed to the project](@/config.md#user-project-specific-settings) outranks the global key of the same name. So `worktree-path` comes from the first of these that sets it: + +1. `--config-set 'worktree-path = …'` +2. `WORKTRUNK_WORKTREE_PATH` +3. `[projects."github.com/owner/repo"]` in the config file +4. global `worktree-path` in the config file + +A `--config-set` that names a project entry applies to that project alone, and beats a global `--config-set` of the same key — it is both the highest layer and the most specific key: + +{{ terminal(cmd="wt --config-set 'projects.__WT_QUOT__github.com/owner/repo__WT_QUOT__.worktree-path = __WT_QUOT__/tmp/scratch__WT_QUOT__' switch --create feature") }} + +Ranking only matters where one value has to win. Hooks, aliases and `step.copy-ignored.exclude` accumulate rather than replace, so an env-set hook and a project's hook both run. + ## Command reference {% terminal() %} diff --git a/plugins/worktrunk/skills/worktrunk/reference/config.md b/plugins/worktrunk/skills/worktrunk/reference/config.md index 2f91dd0d9..1880000a8 100644 --- a/plugins/worktrunk/skills/worktrunk/reference/config.md +++ b/plugins/worktrunk/skills/worktrunk/reference/config.md @@ -341,7 +341,7 @@ User config can include a `[projects]` table for project-specific settings — w Entries are keyed by project identifier — `//` derived from the primary remote URL (no `.git` suffix), or the canonical repo path when there is no remote. Run `wt config show` inside the repo to see the identifier for the current project; it appears in the `PROJECT CONFIG` section as `Identifier: …`. -Scalar values (like `worktree-path`) replace the global value; everything else (hooks, aliases, etc.) appends, global first. +Scalar values (like `worktree-path`) replace the global value; everything else (hooks, aliases, etc.) appends, global first. An entry outranks the global key of the same name, and both lose to a `WORKTRUNK_` env var or `--config-set` — see [how the layers rank](https://worktrunk.dev/config/#precedence). ```toml [projects."github.com/user/repo"] @@ -629,7 +629,7 @@ On first run without shell integration, Worktrunk offers to install it. On first ## Environment variables -All user config options can be overridden with environment variables using the `WORKTRUNK_` prefix. +All user config options can be overridden with environment variables using the `WORKTRUNK_` prefix, which [outrank config files](https://worktrunk.dev/config/#precedence). ### Naming convention @@ -672,7 +672,7 @@ $ WORKTRUNK_COMMIT__GENERATION__COMMAND="echo 'test: automated commit'" wt merge ## Inline config overrides (`--config-set`) -`--config-set ` overrides any user config key for a single invocation, with higher priority than both config files and `WORKTRUNK_` env vars. The value is a TOML fragment, so arrays and tables work directly; the flag is global (works before or after the subcommand), repeatable, and a later `--config-set` replaces an earlier one for the same key. +`--config-set ` overrides any user config key for a single invocation, ahead of every other source. The value is a TOML fragment, so arrays and tables work directly; the flag is global (works before or after the subcommand), repeatable, and a later `--config-set` replaces an earlier one for the same key. ```bash $ wt --config-set list.full=true list @@ -681,6 +681,23 @@ $ wt step copy-ignored --config-set 'step.copy-ignored.exclude=["target", "dist" This composes with aliases — an alias body can invoke `wt --config-set … ` to render a named view without changing the saved config. +## Precedence + +Sources rank by how close they are to the invocation, which puts user config above system config. Within a config file, [an entry keyed to the project](https://worktrunk.dev/config/#user-project-specific-settings) outranks the global key of the same name. So `worktree-path` comes from the first of these that sets it: + +1. `--config-set 'worktree-path = …'` +2. `WORKTRUNK_WORKTREE_PATH` +3. `[projects."github.com/owner/repo"]` in the config file +4. global `worktree-path` in the config file + +A `--config-set` that names a project entry applies to that project alone, and beats a global `--config-set` of the same key — it is both the highest layer and the most specific key: + +```bash +$ wt --config-set 'projects."github.com/owner/repo".worktree-path = "/tmp/scratch"' switch --create feature +``` + +Ranking only matters where one value has to win. Hooks, aliases and `step.copy-ignored.exclude` accumulate rather than replace, so an env-set hook and a project's hook both run. + ## Command reference ``` diff --git a/skills/worktrunk/reference/config.md b/skills/worktrunk/reference/config.md index 2f91dd0d9..1880000a8 100644 --- a/skills/worktrunk/reference/config.md +++ b/skills/worktrunk/reference/config.md @@ -341,7 +341,7 @@ User config can include a `[projects]` table for project-specific settings — w Entries are keyed by project identifier — `//` derived from the primary remote URL (no `.git` suffix), or the canonical repo path when there is no remote. Run `wt config show` inside the repo to see the identifier for the current project; it appears in the `PROJECT CONFIG` section as `Identifier: …`. -Scalar values (like `worktree-path`) replace the global value; everything else (hooks, aliases, etc.) appends, global first. +Scalar values (like `worktree-path`) replace the global value; everything else (hooks, aliases, etc.) appends, global first. An entry outranks the global key of the same name, and both lose to a `WORKTRUNK_` env var or `--config-set` — see [how the layers rank](https://worktrunk.dev/config/#precedence). ```toml [projects."github.com/user/repo"] @@ -629,7 +629,7 @@ On first run without shell integration, Worktrunk offers to install it. On first ## Environment variables -All user config options can be overridden with environment variables using the `WORKTRUNK_` prefix. +All user config options can be overridden with environment variables using the `WORKTRUNK_` prefix, which [outrank config files](https://worktrunk.dev/config/#precedence). ### Naming convention @@ -672,7 +672,7 @@ $ WORKTRUNK_COMMIT__GENERATION__COMMAND="echo 'test: automated commit'" wt merge ## Inline config overrides (`--config-set`) -`--config-set ` overrides any user config key for a single invocation, with higher priority than both config files and `WORKTRUNK_` env vars. The value is a TOML fragment, so arrays and tables work directly; the flag is global (works before or after the subcommand), repeatable, and a later `--config-set` replaces an earlier one for the same key. +`--config-set ` overrides any user config key for a single invocation, ahead of every other source. The value is a TOML fragment, so arrays and tables work directly; the flag is global (works before or after the subcommand), repeatable, and a later `--config-set` replaces an earlier one for the same key. ```bash $ wt --config-set list.full=true list @@ -681,6 +681,23 @@ $ wt step copy-ignored --config-set 'step.copy-ignored.exclude=["target", "dist" This composes with aliases — an alias body can invoke `wt --config-set … ` to render a named view without changing the saved config. +## Precedence + +Sources rank by how close they are to the invocation, which puts user config above system config. Within a config file, [an entry keyed to the project](https://worktrunk.dev/config/#user-project-specific-settings) outranks the global key of the same name. So `worktree-path` comes from the first of these that sets it: + +1. `--config-set 'worktree-path = …'` +2. `WORKTRUNK_WORKTREE_PATH` +3. `[projects."github.com/owner/repo"]` in the config file +4. global `worktree-path` in the config file + +A `--config-set` that names a project entry applies to that project alone, and beats a global `--config-set` of the same key — it is both the highest layer and the most specific key: + +```bash +$ wt --config-set 'projects."github.com/owner/repo".worktree-path = "/tmp/scratch"' switch --create feature +``` + +Ranking only matters where one value has to win. Hooks, aliases and `step.copy-ignored.exclude` accumulate rather than replace, so an env-set hook and a project's hook both run. + ## Command reference ``` diff --git a/src/cli/mod.rs b/src/cli/mod.rs index c4e91f75c..f91452bfa 100644 --- a/src/cli/mod.rs +++ b/src/cli/mod.rs @@ -2232,7 +2232,7 @@ User config can include a `[projects]` table for project-specific settings — w Entries are keyed by project identifier — `//` derived from the primary remote URL (no `.git` suffix), or the canonical repo path when there is no remote. Run `wt config show` inside the repo to see the identifier for the current project; it appears in the `PROJECT CONFIG` section as `Identifier: …`. -Scalar values (like `worktree-path`) replace the global value; everything else (hooks, aliases, etc.) appends, global first. +Scalar values (like `worktree-path`) replace the global value; everything else (hooks, aliases, etc.) appends, global first. An entry outranks the global key of the same name, and both lose to a `WORKTRUNK_` env var or `--config-set` — see [how the layers rank](@/config.md#precedence). ```toml [projects."github.com/user/repo"] @@ -2520,7 +2520,7 @@ On first run without shell integration, Worktrunk offers to install it. On first ## Environment variables -All user config options can be overridden with environment variables using the `WORKTRUNK_` prefix. +All user config options can be overridden with environment variables using the `WORKTRUNK_` prefix, which [outrank config files](@/config.md#precedence). ### Naming convention @@ -2563,7 +2563,7 @@ $ WORKTRUNK_COMMIT__GENERATION__COMMAND="echo 'test: automated commit'" wt merge ## Inline config overrides (`--config-set`) -`--config-set ` overrides any user config key for a single invocation, with higher priority than both config files and `WORKTRUNK_` env vars. The value is a TOML fragment, so arrays and tables work directly; the flag is global (works before or after the subcommand), repeatable, and a later `--config-set` replaces an earlier one for the same key. +`--config-set ` overrides any user config key for a single invocation, ahead of every other source. The value is a TOML fragment, so arrays and tables work directly; the flag is global (works before or after the subcommand), repeatable, and a later `--config-set` replaces an earlier one for the same key. ```console $ wt --config-set list.full=true list @@ -2571,6 +2571,23 @@ $ wt step copy-ignored --config-set 'step.copy-ignored.exclude=["target", "dist" ``` This composes with aliases — an alias body can invoke `wt --config-set … ` to render a named view without changing the saved config. + +## Precedence + +Sources rank by how close they are to the invocation, which puts user config above system config. Within a config file, [an entry keyed to the project](@/config.md#user-project-specific-settings) outranks the global key of the same name. So `worktree-path` comes from the first of these that sets it: + +1. `--config-set 'worktree-path = …'` +2. `WORKTRUNK_WORKTREE_PATH` +3. `[projects."github.com/owner/repo"]` in the config file +4. global `worktree-path` in the config file + +A `--config-set` that names a project entry applies to that project alone, and beats a global `--config-set` of the same key — it is both the highest layer and the most specific key: + +```console +$ wt --config-set 'projects."github.com/owner/repo".worktree-path = "/tmp/scratch"' switch --create feature +``` + +Ranking only matters where one value has to win. Hooks, aliases and `step.copy-ignored.exclude` accumulate rather than replace, so an env-set hook and a project's hook both run. diff --git a/src/config/mod.rs b/src/config/mod.rs index 1c8726354..38867bd43 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -8,8 +8,11 @@ //! 3. **Project config** (`.config/wt.toml`) - Lifecycle hooks, checked into git //! //! System and user configs share the same schema and are merged via -//! `deep_merge_table` (user values override system values at the key level). -//! Project config is independent — different schema, different purpose. +//! `merge_layer`, which ranks each layer above the one beneath it as a whole: +//! a user value overrides the system value for the same key, and a user global +//! key also outranks a system `[projects."…"]` entry that would otherwise be +//! the more specific match. Project config is independent — different schema, +//! different purpose. //! //! See `wt config --help` for complete documentation. diff --git a/src/config/user/mod.rs b/src/config/user/mod.rs index 02cef87fb..175420c7b 100644 --- a/src/config/user/mod.rs +++ b/src/config/user/mod.rs @@ -1,6 +1,21 @@ //! User-level configuration //! //! Personal preferences and per-project approved commands, not checked into git. +//! +//! # Precedence +//! +//! Sources rank by how close they are to the invocation — system config, user +//! config, `WORKTRUNK_*` env vars, `--config-set`, lowest first. Within one of +//! them a `[projects."…"]` entry outranks the global key of the same name, so +//! specificity is a rule inside a layer rather than a second axis over all of +//! them: a lower layer's entry loses to a higher layer's global key. +//! +//! [`UserConfig::load_with_warnings`] flattens the layers into one document and +//! the accessors resolve specificity on the result, which is what costs the +//! order — a document holds one set of entries, and reading it can only let +//! them win. So every layer goes on through [`merge_layer`], which first drops +//! what the layer sets globally from the entries beneath it, and the flattened +//! document then reads back as the ordered one. mod accessors; mod merge; @@ -282,6 +297,182 @@ fn deep_merge_table(base: &mut toml::Table, overlay: toml::Table) { } } +/// Merge one layer over `merged_table`, ranking its global keys above the +/// `[projects."…"]` entries the lower layers left there. +/// +/// Every leaf `layer` sets globally is dropped from the accumulated entries +/// first, so the global key it also set is what answers for them; the layer +/// then merges on top, its own entries included. Dropping before the merge is +/// what keeps specificity local to a layer — a key the layer restates under +/// `projects.""` lands after the drop and wins, with nothing here having +/// to recognize the case. +/// +/// Composing keys are held back, their project-scoped values appending to the +/// global ones rather than replacing them ([`is_compose_only`]). Both already +/// apply, so there is no ranking to settle, and dropping the project's copy +/// would silently stop it running. +/// +/// Only removals precede the merge, but a removal can still leave a document +/// that no longer deserializes: [`exclusive_sibling`] and [`is_atomic_section`] +/// name the sections that have to go as a unit, and the removals degrade as a +/// unit behind them — they land on a candidate, and a candidate that stops +/// deserializing or validating is dropped for the plain merge rather than +/// handed to [`UserConfig::finalize`], which would answer a stranded required +/// field by wiping the config to defaults. The layer itself applies either way. +fn merge_layer(merged_table: &mut toml::Table, layer: toml::Table) { + // Nothing to rank: no entries beneath, or nothing above them. The common + // case is a config with no `[projects]` table at all, and it pays only the + // lookup. + if !merged_table.contains_key("projects") || layer.keys().all(|key| key == "projects") { + deep_merge_table(merged_table, layer); + return; + } + + let mut global = layer.clone(); + global.remove("projects"); + + let mut candidate = merged_table.clone(); + if let Some(projects) = candidate + .get_mut("projects") + .and_then(toml::Value::as_table_mut) + { + for entry in projects + .iter_mut() + .filter_map(|(_, entry)| entry.as_table_mut()) + { + drop_overridden_keys(entry, &global, &mut Vec::new()); + } + } + deep_merge_table(&mut candidate, layer.clone()); + + match deserialize_and_validate(&candidate) { + Ok(()) => *merged_table = candidate, + // Reachable two ways. A partial removal the enumerations above miss — + // none today, but they are enumerations, and the next required field + // would otherwise cost the user their whole config rather than one + // project entry. Or a document that was already invalid before this + // layer: step 3's env probe deserializes without validating, so an + // empty `worktree-path` from the environment lands here. Merging + // without the removals is right for both; `finalize` reports the + // second. + Err(err) => { + log::debug!("keeping project precedence: {err}"); + deep_merge_table(merged_table, layer); + } + } +} + +/// Deserialize `table` into [`UserConfig`] and validate it, reporting the +/// first failure. The probe every layer runs before it commits. +fn deserialize_and_validate(table: &toml::Table) -> Result<(), String> { + match toml::Value::Table(table.clone()).try_into::() { + Ok(config) => config.validate().map_err(|e| e.0), + Err(err) => Err(err.to_string()), + } +} + +/// Remove from `entry` every leaf `overlay` sets. `section` tracks the path +/// walked so far, for [`exclusive_sibling`] and the predicates beside it. +fn drop_overridden_keys<'a>( + entry: &mut toml::Table, + overlay: &'a toml::Table, + section: &mut Vec<&'a str>, +) { + for (key, value) in overlay { + if is_compose_only(section, key) { + continue; + } + + // An exclusive pair goes as a unit, whether or not `entry` carries + // `key` itself: the project's partner alone would still win the merge. + if let Some(sibling) = exclusive_sibling(section, key) { + entry.remove(sibling); + } + + match (entry.get_mut(key.as_str()), value) { + // Both sides are sections: recurse, so an override of one leaf + // leaves the project's sibling leaves alone. An atomic section's + // children are not sections in that sense — they go whole, through + // the arms below. + (Some(toml::Value::Table(entry_table)), toml::Value::Table(overlay_table)) + if !is_atomic_section(section) => + { + section.push(key); + drop_overridden_keys(entry_table, overlay_table, section); + section.pop(); + if entry_table.is_empty() { + entry.remove(key.as_str()); + } + } + (Some(_), _) => { + entry.remove(key.as_str()); + } + (None, _) => {} + } + } +} + +/// The key that `key` clears when both are set under `section`. +/// +/// `[commit.generation]` rejects `template` alongside `template-file` +/// (`UserConfig::validate`), and setting either clears the other when a +/// project entry merges over the global one +/// (`CommitGenerationConfig::merge_with`). +/// So a layer that sets one member has to displace *both* at +/// project scope: dropping only its own key would leave the project's partner +/// to win the merge — the ranking this pass exists to remove. +fn exclusive_sibling(section: &[&str], key: &str) -> Option<&'static str> { + if section != ["commit", "generation"] { + return None; + } + match key { + "template" => Some("template-file"), + "template-file" => Some("template"), + "squash-template" => Some("squash-template-file"), + "squash-template-file" => Some("squash-template"), + _ => None, + } +} + +/// A table whose entries the merge replaces whole, so removing one of an +/// entry's leaves neither removes the precedence nor leaves the entry usable. +/// +/// `[list.custom-columns]` is the case: `ListConfig::merge_with` extends +/// `custom_columns` per *column*, so a project's `Ticket` replaces the global +/// `Ticket` outright — dropping only the overridden leaf would leave the +/// project's column winning anyway, the ranking this pass exists to remove. +/// And `ListColumnConfig::template` is required, so a partial removal can +/// strand a column that no longer deserializes. +/// +/// `section` is the path of the containing table, so this asks "are this +/// table's children atomic", the way [`exclusive_sibling`] asks about a pair. +fn is_atomic_section(section: &[&str]) -> bool { + section == ["list", "custom-columns"] +} + +/// Whether the project-scoped `key` under `section` composes with the global +/// one instead of replacing it, so no layer displaces it. +/// +/// Hook names come from the schema, cached the way +/// `config::is_user_project_override_key` caches its own lookup, so a new hook +/// can't be forgotten. The others are the composing keys elsewhere in the tree +/// — `[aliases]` (`UserConfig::aliases`) and `step.copy-ignored.exclude` +/// (`CopyIgnoredConfig::merged_with` unions the two pattern lists). +fn is_compose_only(section: &[&str], key: &str) -> bool { + static HOOKS: OnceLock> = OnceLock::new(); + match section { + [] => { + key == "aliases" + || HOOKS + .get_or_init(crate::config::schema_top_level_keys::) + .iter() + .any(|hook| hook == key) + } + ["step", "copy-ignored"] => key == "exclude", + _ => false, + } +} + /// Load and validate a single config file. Returns the parsed table for /// merging and validates via `toml::from_str::` for rich errors. fn load_config_file( @@ -468,7 +659,7 @@ impl UserConfig { &result.migrated_content, ConfigFileKind::System, ) { - Ok(table) => deep_merge_table(&mut merged_table, table), + Ok(table) => merge_layer(&mut merged_table, table), Err(e) => warnings.push(e), } } @@ -504,7 +695,7 @@ impl UserConfig { &result.migrated_content, ConfigFileKind::User, ) { - Ok(table) => deep_merge_table(&mut merged_table, table), + Ok(table) => merge_layer(&mut merged_table, table), Err(e) => warnings.push(e), } } @@ -534,7 +725,7 @@ impl UserConfig { // needs Integer for u64, WORKTRUNK_WORKTREE_PATH=42 needs String). let file_table = merged_table.clone(); let env_overlay = migrate_env_overlay(resolve_env_overlay(&file_table, &env_vars)); - deep_merge_table(&mut merged_table, env_overlay); + merge_layer(&mut merged_table, env_overlay); // Env overlay broke deserialization — fall back to file-only config. // Each file was individually validated by load_config_file(), so the @@ -590,40 +781,40 @@ impl UserConfig { return; } - let base = merged_table.clone(); + let mut overlay = toml::Table::new(); for raw in overrides { // `migrate_content` returns the fragment unchanged when it is not // valid TOML, so the parse below still catches a malformed fragment // and drops the whole layer with an attributed warning. let migrated = super::deprecation::migrate_content(raw); match migrated.parse::() { - Ok(fragment) => deep_merge_table(merged_table, fragment), + Ok(fragment) => deep_merge_table(&mut overlay, fragment), Err(err) => { warnings.push(LoadError::CliOverride { err: err.to_string(), overrides: overrides.to_vec(), }); - *merged_table = base; return; } } } + let mut candidate = merged_table.clone(); + merge_layer(&mut candidate, overlay); + // Probe deserialize *and* validate, so a semantically-invalid override // (e.g. an empty worktree-path) drops just this layer rather than // falling through to finalize(), which would wipe the lower layers to // defaults. - let probe = match toml::Value::Table(merged_table.clone()).try_into::() { - Ok(config) => config.validate().map_err(|e| e.0), - Err(err) => Err(err.to_string()), - }; - if let Err(err) = probe { + if let Err(err) = deserialize_and_validate(&candidate) { warnings.push(LoadError::CliOverride { err, overrides: overrides.to_vec(), }); - *merged_table = base; + return; } + + *merged_table = candidate; } /// Deserialize a merged table into `UserConfig`, validate, and collect diff --git a/src/config/user/tests.rs b/src/config/user/tests.rs index 1c4778bc1..dd3830486 100644 --- a/src/config/user/tests.rs +++ b/src/config/user/tests.rs @@ -2645,6 +2645,330 @@ fn test_try_parse_value() { ); } +// ========================================================================= +// merge_layer() — a layer's global keys vs the `[projects]` entries below +// ========================================================================= + +const PROJECT: &str = "github.com/owner/repo"; + +/// A base table with one project entry carrying `body`. +fn base_with_project(body: &str) -> toml::Table { + format!("[projects.\"{PROJECT}\"]\n{body}").parse().unwrap() +} + +fn loaded(table: toml::Table) -> UserConfig { + toml::Value::Table(table).try_into().unwrap() +} + +#[test] +fn test_cli_layer_outranks_project_worktree_path() { + // The reported bug (#3788), on the `--config-set` half: a project entry's + // `worktree-path` no longer beats a global key a higher layer set. + let base = base_with_project("worktree-path = \"/from-project\"\n"); + let (table, warnings) = apply_overrides(base, &["worktree-path = \"/from-cli\""]); + assert!(warnings.is_empty()); + assert_eq!( + loaded(table).worktree_path_for_project(PROJECT), + "/from-cli" + ); +} + +#[test] +fn test_env_layer_outranks_project_worktree_path() { + // The env half of the same fix, driven through the overlay + // `load_with_warnings` builds rather than the process environment. + use super::{EnvVar, migrate_env_overlay, resolve_env_overlay, try_parse_value}; + let var = EnvVar { + name: "WORKTRUNK_WORKTREE_PATH".to_string(), + segments: vec!["worktree-path".to_string()], + typed_value: try_parse_value("/from-env"), + raw_value: "/from-env".to_string(), + }; + let mut table = base_with_project("worktree-path = \"/from-project\"\n"); + let overlay = migrate_env_overlay(resolve_env_overlay(&table, &[var])); + merge_layer(&mut table, overlay); + + assert_eq!( + loaded(table).worktree_path_for_project(PROJECT), + "/from-env" + ); +} + +#[test] +fn test_layer_keeps_an_already_invalid_candidate_untouched() { + // The pass discards a candidate that does not deserialize and validate. + // Step 3's env probe only deserializes, so an empty `worktree-path` from + // the environment reaches here already invalid — and the removals are + // dropped rather than handed to `finalize`, which would answer the same + // failure by wiping the config to defaults. + use super::{EnvVar, migrate_env_overlay, resolve_env_overlay, try_parse_value}; + let empty_path = |value: &str| EnvVar { + name: "WORKTRUNK_WORKTREE_PATH".to_string(), + segments: vec!["worktree-path".to_string()], + typed_value: try_parse_value(value), + raw_value: value.to_string(), + }; + let plain_merge = |overlay: &toml::Table| { + let mut plain = base_with_project("worktree-path = \"/from-project\"\n"); + deep_merge_table(&mut plain, overlay.clone()); + plain + }; + + let mut table = base_with_project("worktree-path = \"/from-project\"\n"); + let overlay = migrate_env_overlay(resolve_env_overlay(&table, &[empty_path("")])); + let plain = plain_merge(&overlay); + merge_layer(&mut table, overlay); + assert_eq!( + table, plain, + "the removals are discarded as a unit, and the layer still applies" + ); + + // Control: the same overlay with a valid value does remove the project's + // key, so the assertion above is the discard and not a pass that found + // nothing to do. + let mut table = base_with_project("worktree-path = \"/from-project\"\n"); + let overlay = migrate_env_overlay(resolve_env_overlay(&table, &[empty_path("/from-env")])); + let plain = plain_merge(&overlay); + merge_layer(&mut table, overlay); + assert_ne!(table, plain); +} + +#[test] +fn test_file_layer_outranks_lower_layer_project_entry() { + // The same rule where neither layer is an invocation one: the user file's + // global key answers for a project the system file keyed an entry to. + let mut table = base_with_project("worktree-path = \"/from-system-project\"\n"); + merge_layer( + &mut table, + "worktree-path = \"/from-user-global\"\n".parse().unwrap(), + ); + + assert_eq!( + loaded(table).worktree_path_for_project(PROJECT), + "/from-user-global" + ); +} + +#[test] +fn test_layer_leaves_untouched_project_keys() { + // Only the overridden key is displaced: a project entry's other settings, + // and its sibling keys inside the same section, still apply. + let base = base_with_project( + r#"worktree-path = "/from-project" + +[projects."github.com/owner/repo".list] +full = true +branches = true +"#, + ); + let (table, warnings) = apply_overrides(base, &["list.full = false"]); + assert!(warnings.is_empty()); + let config = loaded(table); + assert_eq!(config.worktree_path_for_project(PROJECT), "/from-project"); + let list = config.list(Some(PROJECT)); + assert_eq!(list.full, Some(false), "the higher layer wins"); + assert_eq!(list.branches, Some(true), "sibling key survives"); +} + +#[test] +fn test_layer_keeps_its_own_project_scoped_override() { + // Naming the project entry is both the highest layer and the most + // specific key, so it outranks the same layer's global key. + let base = base_with_project("worktree-path = \"/from-project\"\n"); + let (table, warnings) = apply_overrides( + base, + &[ + "worktree-path = \"/from-cli-global\"", + &format!("projects.\"{PROJECT}\".worktree-path = \"/from-cli-project\""), + ], + ); + assert!(warnings.is_empty()); + assert_eq!( + loaded(table).worktree_path_for_project(PROJECT), + "/from-cli-project" + ); +} + +#[test] +fn test_layer_applies_to_pattern_entries() { + // Pattern entries are project entries too — a `*` key must not smuggle a + // project-scoped value past a higher layer. + let base: toml::Table = "[projects.\"github.com/*\"]\nworktree-path = \"/from-pattern\"\n" + .parse() + .unwrap(); + let (table, warnings) = apply_overrides(base, &["worktree-path = \"/from-cli\""]); + assert!(warnings.is_empty()); + assert_eq!( + loaded(table).worktree_path_for_project(PROJECT), + "/from-cli" + ); +} + +#[test] +fn test_layer_leaves_composing_keys_alone() { + // Per-project hooks, aliases and copy-ignored excludes append to the + // global ones rather than replacing them, so both already apply and there + // is no precedence to fix. Dropping the project's copy would silently stop + // it applying. + let base = base_with_project( + r#"pre-merge = "project-hook" + +[projects."github.com/owner/repo".aliases] +ship = "project-alias" + +[projects."github.com/owner/repo".step.copy-ignored] +exclude = ["project-pattern"] +"#, + ); + let (table, warnings) = apply_overrides( + base, + &[ + "pre-merge = \"cli-hook\"", + "aliases.ship = \"cli-alias\"", + "step.copy-ignored.exclude = [\"cli-pattern\"]", + ], + ); + assert!(warnings.is_empty()); + let config = loaded(table); + let templates = |commands: &CommandConfig| { + commands + .commands() + .map(|command| command.template.clone()) + .collect::>() + }; + assert_eq!( + templates(&config.hooks(Some(PROJECT)).pre_merge.unwrap()), + ["cli-hook", "project-hook"] + ); + assert_eq!( + templates(&config.aliases(Some(PROJECT))["ship"]), + ["cli-alias", "project-alias"] + ); + assert_eq!( + config.copy_ignored(Some(PROJECT)).exclude, + ["cli-pattern", "project-pattern"] + ); +} + +#[test] +fn test_layer_displaces_whole_custom_column() { + // `[list.custom-columns]` merges per column, so an override of one leaf + // has to displace the whole column: leaving the rest of the project's + // column would let it replace the global one wholesale anyway, and + // `template` is required — a column stripped of it stops deserializing, + // which would cost the user their whole config rather than one entry. + let base = base_with_project( + r#"[projects."github.com/owner/repo".list.custom-columns.Ticket] +template = "{{ vars.ticket }}" +width = 30 +"#, + ); + let (table, warnings) = apply_overrides( + base, + &["list.custom-columns.Ticket.template = \"from-cli\""], + ); + assert!(warnings.is_empty()); + let column = loaded(table).list(Some(PROJECT)).custom_columns["Ticket"].clone(); + assert_eq!(column.template, "from-cli"); + assert_eq!(column.width, None, "the column went as a unit"); + + // Restating the column at project scope wins, as for any other key — and + // states the whole column, since the layer's global already displaced the + // one below it. A column is the unit on both sides of the boundary, so + // `width` is not carried over from the entry that was displaced. + let base = base_with_project( + r#"[projects."github.com/owner/repo".list.custom-columns.Ticket] +template = "{{ vars.ticket }}" +width = 30 +"#, + ); + let (table, warnings) = apply_overrides( + base, + &[ + "list.custom-columns.Ticket.template = \"from-cli\"", + &format!( + "projects.\"{PROJECT}\".list.custom-columns.Ticket.template = \"from-cli-project\"" + ), + ], + ); + assert!(warnings.is_empty()); + let column = loaded(table).list(Some(PROJECT)).custom_columns["Ticket"].clone(); + assert_eq!(column.template, "from-cli-project"); + assert_eq!(column.width, None, "the column went as a unit here too"); +} + +#[test] +fn test_layer_displaces_exclusive_sibling() { + // Each `[commit.generation]` pair clears itself, so overriding one member + // has to displace both at project scope: leaving the project's partner + // would let it win the merge, and it would fail validation next to the + // global key the same entry now answers with. + let pairs = [ + ("template", "template-file"), + ("template-file", "template"), + ("squash-template", "squash-template-file"), + ("squash-template-file", "squash-template"), + ]; + for (overridden, partner) in pairs { + let base = base_with_project(&format!( + "[projects.\"{PROJECT}\".commit.generation]\n{partner} = \"/project.txt\"\n" + )); + let (table, warnings) = apply_overrides( + base, + &[&format!("commit.generation.{overridden} = \"from-cli\"")], + ); + assert!(warnings.is_empty(), "{overridden}: {warnings:?}"); + let config = loaded(table); + config + .validate() + .unwrap_or_else(|e| panic!("{overridden}: {e}")); + let generation = config.commit_generation(Some(PROJECT)); + let value = |key: &str| match key { + "template" => generation.template.as_deref(), + "template-file" => generation.template_file.as_deref(), + "squash-template" => generation.squash_template.as_deref(), + _ => generation.squash_template_file.as_deref(), + }; + assert_eq!(value(overridden), Some("from-cli"), "{overridden}"); + assert_eq!(value(partner), None, "{overridden} should clear {partner}"); + } + + // A `[commit.generation]` key that is in no pair displaces only itself. + let base = base_with_project(&format!( + "[projects.\"{PROJECT}\".commit.generation]\ncommand = \"project-llm\"\ntemplate-file = \"/project.txt\"\n" + )); + let (table, warnings) = apply_overrides(base, &["commit.generation.command = \"cli-llm\""]); + assert!(warnings.is_empty()); + let generation = loaded(table).commit_generation(Some(PROJECT)); + assert_eq!(generation.command.as_deref(), Some("cli-llm")); + assert_eq!(generation.template_file.as_deref(), Some("/project.txt")); +} + +#[test] +fn test_layer_noop_without_overrides() { + // No higher layer, no change: a project entry keeps every key. + let base = base_with_project("worktree-path = \"/from-project\"\n"); + let (table, warnings) = apply_overrides(base, &[]); + assert!(warnings.is_empty()); + assert_eq!( + loaded(table).worktree_path_for_project(PROJECT), + "/from-project" + ); +} + +#[test] +fn test_dropped_layer_leaves_projects_intact() { + // A `--config-set` layer that rolls back (malformed fragment) overrides + // nothing, so it must not displace the project entry either. + let base = base_with_project("worktree-path = \"/from-project\"\n"); + let (table, warnings) = apply_overrides(base, &["worktree-path = \"/from-cli\"", "garbage"]); + assert_eq!(warnings.len(), 1); + assert_eq!( + loaded(table).worktree_path_for_project(PROJECT), + "/from-project" + ); +} + #[test] fn test_env_overlay_migrates_deprecated_key() { use super::{EnvVar, migrate_env_overlay, resolve_env_overlay, try_parse_value}; diff --git a/tests/integration_tests/switch.rs b/tests/integration_tests/switch.rs index 3cd32fcb7..c93c7651b 100644 --- a/tests/integration_tests/switch.rs +++ b/tests/integration_tests/switch.rs @@ -8,7 +8,7 @@ use ansi_str::AnsiStr; use insta_cmd::assert_cmd_snapshot; use rstest::rstest; use std::fs; -use std::path::Path; +use std::path::{Path, PathBuf}; use tempfile::TempDir; // Snapshot helpers @@ -2432,6 +2432,130 @@ worktree-path = "{{ repo_path }}/../{{ branch | sanitize }}" ); } +/// A layer's global key outranks the `[projects."…"]` entries below it, at +/// every boundary: `WORKTRUNK_WORKTREE_PATH`, `--config-set worktree-path` and +/// the user file's global key each beat an entry a lower layer set, while an +/// entry still outranks the global key of its own layer (#3788). +/// +/// End-to-end because the rule lives in config *loading*: only a real process +/// reads `WORKTRUNK_WORKTREE_PATH` off the environment and stacks a system file +/// under the user's. +#[rstest] +fn test_switch_create_layers_outrank_project_worktree_path(repo: TestRepo) { + set_github_remote_url(&repo); + + let created_path = |args: &[&str], env: &[(&str, &str)]| { + let mut cmd = repo.wt_command(); + cmd.args(args); + for (key, value) in env { + cmd.env(key, value); + } + let output = cmd.output().unwrap(); + assert!( + output.status.success(), + "switch --create should succeed, stderr: {}", + String::from_utf8_lossy(&output.stderr) + ); + let json: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap(); + PathBuf::from(json["path"].as_str().unwrap()) + }; + let switch = + |branch: &'static str| vec!["switch", "--create", branch, "--format=json", "--no-cd"]; + const CLI_TEMPLATE: &str = + r#"worktree-path = "{{ repo_path }}/../from-cli-{{ branch | sanitize }}""#; + + repo.write_test_config( + r#" +worktree-path = "{{ repo_path }}/../from-global-{{ branch | sanitize }}" + +[projects."github.com/owner/test-repo"] +worktree-path = "{{ repo_path }}/../from-project-{{ branch | sanitize }}" +"#, + ); + + // Control: within the config file, the project entry still outranks the + // global key. Without this the assertions below would hold equally if + // project entries had stopped applying at all. + let file_path = created_path(&switch("file-layer"), &[]); + assert_eq!( + file_path.file_name().unwrap(), + "from-project-file-layer", + "project entry should outrank the file's global key, got {}", + file_path.display() + ); + + let env_path = created_path( + &switch("env-layer"), + &[( + "WORKTRUNK_WORKTREE_PATH", + "{{ repo_path }}/../from-env-{{ branch | sanitize }}", + )], + ); + assert_eq!( + env_path.file_name().unwrap(), + "from-env-env-layer", + "WORKTRUNK_WORKTREE_PATH should outrank the project entry, got {}", + env_path.display() + ); + + let mut cli_args = vec!["--config-set", CLI_TEMPLATE]; + cli_args.extend(switch("cli-layer")); + let cli_path = created_path(&cli_args, &[]); + assert_eq!( + cli_path.file_name().unwrap(), + "from-cli-cli-layer", + "--config-set should outrank the project entry, got {}", + cli_path.display() + ); + + // An entry outranks the global key of its own layer, which is why naming + // the project entry pins a `--config-set` to one repo. + let mut pinned_args = vec![ + "--config-set", + CLI_TEMPLATE, + "--config-set", + r#"projects."github.com/owner/test-repo".worktree-path = "{{ repo_path }}/../from-pin-{{ branch | sanitize }}""#, + ]; + pinned_args.extend(switch("pinned")); + let pinned_path = created_path(&pinned_args, &[]); + assert_eq!( + pinned_path.file_name().unwrap(), + "from-pin-pinned", + "--config-set on the project entry should win, got {}", + pinned_path.display() + ); + + // The rule is the same one layer down, where no invocation is involved: + // with the entry moved to the system file, the user file's global key + // answers for this repo. Written last because it rewrites the user config. + let system_dir = tempfile::tempdir().unwrap(); + let system_config = system_dir.path().join("config.toml"); + fs::write( + &system_config, + r#" +[projects."github.com/owner/test-repo"] +worktree-path = "{{ repo_path }}/../from-system-project-{{ branch | sanitize }}" +"#, + ) + .unwrap(); + repo.write_test_config( + r#"worktree-path = "{{ repo_path }}/../from-user-global-{{ branch | sanitize }}""#, + ); + let system_path = created_path( + &switch("system-layer"), + &[( + "WORKTRUNK_SYSTEM_CONFIG_PATH", + system_config.to_str().unwrap(), + )], + ); + assert_eq!( + system_path.file_name().unwrap(), + "from-user-global-system-layer", + "the user file's global key should outrank the system file's project entry, got {}", + system_path.display() + ); +} + // ============================================================================ // PR Syntax Tests (pr:) // ============================================================================ diff --git a/tests/snapshots/integration__integration_tests__help__help_config_create.snap b/tests/snapshots/integration__integration_tests__help__help_config_create.snap index 441405d99..875b55fe0 100644 --- a/tests/snapshots/integration__integration_tests__help__help_config_create.snap +++ b/tests/snapshots/integration__integration_tests__help__help_config_create.snap @@ -36,6 +36,7 @@ info: WORKTRUNK_TEST_PARENT_SHELL: "" WORKTRUNK_TEST_POWERSHELL_ENV: "0" WORKTRUNK_TEST_POWERSHELL_INSTALLED: "0" + WORKTRUNK_TEST_PROBE_TIMEOUT_MS: "60000" WORKTRUNK_TEST_SKIP_URL_HEALTH_CHECK: "1" WORKTRUNK_TEST_ZSH_INSTALLED: "0" --- @@ -313,7 +314,7 @@ Creates ~/.config/worktrunk/config.toml with the following content:   #   # Entries are keyed by project identifier — `//` derived from the primary remote URL (no `.git` suffix), or the canonical repo path when there is no remote. Run `wt config show` inside the repo to see the identifier for the current project; it appears in the `PROJECT CONFIG` section as `Identifier: …`.   # -  # Scalar values (like `worktree-path`) replace the global value; everything else (hooks, aliases, etc.) appends, global first. +  # Scalar values (like `worktree-path`) replace the global value; everything else (hooks, aliases, etc.) appends, global first. An entry outranks the global key of the same name, and both lose to a `WORKTRUNK_` env var or `--config-set` — see how the layers rank (https://worktrunk.dev/config/#precedence).   #   # [projects."github.com/user/repo"]   # worktree-path = ".worktrees/{{ branch | sanitize }}" diff --git a/tests/snapshots/integration__integration_tests__help__help_config_long.snap b/tests/snapshots/integration__integration_tests__help__help_config_long.snap index c0fe08fef..afe00f020 100644 --- a/tests/snapshots/integration__integration_tests__help__help_config_long.snap +++ b/tests/snapshots/integration__integration_tests__help__help_config_long.snap @@ -35,6 +35,7 @@ info: WORKTRUNK_TEST_PARENT_SHELL: "" WORKTRUNK_TEST_POWERSHELL_ENV: "0" WORKTRUNK_TEST_POWERSHELL_INSTALLED: "0" + WORKTRUNK_TEST_PROBE_TIMEOUT_MS: "60000" WORKTRUNK_TEST_SKIP_URL_HEALTH_CHECK: "1" WORKTRUNK_TEST_ZSH_INSTALLED: "0" --- @@ -361,7 +362,7 @@ User config can include a [projects] table for project-specific settings Entries are keyed by project identifier — // derived from the primary remote URL (no .git suffix), or the canonical repo path when there is no remote. Run wt config show inside the repo to see the identifier for the current project; it appears in the PROJECT CONFIG section as Identifier: …. -Scalar values (like worktree-path) replace the global value; everything else (hooks, aliases, etc.) appends, global first. +Scalar values (like worktree-path) replace the global value; everything else (hooks, aliases, etc.) appends, global first. An entry outranks the global key of the same name, and both lose to a WORKTRUNK_ env var or --config-set — see how the layers rank.   [projects."github.com/user/repo"]   worktree-path = ".worktrees/{{ branch | sanitize }}" @@ -615,7 +616,7 @@ On first run without shell integration, Worktrunk offers to install it. On first Environment variables -All user config options can be overridden with environment variables using the WORKTRUNK_ prefix. +All user config options can be overridden with environment variables using the WORKTRUNK_ prefix, which outrank config files. Naming convention @@ -656,11 +657,26 @@ Override the LLM command in CI to use a mock: Inline config overrides (--config-set) ---config-set  overrides any user config key for a single invocation, with higher priority than both config files and WORKTRUNK_ env vars. The value is a TOML fragment, so arrays and tables work directly; the flag is global (works before or after the subcommand), repeatable, and a later --config-set replaces an earlier one for the same key. +--config-set  overrides any user config key for a single invocation, ahead of every other source. The value is a TOML fragment, so arrays and tables work directly; the flag is global (works before or after the subcommand), repeatable, and a later --config-set replaces an earlier one for the same key.   wt --config-set list.full=true list   wt step copy-ignored --config-set 'step.copy-ignored.exclude=["target", "dist"]' This composes with aliases — an alias body can invoke wt --config-set …  to render a named view without changing the saved config. +Precedence + +Sources rank by how close they are to the invocation, which puts user config above system config. Within a config file, an entry keyed to the project outranks the global key of the same name. So worktree-path comes from the first of these that sets it: + +1. --config-set 'worktree-path = …' +2. WORKTRUNK_WORKTREE_PATH +3. [projects."github.com/owner/repo"] in the config file +4. global worktree-path in the config file + +A --config-set that names a project entry applies to that project alone, and beats a global --config-set of the same key — it is both the highest layer and the most specific key: + +  wt --config-set 'projects."github.com/owner/repo".worktree-path = "/tmp/scratch"' switch --create feature + +Ranking only matters where one value has to win. Hooks, aliases and step.copy-ignored.exclude accumulate rather than replace, so an env-set hook and a project's hook both run. + ----- stderr -----