diff --git a/docs/content/hook.md b/docs/content/hook.md index 0975dcee8..df00c75ce 100644 --- a/docs/content/hook.md +++ b/docs/content/hook.md @@ -102,7 +102,7 @@ server = "npm run dev" Here `install` runs first, then `build` and `server` run together. -Templates are syntax-checked before the pipeline starts and rendered as each step runs, so a step can store [per-branch vars](@/config.md#wt-config-state-vars) that later steps read via `{{ vars. }}`. +Templates are syntax-checked before the pipeline starts and rendered as each step runs, so a step can store [per-branch vars](@/config.md#wt-config-state-vars) that later steps read via `{{ vars. }}`. Because an earlier step can still change those values, a preview leaves them alone: `wt hook --dry-run` and `wt hook show --expanded` render `{{ vars. }}` as itself while every other variable expands. Most hooks don't need `[[hook]]` blocks. Reach for them when there's a dependency chain — typically setup that must complete before later steps, like installing dependencies before running a build and dev server concurrently. diff --git a/plugins/worktrunk/skills/worktrunk/reference/hook.md b/plugins/worktrunk/skills/worktrunk/reference/hook.md index 9dc5e9ff0..9e4fc8143 100644 --- a/plugins/worktrunk/skills/worktrunk/reference/hook.md +++ b/plugins/worktrunk/skills/worktrunk/reference/hook.md @@ -93,7 +93,7 @@ server = "npm run dev" Here `install` runs first, then `build` and `server` run together. -Templates are syntax-checked before the pipeline starts and rendered as each step runs, so a step can store [per-branch vars](https://worktrunk.dev/config/#wt-config-state-vars) that later steps read via `{{ vars. }}`. +Templates are syntax-checked before the pipeline starts and rendered as each step runs, so a step can store [per-branch vars](https://worktrunk.dev/config/#wt-config-state-vars) that later steps read via `{{ vars. }}`. Because an earlier step can still change those values, a preview leaves them alone: `wt hook --dry-run` and `wt hook show --expanded` render `{{ vars. }}` as itself while every other variable expands. Most hooks don't need `[[hook]]` blocks. Reach for them when there's a dependency chain — typically setup that must complete before later steps, like installing dependencies before running a build and dev server concurrently. diff --git a/skills/worktrunk/reference/hook.md b/skills/worktrunk/reference/hook.md index 9dc5e9ff0..9e4fc8143 100644 --- a/skills/worktrunk/reference/hook.md +++ b/skills/worktrunk/reference/hook.md @@ -93,7 +93,7 @@ server = "npm run dev" Here `install` runs first, then `build` and `server` run together. -Templates are syntax-checked before the pipeline starts and rendered as each step runs, so a step can store [per-branch vars](https://worktrunk.dev/config/#wt-config-state-vars) that later steps read via `{{ vars. }}`. +Templates are syntax-checked before the pipeline starts and rendered as each step runs, so a step can store [per-branch vars](https://worktrunk.dev/config/#wt-config-state-vars) that later steps read via `{{ vars. }}`. Because an earlier step can still change those values, a preview leaves them alone: `wt hook --dry-run` and `wt hook show --expanded` render `{{ vars. }}` as itself while every other variable expands. Most hooks don't need `[[hook]]` blocks. Reach for them when there's a dependency chain — typically setup that must complete before later steps, like installing dependencies before running a build and dev server concurrently. diff --git a/src/cli/mod.rs b/src/cli/mod.rs index 3513134ba..6eb7c9992 100644 --- a/src/cli/mod.rs +++ b/src/cli/mod.rs @@ -1613,7 +1613,7 @@ server = "npm run dev" Here `install` runs first, then `build` and `server` run together. -Templates are syntax-checked before the pipeline starts and rendered as each step runs, so a step can store [per-branch vars](@/config.md#wt-config-state-vars) that later steps read via `{{ vars. }}`. +Templates are syntax-checked before the pipeline starts and rendered as each step runs, so a step can store [per-branch vars](@/config.md#wt-config-state-vars) that later steps read via `{{ vars. }}`. Because an earlier step can still change those values, a preview leaves them alone: `wt hook --dry-run` and `wt hook show --expanded` render `{{ vars. }}` as itself while every other variable expands. Most hooks don't need `[[hook]]` blocks. Reach for them when there's a dependency chain — typically setup that must complete before later steps, like installing dependencies before running a build and dev server concurrently. diff --git a/src/commands/command_executor.rs b/src/commands/command_executor.rs index 55ac69f68..22d1ce2c0 100644 --- a/src/commands/command_executor.rs +++ b/src/commands/command_executor.rs @@ -5,8 +5,8 @@ use anyhow::Result; use color_print::cformat; use worktrunk::HookType; use worktrunk::config::{ - Command, CommandConfig, HookStep, TemplateContext, UserConfig, VarScope, format_hook_variables, - template_references_var, validate_template_syntax, + Command, CommandConfig, HookStep, TemplateContext, UserConfig, VarScope, VarsMode, + format_hook_variables, validate_template_syntax, }; use worktrunk::git::{ErrorExt, Repository, WorktrunkError}; use worktrunk::path::{format_path_for_display, to_posix_path}; @@ -26,8 +26,9 @@ use crate::output::{DirectivePassthrough, execute_shell_command}; pub struct PreparedCommand { pub name: Option, /// Raw template, rendered against `context` when the command runs. - /// Syntax is validated at preparation; rendering is deferred so `vars.*` - /// set by earlier pipeline steps are read fresh from git config. + /// Execution paths validate syntax via [`PreparedPipeline::validated`]; + /// rendering is deferred so `vars.*` set by earlier pipeline steps are + /// read fresh from git config. pub template: String, /// Template variables, frozen at preparation. Serialized to JSON only at /// the process boundary (child stdin, background pipeline spec). @@ -434,24 +435,26 @@ fn resolve_command_str(cmd: &PreparedCommand, repo: &Repository) -> Result }}`, which renders back as itself: those values are read from +/// git config when the step runs, possibly written by an earlier step, so a +/// value resolved now could differ from the one the run uses. Expansion is +/// side-effect-free, so previewing never perturbs the real run. pub fn render_template_preview( template: &str, context: &TemplateContext, repo: &Repository, name: &str, ) -> Result { - if template_references_var(template, "vars") { - validate_template_syntax(template, name)?; - Ok(template.to_string()) - } else { - expand_shell_template(template, context, repo, name) - } + Ok(context.expand_with( + template, + ShellEscapeMode::Posix, + repo, + name, + VarsMode::Literal, + )?) } /// Short summary name: "user:name" for named commands, "user" otherwise. @@ -735,20 +738,36 @@ pub fn map_config_steps( .collect() } -/// Reject a prepared pipeline whose templates cannot parse, before its first -/// command runs. +/// A prepared pipeline that has not yet chosen a syntax-error policy. /// -/// Separate from [`prepare_steps`] because it is an execution policy rather -/// than part of building a command: every path that runs hooks calls it, while -/// `wt hook show --expanded` prepares the same commands only to display them -/// and annotates a broken template in place instead of blanking the listing. -/// Semantic errors (undefined variable, filter failure) are not checked here — -/// rendering is deferred, so they surface at the failing step. -pub fn validate_pipeline_syntax(steps: &[PreparedStep]) -> Result<()> { - for cmd in steps.iter().flat_map(PreparedStep::commands) { - validate_template_syntax(&cmd.template, &cmd.template_name)?; +/// [`prepare_steps`] returns this rather than the steps themselves so the +/// choice is a method call the compiler demands: running hooks takes +/// [`validated`](Self::validated), and the one caller that must not abort takes +/// [`into_unvalidated`](Self::into_unvalidated). A new execution path can no +/// longer skip the check by forgetting a line — the same reason `ApprovedHookPlan` +/// makes hook approval unforgeable. +#[must_use] +pub struct PreparedPipeline(Vec); + +impl PreparedPipeline { + /// The steps, rejected if any template cannot parse — so a pipeline that + /// cannot render in full never starts. + /// + /// Semantic errors (undefined variable, filter failure) are not checked: + /// rendering is deferred, so they surface at the failing step. + pub fn validated(self) -> Result> { + for cmd in self.0.iter().flat_map(PreparedStep::commands) { + validate_template_syntax(&cmd.template, &cmd.template_name)?; + } + Ok(self.0) + } + + /// The steps with no syntax check, for `wt hook show --expanded`: a + /// listing annotates a broken template in place rather than blanking the + /// rest of the listing. + pub fn into_unvalidated(self) -> Vec { + self.0 } - Ok(()) } /// Prepare hook pipeline steps, preserving serial/concurrent structure. Sole @@ -758,16 +777,16 @@ pub fn validate_pipeline_syntax(steps: &[PreparedStep]) -> Result<()> { /// /// Each command freezes its context as JSON and keeps its raw template; /// rendering happens when the command runs, so semantic errors (undefined -/// variable, filter failure) surface at the failing step. Execution paths -/// follow up with [`validate_pipeline_syntax`], which aborts an unparsable -/// pipeline before its first step runs. +/// variable, filter failure) surface at the failing step. The returned +/// [`PreparedPipeline`] makes the caller choose what an unparsable template +/// does. pub fn prepare_steps( command_config: &CommandConfig, ctx: &CommandContext<'_>, extra_vars: &[(&str, &str)], hook_type: HookType, source: HookSource, -) -> anyhow::Result> { +) -> anyhow::Result { // Built once per pipeline — build_hook_context spawns git subprocesses. let mut base_context = build_hook_context(ctx, extra_vars, VarScope::All)?; @@ -785,7 +804,7 @@ pub fn prepare_steps( base_context.insert(worktrunk::config::ALIAS_ARGS_KEY, "[]"); } - map_config_steps(command_config, |cmd| { + let steps = map_config_steps(command_config, |cmd| { // hook_name is per-command: available as template variable and in JSON context let mut cmd_context = base_context.clone(); if let Some(ref name) = cmd.name { @@ -804,7 +823,8 @@ pub fn prepare_steps( template_name, label: command_summary_name(cmd.name.as_deref(), source), }) - }) + })?; + Ok(PreparedPipeline(steps)) } #[cfg(test)] @@ -937,26 +957,4 @@ mod tests { let result = handle_command_error(err, &cmd, &wrapper, FailureStrategy::Warn); assert!(result.is_ok()); } - - #[test] - fn test_template_references_var_for_vars() { - // Real vars references - assert!(template_references_var("{{ vars.container }}", "vars")); - assert!(template_references_var("{{vars.container}}", "vars")); - assert!(template_references_var( - "docker run --name {{ vars.name }}", - "vars" - )); - assert!(template_references_var( - "{% if vars.key %}yes{% endif %}", - "vars" - )); - - // Literal text — not a template reference - assert!(!template_references_var( - "echo hello > template_vars.txt", - "vars" - )); - assert!(!template_references_var("no vars references here", "vars")); - } } diff --git a/src/commands/hook_commands.rs b/src/commands/hook_commands.rs index 514667ac9..bd39eed45 100644 --- a/src/commands/hook_commands.rs +++ b/src/commands/hook_commands.rs @@ -85,18 +85,18 @@ fn run_post_hook( /// (execution + dry-run) and [`hook_command_rows`] (`hook show --expanded`) use /// this function. Returns a `TemplateVars` so callers can extend with /// additional bindings (e.g. CLI shorthand) before materializing. -fn build_manual_hook_template_vars( - ctx: &CommandContext, - hook_type: HookType, - default_branch: Option<&str>, -) -> TemplateVars { +fn build_manual_hook_template_vars(ctx: &CommandContext, hook_type: HookType) -> TemplateVars { let branch = ctx.branch_or_head(); let worktree_path = ctx.worktree_path; match hook_type { - // Merge/commit hooks: target = merge target (default branch for commit, current for merge) - HookType::PreCommit | HookType::PostCommit => { - default_branch.map_or_else(TemplateVars::new, |t| TemplateVars::new().with_target(t)) - } + // Merge/commit hooks: target = merge target (default branch for commit, + // current for merge). Only this arm needs the default branch, and + // resolving it can cost a `git ls-remote` on a fresh clone — so it is + // fetched here rather than up front. + HookType::PreCommit | HookType::PostCommit => ctx + .repo + .default_branch() + .map_or_else(TemplateVars::new, |t| TemplateVars::new().with_target(&t)), HookType::PreMerge | HookType::PostMerge => TemplateVars::new() .with_target(branch) .with_target_worktree_path(worktree_path), @@ -271,14 +271,13 @@ pub fn run_hook( .collect(); // Build extra vars per hook type (shared by dry-run and execution paths) - let default_branch = repo.default_branch(); // Splice `args` into the template context as a JSON-encoded sequence. // `expand_template` rehydrates it as `ShellArgs` so bare `{{ args }}` // renders space-joined with per-element shell escaping. Mirrors // `run_alias` at `src/commands/alias.rs`. let args_json = serde_json::to_string(&args).expect("Vec serialization should never fail"); - let template_vars = build_manual_hook_template_vars(&ctx, hook_type, default_branch.as_deref()); + let template_vars = build_manual_hook_template_vars(&ctx, hook_type); let mut extra_vars = template_vars.as_extra_vars(); extra_vars.extend(custom_vars_refs.iter().copied()); // Forward positional CLI args as `{{ args }}` (empty sequence when @@ -418,7 +417,7 @@ pub fn handle_hook_show( /// Each record carries the hook type, source (user or project), optional name, /// raw template, project approval status, and — when `--expanded` was passed — /// the rendered command preview. `handle_hook_show` builds `ctx` only under -/// `--expanded`, so its presence is that flag. +/// `--expanded`, and each row carries whether it was expanded. fn emit_hook_show_json( user_config: &UserConfig, project_config: Option<&ProjectConfig>, @@ -440,8 +439,8 @@ fn emit_hook_show_json( "needs_approval": needs_approval(source, approvals, project_id, &row.template), }); - if ctx.is_some() { - obj["expanded"] = serde_json::Value::String(row.display); + if let Some(expanded) = row.expanded { + obj["expanded"] = serde_json::Value::String(expanded); } entries.push(obj); @@ -624,7 +623,8 @@ fn render_hook_commands( }; writeln!(out, "{emoji} {label}{suffix}")?; - writeln!(out, "{}", format_bash_with_gutter(&row.display))?; + let shown = row.expanded.as_deref().unwrap_or(&row.template); + writeln!(out, "{}", format_bash_with_gutter(shown))?; } Ok(()) @@ -653,9 +653,10 @@ fn needs_approval( struct HookCommandRow { name: Option, template: String, - /// What to print: the command as it would run under `--expanded`, - /// otherwise the raw template. - display: String, + /// The command as it would run, under `--expanded`. `None` without it, so + /// the listing prints the raw template and the JSON omits the field — + /// neither has to re-derive which mode it is in. + expanded: Option, } /// The rows for one hook config, expanded when `ctx` is present — @@ -688,12 +689,11 @@ fn hook_command_rows( if let Some(ctx) = ctx && config.commands().next().is_some() { - let default_branch = ctx.repo.default_branch(); - let template_vars = - build_manual_hook_template_vars(ctx, hook_type, default_branch.as_deref()); + let template_vars = build_manual_hook_template_vars(ctx, hook_type); let extra_vars = template_vars.as_extra_vars(); return Ok(prepare_steps(config, ctx, &extra_vars, hook_type, source)? + .into_unvalidated() .into_iter() .flat_map(PreparedStep::into_commands) .map(|cmd| { @@ -704,7 +704,7 @@ fn hook_command_rows( HookCommandRow { name: cmd.name, template, - display, + expanded: Some(display), } }) .collect()); @@ -715,7 +715,7 @@ fn hook_command_rows( .map(|cmd| HookCommandRow { name: cmd.name.clone(), template: cmd.template.clone(), - display: cmd.template.clone(), + expanded: None, }) .collect()) } diff --git a/src/commands/hook_plan.rs b/src/commands/hook_plan.rs index 918a3026c..df42927d8 100644 --- a/src/commands/hook_plan.rs +++ b/src/commands/hook_plan.rs @@ -52,7 +52,6 @@ use worktrunk::git::add_hook_skip_hint; use super::command_approval::approve_command_batch; use super::command_executor::{ CommandContext, FailureStrategy, PipelineKind, execute_pipeline_foreground, prepare_steps, - validate_pipeline_syntax, }; use super::hook_announcement::SourcedStep; use super::hook_filter::HookSource; @@ -359,8 +358,7 @@ fn render_planned( ) -> anyhow::Result> { let mut out = Vec::new(); for (source, cfg) in entries { - let steps = prepare_steps(cfg, ctx, extra_vars, hook_type, *source)?; - validate_pipeline_syntax(&steps)?; + let steps = prepare_steps(cfg, ctx, extra_vars, hook_type, *source)?.validated()?; for step in steps { out.push(SourcedStep { step, diff --git a/src/commands/hooks.rs b/src/commands/hooks.rs index 7636a1e71..c713165ca 100644 --- a/src/commands/hooks.rs +++ b/src/commands/hooks.rs @@ -77,7 +77,6 @@ use worktrunk::styling::{ use super::command_executor::{ CommandContext, FailureStrategy, ForegroundStep, PipelineKind, PreparedCommand, PreparedStep, alias_error_wrapper, execute_pipeline_foreground, hook_error_wrapper, prepare_steps, - validate_pipeline_syntax, }; use super::hook_announcement::{SourcedStep, format_pipeline_summary}; use crate::commands::process::{HookLog, spawn_detached_exec}; @@ -127,8 +126,7 @@ pub(crate) fn prepare_and_check( continue; } - let steps = prepare_steps(config, ctx, extra_vars, hook_type, source)?; - validate_pipeline_syntax(&steps)?; + let steps = prepare_steps(config, ctx, extra_vars, hook_type, source)?.validated()?; for step in steps { if let Some(filtered) = filter_step_by_name(step, source, &parsed_filters) { result.push(SourcedStep { @@ -463,10 +461,7 @@ fn print_background_variable_table(pipelines: &[PendingPipeline], hook_type: Hoo continue; } // Pipelines carry non-empty steps by construction — `steps[0]` is safe. - let cmd = match &pipeline.steps[0].step { - PreparedStep::Single(cmd) => cmd, - PreparedStep::Concurrent(cmds) => &cmds[0], - }; + let cmd = &pipeline.steps[0].step.commands()[0]; eprintln!( "{}", info_message(cformat!("{hook_type} template variables:")) @@ -492,10 +487,7 @@ fn spawn_hook_pipeline_quiet(repo: &Repository, pipeline: &PendingPipeline) -> a // step). let steps = &pipeline.steps; let source = steps[0].source; - let first_cmd = match &steps[0].step { - PreparedStep::Single(cmd) => cmd, - PreparedStep::Concurrent(cmds) => &cmds[0], - }; + let first_cmd = &steps[0].step.commands()[0]; let mut context = first_cmd.context.clone(); context.remove("hook_name"); diff --git a/src/commands/worktree/switch.rs b/src/commands/worktree/switch.rs index ce73ac6eb..f3cb4d01d 100644 --- a/src/commands/worktree/switch.rs +++ b/src/commands/worktree/switch.rs @@ -2078,7 +2078,7 @@ fn validate_switch_templates( // Skip full validation for templates referencing {{ vars.X }} — // those values come from git config at execution time, after // prior pipeline steps set them. Syntax is still checked by - // prepare_steps. + // PreparedPipeline::validated. if template_references_var(&cmd.template, "vars") { continue; } diff --git a/src/config/expansion.rs b/src/config/expansion.rs index bbb2b84b7..dba7c919e 100644 --- a/src/config/expansion.rs +++ b/src/config/expansion.rs @@ -152,13 +152,25 @@ impl TemplateContext { escape_mode: ShellEscapeMode, repo: &Repository, name: &str, + ) -> Result { + self.expand_with(template, escape_mode, repo, name, VarsMode::Resolve) + } + + /// [`Self::expand`], with control over how `{{ vars. }}` resolves. + pub fn expand_with( + &self, + template: &str, + escape_mode: ShellEscapeMode, + repo: &Repository, + name: &str, + vars_mode: VarsMode, ) -> Result { let vars: HashMap<&str, &str> = self .0 .iter() .map(|(k, v)| (k.as_str(), v.as_str())) .collect(); - expand_template(template, &vars, escape_mode, repo, name) + expand_template_with(template, &vars, escape_mode, repo, name, vars_mode) } /// The JSON form piped to a child's stdin. @@ -258,8 +270,8 @@ fn hook_extras(hook_type: HookType) -> &'static [&'static str] { } } -/// Vars added by the hook execution infrastructure itself (`expand_commands` -/// / `expand_command_template`), regardless of hook type. +/// Vars added by the hook execution infrastructure itself (`prepare_steps`), +/// regardless of hook type. const HOOK_INFRASTRUCTURE_VARS: &[&str] = &["hook_type", "hook_name"]; /// All template variables available in a given scope. @@ -445,6 +457,41 @@ impl Object for ShellArgs { } } +/// Stands in for the `vars` map in a preview, rendering each reference back as +/// the `{{ vars. }}` that produced it. +/// +/// A preview shows what a command *will* run, and `vars.*` values are read from +/// git config when the step runs — after any earlier step in the pipeline has +/// written them. Resolving one at preview time would show a value the run may +/// not use, so the reference stands for itself while everything around it +/// expands normally. +/// +/// Each key access returns another `LiteralVars` carrying the path so far, so +/// nested access (`{{ vars.config.port }}`) round-trips too. The formatter +/// installed by `expand_template` writes it through unescaped, as it does +/// `ShellArgs`. +#[derive(Debug)] +struct LiteralVars(String); + +impl Object for LiteralVars { + fn get_value(self: &Arc, key: &Value) -> Option { + Some(Value::from_object(LiteralVars(format!("{}.{key}", self.0)))) + } + + fn render(self: &Arc, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{{{{ {} }}}}", self.0) + } +} + +/// How template expansion resolves `{{ vars. }}`. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum VarsMode { + /// Read values from git config — what a command runs with. + Resolve, + /// Render each reference back as `{{ vars. }}`, for a preview. + Literal, +} + /// Space-join POSIX-shell-escaped args — the canonical rendering of /// `{{ args }}` used by both `ShellArgs::render` (template expansion) and the /// alias `-v` variable table. Always POSIX: see [`ShellArgs`]. @@ -929,10 +976,12 @@ pub fn referenced_vars_for_config( /// Parse-only syntax check for a template. /// -/// Hook and alias preparation runs this on every template so syntax errors -/// (e.g. `{{ vars..foo }}`) abort before the first pipeline step runs; -/// rendering — and with it semantic errors like undefined variables — is -/// deferred to execution time. The error matches [`expand_template`]'s +/// Every path that runs hooks calls this on each template, via +/// `PreparedPipeline::validated`, so syntax errors (e.g. `{{ vars..foo }}`) abort +/// before the first pipeline step runs; rendering — and with it semantic errors +/// like undefined variables — is deferred to execution time. Alias dispatch has +/// already parsed each template for argument routing by then, so it skips the +/// check. The error matches [`expand_template`]'s /// parse-failure shape so syntax errors render identically wherever they /// surface. pub fn validate_template_syntax(template: &str, name: &str) -> Result<(), TemplateExpandError> { @@ -1048,6 +1097,22 @@ pub fn expand_template( escape_mode: ShellEscapeMode, repo: &Repository, name: &str, +) -> Result { + expand_template_with(template, vars, escape_mode, repo, name, VarsMode::Resolve) +} + +/// [`expand_template`], with control over how `{{ vars. }}` resolves. +/// +/// Previews pass [`VarsMode::Literal`] so a `vars.*` reference renders back as +/// itself while every other variable expands; execution passes +/// [`VarsMode::Resolve`]. +pub fn expand_template_with( + template: &str, + vars: &HashMap<&str, &str>, + escape_mode: ShellEscapeMode, + repo: &Repository, + name: &str, + vars_mode: VarsMode, ) -> Result { // Build context map with raw values (shell escaping is applied at output time via formatter). // The `args` key is reserved: run_alias encodes positional CLI args as a JSON list string, @@ -1083,7 +1148,11 @@ pub fn expand_template( // output avoids re-escaping the whole joined string as one // opaque token. Iteration and indexing yield plain string // values that still flow through the generic escape branch. - if value.downcast_object_ref::().is_some() { + // `LiteralVars` renders the `{{ vars. }}` reference itself, + // which must reach the preview unquoted. + if value.downcast_object_ref::().is_some() + || value.downcast_object_ref::().is_some() + { write!(out, "{value}")?; return Ok(()); } @@ -1125,13 +1194,25 @@ pub fn expand_template( // Only look up vars data if the parsed template references the top-level // `vars` object (avoids a git process spawn per expansion while supporting // every MiniJinja access form without false positives from literal text). - if tmpl.undeclared_variables(false).contains("vars") - && let Some(branch) = vars.get("branch") - { - context.insert( - "vars".to_string(), - vars_map_to_value(&repo.vars_entries(branch)), - ); + // A preview injects `LiteralVars` instead, which needs no branch and no + // git config read. + if tmpl.undeclared_variables(false).contains("vars") { + match vars_mode { + VarsMode::Literal => { + context.insert( + "vars".to_string(), + Value::from_object(LiteralVars("vars".to_string())), + ); + } + VarsMode::Resolve => { + if let Some(branch) = vars.get("branch") { + context.insert( + "vars".to_string(), + vars_map_to_value(&repo.vars_entries(branch)), + ); + } + } + } } let result = tmpl @@ -1273,6 +1354,28 @@ mod tests { TestRepo::new() } + #[test] + fn test_template_references_var_for_vars() { + // Real vars references + assert!(template_references_var("{{ vars.container }}", "vars")); + assert!(template_references_var("{{vars.container}}", "vars")); + assert!(template_references_var( + "docker run --name {{ vars.name }}", + "vars" + )); + assert!(template_references_var( + "{% if vars.key %}yes{% endif %}", + "vars" + )); + + // Literal text — not a template reference + assert!(!template_references_var( + "echo hello > template_vars.txt", + "vars" + )); + assert!(!template_references_var("no vars references here", "vars")); + } + #[test] fn test_sanitize_branch_name() { let cases = [ diff --git a/src/config/mod.rs b/src/config/mod.rs index 3e690be40..51677592b 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -161,9 +161,9 @@ pub use deprecation::{ pub use deprecation::{DeprecationKind, Deprecations}; pub use expansion::{ ACTIVE_VARS, ALIAS_ARGS_KEY, DEPRECATED_TEMPLATE_VARS, EXEC_BASE_VARS, REPO_VARS, - TemplateContext, TemplateExpandError, ValidationScope, VarScope, alias_context_filter, - base_vars, expand_template, format_alias_variables, format_base_variables, - format_hook_variables, redact_credentials, referenced_vars_for_config, + TemplateContext, TemplateExpandError, ValidationScope, VarScope, VarsMode, + alias_context_filter, base_vars, expand_template, format_alias_variables, + format_base_variables, format_hook_variables, redact_credentials, referenced_vars_for_config, referenced_vars_for_templates, sanitize_branch_name, sanitize_db, short_hash, template_environment, template_references_var, validate_list_column_template, validate_template, validate_template_syntax, vars_available_in, vars_map_to_value, diff --git a/tests/integration_tests/hook_show.rs b/tests/integration_tests/hook_show.rs index 093ba0eda..58103cd29 100644 --- a/tests/integration_tests/hook_show.rs +++ b/tests/integration_tests/hook_show.rs @@ -571,6 +571,11 @@ project-deps = "echo deps" /// the per-command `hook_name`, and the `args` sequence a listing leaves empty /// — render in the listing exactly as they do in `wt hook --dry-run`, /// which previews what would actually run. +/// +/// The `varsy` command pins the one variable a preview deliberately leaves +/// alone: `vars.*` is read from git config when the step runs, so it renders +/// back as itself even though a value is set here, while `{{ branch }}` beside +/// it still expands. #[rstest] fn test_hook_show_expanded_matches_dry_run(repo: TestRepo, temp_home: TempDir) { let global_config_dir = temp_home.path().join(".config").join("worktrunk"); @@ -582,11 +587,14 @@ fn test_hook_show_expanded_matches_dry_run(repo: TestRepo, temp_home: TempDir) { [pre-commit] context = "echo type={{ hook_type }} name={{ hook_name }} args=[{{ args }}]" +varsy = "deploy --branch={{ branch }} --env={{ vars.env }}" "#, ) .unwrap(); + repo.run_git(&["config", "worktrunk.state.main.vars.env", "staging"]); let expected = "echo type=pre-commit name=context args=[]"; + let expected_varsy = "deploy --branch=main --env={{ vars.env }}"; let mut show = wt_command(); repo.configure_wt_cmd(&mut show); @@ -603,6 +611,7 @@ context = "echo type={{ hook_type }} name={{ hook_name }} args=[{{ args }}]" let parsed: serde_json::Value = serde_json::from_str(&String::from_utf8_lossy(&output.stdout)).expect("valid JSON"); assert_eq!(parsed[0]["expanded"], expected); + assert_eq!(parsed[1]["expanded"], expected_varsy); let mut dry_run = wt_command(); repo.configure_wt_cmd(&mut dry_run); @@ -621,10 +630,12 @@ context = "echo type={{ hook_type }} name={{ hook_name }} args=[{{ args }}]" String::from_utf8_lossy(&output.stderr) ); let stdout = String::from_utf8_lossy(&output.stdout); - assert!( - stdout.contains(expected), - "dry-run should render the listing's command: {stdout}" - ); + for want in [expected, expected_varsy] { + assert!( + stdout.contains(want), + "dry-run should render the listing's command `{want}`: {stdout}" + ); + } } /// Test that valid templates expand correctly with --expanded. diff --git a/tests/integration_tests/user_hooks.rs b/tests/integration_tests/user_hooks.rs index 3fd7e13c3..56f0ad3e9 100644 --- a/tests/integration_tests/user_hooks.rs +++ b/tests/integration_tests/user_hooks.rs @@ -1206,7 +1206,7 @@ fn test_foreground_pipeline_undefined_var_runs_earlier_steps(repo: TestRepo) { /// The other half of that contract: a *syntax* error anywhere in the pipeline /// aborts before step 1. Preparation parses every template -/// (`validate_pipeline_syntax`), so a pipeline that can't render in full never +/// (`PreparedPipeline::validated`), so a pipeline that can't render in full never /// starts — where a semantic error, rendered per step, lets earlier steps run. /// /// Driven through `wt merge`'s pre-commit hooks rather than `wt hook