Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/content/hook.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.<key> }}`.
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.<key> }}`. Because an earlier step can still change those values, a preview leaves them alone: `wt hook <type> --dry-run` and `wt hook show --expanded` render `{{ vars.<key> }}` 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.

Expand Down
2 changes: 1 addition & 1 deletion plugins/worktrunk/skills/worktrunk/reference/hook.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.<key> }}`.
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.<key> }}`. Because an earlier step can still change those values, a preview leaves them alone: `wt hook <type> --dry-run` and `wt hook show --expanded` render `{{ vars.<key> }}` 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.

Expand Down
2 changes: 1 addition & 1 deletion skills/worktrunk/reference/hook.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion src/cli/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.<key> }}`.
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.<key> }}`. Because an earlier step can still change those values, a preview leaves them alone: `wt hook <type> --dry-run` and `wt hook show --expanded` render `{{ vars.<key> }}` 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.

Expand Down
110 changes: 54 additions & 56 deletions src/commands/command_executor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand All @@ -26,8 +26,9 @@ use crate::output::{DirectivePassthrough, execute_shell_command};
pub struct PreparedCommand {
pub name: Option<String>,
/// 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).
Expand Down Expand Up @@ -434,24 +435,26 @@ fn resolve_command_str(cmd: &PreparedCommand, repo: &Repository) -> Result<Strin
expand_shell_template(&cmd.template, &cmd.context, repo, &cmd.template_name)
}

/// Render a template for dry-run / preview display. Mirrors execution-time
/// semantics: a template referencing `vars.*` is shown raw after a syntax
/// check — its values resolve from git config when the step runs, possibly
/// written by earlier pipeline steps — while everything else renders against
/// `context`. Expansion is side-effect-free, so previewing never perturbs the
/// real run.
/// Render a template for dry-run / preview display.
///
/// Everything renders against `context` as it would at execution time, except
/// `{{ vars.<key> }}`, 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<String> {
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.
Expand Down Expand Up @@ -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<PreparedStep>);

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<Vec<PreparedStep>> {
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<PreparedStep> {
self.0
}
Ok(())
}

/// Prepare hook pipeline steps, preserving serial/concurrent structure. Sole
Expand All @@ -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<Vec<PreparedStep>> {
) -> anyhow::Result<PreparedPipeline> {
// Built once per pipeline — build_hook_context spawns git subprocesses.
let mut base_context = build_hook_context(ctx, extra_vars, VarScope::All)?;

Expand 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 {
Expand All @@ -804,7 +823,8 @@ pub fn prepare_steps(
template_name,
label: command_summary_name(cmd.name.as_deref(), source),
})
})
})?;
Ok(PreparedPipeline(steps))
}

#[cfg(test)]
Expand Down Expand Up @@ -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"));
}
}
46 changes: 23 additions & 23 deletions src/commands/hook_commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down Expand Up @@ -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<String> 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
Expand Down Expand Up @@ -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>,
Expand All @@ -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);
Expand Down Expand Up @@ -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(())
Expand Down Expand Up @@ -653,9 +653,10 @@ fn needs_approval(
struct HookCommandRow {
name: Option<String>,
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<String>,
}

/// The rows for one hook config, expanded when `ctx` is present —
Expand Down Expand Up @@ -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| {
Expand All @@ -704,7 +704,7 @@ fn hook_command_rows(
HookCommandRow {
name: cmd.name,
template,
display,
expanded: Some(display),
}
})
.collect());
Expand All @@ -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())
}
4 changes: 1 addition & 3 deletions src/commands/hook_plan.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -359,8 +358,7 @@ fn render_planned(
) -> anyhow::Result<Vec<SourcedStep>> {
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,
Expand Down
14 changes: 3 additions & 11 deletions src/commands/hooks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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!("<bold>{hook_type}</> template variables:"))
Expand All @@ -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");

Expand Down
Loading
Loading