From 6f74479017d7c0a4986fcb29a7fa5a8e40faf480 Mon Sep 17 00:00:00 2001 From: worktrunk-bot Date: Fri, 19 Jun 2026 00:52:09 +0000 Subject: [PATCH 1/2] feat(hooks): connect foreground hooks to the terminal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Foreground (`pre-*`) hooks now inherit the parent's stdin, exactly as aliases already do, so an interactive child keeps the controlling terminal — a hook can prompt before continuing (e.g. `gum confirm` before `mise trust`). Previously every foreground hook had the JSON context piped to its stdin, which stole the tty and made interactive hooks impossible. The lever is the one aliases already use: `sourced_steps_to_foreground` hard-coded `pipe_stdin = true` for hooks and `false` for aliases. With both sides now inheriting stdin in the single-step path, the flag was uniformly false, so the `pipe_stdin` field is removed rather than left vestigial. The JSON context is unchanged for the paths that can't be interactive: concurrent hook groups and background (`post-*`) detached hooks still receive it on stdin. Template variables (`{{ }}`) reach every hook regardless of form. Closes #3093 Co-Authored-By: Claude Opus 4.8 --- .claude/settings.local.json | 1 + docs/content/extending.md | 2 +- docs/content/hook.md | 15 ++- skills/worktrunk/reference/extending.md | 2 +- skills/worktrunk/reference/hook.md | 15 ++- src/cli/mod.rs | 15 ++- src/commands/command_executor.rs | 17 ++-- src/commands/hooks.rs | 19 ++-- .../integration_tests/post_start_commands.rs | 99 +++++++++---------- 9 files changed, 105 insertions(+), 80 deletions(-) create mode 100644 .claude/settings.local.json diff --git a/.claude/settings.local.json b/.claude/settings.local.json new file mode 100644 index 0000000000..b28ffd45fb --- /dev/null +++ b/.claude/settings.local.json @@ -0,0 +1 @@ +{"permissions":{"defaultMode":"bypassPermissions","allow":["Bash","Edit","Read","Write","Glob","Grep","WebSearch","WebFetch","Task","Skill"]},"skipDangerousModePermissionPrompt":true} diff --git a/docs/content/extending.md b/docs/content/extending.md index 77349b7665..e671146f5c 100644 --- a/docs/content/extending.md +++ b/docs/content/extending.md @@ -224,7 +224,7 @@ Aside from the differences below, hooks and aliases behave the same. | Force-bind escape | `--var KEY=VALUE` (deprecated in favor of `--KEY=VALUE`, but still force-binds) | None; smart routing is the only path | | `--help` | `wt hook --help` lists hook types; `wt hook --help` shows flags and arguments for that type | The template body is the documentation: `wt --help` redirects to `wt config alias show` / `dry-run`. `wt --help` and `wt step --help` list configured aliases alongside built-in commands | | Inspection | `wt hook show [type] [--expanded]` | `wt config alias show ` / `wt config alias dry-run ` | -| Stdin | All template variables as JSON (parse with `json.load(sys.stdin)`) | Inherits parent stdin (pipes pass through; interactive TUIs like `wt switch` keep the tty) | +| Stdin | Foreground (`pre-*`) inherit parent stdin, same as aliases (interactive prompts work); background (`post-*`) receive all template variables as JSON (parse with `json.load(sys.stdin)`) | Inherits parent stdin (pipes pass through; interactive TUIs like `wt switch` keep the tty) | | Template-context extras | `hook_type`, `hook_name`, per-type operation vars (`base`, `target`, `pr_number`, …) | `args` on top of the shared base variables | diff --git a/docs/content/hook.md b/docs/content/hook.md index af44c25875..30c65a5501 100644 --- a/docs/content/hook.md +++ b/docs/content/hook.md @@ -257,13 +257,20 @@ The `worktree_path_of_branch` function returns the filesystem path of a worktree setup = "cp {{ worktree_path_of_branch('main') }}/config.local {{ worktree_path }}" ``` -## JSON context +## Interactive hooks and JSON context -Hooks receive all template variables as JSON on stdin, enabling complex logic that templates can't express: +Foreground (`pre-*`) hooks run connected to your terminal, so a hook can prompt before continuing — for example, confirming an action before running it: ```toml [pre-start] -setup = "python3 scripts/pre-start-setup.py" +trust = "gum confirm 'trust this worktree?' && mise trust" +``` + +Background (`post-*`) hooks run detached, with no terminal. They receive all template variables as JSON on stdin, enabling complex logic that templates can't express: + +```toml +[post-start] +setup = "python3 scripts/post-start-setup.py" ``` ```python @@ -273,6 +280,8 @@ if ctx['branch'].startswith('feature/') and 'backend' in ctx['repo']: subprocess.run(['make', 'seed-db']) ``` +Every hook also receives its template variables through `{{ }}` substitution, regardless of form. + ## Copying untracked files One specific command worth calling out: [`wt step copy-ignored`](@/step.md#wt-step-copy-ignored). Git worktrees share the repository but not untracked files, and this copies gitignored files between worktrees: diff --git a/skills/worktrunk/reference/extending.md b/skills/worktrunk/reference/extending.md index 101ac86e72..d0d8665275 100644 --- a/skills/worktrunk/reference/extending.md +++ b/skills/worktrunk/reference/extending.md @@ -231,7 +231,7 @@ Aside from the differences below, hooks and aliases behave the same. | Force-bind escape | `--var KEY=VALUE` (deprecated in favor of `--KEY=VALUE`, but still force-binds) | None; smart routing is the only path | | `--help` | `wt hook --help` lists hook types; `wt hook --help` shows flags and arguments for that type | The template body is the documentation: `wt --help` redirects to `wt config alias show` / `dry-run`. `wt --help` and `wt step --help` list configured aliases alongside built-in commands | | Inspection | `wt hook show [type] [--expanded]` | `wt config alias show ` / `wt config alias dry-run ` | -| Stdin | All template variables as JSON (parse with `json.load(sys.stdin)`) | Inherits parent stdin (pipes pass through; interactive TUIs like `wt switch` keep the tty) | +| Stdin | Foreground (`pre-*`) inherit parent stdin, same as aliases (interactive prompts work); background (`post-*`) receive all template variables as JSON (parse with `json.load(sys.stdin)`) | Inherits parent stdin (pipes pass through; interactive TUIs like `wt switch` keep the tty) | | Template-context extras | `hook_type`, `hook_name`, per-type operation vars (`base`, `target`, `pr_number`, …) | `args` on top of the shared base variables | diff --git a/skills/worktrunk/reference/hook.md b/skills/worktrunk/reference/hook.md index f37ed069fa..997b12871b 100644 --- a/skills/worktrunk/reference/hook.md +++ b/skills/worktrunk/reference/hook.md @@ -248,13 +248,20 @@ The `worktree_path_of_branch` function returns the filesystem path of a worktree setup = "cp {{ worktree_path_of_branch('main') }}/config.local {{ worktree_path }}" ``` -## JSON context +## Interactive hooks and JSON context -Hooks receive all template variables as JSON on stdin, enabling complex logic that templates can't express: +Foreground (`pre-*`) hooks run connected to your terminal, so a hook can prompt before continuing — for example, confirming an action before running it: ```toml [pre-start] -setup = "python3 scripts/pre-start-setup.py" +trust = "gum confirm 'trust this worktree?' && mise trust" +``` + +Background (`post-*`) hooks run detached, with no terminal. They receive all template variables as JSON on stdin, enabling complex logic that templates can't express: + +```toml +[post-start] +setup = "python3 scripts/post-start-setup.py" ``` ```python @@ -264,6 +271,8 @@ if ctx['branch'].startswith('feature/') and 'backend' in ctx['repo']: subprocess.run(['make', 'seed-db']) ``` +Every hook also receives its template variables through `{{ }}` substitution, regardless of form. + ## Copying untracked files One specific command worth calling out: [`wt step copy-ignored`](https://worktrunk.dev/step/#wt-step-copy-ignored). Git worktrees share the repository but not untracked files, and this copies gitignored files between worktrees: diff --git a/src/cli/mod.rs b/src/cli/mod.rs index db10fc7629..2b48e1a349 100644 --- a/src/cli/mod.rs +++ b/src/cli/mod.rs @@ -1533,13 +1533,20 @@ The `worktree_path_of_branch` function returns the filesystem path of a worktree setup = "cp {{ worktree_path_of_branch('main') }}/config.local {{ worktree_path }}" ``` -## JSON context +## Interactive hooks and JSON context -Hooks receive all template variables as JSON on stdin, enabling complex logic that templates can't express: +Foreground (`pre-*`) hooks run connected to your terminal, so a hook can prompt before continuing — for example, confirming an action before running it: ```toml [pre-start] -setup = "python3 scripts/pre-start-setup.py" +trust = "gum confirm 'trust this worktree?' && mise trust" +``` + +Background (`post-*`) hooks run detached, with no terminal. They receive all template variables as JSON on stdin, enabling complex logic that templates can't express: + +```toml +[post-start] +setup = "python3 scripts/post-start-setup.py" ``` ```python @@ -1549,6 +1556,8 @@ if ctx['branch'].startswith('feature/') and 'backend' in ctx['repo']: subprocess.run(['make', 'seed-db']) ``` +Every hook also receives its template variables through `{{ }}` substitution, regardless of form. + ## Copying untracked files One specific command worth calling out: [`wt step copy-ignored`](@/step.md#wt-step-copy-ignored). Git worktrees share the repository but not untracked files, and this copies gitignored files between worktrees: diff --git a/src/commands/command_executor.rs b/src/commands/command_executor.rs index 73ecf96e8c..e3993c087f 100644 --- a/src/commands/command_executor.rs +++ b/src/commands/command_executor.rs @@ -84,7 +84,7 @@ pub type ErrorWrapper = Box) -> any /// Supplied at conversion time (`sourced_steps_to_foreground`) so a single /// `SourcedStep` shape can be produced by both alias and hook resolution. /// Drives the per-step trust model (EXEC passthrough), announce policy, -/// stdin handling, and error wrapping. Hook-only metadata +/// stdout redirection, and error wrapping. Hook-only metadata /// (`hook_type`, `display_path`) lives on the `Hook` variant — it's /// per-pipeline, not per-step, so the per-step shape stays neutral. #[derive(Clone)] @@ -117,10 +117,6 @@ pub struct ForegroundStep { /// line plus the bash gutter; aliases stay silent (the caller emits one /// pipeline summary line). pub announce: PipelineKind, - /// Pipe `context_json` to the child's stdin (hooks); when `false`, inherit - /// the parent's stdin so interactive children keep the controlling tty - /// (aliases). - pub pipe_stdin: bool, /// Merge the child's stdout onto wt's stderr (`true`, hooks) or pass it /// through unchanged (`false`, aliases). Hooks merge so their output stays /// ordered with wt's own stderr "Running …" lines; aliases pass through so @@ -569,15 +565,16 @@ fn run_one_command( }; announce_command(cmd, &fg_step.announce, &command_str); - // Hooks get a documented JSON context on stdin; aliases inherit stdin so - // interactive children (e.g. `wt switch`'s picker) keep their controlling - // terminal. Piping JSON into an interactive alias body steals the tty. - let stdin_json = fg_step.pipe_stdin.then(|| cmd.context_json()); + // Foreground steps inherit the parent's stdin so an interactive child keeps + // its controlling terminal — a `pre-*` hook can prompt (e.g. `gum confirm` + // before `mise trust`), and an alias body's `wt switch` picker keeps the + // tty. The JSON context still reaches concurrent and background (`post-*`) + // hooks, which can't be interactive, via their own stdin pipe. let log_label = fg_step.announce.log_label(cmd); let result = execute_shell_command( wt_path, &command_str, - stdin_json.as_deref(), + None, log_label.as_deref(), directives.clone(), fg_step.redirect_stdout_to_stderr, diff --git a/src/commands/hooks.rs b/src/commands/hooks.rs index 9b9d9078c0..500b2ef529 100644 --- a/src/commands/hooks.rs +++ b/src/commands/hooks.rs @@ -561,10 +561,16 @@ fn spawn_hook_pipeline_quiet(repo: &Repository, pipeline: &PendingPipeline) -> a /// Convert source-tagged steps into foreground steps with pipeline-kind policy. /// /// Shared between hook and alias dispatch. The `kind` argument supplies the -/// per-call-site policy (announce style, stdin handling, error wrapping) while -/// the `source` field on each step drives the per-step trust model +/// per-call-site policy (announce style, stdout redirection, error wrapping) +/// while the `source` field on each step drives the per-step trust model /// (`DirectivePassthrough`). /// +/// Foreground steps — hook and alias alike — inherit the parent's stdin so an +/// interactive child keeps the controlling terminal (a `pre-*` hook can prompt; +/// an alias body's `wt switch` picker works). The JSON context still reaches +/// concurrent and background (`post-*`) hooks, which can't be interactive, via +/// their own per-child stdin pipe. +/// /// Trust model: /// - User-source alias steps pass EXEC through. The body lives in the user's /// own config, so a nested `wt switch --execute …` is no different from the @@ -588,16 +594,13 @@ pub(crate) fn sourced_steps_to_foreground( } _ => DirectivePassthrough::inherit_from_env(), }; - let (pipe_stdin, redirect_stdout_to_stderr, error_wrapper) = match kind { - PipelineKind::Hook { hook_type, .. } => { - (true, true, hook_error_wrapper(*hook_type)) - } - PipelineKind::Alias { name } => (false, false, alias_error_wrapper(name.clone())), + let (redirect_stdout_to_stderr, error_wrapper) = match kind { + PipelineKind::Hook { hook_type, .. } => (true, hook_error_wrapper(*hook_type)), + PipelineKind::Alias { name } => (false, alias_error_wrapper(name.clone())), }; ForegroundStep { step: sourced.step, announce: kind.clone(), - pipe_stdin, redirect_stdout_to_stderr, error_wrapper, directives, diff --git a/tests/integration_tests/post_start_commands.rs b/tests/integration_tests/post_start_commands.rs index d93f45e70a..82e0e14690 100644 --- a/tests/integration_tests/post_start_commands.rs +++ b/tests/integration_tests/post_start_commands.rs @@ -548,31 +548,48 @@ approved-commands = [ } #[rstest] -fn test_pre_start_json_stdin(repo: TestRepo) { +fn test_pre_start_inherits_stdin(repo: TestRepo) { use crate::common::wt_command; + use std::io::Write; + use std::process::Stdio; - // Create project config with a command that reads JSON from stdin - // Use cat to capture stdin to a file - repo.write_project_config(r#"pre-start = "cat > context.json""#); + // Foreground (`pre-*`) hooks inherit the parent's stdin — same as aliases — + // so an interactive child keeps the controlling terminal. The legacy + // JSON-context-on-stdin contract no longer applies here (it survives only + // for concurrent and background `post-*` hooks). Capture whatever the hook + // sees on stdin to a file and verify it's the parent's raw stdin, not JSON. + repo.write_project_config(r#"pre-start = "cat > captured.txt""#); repo.commit("Add config"); // Pre-approve the command repo.write_test_approvals( r#"[projects."../origin"] -approved-commands = ["cat > context.json"] +approved-commands = ["cat > captured.txt"] "#, ); - // Create worktree - this should pipe JSON to the hook's stdin + // Create worktree, piping a sentinel to wt's stdin. The pre-start hook + // inherits that stdin, so `cat` captures the sentinel verbatim. let temp_home = TempDir::new().unwrap(); let mut cmd = wt_command(); - cmd.args(["switch", "--create", "feature-json"]) + cmd.args(["switch", "--create", "feature-stdin"]) .current_dir(repo.root_path()) .env("WORKTRUNK_CONFIG_PATH", repo.test_config_path()) - .env("WORKTRUNK_APPROVALS_PATH", repo.test_approvals_path()); + .env("WORKTRUNK_APPROVALS_PATH", repo.test_approvals_path()) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()); set_temp_home_env(&mut cmd, temp_home.path()); - let output = cmd.output().expect("failed to run wt switch"); + + let mut child = cmd.spawn().expect("failed to spawn wt switch"); + child + .stdin + .take() + .expect("stdin piped") + .write_all(b"sentinel-from-parent-stdin\n") + .expect("failed to write to wt stdin"); + let output = child.wait_with_output().expect("failed to run wt switch"); assert!( output.status.success(), @@ -580,46 +597,26 @@ approved-commands = ["cat > context.json"] String::from_utf8_lossy(&output.stderr) ); - // Find the worktree and read the JSON - let worktree_path = repo.root_path().parent().unwrap().join("repo.feature-json"); - let json_file = worktree_path.join("context.json"); + let worktree_path = repo + .root_path() + .parent() + .unwrap() + .join("repo.feature-stdin"); + let captured = worktree_path.join("captured.txt"); assert!( - json_file.exists(), - "context.json should have been created from stdin" + captured.exists(), + "captured.txt should have been created from the inherited stdin" ); - let contents = fs::read_to_string(&json_file).unwrap(); - - // Parse and verify the JSON contains expected fields - let json: serde_json::Value = serde_json::from_str(&contents) - .unwrap_or_else(|e| panic!("Should be valid JSON: {}\nContents: {}", e, contents)); - - assert!( - json.get("repo").is_some(), - "JSON should contain 'repo' field" - ); - assert!( - json.get("branch").is_some(), - "JSON should contain 'branch' field" - ); + let contents = fs::read_to_string(&captured).unwrap(); assert_eq!( - json["branch"].as_str(), - Some("feature-json"), - "Branch should be sanitized (feature-json)" + contents, "sentinel-from-parent-stdin\n", + "Foreground hook should receive the parent's raw stdin, not the JSON context" ); assert!( - json.get("worktree").is_some(), - "JSON should contain 'worktree' field" - ); - assert!( - json.get("repo_root").is_some(), - "JSON should contain 'repo_root' field" - ); - assert_eq!( - json["hook_type"].as_str(), - Some("pre-start"), - "JSON should contain hook_type" + !contents.contains("\"branch\""), + "The JSON context must not be piped to a foreground hook: {contents}" ); } @@ -654,9 +651,12 @@ with open('hook_output.txt', 'w') as f: fs::write(&script_path, script_content).unwrap(); fs::set_permissions(&script_path, fs::Permissions::from_mode(0o755)).unwrap(); - // Create project config that runs the script + // Create project config that runs the script. Background (`post-*`) hooks + // run detached and still receive the JSON context on stdin — that's the + // path this script exercises. (Foreground `pre-*` hooks now inherit the + // terminal instead; see `test_pre_start_inherits_stdin`.) repo.write_project_config( - r#"[pre-start] + r#"[post-start] setup = "./scripts/setup.py" "#, ); @@ -686,18 +686,15 @@ approved-commands = ["./scripts/setup.py"] String::from_utf8_lossy(&output.stderr) ); - // Find the worktree and verify the script wrote the expected output + // Find the worktree and verify the script wrote the expected output. The + // hook is detached, so poll until it finishes writing. let worktree_path = repo .root_path() .parent() .unwrap() .join("repo.feature-script"); let output_file = worktree_path.join("hook_output.txt"); - - assert!( - output_file.exists(), - "Script should have created hook_output.txt" - ); + wait_for_file_content(&output_file); let contents = fs::read_to_string(&output_file).unwrap(); assert!( @@ -711,7 +708,7 @@ approved-commands = ["./scripts/setup.py"] contents ); assert!( - contents.contains("hook_type=pre-start"), + contents.contains("hook_type=post-start"), "Output should contain hook_type: {}", contents ); From a9f7bc22ee7408abbe8dd6f40af40cf512edd932 Mon Sep 17 00:00:00 2001 From: worktrunk-bot Date: Fri, 19 Jun 2026 01:00:27 +0000 Subject: [PATCH 2/2] chore: drop accidentally-committed .claude/settings.local.json This bypass-permissions agent-sandbox settings file was committed unintentionally and is unrelated to the foreground-hooks change. Remove it from tracking and gitignore it (per the .local.json per-machine convention). Co-Authored-By: Claude Opus 4.8 --- .claude/settings.local.json | 1 - .gitignore | 3 +++ 2 files changed, 3 insertions(+), 1 deletion(-) delete mode 100644 .claude/settings.local.json diff --git a/.claude/settings.local.json b/.claude/settings.local.json deleted file mode 100644 index b28ffd45fb..0000000000 --- a/.claude/settings.local.json +++ /dev/null @@ -1 +0,0 @@ -{"permissions":{"defaultMode":"bypassPermissions","allow":["Bash","Edit","Read","Write","Glob","Grep","WebSearch","WebFetch","Task","Skill"]},"skipDangerousModePermissionPrompt":true} diff --git a/.gitignore b/.gitignore index bf6d9744fe..dcd93c8abf 100644 --- a/.gitignore +++ b/.gitignore @@ -12,6 +12,9 @@ test-concurrent.sh logo-variant-*.png test-results/ +# Local per-machine agent settings (gitignore'd by convention) +.claude/settings.local.json + # Nix build outputs /result /result-*