mirror of
https://github.com/max-sixty/worktrunk.git
synced 2026-09-14 20:00:38 +08:00
codex/remove-codex-cloud-specific-tests
102 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Maximilian Roos
|
9e7ad0144d |
fix(hooks): expand everything but vars.* in a preview (#3638)
Follow-up to #3635, from two reviews that landed after it merged. ## The `vars.*` preview was worse than #3635 claimed `render_template_preview` short-circuits on `template_references_var(template, "vars")`, returning the raw template. So one `vars.` token disabled expansion for the entire command: ``` pre-commit = "deploy --branch={{ branch }} --repo={{ repo }} --env={{ vars.env }}" before: deploy --branch={{ branch }} --repo={{ repo }} --env={{ vars.env }} after: deploy --branch=main --repo=repo --env={{ vars.env }} ``` #3635 described this as "a `vars.*` template renders raw", which is true but understates it: `{{ branch }}` and `{{ repo }}` stopped expanding too, in a command whose whole job is to show the expansion. That short-circuit predates #3635 and has been degrading `wt hook <type> --dry-run` the same way; #3635 only extended it to `wt hook show --expanded`. The fix is at the source rather than at either caller. A preview now injects a stand-in object for `vars` that renders each reference back as itself, nested access included (`{{ vars.config.port }}` round-trips), while every other variable expands normally. `VarsMode::Resolve` keeps execution reading real values from git config; only previews pass `VarsMode::Literal`. A preview also no longer spawns the git read that resolving `vars` required. `vars.*` stays literal on purpose: those values are read when the step runs, after an earlier step in the pipeline may have written them, so a value resolved at preview time can differ from the one the run uses. Nothing covered this, which is why the suite stayed green through the regression. `test_hook_show_expanded_matches_dry_run` now sets a var and asserts the listing and the dry-run both leave it alone while expanding `{{ branch }}` beside it. ## The syntax gate is a type error now #3635 moved the template syntax check out of `prepare_steps` into a free `validate_pipeline_syntax` that both execution funnels had to remember to call. `prepare_steps` now returns a `PreparedPipeline` the caller must resolve: `.validated()` for the paths that run hooks, `.into_unvalidated()` for the listing, which annotates a broken template in place rather than blanking itself. Forgetting is a compile error, the same property `ApprovedHookPlan` gives hook approval. ## Smaller items Four cross-references went stale when the syntax check moved: `PreparedCommand.template`, `validate_template_syntax`, the `switch.rs` skip comment, and `HOOK_INFRASTRUCTURE_VARS` (which still named two deleted functions). The `--expanded` behavior is now documented in the sentence that already owns `{{ vars.<key> }}` semantics, with its three generated mirrors regenerated. `PreparedStep::commands()` replaces two hand-rolled matches in `hooks.rs`. `default_branch` moves inside `build_manual_hook_template_vars` — only the commit-hook arm reads it, and resolving it can cost a `git ls-remote` on a fresh clone, so the other eight hook types no longer pay for it. The listing carries its expansion state in an `Option<String>` instead of re-deriving "was this expanded?" from whether a context exists. > _This was written by Claude Code on behalf of max_ --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Maximilian Roos
|
4888d5246f |
refactor(hooks): derive the hook preview from the execution path (#3635)
`wt hook show --expanded` rebuilt the hook pipeline's template context by hand in `expand_command_template`, duplicating what `prepare_steps` already does for what actually runs: the same `build_hook_context` call, the same `hook_type` and `hook_name` inserts, the same `args` default, the same POSIX escape mode. Two copies of one rule, so the preview drifted from the executed command whenever the execution path gained a context key, with nothing to catch it. The listing now prepares its commands through `prepare_steps` itself (`hook_command_rows` in `hook_commands.rs`) and renders them through `render_template_preview`, the renderer `wt hook <type> --dry-run` already used. `prepare_steps` is the sole producer of hook command contexts, so a key added there reaches both with no second edit. ## The `args` divergence The old preview inserted `args = "[]"` unconditionally; `prepare_steps` defaults it only when unset, because manual `wt hook <type>` supplies real args upstream via `extra_vars`. The shared path keeps the conditional default and the listing simply does not supply `args`. The values coincide (a listing has no CLI args to forward, which is exactly what the default encodes), and keeping the conditional form means the rule stays written once, in the place that has a caller who needs the other branch. ## Where the syntax check went `prepare_steps` used to reject an unparsable template, so a pipeline that could not render in full never started. A listing wants the opposite: `wt hook show` is what you run when your hooks are broken, so it annotates the bad template in place and shows the rest. That check is an execution policy rather than part of building a command, so it moved out of `prepare_steps` into `validate_pipeline_syntax`, called by the two funnels every hook-running path goes through: `prepare_and_check` (foreground, background, dry-run, filtered) and `render_planned` (the plan-backed hooks behind `execute_planned_hook` and `register_planned`). Both are mutation-verified: removing either call fails a test. A newtype that made forgetting the gate a compile error would be stronger, but it threads a wrapper through `SourcedStep`, `ForegroundStep`, and the background pipeline spec for a guard whose failure mode is degraded fail-fast rather than incorrectness (a syntax error still surfaces when its step renders). ## User-visible changes Preview expansion errors now name the hook (`Failed to expand project:lint: ...`) instead of the generic `hook preview`. A template referencing `vars.*` renders raw in `--expanded`, matching `--dry-run`, where before it resolved against git config at preview time. Raw is the honest preview: those values resolve when the step runs, and an earlier step in the pipeline may write them. ## Tests `test_hook_show_expanded_matches_dry_run` pins the listing and the dry-run to the same rendering of `hook_type`, `hook_name`, and `args`. `test_foreground_pipeline_syntax_error_aborts_before_first_step` pins the relocated gate; it runs through `wt merge`'s pre-commit hooks because `wt hook <type>` cannot reach it (the CLI pre-parses every template for shorthand-argument routing and errors first). Also folded in: adding `source: HookSource` to the listing renderer left `approval_context: Option<(&Approvals, Option<&str>)>` encoding the same user-vs-project discriminator, so both sites now call one `needs_approval`. > _This was written by Claude Code on behalf of max_ --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Maximilian Roos
|
a285ed308d |
refactor(tests): bring the mock-stub env vars under the WORKTRUNK_TEST_ prefix (#3621)
`MOCK_CONFIG_DIR` and `MOCK_CALL_LOG_DIR` were the only two worktrunk-invented environment variables without the `WORKTRUNK_` prefix. That is not just a naming inconsistency: `isolate_subprocess_env` scrubs the parent environment by prefix — `GIT_*` and `WORKTRUNK_*` — so an unprefixed name is the one thing a test child inherits from whoever ran the suite. Renaming them brings them under that scrub. - `MOCK_CONFIG_DIR` → `WORKTRUNK_TEST_MOCK_CONFIG_DIR` - `MOCK_CALL_LOG_DIR` → `WORKTRUNK_TEST_MOCK_CALL_LOG_DIR` `TEST` rather than a bare `WORKTRUNK_` because both are read only by `tests/helpers/mock-stub` — they are the protocol between the harness and its helper binary, and `wt` itself never reads either one. That matches the ~15 existing `WORKTRUNK_TEST_*` knobs. ## The snapshot half Mechanical but not a substitution: the `env:` block is byte-sorted by key, so the renamed entry moves position within it and a `sed` in place would leave it where the old name sorted. All 971 affected blocks were rewritten by dropping the old line, inserting the new one, and re-sorting — the aggregate diff is exactly one removed and one added line per file: ``` 971 files changed, 971 insertions(+), 971 deletions(-) - MOCK_CONFIG_DIR: "[MOCK_CONFIG_DIR]" + WORKTRUNK_TEST_MOCK_CONFIG_DIR: "[TEST_MOCK_CONFIG]" ``` The redaction placeholder follows its neighbours' convention in `add_standard_env_redactions` (`WORKTRUNK_TEST_NU_VENDOR_AUTOLOAD_DIR` → `[TEST_NU_VENDOR_AUTOLOAD]`), so it now reads `[TEST_MOCK_CONFIG]` rather than repeating the full key. `MOCK_CALL_LOG_DIR` appears in no snapshot — its two call sites are `.output()` assertion tests — so it needs no redaction. ## Keeping it from regressing `tests/CLAUDE.md` gains the rule under "Where a new environment variable goes": name it `WORKTRUNK_TEST_*`, and the rule covers the harness↔helper-binary protocol, not just knobs `wt` itself reads. Without it the next helper-binary variable gets named `MOCK_*` again and the hermeticity hole reopens. ## Verification `cargo run -- hook pre-merge --yes` passes (exit 0) on the merged tree: 4607 tests including `--features shell-integration-tests`, `pre-commit run --all-files`, clippy, doctests. No pending snapshots. A repo-wide sweep finds no remaining unprefixed spelling. ## Merged main #3620 landed while this was in flight and regenerated several `for_each` snapshots that still carried the old key, so main is merged in here. It resolved with no conflicts, and the result is what you'd want rather than what git happened to produce: those blocks now carry #3620's new keys (`GIT_ALLOW_PROTOCOL`, `CLAUDE_CONFIG_DIR`, `WORKTRUNK_TEST_PARENT_SHELL`) *and* the renamed key, each in sorted position. The sweep and the gate above both ran after the merge. #3620 also rewrote the "Where a new environment variable goes" section this branch adds to — three layers became four. Both edits survived; the new naming paragraph follows the updated layer list. The two advisory `affected tests` checks are red for the same reason, and merging clears them: `cargo affected` errors on `git diff stdout was not valid UTF-8`, because the diff from this PR's base contains 16 binary files — the `tests/fixtures/standard/` git objects and index files that #3620 deleted. This branch's own commit contributes none. A PR based after #3620 won't see them. 🤖 Generated with [Claude Code](https://claude.com/claude-code) > _This was written by Claude Code on behalf of Maximilian Roos_ Co-authored-by: Claude Opus 5 <noreply@anthropic.com> |
||
|
Worktrunk Bot
|
c6eb148e62 | docs(hook): correct pre-switch {{ branch }} resolution note (#3516) | ||
|
Worktrunk Bot
|
89e858e9c3 |
fix(hooks): scrub inherited GIT_* discovery vars from user hooks (#3374)
## Problem `wt`'s command runner forwards inherited `GIT_*` discovery vars — `GIT_DIR`, `GIT_WORK_TREE`, `GIT_COMMON_DIR`, `GIT_INDEX_FILE`, `GIT_OBJECT_DIRECTORY` (`INHERITED_GIT_PATH_VARS`) — into every child it spawns, including **user hooks**. A hook is meant to operate on the worktree `wt` sets as its cwd, but when it shells out to `git` it instead discovers the *inherited* repo/worktree (e.g. `wt` run as a `!wt` git alias, or nested under another tool's git hook). The concrete harm the reporter called out: with both `GIT_DIR` and `GIT_WORK_TREE` present, a hook that runs `git init` (common in test-harness fixtures) writes `core.worktree` into the **inherited** repo's config, silently redirecting every later plain git command in that repo. Reported in #3373 with a full mechanism trace and minimal plain-git repro. ## Solution Scrub the git-discovery vars at every **user-hook** spawn site, so a hook's `git` commands discover the repo from the working directory `wt` sets: - `execute_shell_command` — foreground serial hooks (gated on hook vs alias via `PipelineKind::is_hook()`) - `output/concurrent.rs::spawn_child` — foreground concurrent hook groups (new `ConcurrentCommand::scrub_git_discovery`) - `commands/run_pipeline.rs::spawn_shell_command` — background hook pipelines (unconditional — that runner only ever executes hooks) `wt`'s **own** internal git plumbing (`Repository::run_command`) keeps the inherited context on purpose — that's the absolutize-and-forward behavior #1914 added for git-alias support — so the scrub is confined to the hook spawn sites. Aliases likewise keep the inherited context: a top-level `wt <alias>` is the user's own command, like typing it directly. The shared logic lives in `shell_exec::scrub_git_discovery_env_vars` (raw `Command`) and `Cmd::scrub_git_discovery_env` (builder). ## Testing Three new integration tests in `tests/integration_tests/user_hooks.rs`, one per spawn path (foreground serial, background, foreground concurrent). Each sets a repo-consistent `GIT_DIR`/`GIT_WORK_TREE` (so `wt` itself runs normally) and asserts the spawned hook sees neither var. All three fail on `main` (hook records `[<git_dir>][<work_tree>]`) and pass with the fix (`[][]`). `cargo clippy --all-targets` clean; the `user_hooks`, `step_alias`, `post_start`, and `for_each` suites pass unchanged. --- Closes #3373 — automated triage --------- Co-authored-by: worktrunk-bot <254187624+worktrunk-bot@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Maximilian Roos
|
9be99ee21b |
test: consolidate absence-window sleeps behind a shared SLEEP_FOR_ABSENCE_CHECK (#3206)
Absence assertions (verifying something did NOT happen) need a fixed window rather than polling, since there's no event to wait for. That window was expressed three inconsistent ways across the test suite: a per-file `const SLEEP_FOR_ABSENCE_CHECK` duplicated in `user_hooks.rs` and `post_start_commands.rs`, and bare `Duration::from_millis(500)` literals scattered through `switch.rs`, `step_alias.rs`, `remove.rs`, `bare_repository.rs`, and `merge.rs` — none greppable as a group, and `merge.rs` sitting below the documented 500ms floor at 200ms. This promotes `SLEEP_FOR_ABSENCE_CHECK` to `src/testing/mod.rs`, next to the presence helper `wait_for_file`, re-exported via `tests/common`, and routes all 15 absence sleeps through it. The constant's doc comment states the discriminator that decides the tool: a *presence* assertion polls (and returns the instant the event lands); an *absence* assertion has no event to wait for, so it holds a bounded window. The three sleeps left as literals are genuinely different roles — bash startup, signal sequencing between two SIGINTs, and PTY output sequencing — not absence checks. It also rewrites the `tests/CLAUDE.md` timing section around that polarity discriminator (split into Presence / Absence subsections), with the absence example using the constant, plus two traps: pairing one sleep with both a presence and an absence assertion (the presence half goes flaky), and structural absence — when the event is gated on a condition the test never sets up, drop the window and poll the positive precondition instead. This is the consistency-and-guidance follow-up to the watchdog de-flaking in #3187: the marker convention makes legitimate absence sleeps self-labeling, which is what lets a reviewer (or the nightly sweep) tell them apart from a flaky fixed sleep before a presence assertion. ## Testing No behavior change — test-infrastructure and docs only. Each of the 15 conversions was verified to be followed by a negative assertion (adversarial review, 20/20 confirmed); the full `pre-merge` gate passes (4170 tests). `merge.rs`'s window widened 200ms → 500ms, which only makes its absence check more conservative. > _This was written by Claude Code on behalf of max_ --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Maximilian Roos
|
22768d425f |
feat(hooks)!: table-form pre-* hooks run concurrently — remove the serial path (#3052)
## Summary Lands the cutover announced in v0.37.0 (#2135): a multi-entry table hook now runs its commands concurrently for every hook type. `[pre-merge]` with several keys behaves like `[post-start]` with several keys and like a single `[[pre-merge]]` block — one execution semantic for `Concurrent` steps. The serial behavior was not implemented where one might expect. The executor's serial branch (`ForegroundStep.concurrent` / `SourcedStep.is_pipeline` / `CommandConfig::is_pipeline()`) was mostly dead: the deprecation shipped with a load-time TOML migration that rewrote multi-entry pre-* tables into serial pipeline form in memory on every config load, so affected configs never reached the executor as `Concurrent` steps. Removing that migration (now a `DEPRECATION_RULES` row, after #3045) is the actual behavior change; the executor flags come out with it. ## Why no replacement runtime warning Affected configs have printed a warning on every `wt` invocation for twenty minor releases (v0.37.0, April 12 → v0.57.0), with `wt config update` offering a one-command migration that preserved serial behavior explicitly ("migrate now to keep the current serial behavior once the table form is repurposed"). A post-cutover warning has no coherent shape: table form is now legitimate concurrent config, so it would nag users who want exactly that behavior with no way to silence it, and the `wt config update` rewrite would no longer be behavior-preserving. ## Parse normalization A one-entry top-level table previously parsed as a `Concurrent` group of one; with the serial branch gone it would have picked up `name │` output prefixes. One-entry maps now parse as `Single(named)` everywhere, matching one-entry maps inside pipeline lists (removing the documented dict-at-top vs dict-in-list asymmetry), and a new `Serialize` arm keeps one-step named configs round-tripping as named tables. Single-entry table hooks render exactly as before. ## Behavior changes 1. **Multi-entry table-form pre-* hooks: serial → concurrent.** The announced change. 2. **`wt hook <post-type>` foreground runs of multi-entry table hooks: serial → concurrent.** Previously an undocumented inconsistency — the same config already ran concurrently via the background path. 3. **Single-entry table aliases (`[aliases.x]` with one key): prefixed-stderr → stdout passthrough.** Now matches the string and `[[aliases.x]]` spellings, and makes `wt <alias> | …` work for this spelling too. (2) and (3) were never deprecation-warned; all three belong in the release notes. ## Tests - `test_pre_merge_deprecated_table_runs_serially` → rewritten as `test_pre_merge_table_form_runs_concurrently`; the single-`[[pre-merge]]`-block test is deleted (both forms now parse identically, so it duplicated the rewritten test). - Fixtures that genuinely need serial ordering (`>`/`>>` chains, the signal-abort "second must not run" assertion) converted to pipeline form. - `test_user_hooks_preserve_toml_order` (#737) and `test_post_start_named_commands` keep table form and pin ordering via `WORKTRUNK_TEST_SERIAL_CONCURRENT=1`, so insertion order is still asserted through the real concurrent input ordering. - New: one-entry-table parse/round-trip unit test; `test_alias_single_entry_table_writes_to_stdout` pinning change (3). - Regenerated snapshots drop a stale `RUST_LOG: warn` env line (the harness stopped setting it in #2901). ## Known follow-up Concurrent-group announcements expose a pre-existing ANSI dim-bleed: `format_bash_with_gutter` output ends without closing the dim attribute, so back-to-back `◎ Running …` lines render dim (visible today on pipeline-form hooks and `--execute` verbose). Now that this rendering is the table-form default it's worth fixing, but the fix lives in the gutter formatter and churns every bash-gutter snapshot — separate PR. > _This was written by Claude Code on behalf of max_ --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
Maximilian Roos
|
96c3af4853 |
fix(hooks): background runner labels template errors like the foreground (#3047)
Follow-up from review on #3042. The background pipeline runner labeled template-expansion errors with `name.unwrap_or("pipeline step")`, so a runner log read `Failed to expand broken: …` (or `Failed to expand pipeline step: …`) where the foreground says `Failed to expand user:broken: …`. The prep-computed `template_name` ("user:foo" for named hook commands, "user pre-merge hook" for unnamed ones) now travels through `PipelineStepSpec` / `PipelineCommandSpec` and is used as the render label in the runner, so the label is computed once in `prepare_steps` and the two paths can't drift. The spec is an internal JSON blob piped to the detached `wt hook run-pipeline` process spawned from the same binary, so the new required field has no compatibility concerns. Command-failure messages intentionally keep the bare command name rather than `template_name`: that mirrors the foreground, where `hook_error_wrapper` puts `cmd.name` into `HookCommandFailed` and only expansion errors get the source-qualified name. The serial path's failure label is aligned with the concurrent path's existing convention (`name.unwrap_or(expanded)` — it previously used the expanded command even for named steps). Runner-log error renderings are now pinned by inline snapshots: the expansion error (matching the committed foreground snapshot byte-for-byte) and both command-failure label shapes (named and unnamed). The concurrent-group failure label isn't separately snapshotted — it's the same `failure_error` call and label choice the new test pins, unchanged by this PR. > _This was written by Claude Code on behalf of max_ Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
Maximilian Roos
|
371d286628 |
refactor(hooks): always-lazy template expansion for hooks and aliases (#3042)
Hook templates expanded eagerly at prep time unless they referenced `vars.*` (#1840); aliases were unconditionally lazy. The dual representation (`expanded` + `lazy_template` on `PreparedCommand`) put a mode choice in front of every consumer, and the seams produced real bugs (#1855, #1910). The background runner also re-rendered eagerly-expanded strings: a latent double-expansion whenever expansion output contained `{{`. This PR unifies on one rule: syntax is validated at preparation (before step 1), templates render when each step runs, and `vars.*` always reads fresh from git config. This is the direction #2128 recorded ("treat hooks as aliases bound to triggers"). **Navigating the diff:** - `src/commands/command_executor.rs`: `PreparedCommand` now carries the raw template plus the frozen context map; JSON is produced only at the process boundary. A `template_name` field keeps the moved errors' text identical to the old prep-time messages. `prepare_steps` validates syntax and freezes context; `resolve_command_str` renders at execution; `map_config_steps` is the structural walker shared with aliases. - `src/commands/alias.rs`: alias prep shares the walker. The residual fork is genuinely alias-specific: context construction (filtered to referenced vars, `{{ args }}` injection) and labels. A prep-time syntax check turned out to be dead code (the arg-routing parse already aborts on syntax errors), so `referenced_vars_for_config` now produces the rich `TemplateExpandError` instead. - `src/commands/hooks.rs`: the background spec always ships raw templates; the runner renders each step (it already did for vars steps). - Dry-run paths (`wt hook --dry-run`, `wt config alias dry-run`) share `render_template_preview`: vars-referencing templates shown raw after a syntax check, everything else rendered at display time. **Behavior changes** (each pinned by a test or snapshot): 1. A foreground pipeline whose step N has a semantic template error (undefined variable) runs steps 1..N-1 before failing, matching what vars-referencing steps already did. New test: `test_foreground_pipeline_undefined_var_runs_earlier_steps`. 2. A semantic error in a background hook template no longer fails the foreground command; it surfaces in the runner log. Syntax errors still abort at prep. New test: `test_background_hook_undefined_var_fails_in_runner`; `test_merge_drops_pending_hooks_when_post_merge_fails` switched to a syntax-error fixture since that is what now reaches its subject (the announcer Drop-flush). 3. `-v` no longer prints the parent-side rendered command for background hooks (rendering happens in the runner). The variables table remains and `wt hook <type> --dry-run` previews commands; help text updated. Three snapshots changed by exactly this. 4. Announce gutters for vars-referencing foreground steps show the rendered command instead of the raw template. 5. Syntax errors caught by the arg-routing parse (aliases, `wt hook` `--KEY=VALUE` routing) render as the rich template error with the source-line gutter, naming the alias or hook. `ApprovedHookPlan` freezing, approval semantics (keyed and displayed by template), config file format, and CLI flags are unchanged. **Testing:** full local gate green (3917 tests + lints). The deferral semantics in changes 1 and 2 previously had no coverage; both are now pinned. > _This was written by Claude Code on behalf of max_ Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
Worktrunk Bot
|
ac5877624c |
test: correct detached-HEAD assertion and isolate raw Command usages (#3032)
Three test-hygiene fixes surfaced during the nightly survey, all in
integration tests.
## `test_statusline_detached_head` asserted the wrong behavior
The test claimed "we show 'HEAD' as the branch name", but the statusline
actually renders `(detached)` for a detached HEAD — it never emits the
string `HEAD`. The assertion was `output.contains("HEAD") ||
!output.contains("feature")`, which passed entirely on the right-hand
side (the output contains neither `HEAD` nor `feature`), so the `HEAD`
claim was never exercised. Captured output for the detached case is `
(detached) _`.
Now split into two real assertions: the output **must** contain
`(detached)`, and **must not** contain the prior branch name. The
corrected test would have failed against the old (wrong) expectation,
confirming it now checks real behavior.
## Stale `COLUMNS=80` comment in
`test_statusline_rate_limit_drops_at_narrow_width`
The doc-comment said `COLUMNS=80`, but the test passes `Some(40)` and
the very next inline comment says `COLUMNS=40`. Corrected the
doc-comment to match.
## Two tests bypassed the isolation helper
`test_var_flag_invalid_format_fails` and
`test_var_shorthand_does_not_leak_into_hook_show` invoked the binary via
`std::process::Command::new(env!("CARGO_BIN_EXE_wt"))`, the pattern
`tests/CLAUDE.md` explicitly flags as the BAD form because it inherits
the host environment (`WORKTRUNK_CONFIG_PATH`, `HOME`, `GIT_*`). Both
only assert argv-parse errors, so behavior is unchanged, but they now
use the isolated free `crate::common::wt_command()` like the rest of the
file.
## Test plan
```
cargo test --test integration -- \
test_statusline_detached_head \
test_statusline_rate_limit_drops_at_narrow_width \
test_var_flag_invalid_format_fails \
test_var_shorthand_does_not_leak_into_hook_show
```
All four pass.
Co-authored-by: worktrunk-bot <254187624+worktrunk-bot@users.noreply.github.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
|
||
|
Maximilian Roos
|
d8d94bd9d3 |
fix(hooks): correct pre-start docstring; rename mismatched test (#2879)
Two small follow-ups noticed while fixing the test comments in #2877. **`pre_create` docstring was wrong.** It read "Commands to execute before worktree creation (blocking)", but `pre-start` actually runs *after* the worktree is created (`switch.rs:1142` executes it against "the new worktree (created just above)", and the renamed test below asserts the worktree exists after a failing pre-start). The wrong wording entered in #2840 as part of the mechanical `pre-start` → `pre-create` rename — someone "corrected" the docstring to match the new name. #2857 reverted the name but left the docstring. Restoring "after worktree creation (blocking, fail-fast)" matches the actual behavior, `docs/content/hook.md` ("Runs once when a new worktree is created, blocking…"), and the parallel `post_create` docstring. Because `HooksConfig` derives `JsonSchema`, this docstring is user-visible in generated schema descriptions. **`test_user_post_start_hook_failure` was misnamed.** The fixture is `[pre-start]`, the comment and assertion describe a failing pre-start, and only the function name said post-start. Renamed to `test_user_pre_start_hook_failure` along with its snapshot name and file (`git mv`, so the rename shows as a rename). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
Maximilian Roos
|
9131aa6644 |
test(hooks): exercise the pre-create alias direction post-revert (#2877)
Follow-up to #2857. That PR reverted the docs half of the `pre-start`/`post-start` → `pre-create`/`post-create` hook rename while keeping the code that accepts both names. Its self-review landed 26 seconds after the merge, so none of its findings were addressed — several tests still described the pre-revert direction and exercised only canonical names, so they would pass even if the deprecated alias broke. ## Changes The three deprecated-alias tests now use `pre-create`/`post-create` in their fixtures (or invoke `wt hook pre-create`), so they exercise the migration and the CLI alias rather than canonical names: `test_hook_show_accepts_deprecated_create_hooks`, `test_deprecated_create_hook_key_runs_silently`, `test_standalone_hook_create_alias_runs_silently`. `test_parse_hook_type_aliases` had the same defect — it parsed only the canonical names — and now parses both forms and asserts they map to the same `HookType`. Added unit tests for `migrate_create_hooks_doc` (every value shape, per-project tables, skip-when-canonical-exists, invalid TOML) plus a migration-diff snapshot; the revert removed the `migrate_start_hooks_doc` tests with no equivalent. Deleted `Deprecations::pre_start`/`post_start` — always-false dead fields once the revert removed their detection. Deleted `test_config_show_displays_start_hook_migration`: post-revert there is no detection for `pre-create`, so `wt config show` shows no migration diff and the test verified nothing (its regenerated snapshot confirmed this). Also corrected reversed-direction comments in `deprecation.rs` and `hook_commands.rs`. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
Worktrunk Bot
|
ec62580c29 |
revert(hooks): keep docs on pre-start/post-start; code accepts both (#2857)
Per @max-sixty's [direction in #2838](https://github.com/max-sixty/worktrunk/issues/2838#issuecomment-4509447593): revert the docs portion of #2840 and keep the code. Docs continue to recommend `pre-start`/`post-start`; both names work in code so anyone who already followed the briefly-changed docs (e.g. @EcksDy) isn't stranded once a release ships these aliases. ## User-visible — back to `pre-start`/`post-start` - README, docs site, skill mirrors, `dev/*.example.toml`, `plugins/worktrunk/README.md`, `flake.nix`, `.config/wt.toml` - `src/cli/mod.rs` / `src/cli/config.rs` / `src/cli/step.rs` / `src/help.rs` after_long_help and example snippets — and the auto-synced `docs/content/` and `skills/worktrunk/reference/` mirrors - `wt hook --help` canonical subcommand names; completion advertises `-start` only - `HookType` Display via strum, serde `rename`, and clap `ValueEnum` name — all `pre-start`/`post-start`. The Rust variant identifiers stay `PreCreate`/`PostCreate` (internal; we already paid for that rename in #2840, and now the eventual flip is a Display-only change) - `HooksConfig` serde canonical fields ## `*-create` still works (kept code) - `wt hook pre-create` / `post-create` — CLI alias on the canonical subcommand - `pre-create` / `post-create` in config: top-level, `[hooks.*]`, and per-project, in string, `[table]`, and `[[array-of-tables]]` form. Mechanism: serde `alias = ...` on the field, plus a silent in-memory rename in `migrate_content()` so the round-trip in `unknown_tree` doesn't flag table forms as schema-unknown. - The pre-0.32.0 `post-create` fatal-load-error machinery stays removed — the name is reclaimed, and both forms load without error. ## Smaller bits - `valid_user_config_keys()` / `valid_project_config_keys()` append `pre-create` / `post-create` so the unknown-field round-trip skips them. `test_valid_*_keys_all_deserialize` skips both aliases (they can't sit alongside the canonical without a duplicate-field error). - `DEPRECATED_SECTION_KEYS` drops the `pre-start`/`post-start` entries #2840 added — `pre-start`/`post-start` are canonical again. - `find_pre_start_from_doc` / `find_post_start_from_doc` / `find_renamed_hook_key` / `is_non_empty_item` / `migrate_start_hooks_doc` and their tests are removed; the migration direction flips via a new `migrate_create_hooks_doc` (silent, mirrors the prior shape). - Test files `e2e_shell_post_create.rs` and `post_create_commands.rs` rename back to `_post_start_` (via `git mv`, so the rename shows as a rename). ## Testing `cargo run -- hook pre-merge --yes` — 3806 tests pass; the 10 failures are all `case_4` of `shell_wrapper::unix_tests::*` (nu-shell case; `nu` isn't installed in this runner; same failures occur on `main`). Also manually verified that a fresh `wt switch --create` against a project config with `[post-create]` loads cleanly with no unknown-field warning and the hook fires as `post-start`. ## Follow-up Per @max-sixty: in a couple of weeks, once a release with both-names-work is out and users have had a chance to upgrade, the docs flip is straightforward (most of it is in `src/cli/mod.rs`'s `after_long_help` and the doc-sync test propagates). Re #2838. Co-authored-by: worktrunk-bot <254187624+worktrunk-bot@users.noreply.github.com> Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com> |
||
|
Maximilian Roos
|
d7e3f88422 |
feat(hooks): rename worktree-creation hooks to pre-create/post-create (#2840)
Phase 1 of the staged hook rename tracked in #2838: the worktree-creation hooks `pre-start`/`post-start` become `pre-create`/`post-create`. The old names keep working with no deprecation warning yet (Phase 2, months out, adds the warning). ## What changes - `pre-create`/`post-create` are canonical everywhere: the `HookType` enum, the `HooksConfig` serde fields, the `wt hook` CLI, completion, and all docs. - Old names keep working: `migrate_content()` rewrites `pre-start`/`post-start` config keys to `-create` before serde, and `parse_hook_type` accepts the old CLI names as silent aliases. `wt config update` rewrites them on disk; `wt config show` shows the migration diff. `wt hook <type>` execution and `wt hook show` both accept the old names; completion and `--help` advertise only the canonical names. - `detect_deprecations()` flags the old keys so `update`/`show` act on them, but `format_deprecation_warnings()` stays silent (Phase 2 adds the warning). A new empty-warnings guard in `check_and_migrate` keeps a `-start`-only config from emitting a stray hint. - The dead pre-0.32.0 `post-create` machinery is removed: the fatal `POST_CREATE_REMOVED_MSG` load error, the vestigial `HooksConfig.post_create` merge-fold, and `find_post_create_from_doc`. `post-create` is reclaimed as the canonical background creation hook. ## Semantic flip Before v0.32.0, the key `post-create` named a *blocking* hook. It now names the *background* one. Since v0.44.0, a pre-0.32.0 `post-create` config is a fatal load error on the `check_and_migrate` paths: `ProjectConfig::load` and user/system config loading, which fire on essentially every `wt` command. A repo carrying one has been unusable ever since. The one path that skips that check is `project_config_at_ref` (the base-ref read behind `wt switch --create`), which applies only structural migration. A pre-0.32.0 `post-create` surviving solely on a base ref, never checked out into a worktree, would now load as a background hook rather than folding into the blocking `pre-start`. That edge case is accepted: once `post-create` is valid again, reclaiming the name and detecting the dead key are mutually exclusive. ## Reviewing this diff 205 files, but the substance is ~36 files under `src/`. The rest is regenerated snapshots and auto-synced doc mirrors. Start with: - `src/config/deprecation.rs` — detection (`find_renamed_hook_key`), migration (`rename_hook_key`), removal of the fatal block, the empty-warnings guard, and the `DEPRECATED_SECTION_KEYS` entries that stop unknown-field detection from flagging the migrated keys. - `src/config/hooks.rs`, `src/git/mod.rs` — the serde field and enum renames. - `src/config/project.rs` — `ProjectConfig::load` deserializes `check_and_migrate`'s migrated content, so a current-worktree config using the old keys loads into the canonical fields. - `src/cli/hook.rs`, `src/commands/hook_commands.rs`, `src/completion.rs`, `src/main.rs` — the CLI alias layer; `wt hook show` accepts the old type names as hidden value-parser aliases. - `src/cli/mod.rs` — the `wt hook` docs, including the soft-deprecation note linking #2838. The ~93 modified snapshots also pick up deterministic env-block lines (`GIT_*: ""`, `LLVM_PROFILE_FILE`) that pre-existing snapshots already carry. That is stale-snapshot drift surfaced by the regeneration, not a behavior change. ## Testing Full suite green (3799 tests). New coverage: `snapshot_migrate_start_to_create` (migration preserves value shape and position), `test_deprecated_start_hook_key_runs_silently` and `test_standalone_hook_start_alias_runs_silently` (old config and CLI names run with no warning), `test_config_show_displays_start_hook_migration` (`config show` reveals the diff without an "unknown field" warning), and `test_hook_show_accepts_deprecated_start_hooks` (a current-worktree config using the old keys loads, and `wt hook show` takes both the canonical and the deprecated type arguments). Part of #2838. 🤖 Generated with [Claude Code](https://claude.com/claude-code) |
||
|
Maximilian Roos
|
2d093ffc83 |
refactor(hooks/alias): lift hook metadata to PipelineKind, unify source enum (#2521)
Two follow-ups deferred from #2474, plus a coverage refinement on review feedback. ## Lift hook-only metadata onto `PipelineKind::Hook` `SourcedStep::hook_type: Option<HookType>` and `display_path: Option<PathBuf>` were a smell — the invariant is per-pipeline, not per-step, but living on `SourcedStep` forced four `.expect("hook pipelines always set hook_type")` sites. Lifted both onto `PipelineKind::Hook { hook_type, display_path }`, mirroring `Alias { name }`. The invariant now lives on the type, and `sourced_steps_to_foreground` reads the metadata directly from `kind`. Background flow doesn't need `PipelineKind` at all — it only ever handles hooks (aliases run in the foreground). Threading `PipelineKind` through it forced two `unreachable!()` and one defensive `continue` for the structurally unreachable `Alias` variant. Background functions now take a `BackgroundPipeline` type alias for `(CommandContext, HookType, Option<PathBuf>, Vec<SourcedStep>)` directly. Touches `prepare_background_pipelines`, `run_hooks_background`, `print_background_variable_table`, `spawn_hook_pipeline_quiet`, `sourced_steps_to_foreground`, `run_hooks_foreground`, and the alias and hook-commands call sites. ## Unify `AliasSource` into `HookSource` `AliasSource` and `HookSource` were near-identical `User`/`Project` enums. Removed `AliasSource`; all alias call sites now use `HookSource`. The shared enum gains `PartialOrd`/`Ord` derives (needed for `(name, source)` listing sort) and an updated docstring covering dual hook+alias usage. `source.label()` becomes `{source}` via the existing `strum::Display`. ## Test plan - [x] `cargo nextest run` — 3261 tests pass (incl. new `test_combined_post_remove_and_post_switch_hooks_verbose` covering the multi-hook-type filter in `print_background_variable_table`) - [x] `pre-commit run --all-files` — green - [x] Snapshot tests unchanged for existing flows 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Maximilian Roos
|
f499ec0590 |
refactor(hooks): route post-commit and post-switch through HookAnnouncer (#2482)
Extends the `HookAnnouncer` coordinator (introduced in #2457, refined in #2477) to more hot paths so multi-phase background hooks share a single `◎ Running …` line. **Migrated sites:** - `wt switch --create` — post-switch + post-start (`spawn_switch_background_hooks` in `handle_switch.rs`). - `wt merge` — auto-commit (when dirty) + post-remove + post-switch + post-merge. - `wt merge --squash` — post-commit (from squash) + post-remove + post-switch + post-merge. **Plumbing:** `CommitOptions::commit` and `handle_squash` gained an optional `&mut HookAnnouncer<'_>` parameter. `handle_merge` constructs one announcer early and threads it through the commit, squash, and remove paths; `flush()` runs once at the end. Standalone `wt commit` / `wt step squash` pass `None` and self-announce as before. **No new abstractions** — the `Some` arm calls `extend`, the `None` arm calls `run_hooks_background`, mirroring the existing precedent in `output/handlers.rs::spawn_hooks_after_remove`. As a follow-on, `spawn_background_hooks` (whose only caller after migration was `picker::do_removal`, gated `#[cfg(unix)]`) is dropped and the picker's call site is inlined to match — fixing a Windows dead-code build error and unifying all four single-shot post-hook sites on the same shape. **Test coverage:** new `test_merge_squash_combines_post_commit_post_remove_post_switch_post_merge` and `test_merge_auto_commit_combines_post_commit_post_remove_post_switch_post_merge` snapshot the combined four-phase announce line for both squash and non-squash auto-commit paths. Existing `test_merge_combines_post_remove_post_switch_post_merge`, `test_merge_drops_pending_hooks_when_post_merge_fails`, and `test_switch_combined_post_switch_and_post_start_hooks` remain green. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
Maximilian Roos
|
d2ef2ee90c |
refactor(hooks): unify dispatch and replace CommandOrigin with closures (#2472)
## Summary - Replaces the `CommandOrigin` enum in `command_executor.rs` with explicit fields on `ForegroundStep` (`AnnouncePolicy`, `pipe_stdin: bool`, `error_wrapper: ErrorWrapper`) and per-command `label` / `log_label` set at prep time. `handle_command_error` collapses from a 4-variant case-split to a single branch (`FailFast` calls the closure, `Warn` prints). Resolves the pre-existing TODO at `command_executor.rs:308-316`. - Tightens `hooks.rs` around two pub entries (`run_hooks_foreground`, `run_hooks_background`) plus `pub(crate)` shortcuts (`execute_hook`, `spawn_background_hooks`) that absorb the auto-config-lookup boilerplate. `execute_hook` is the canonical operation-driven path and applies `add_hook_skip_hint` centrally; `wt hook <type>` calls `run_hooks_foreground` directly so failures don't carry the misleading `--no-hooks` reminder. - Adds `HookType::is_pre()` and `FailureStrategy::default_for(hook_type)` so `run_hook`'s pre/post dispatch is one `if`. Adds `approve_or_skip` for the "approve hooks → fall through if declined" pattern (used at 4 sites). - Collapses `run_post_hook`'s filter/no-filter dispatch to a single `is_empty()` branch. Eliminates `prepare_background_hooks` duplication via `into_source_groups` over the flat `prepare_sourced_steps` result. Tightens `spawn_hook_pipeline_quiet`'s base-context extraction (caller invariants guarantee non-empty steps). - Integrates with `HookAnnouncer` (#2457): `register` calls `prepare_background_pipelines`, `flush` calls `run_hooks_background`. Net: 500 insertions, 593 deletions across 16 files. 3391 tests pass; lints and snapshots clean. New regression test `test_standalone_hook_failure_omits_skip_hint` locks in the `wt hook <type>` skip-hint behaviour. ## Test plan - [x] \`cargo run -- hook pre-merge --yes\` (3391 tests, lints, snapshots clean) - [x] \`cargo insta test --accept -- --test integration "test_help"\` (no snapshot drift) - [x] Regression test verified by re-introducing the skip-hint over-application — test fails; reverting fixes it. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Maximilian Roos
|
091a3d6ed6 |
hooks: combine background hook announces per command (#2457)
Today `wt merge` (with removal) emits two or three separate `◎ Running …` lines — one for post-remove + post-switch (already batched), then another for post-merge. Collapse them into one combined announce so a single `wt` command produces a single status line for its background hooks. ``` # before ◎ Running post-remove: user:cleanup; post-switch: user:notify ◎ Running post-merge: user:sync # after ◎ Running post-remove: user:cleanup; post-switch: user:notify; post-merge: user:sync ``` ## Approach `HookAnnouncer` (in `src/commands/hooks.rs`) registers pipelines from multiple phases and flushes once. It stores owned `PendingPipeline` data so registration sites at different points in the command's lifecycle can pass short-lived `CommandContext`s without lifetime gymnastics; `flush()` rebuilds contexts from owned data and delegates to the existing combined formatter. A `Drop` impl flushes pending hooks on early-return errors. Without it, a later registration failure would silently swallow earlier-registered pipelines — a regression from the prior fire-and-forget pattern. On the success path, explicit `flush()` runs first so `Drop` sees empty pending and is a no-op. ## Wiring - `handle_remove_output` gains an `announcer: Option<&mut HookAnnouncer<'_>>` parameter, threaded through the internal handlers (`handle_named_removed_worktree_*`, `handle_detached_removed_worktree_output`, `spawn_hooks_after_remove`). When `Some`, pipelines register on the announcer instead of self-spawning. - `wt merge` constructs one announcer, passes it through the remove block, registers post-merge after, calls `flush()` once before json output. - Standalone `wt remove`, prune, and the picker remove path pass `None` — output unchanged. - `wt switch --create` (already one-line via `announce_and_spawn_background_hooks`) is unchanged. Migrating it to `HookAnnouncer` for consistency is a follow-up. ## Testing - `test_merge_combines_post_remove_post_switch_post_merge` snapshots the combined announce line. - `test_merge_drops_pending_hooks_when_post_merge_fails` covers the Drop fallback: post-merge template prep errors after post-remove + post-switch register, and the hooks still fire via Drop. `codecov/patch` reports 96.40% vs 96.73% target. The five missed lines are the inner `eprintln!` arm of the Drop fallback (fires only when `flush()` itself errors during Drop — requires `spawn_hook_pipeline_quiet` to fail) and a few trailing `)?;` lines that llvm-cov misattributes. Defensive logging path, not meaningfully testable; merging with the gap. --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Maximilian Roos
|
acb985c2d6 |
fix(deprecate): warn on --claude-code flag and wt hook post-create alias (#2436)
Both surfaces previously mapped silently to their canonical replacements, so users had no signal to migrate before eventual removal. Emit per-invocation warnings to stderr matching the existing pattern used by `wt select`, `--no-verify`, and `wt hook approvals`. The config-section `[hooks.post-create]` warning path is unchanged — that already errors at config load (#2361). This only adds a warning for the bare CLI alias `wt hook post-create`, which still maps to `pre-start` after warning. One UX consideration: `--claude-code` is read on every Claude Code statusline redraw, so users with the deprecated form in their statusline integration will see the warning each redraw until they migrate. That's the intent (surface the deprecation), but worth noting. Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Maximilian Roos
|
8abefbce82 |
refactor(docs): unify AUTO-GENERATED marker form; share constants across crate boundary (#2418)
## Summary Follow-ups from #2417 review, plus adjacent simplifications. - **Cross-crate share of marker constants.** `MARKER_OPEN_PREFIX` and `MARKER_CLOSE` now live in `src/docs.rs`. `src/help.rs` (the `--help-page` producer of help-region markers) and `tests/integration_tests/readme_sync.rs` (the consumer + producer of snapshot/section markers) both reference them — drift becomes a build error rather than a silent mismatch. - **Drop `MARKER_OPEN_HTML_PREFIX`.** The `-HTML` variant emitted visually identical wrapping; the suffix only signalled "in-place refreshable" but the `.snap` ID + terminal-shortcode body in `DOCS_SNAPSHOT_MARKER_PATTERN` already discriminate that. Standardised on a single open prefix and stripped the `-HTML` literals from `README.md`, the four standalone docs files, and the test file. - **Help-page marker uses the shared prefix.** The mirrored close (`<!-- END AUTO-GENERATED from \`wt <cmd> --help-page\` -->`) stays as-is because adjacent regions need unambiguous pairing, but the open prefix now goes through `MARKER_OPEN_PREFIX`. - **Drop `MarkerType::output_format()` / `extract_inner()`.** Both had a single trivial non-panic branch that existed only to enforce "no Snapshot markers in README" via `unreachable!()`. Replaced with one explicit assertion in `sync_readme_markers` — surfaces the invariant as an actionable error message instead of a panic. - **Collapse `format_replacement`.** `wrap_in_marker` is invoked once for both output formats; only body construction varies. - **`__WT_QUOT__` rename in `tests/integration_tests/user_hooks.rs`.** Inlined the single quotes — the `.replace('__WT_QUOT__', \"'\")` was unnecessary indirection (the outer raw-string literal already tolerates `'`, and TOML / Tera don't care about embedded `'`). Removes a name-conflation footgun with the unrelated `__WT_QUOT__` placeholder in `src/docs.rs`. Net diff: -7 lines. ## Test plan - [x] `cargo test --test integration` — full integration suite (1559 tests) pass - [x] `cargo test --test integration readme_sync` — all 13 sync tests pass; `test_readme_examples_are_in_sync` now produces a stable README on a clean run (verified by re-running multiple times — no further updates after the initial regeneration) - [x] `cargo test --test integration test_args_indexing_and_length_in_hook_template` — confirms the user_hooks single-quote inlining works through TOML and Tera - [x] Visual check via local Zola dev server: \`/worktrunk/\`, \`/llm-commits/\`, \`/claude-code/\`, \`/tips-patterns/\`, \`/merge/\`, \`/step/\`, \`/remove/\`, \`/hook/\`, \`/list/\` all render terminal blocks cleanly with no leaked marker strings (`AUTO-GENERATED-HTML`, `__WT_OPEN2__`, trailing `|||`) - [x] `cargo clippy --all-targets --all-features` — clean - [x] `cargo fmt --check` — clean 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Worktrunk Bot
|
5f126d21fe |
docs(pages): add static command output to sections dominated by GIFs (#2405)
## Summary Adds static command-output blocks to the docs pages dominated by GIFs (addresses #2403). The blocks are **driven from insta snapshots** so they stay in lockstep with what `wt` actually prints — one source flows to all three surfaces: - terminal `wt <cmd> --help` (plain text, gutter-formatted) - `docs/content/*.md` (colorized `{% terminal(cmd="...") %}` shortcode) - `skills/worktrunk/reference/*.md` (plain `$ cmd\noutput\n` block) ## What changed - **Six scripted snapshot tests** produce realistic output (`cargo nextest run`, `flyctl scale count 0`, LLM-generated commit messages): - `test_docs_merge_pre_merge_hook` — `wt merge` with pre-merge hook - `test_docs_step_commit_llm` — `wt step commit` with LLM - `test_docs_step_squash_llm` — three-commit squash with LLM - `test_docs_merge_squash_llm` — `wt merge` (squash + LLM + merge) for `llm-commits.md` - `test_docs_remove_pre_remove_hook` — `wt remove` with pre-remove hook - `test_docs_hook_pre_merge` — `wt hook pre-merge` direct invocation - **Sync pipeline extension** in `tests/integration_tests/readme_sync.rs`: - New write-back pass `sync_cli_mod_example_bodies` fills the ```console``` body in each `<!-- wt <cmd> (docs-example) -->` placeholder in `src/cli/mod.rs` from the registered snapshot. Runs before `--help-page` reads the file. - `COMMAND_PLACEHOLDER_PATTERN` extended to match three forms (```bash```, `{{ terminal() }}` self-closing, `{% terminal %} body {% end %}`). This **fixes a pre-existing bug** in `docs/content/list.md` where the HTML-mode expansion was silently broken. - Stripped trailing `|||` corruption that arises when blank lines in snapshot bodies are interpreted as empty commands by `convert_dollar_console_to_terminal`. - **Docs page migration**: - `src/cli/mod.rs` — replaced four hand-written ```console``` blocks (merge, step, remove, hook) with `<!-- wt <cmd> (docs-example) -->` markers. - `docs/content/llm-commits.md` — replaced three hand-crafted HTML blocks with `<!-- ⚠️ AUTO-GENERATED-HTML from X.snap -->` markers. - **Refactor follow-up** in a separate commit: - `BADGE_EXPERIMENTAL_HTML`, `SUBDOC_MARKER_PREFIX`, `DEMO_MARKER_PREFIX` hoisted to `worktrunk::docs` so producer and consumer stay in lockstep. - `normalize_clap_help_fences()` consolidates the `text→` + `console→bash` replacement pair shared by `--help-md` and `help_reference_inner`. - **CLAUDE.md note** documenting the `.gitattributes` `linguist-generated=false` exemption requirement when adding skill-only files (carryover from #2409). ## Test plan - [x] `cargo test --test integration readme_sync` — all 13 tests pass, idempotent - [x] `cargo test --test integration test_help` — help snapshots updated - [x] `cargo run -- {merge,remove,step,hook} --help` — clean gutter formatting, no visible HTML comments - [x] `cargo run -- hook pre-merge --yes` — full project gate green (3355 tests) - [ ] Visual check on dev site (maintainer — terminal shortcodes now render with colors via ANSI→HTML; dark/light variants both expected to look like the GIFs they replace) 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: worktrunk-bot <254187624+worktrunk-bot@users.noreply.github.com> Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com> Co-authored-by: Maximilian Roos <m@maxroos.com> |
||
|
Worktrunk Bot
|
04db9ab801 |
fix(config): reject post-create hook instead of silently migrating (#2361)
## Summary - Converts the `post-create` → `pre-start` silent migration into a fatal load error. Paves the way to later reclaim `post-create` as a background-semantics counterpart to `post-start` without the silent flip risk. - Project config: rejects with `ConfigError`. User config: surfaces as `LoadError::Validation` and best-effort load continues without the file. - `wt config show` renders the error inline instead of hiding it. Marked as **draft for consideration** — this is step 1 of the two-PR sequence discussed in [#1571 (comment)](https://github.com/max-sixty/worktrunk/issues/1571#issuecomment-4291059416). A follow-up PR (one release later) would perform the actual `pre-start` → `pre-create` / `post-start` → `post-create` rename. Supersedes the closed [#2359](https://github.com/max-sixty/worktrunk/pull/2359). ## Test plan - [x] `cargo test --lib --bins` (601 passing) - [x] `cargo test --test integration` (1540 passing; 10 `case_4` failures are local `nu`-not-installed environmental, unrelated to this PR) - [x] `cargo clippy --all-targets --all-features` clean - [x] New integration tests: `test_post_create_in_project_config_is_fatal` and `test_post_create_in_user_config_warns_and_skips` --------- Co-authored-by: worktrunk-bot <254187624+worktrunk-bot@users.noreply.github.com> Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com> |
||
|
Worktrunk Bot
|
ea505bebf9 |
fix(step): gate copy-ignored self-lower on WORKTRUNK_FOREGROUND=-1 (#2358)
## Summary Addresses #2342 — `wt step copy-ignored` has been much slower on macOS since v0.37.0 because `taskpolicy -b` throttles disk I/O, and that hits foreground callers (interactive use and `pre-*` hooks) as well as background ones. Per the [design proposed in the issue](https://github.com/max-sixty/worktrunk/issues/2342#issuecomment-4290580468): - Export `WORKTRUNK_FOREGROUND=-1` when spawning a background hook pipeline (detached `wt hook run-pipeline`). The env var is inherited by every descendant — shell, user command, nested `wt` invocations. - `wt step copy-ignored` self-lowers only when it sees that sentinel; interactive runs and synchronous `pre-*` hooks run at normal priority. - Documented in `wt step copy-ignored --help` under a "Background-hook priority (experimental)" section. Variable name and value flagged as not-yet-stable. Surface area: - `src/priority.rs` — new `FOREGROUND_ENV_VAR` / `BACKGROUND_HOOK_VALUE` consts and `in_background_hook()` helper, with a testable inner fn (`is_background_hook_value`) so we can unit-test the match without mutating process-global env. - `src/commands/process.rs` — `spawn_detached_exec` now sets the env var on the detached runner when the log variant is `HookLog::Hook`. Internal ops (removal, trash-sweep) don't set it. - `src/commands/step_commands.rs` — `copy-ignored` gates the existing `lower_current_process()` call on `in_background_hook()`. - `src/cli/step.rs` + auto-synced docs — experimental note, and removed a now-stale paragraph that claimed unconditional `nice 19`. - Integration test verifies `pre-start` sees the var unset and `post-start` sees `-1`. ## Test plan - [x] `cargo test --lib --bins` — 601 passed - [x] `cargo test --test integration test_background_hook_sees_worktrunk_foreground_env_var` — passes - [x] `cargo test --test integration step_copy_ignored` — 44 passed - [x] `cargo test --test integration user_hooks` — 106 passed - [x] `cargo test --test integration test_command_pages_and_skill_files_are_in_sync` — passes (docs auto-synced) - [x] `cargo test --test integration test_help` — 40 help snapshots pass - [x] `cargo clippy --all-targets` — clean --------- Co-authored-by: worktrunk-bot <254187624+worktrunk-bot@users.noreply.github.com> Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com> |
||
|
Maximilian Roos
|
eec3b24883 |
Show resolved template variables under -v for hooks (#2316)
Closes #2309. When a hook fires under `-v`, `wt` now prints a `template variables:` block listing every variable in scope for that hook type and the value it resolved to for this specific invocation. Vars that are in scope but not populated render as `(unset)` — which is exactly how `target_worktree_path` surfaces during `wt switch -`, the thing the issue reporter hand-rolled an echo-hook to discover. The block prints *before* the `◎ Running …` announce line so it describes what the hook is about to see, not what already ran. Works in both paths: the foreground path (`announce_command`, one block per command) and the background path (`announce_and_spawn_background_hooks`, one block per distinct hook type in the pipeline batch). ## Key files - `src/config/expansion.rs` — `format_hook_variables(hook_type, ctx)` emits the aligned `name = value` block ordered per the docs table (active → operation → repo → exec). `BASE_VARS` is split into `ACTIVE_VARS` / `REPO_VARS` / `EXEC_BASE_VARS` with a `base_vars()` helper, so `vars_available_in` and the printer share one source of truth. - `src/commands/command_executor.rs` — foreground wiring in `announce_command`, gated on `verbosity() >= 1`. - `src/commands/hooks.rs` — background wiring in `announce_and_spawn_background_hooks`; prints one table per distinct hook type (since within a pipeline only `hook_name` varies). - `src/cli/mod.rs` — updates the global `-v` help text and adds a pointer under `## Template variables` in the hook help. ## Testing - Unit: `test_format_hook_variables_groups_and_unset` snapshots a pre-switch context with `target_worktree_path` omitted so `(unset)` is covered; `test_format_hook_variables_scope_filters_operation` confirms pre-commit's narrower operation scope. - Integration: `test_hook_verbose_prints_variable_table` covers the foreground path; the existing `test_post_start_verbose_shows_per_hook_output` now also exercises the background path. No new CLI surface, no new config keys — only behavior added behind the existing `-v` flag. --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
Maximilian Roos
|
e4fab6c791 |
feat(hook): unify hook argument syntax with alias smart routing (#2313)
## Summary
Hooks and aliases now share a single mental model for CLI variable
binding: `--KEY=VALUE` binds `{{ KEY }}` if the template references it,
else forwards to `{{ args }}`. Tokens after `--` forward
unconditionally. `{{ args }}` is now available in hook templates.
Internally, the 11 duplicated hook clap variants collapse into a single
`#[command(external_subcommand)]` mirroring the alias pattern, with
completion and help injection grafting hook-type stubs onto the
augmented `Command` tree.
- `--var KEY=VALUE` still works but emits a deprecation warning pointing
at `--KEY=VALUE`.
- `wt hook --help` lists all hook types (via help-tree injection).
- New `## Reference: hooks vs. aliases` section in
`docs/content/extending.md` documents remaining interface differences.
## Test plan
- [x] `cargo run -- hook pre-merge --yes` (all tests + lints, 3283 tests
passing)
- [x] `wt hook pre-merge --branch=foo --yes` — smart-binds when
referenced
- [x] `wt hook pre-merge --KEY=VALUE --yes` — forwards to `{{ args }}`
when not referenced
- [x] `wt hook pre-merge -- --extra --yes` — post-`--` tokens forward
unconditionally
- [x] `wt hook pre-merge --var branch=foo --yes` — deprecation warning +
force-bind
- [x] `wt hook --help` shows all 10 hook types
- [x] Shell completion: `wt hook <TAB>` lists hook types
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude <noreply@anthropic.com>
|
||
|
Worktrunk Bot
|
d97a6c4b18 | fix(switch): resolve symbolic target (-, @, ^) before pre-switch hooks (#2310) | ||
|
Maximilian Roos
|
39cbbede62 |
refactor(hooks): consolidate per-source loop into spawn_background_hooks (#2298)
Follow-up to #2294. That PR fixed the two-line `Running post-merge:` announce inside `wt merge`, but the same bug lived at every other caller of `prepare_background_hooks`. `wt step landed`'s own post-merge (which routes through `wt hook post-merge`) still printed two lines, and the post-commit paths (step, commit) and the TUI picker's post-remove would too. Root cause is the pattern, not the site. Each caller iterated the source groups returned by `prepare_background_hooks` and called `spawn_hook_pipeline` per group — which announces per call. Missing the collect-then-dispatch wrapper meant one announce per source. Canonicalized: `spawn_background_hooks(ctx, hook_type, extra, display_path)` wraps prepare + announce, and every single-hook-type caller now goes through it. `announce_and_spawn_background_hooks` stays public for the legitimate multi-hook-type batch case (switch, remove). `spawn_hook_pipeline` stays for the name-filter path in `wt hook <type> <name>`. Snapshot test covers `wt hook post-merge` with both user and project configs — the exact path still broken after #2294. > _This was written by Claude Code on behalf of Maximilian_ --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Maximilian Roos
|
4e566560cf |
feat(merge): combine user+project post-merge into one announce line (#2294)
When both user and project hooks fire on post-merge, the pre-change output showed two separate announce lines: ``` ◎ Running post-merge: user:sync ◎ Running post-merge: project:install; project:write, project:publish ``` The merge loop was calling `spawn_hook_pipeline` once per source group. Switching to `announce_and_spawn_background_hooks` — the same pattern `handle_switch` already uses for post-switch + post-start — collapses both into: ``` ◎ Running post-merge: user:sync, project:install; project:write, project:publish @ <path> ``` Also adds `test_combined_user_and_project_post_merge`; no existing post-merge test exercised both sources together, which is why the original behavior survived unnoticed. > _This was written by Claude Code on behalf of Maximilian_ Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Maximilian Roos
|
ea9a52dfdc |
feat(hooks): enable concurrent execution in pre-* pipeline blocks (#2249)
Pipeline blocks (`[[pre-start]]`, `[[pre-merge]]`, etc.) now run their concurrent commands in parallel for foreground (pre-*) hooks, matching the existing behavior in post-* hooks and aliases. The deprecated single-table form (`[pre-start]`) remains serial. Moves the `concurrent` flag from a function parameter on `execute_pipeline_foreground` to a per-step field on `ForegroundStep`, so mixed configs (e.g., user config is deprecated table, project config is pipeline) get correct per-step behavior. `CommandConfig` now tracks whether it was deserialized from a pipeline form (seq visitor) vs a table form (map visitor), so even a single `[[hook]]` block is correctly identified as a pipeline. Also cleans up the "How it works" subsection from the hook docs — the content was either redundant with surrounding text or now incorrect (the pre-* serial restriction). > _This was written by Claude Code on behalf of Maximilian Roos_ Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Maximilian Roos
|
a0157c3343 |
Enforce Ctrl-C cancellation policy across command loops (#2182)
Adds a project-wide policy that signal-derived child exits (SIGINT/SIGTERM) abort whatever loop wt is iterating — hook pipelines, alias steps, concurrent groups, and the for-each worktree loop. Without this, wt's `signal_hook` handler intercepts the user's Ctrl-C and forwards it to the current child, but wt itself survives and the loop charges through remaining steps. `FailureStrategy::Warn` (post-* hooks) silently drops each interrupt, turning a single Ctrl-C against `wt merge` into N extra hook invocations. The policy is documented under "Command Execution Principles" in `CLAUDE.md` and enforced via a single helper `worktrunk::git::interrupt_exit_code`. `handle_command_error` short-circuits to `AlreadyDisplayed` before the `FailureStrategy` branch so both FailFast and Warn abort. `for_each.rs` is refactored to use the same helper for consistency. Builds on #2174, which added the `signal: Option<i32>` field that this PR consumes. Test coverage: - Unit test for `interrupt_exit_code` covering every error variant - New integration test `test_pre_merge_pipeline_aborts_on_signal_exit` verifies the second hook step does not run after the first dies from SIGTERM (mirrors `test_for_each_aborts_on_signal_exit` from #2174) > _This was written by Claude Code on behalf of Maximilian_ Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> |
||
|
Maximilian Roos
|
b6ae21d6e1 |
refactor: prefer raw strings over escaped literals (#2150)
Converts ~29 string literals across 16 files from escaped form
(`"\\d+"`, `"{\"name\": 1}"`) to raw-string form (`r"\d+"`, `r#"{"name":
1}"#`) where possible. Strings containing control escapes (`\n`, `\t`,
`\u{1b}`, etc.) are left alone — raw strings can't represent those.
No behavior change; `cargo check/clippy -D warnings/fmt/test` all clean
locally.
> _This was written by Claude Code on behalf of max-sixty_
Co-authored-by: Claude <noreply@anthropic.com>
|
||
|
Maximilian Roos
|
37fb27bc97 |
Deprecate table form for pre-* hooks (#2135)
## Summary
Multi-entry table form for pre-* hooks currently runs serially, while
the same form for post-* hooks runs concurrently. The parser produces
`HookStep::Concurrent` either way, but the foreground executor flattens
steps and runs them serially. To unify the semantics — table form will
run concurrently for all hook types in a future version — this
deprecates the current form for pre-* hooks and auto-migrates it to
pipeline form, which is explicitly serial.
## Implementation
Follows the existing deprecation recipe in `src/config/deprecation.rs`:
- **Detection** and **migration** for top-level hooks (user/project
config) and per-project overrides (`[projects."id".pre-*]`).
- **Warning**: matches the terse `{old} → {new}` pattern of existing
deprecations (`[merge] no-ff → ff`, `post-create → pre-start`, etc.).
- **Auto-migration** at load time: table form rewrites to pipeline of
inline tables so current behavior (serial) is preserved until users run
`wt config update`.
## Docs
Replaces the transitional "concurrent for post-*, sequential for pre-*"
framing with a neutral three-form description (string / table /
pipeline), plus a note recommending pipeline form for pre-* hooks to
avoid the upcoming behavior change.
## Tests
- `snapshot_migrate_pre_hook_table_form` — TOML migration diff
- `test_config_show_displays_pre_hook_table_form_deprecation` — full
user-facing `wt config show` output, covering the "Project config" label
and multi-hook list form
- Unit tests for detection/migration of top-level and per-project
variants
- Existing integration test fixtures migrated to canonical pipeline form
> _This was written by Claude Code on behalf of max-sixty_
Co-authored-by: Claude <noreply@anthropic.com>
|
||
|
Maximilian Roos
|
b174658e29 |
Split directive file into CD (raw path) and EXEC (shell) files (#2118)
The shell wrapper previously used a single `WORKTRUNK_DIRECTIVE_FILE` where wt wrote shell commands (`cd '/path'`, arbitrary `--execute` payloads). This meant the cd path went through shell parsing — any content wt wrote was sourced as shell. This splits the protocol into two files with different trust levels: - **`WORKTRUNK_DIRECTIVE_CD_FILE`** — raw path, read with `cd -- "$(< file)"`. No shell parsing, no escaping, no injection surface. Safe to pass through to alias/hook child processes. - **`WORKTRUNK_DIRECTIVE_EXEC_FILE`** — arbitrary shell (from `--execute`), sourced by the wrapper. Scrubbed from alias/hook child environments so hook bodies cannot inject shell into the parent session. When a nested `wt` inside an alias body tries `--execute` without the EXEC file, the command is dropped with a warning linking to #2101 for user feedback. The old `WORKTRUNK_DIRECTIVE_FILE` is silently honored for one release (users who upgrade wt without restarting their shell). Bash, zsh, fish, and PowerShell self-update on restart; nushell requires `wt config shell install`. Closes #2101 > _This was written by Claude Code on behalf of @max-sixty_ --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Maximilian Roos
|
b2a42bbfc8 |
feat(hook): --KEY=VALUE shorthand, custom vars, unreferenced-var warning (#2117)
`--KEY=VALUE` shorthand for `wt hook --var KEY=VALUE`, custom template
variables in hooks (matching alias behavior), and a warning when a
`--var` isn't referenced by any hook template.
**Shorthand**: `wt hook pre-start --branch=feature/test` rewrites to
`--var branch=feature/test` before clap sees it. Known flags (`--yes`,
`--dry-run`, etc.) are preserved. `--` stops rewriting.
**Custom vars**: Hooks previously restricted `--var` to known
TEMPLATE_VARS while aliases accepted arbitrary names. Now both use
`parse_key_val` with hyphen→underscore canonicalization, so hooks can
inject custom variables like `{{ my_env }}` into templates.
**Unreferenced-var warning**: When a `--var` key isn't referenced by any
template in the hooks being run, a warning is emitted (catches typos
like `--brnach=feature`). Respects name filters — if only "test" runs
and "build" uses the var, we still warn.
**Sync tests**: `HOOK_SUBCOMMANDS_WITH_VARS` and `KNOWN_HOOK_LONG_FLAGS`
are both validated against clap's command tree to catch drift.
> _This was written by Claude Code on behalf of @max-sixty_
---------
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
||
|
Maximilian Roos
|
73b4f8305d |
Unify hook and alias shell execution (#2089)
Hooks, aliases, and `for-each` all run shell commands in a worktree but used two separate execution functions with different capabilities. This collapses them into one (`execute_shell_command`), then builds on the shared foundation. **Phase 1** — `run_command_streaming` and `CommandError` deleted. All three consumers call `execute_shell_command` (renamed from `execute_command_in_worktree`) with a new `directive_file` parameter. `Cmd` gains a `.directive_file()` builder method that re-adds the env var after the security scrub. Aliases and `for-each` gain signal forwarding and ANSI reset. **Phase 2** — Aliases iterate `cmd_config.steps()` instead of flattening via `commands()`. `HookStep::Concurrent` steps spawn threads via `thread::scope`. Lazy `vars.*` expansion supported in pipelines. **Phase 3** — Foreground hooks (pre-\*, post-\* with `--foreground`) pass the directive file through to child processes. `wt switch --create` inside a pre-start hook body now lands the shell in the new worktree. Background hooks continue to scrub. Follow-up: alias announcements could show pipeline summary (e.g., `Running alias deploy: install; build, lint`). > _This was written by Claude Code on behalf of @max-sixty_ --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: worktrunk-bot <254187624+worktrunk-bot@users.noreply.github.com> |
||
|
Worktrunk Bot
|
abe8854ae7 |
fix: exit 0 when wt hook has no hooks configured (#2056)
## Summary - `wt hook <type>` now prints a warning and exits 0 when neither the user nor project config defines hooks of that type, instead of erroring out. - Scripts and CI can invoke `wt hook` unconditionally without special-casing empty configuration. ## Context Previously `require_hooks` in `src/commands/hook_commands.rs` turned an unconfigured hook type into a `GitError::Other` (`"No <type> hook configured; checked both user and project"`), which propagated as a non-zero exit. That behavior was introduced in #916 to unify all hook types on a single error path — but as #2055 notes, "run whatever's there" is the more useful default for the manual `wt hook` command. The fix replaces the error with a `warning_message` and an early `Ok(())`: ``` ▲ No pre-merge hooks configured ``` Stylistically this matches how other `hook_type` references are rendered in this file (no `<bold>`). Name-filter mismatches (`wt hook pre-merge --name doesnt-exist`) still error — that path is unchanged because the user explicitly asked for a named hook. ## Test plan - [x] Updated `test_standalone_hook_no_hooks_configured` to assert `success()` + the warning substring; confirmed it failed before the fix and passes after. - [x] All 148 `hook` integration tests pass (`cargo test --test integration hook`). - [x] `cargo clippy --all-targets -- -D warnings` clean. - [x] `cargo fmt --check` clean. Closes #2055 Co-authored-by: worktrunk-bot <254187624+worktrunk-bot@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Maximilian Roos
|
f8f291b372 |
refactor(logs): nest hook output by branch/source/hook-type/name (#2041)
Flattens categorization of `.git/wt/logs/` by making the filesystem
encode what the old scheme crammed into filenames.
**Layout:** top-level *files* are shared logs (`commands.jsonl*`,
`verbose.log`, `diagnostic.md`); top-level *directories* are per-branch
log trees — `{branch}/{source}/{hook-type}/{name}.log` for hook output,
`{branch}/internal/remove.log` for background removal,
`wt/internal/trash-sweep.log` for the trash sweeper. Categorization
becomes a trivial file-vs-directory check, eliminating the exclusion
rule `ends_with(".log") && !is_diagnostic_file(name)`.
**Wins:** per-branch listing/clearing is now O(that branch) instead of
O(all logs); orphan cleanup for a removed branch is a single
`remove_dir_all`; filenames drop the joined-tuple collision hashes they
only needed to disambiguate flat keys.
**Transition:** `clear_logs` keeps a self-healing sweep of legacy
top-level `.log` files so users transition without an explicit
migration. A pinning test
(`test_state_clear_logs_sweeps_legacy_flat_files`) guards that behavior.
**Observable change:** `logs get --format=json` now puts relative paths
in the `file` field (e.g. `main/user/post-start/server.log`). Log
locations are listed as "flexible" in `CLAUDE.md`, so this is in scope.
**Reviewer orientation:**
- `src/commands/process.rs` — `HookLog::path()` rewritten; `suffix()` /
`filename()` deleted.
- `src/commands/config/state.rs` — new `walk_hook_output_files` /
`walk_branch_dir` / `HookOutputEntry`; `clear_logs` handles legacy
sweep; `partition_log_files_json` + `render_*` split along the top-level
vs hook-output seam; module docstring pins the invariant.
- `src/testing/mod.rs` — `wait_for_file_count` walks recursively.
- Test fixtures in `tests/integration_tests/config_state.rs` use new
`hook_log_rel_path` / `internal_log_rel_path` / `write_log_at` helpers.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-authored-by: worktrunk-bot <254187624+worktrunk-bot@users.noreply.github.com>
|
||
|
Maximilian Roos
|
2514a79b3a |
test: add snapshot test for multi-remove hook branch context (#2016)
Follow-up to #2014. Adds snapshot coverage for the `wt remove branch1 branch2` path, where hook announcements include the branch name for disambiguation. > _This was written by Claude Code on behalf of @max-sixty_ Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Worktrunk Bot
|
446e206ed6 |
feat: accept multiple NAME filters in hook subcommands (#2013)
## Problem All hook subcommands (`pre-switch`, `post-switch`, `pre-merge`, etc.) accept at most one `NAME` filter, so running a subset of hooks requires chaining separate invocations: ``` wt hook pre-merge --yes insta && wt hook pre-merge --yes doctest && wt hook pre-merge --yes doc ``` ## Solution Changed `name: Option<String>` to `name: Vec<String>` across all 10 hook subcommands, threading the multi-filter through the entire hook execution pipeline: - `src/cli/hook.rs` — positional arg becomes `Vec<String>` - `src/commands/hooks.rs` — `HookCommandSpec`, `filter_by_name`, `check_name_filter_matched` accept slices - `src/commands/hook_commands.rs` — `run_hook`, `run_filtered_hook`, `run_post_hook` signatures updated - `src/main.rs` — dispatch helpers updated A command matches if **any** filter matches it. Empty list (no names given) still runs all hooks. Source prefixes (`user:`, `project:`) work per-filter. ``` wt hook pre-merge --yes insta doctest doc ``` ## Testing - All 1390 integration tests pass - All 495 unit tests pass - Clippy clean - Backward compatible — single name still works identically --- Closes #2012 — automated triage --------- Co-authored-by: worktrunk-bot <254187624+worktrunk-bot@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Maximilian Roos
|
1566f51818 |
test: combined post-remove + post-switch hook announcement (#1988)
Adds an integration test exercising the combined announcement path in `spawn_hooks_after_remove` when both post-remove and post-switch hooks fire together (removing the current worktree triggers cd-back to main). Covers lines 860/882 in `handlers.rs` flagged by codecov/patch. > _This was written by Claude Code on behalf of @max-sixty_ Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Maximilian Roos
|
e5665a87af |
feat: per-command log files for background hooks (#1934)
Two improvements to background hook execution:
**Per-command log files** — Each background hook command now writes to
its own log file instead of all commands sharing a single pipeline log.
This matches the convention already documented in `wt config state logs
--help`: `{branch}-{source}-{hook_type}-{name}.log`. Previously a map
config like `[post-start] / task1 = "..." / task2 = "..."` interleaved
both outputs in one file; now each gets its own.
**Combined hook-type display** — When multiple hook types fire together
(e.g., post-switch + post-start on create), they display on one line:
`Running post-switch: zellij-tab; post-start: deps, assets, docs`.
Same-type groups from different sources are merged: `Running post-start:
user_bg, project`.
Key changes:
- `PipelineSpec` gains `log_dir` field; `hook_type`/`source` changed
from `String` to typed enums with serde derives
- Pipeline runner creates per-command log files and redirects each
child's stdout/stderr there
- `announce_and_spawn_background_hooks` collects groups across hook
types for combined display
- Runner process keeps a minimal "runner" log for orchestrator-level
errors
> _This was written by Claude Code on behalf of @max-sixty_
---------
Co-authored-by: Claude <noreply@anthropic.com>
|
||
|
Maximilian Roos
|
679fe539fe |
Deprecate --no-verify in favor of --no-hooks (#1932)
`--no-hooks` describes what the flag does — skip hooks. `--no-verify` was inherited from git's naming but doesn't match worktrunk's semantics (there's no "verification" step being skipped). `--no-verify` remains as a hidden alias that emits a deprecation warning, retained for at least one release cycle per the project's deprecation policy. Changes across switch, remove, merge, step commit, and step squash: - `--no-hooks` is the canonical visible flag - `--no-verify` hidden, emits `▲ --no-verify is deprecated; use --no-hooks instead` - Error hints (`↳ To skip pre-merge hooks, re-run with --no-hooks`), info messages, help text, docs, and config examples all updated - `resolve_verify()` helper in main.rs deduplicates the deprecation logic - Backward-compatibility test verifies `--no-verify` still works > _This was written by Claude Code on behalf of @max-sixty_ --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Maximilian Roos
|
268882ee6d |
refactor: always use pipeline runner for background hooks (#1912)
The hooks system had two background execution paths: flat (independent detached processes per command via `spawn_background_hooks`) and pipeline (one `wt hook run-pipeline` orchestrator via `spawn_hook_pipeline`). The split was determined by `CommandConfig::is_pipeline()` — true when `steps.len() > 1`. This routes everything through the pipeline runner, eliminating ~260 lines of branching: the `PreparedHooks` enum, match arms in 4 caller files, lazy template expansion duplicated in `spawn_background_hooks`, and combined-hook-type display batching logic. `prepare_background_hooks` now returns per-source groups of `Vec<SourcedStep>`; callers spawn each group as an independent pipeline to preserve source isolation (user hook failure doesn't abort project hooks). Behavioral changes: - Map configs produce one `pipeline.log` instead of per-command log files - Combined hook-type messages split into separate lines (`post-switch: X; post-start: Y` → two messages) - Display drops source prefix for named steps (`project:task1` → `task1`); unnamed steps show source (`project`) Includes a TODO noting display presentation issues to revisit (arrow notation, repeated source labels for unnamed multi-step pipelines). > _This was written by Claude Code on behalf of [user]_ --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Maximilian Roos
|
5cd7da344e |
fix: route all background hooks through pipeline-aware API (#1910)
Three bugs in the background hooks system, found by Codex review:
**Post-merge and post-remove bypassed the pipeline path.** `merge.rs`
called `prepare_hook_commands` + `spawn_background_hooks` directly, and
`prepare_post_remove_commands` returned flat `Vec<SourcedCommand>`.
List-form configs lost serial/concurrent semantics — commands raced
instead of running in order.
**Pipeline context leaked per-step `hook_name`.** `spawn_hook_pipeline`
deserialized the first command's `context_json` as the shared pipeline
context, which included `hook_name`. Later steps saw step 1's name
instead of their own. Additionally, `handle_switch.rs` merged PostSwitch
and PostStart pipeline steps into one pipeline, giving PostStart steps
the wrong `hook_type`.
**Lazy template expansion broken in flat spawn path.** When
name-filtering pipeline commands via `wt hook post-start db`,
`run_post_hook` fell through to `spawn_background_hooks`, which passed
the raw `{{ vars.name }}` template to the shell instead of expanding it.
Fixes:
- All background hook callers now go through
`prepare_background_hooks`/`spawn_prepared_hooks`, which auto-detects
pipeline vs flat configs
- Pipeline context strips `hook_name`; the background runner injects it
per-step via `build_step_context_json`
- `spawn_background_hooks` expands lazy templates before detaching
(shared `expand_lazy_template` helper)
- `handle_switch.rs` spawns each hook type's pipeline independently
- `prepare_post_remove_commands` replaced by `PostRemoveContext` struct
+ unified API
- `run_post_hook` simplified to use unified API (removes manual pipeline
detection)
- `prepare_pipeline_hooks_with_configs` made private (only used by
`prepare_background_hooks`)
4 regression tests: pipeline `hook_name` isolation, post-merge pipeline
ordering, post-remove pipeline ordering, name-filtered lazy template
expansion.
> _This was written by Claude Code on behalf of @max-sixty_
---------
Co-authored-by: Claude <noreply@anthropic.com>
|
||
|
Maximilian Roos
|
c1815cf258 |
fix: spawn background pipelines per hook-type (#1904)
When `wt switch --create` fires both post-switch and post-start hooks,
pipeline steps were accumulated into a single `wt hook run-pipeline`
background process. `spawn_hook_pipeline` takes
`hook_type`/`source`/`context` from the first step, so post-start steps
got post-switch's template variables — `{{ hook_type }}` expanded to
`post-switch` instead of `post-start`.
Spawn each hook type's pipeline independently. Flat hooks are still
accumulated for a combined display message since they're independent
processes. Added integration test for post-switch pipelines via `switch
--create`.
> _This was written by Claude Code on behalf of @max-sixty_
---------
Co-authored-by: Claude <noreply@anthropic.com>
|
||
|
Maximilian Roos
|
5913e3d56b |
feat: Rust-orchestrated pipeline execution with stdin spec passing (#1893)
Background pipelines no longer compile to compound shell strings with `__WT_TPL_*` env vars and `eval "$(wt step eval --shell-escape ...)"` wrapping. Instead, the parent `wt` process serializes a `PipelineSpec` to JSON and spawns `wt hook run-pipeline` as a detached background process, piping the spec to stdin. The background runner expands templates just-in-time and spawns shell children per step. Key changes: - **New `spawn_detached_exec`** in `process.rs` — spawns a binary directly (no intermediate shell), pipes data to stdin, with proper detachment on both Unix (`process_group(0)`) and Windows (`CREATE_NEW_PROCESS_GROUP | DETACHED_PROCESS`). Shared log setup extracted into `create_detach_log` helper. - **New `pipeline_spec.rs`** — serde types for the JSON spec (`PipelineSpec`, `PipelineStepSpec`, `PipelineCommandSpec`) with roundtrip test. - **New `run_pipeline.rs`** — the background orchestrator. Reads spec from stdin, walks steps in order (serial abort-on-failure, concurrent spawn-then-wait-for-all), expands templates with `shell_escape=true`, pipes context JSON to each shell child's stdin. Module docstring specifies the full execution model. - **`wt hook run-pipeline`** — hidden subcommand (not in `--help` or autocomplete) replacing the old top-level `_run-pipeline`. - **Deleted**: `build_pipeline_command`, `format_cmd`, `--shell-escape` flag, `__WT_TPL_N` env vars, `extra_env` parameter on `spawn_detached`, 7 unit tests for the old shell builder, 2 integration tests + snapshots for `--shell-escape`. - **Updated docs** — "How it works" sections in CLI help, `docs/content/hook.md`, and skill reference no longer show the compound shell command example. Test comments updated to describe the new execution model. - **New tests** — concurrent group execution (both commands run), concurrent partial failure (sibling completes, later steps abort), and shell escaping of metacharacters (spaces, quotes, `$`). > _This was written by Claude Code on behalf of maximilian_ --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Maximilian Roos
|
bd87f2a146 |
fix: background removal blocks for 1s due to shell parsing bug (#1858)
`spawn_detached_unix` constructed the shell command as `sh -c "sleep 1
&& rmdir ...; rm -rf ... &"`. In POSIX shell, `;` has lower precedence
than `&`, so this parses as two statements: `sleep 1 && rmdir ...` runs
**synchronously** (1 second block), then only `rm -rf ... &` is
backgrounded. Every `wt remove` paid a 1-second penalty.
The fix wraps compound commands in braces — `{ sleep 1 && rmdir ...; rm
-rf ...; } &` — so `&` backgrounds the entire group. This affects all
callers of `spawn_detached` (remove, prune, merge, hooks).
Tests that asserted `!path.exists()` after removal now use
`assert_worktree_removed()` which accepts an empty placeholder directory
(the placeholder is cleaned up by the now-correctly-backgrounded `sleep
1 && rmdir`). Also fixes a pre-existing race in
`test_standalone_hook_post_merge` / `post_create` where background hooks
were checked immediately instead of polled.
Also adds `benches/remove.rs` for measuring end-to-end remove
performance.
> _This was written by Claude Code on behalf of maximilian_
---------
Co-authored-by: Claude <noreply@anthropic.com>
|
||
|
Maximilian Roos
|
346e27431f |
fix: background lazy pipeline vars expansion (#1855)
Two bugs from #1840 broke the background path for lazy template expansion (via `wt switch --create`): 1. **Raw string literal bug in `format_cmd()`** — `r#"..."#` consumed the closing `"` of the shell command, producing `eval "$(wt step eval --shell-escape "$__WT_TPL_0)` (missing closing quotes). The `"#` terminator matched the `"` that was supposed to be part of the output. Fixed by doubling the `"` before `"#`. 2. **`validate_switch_templates()` eagerly failed on `{{ vars.name }}`** — pre-flight validation renders templates with `vars` as an empty map, so accessing `vars.name` errors under SemiStrict mode. But these templates are lazily expanded at runtime after prior pipeline steps set the vars. Fixed by skipping full validation for templates that reference `vars.` (syntax is still checked by `expand_commands`). Also adds the missing integration test for the background lazy vars path and strengthens unit test assertions to catch the quoting regression. > _This was written by Claude Code on behalf of @max-sixty_ Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Maximilian Roos
|
79fa6a8b73 |
feat: lazy template expansion for pipeline vars (#1840)
Pipeline steps referencing `{{ vars.* }}` are expanded at execution time
rather than upfront, so vars set by step N are available to step N+1.
This enables DRY patterns like deriving a container name once and
reusing it across pipeline steps and across hooks
(post-start/post-remove).
A pipeline that sets vars in step 1 and uses them in step 2:
```toml
post-start = [
"wt config state vars set container='{{ repo }}-{{ branch | sanitize }}-postgres'",
{ db = "docker run --name {{ vars.container }} ..." },
]
[post-remove]
db-stop = "docker stop {{ vars.container }} 2>/dev/null || true"
```
**Background pipelines** wrap lazy steps in `eval "$(wt step eval
--shell-escape "$__WT_TPL_N")"` with templates passed as env vars on the
spawned process. **Foreground mode** (`--foreground`) re-expands
templates in-process for structured error reporting.
Detection uses `minijinja::undeclared_variables` (via new shared
`template_references_var()` helper) — no string heuristics. Syntax
errors are caught at prepare time; only var resolution is deferred.
`--shell-escape` on `wt step eval` is hidden from `--help` (internal
mechanism).
Updates database examples in hook docs and tips-patterns to use the
pipeline pattern.
> _This was written by Claude Code on behalf of @max-sixty_
---------
Co-authored-by: Claude <noreply@anthropic.com>
|
||
|
Maximilian Roos
|
8ae8fbde14 |
feat: structure-driven hook execution ordering (#1713)
Post-* hooks all run concurrently in the background. When one hook
depends on another (e.g., `npm run build` needs `npm install` to finish
first), there's no way to express ordering today.
Rather than per-command flags, the TOML data structure itself determines
execution order:
- **String** — single command (unchanged)
- **Map** (table) — concurrent commands (unchanged)
- **List** (array) — serial pipeline, steps execute in order
```toml
[hooks]
post-start = [
{ install = "npm install" },
{ build = "npm run build", lint = "npm run lint" }
]
```
`install` runs first. After it completes, `build` and `lint` run
concurrently. The entire pipeline runs in the background as one detached
compound shell command — the user sees a summary line and nothing else.
**Key files:** `src/config/commands.rs` (HookStep enum, 3-form
deserialization), `src/commands/hooks.rs` (compound shell command
building, pipeline spawning), `src/commands/command_executor.rs`
(PreparedStep).
**Design decisions:**
- Backward compatible — existing string/map configs route through the
unchanged flat path. Pipeline path only activates for multi-step list
configs.
- User and project pipelines run independently (user first, project
second). No cross-source merging of pipeline structure.
- Pre-* hooks unchanged — always serial, fail-fast regardless of
structure.
- `commands()` returns `impl Iterator` for zero-allocation flat access.
`steps()` returns `&[HookStep]` for pipeline-aware execution.
- Compound shell commands wrap each step in `{ ...; }` to prevent
operator precedence issues between steps.
**Testing:** Unit tests cover all deserialization forms, serialization
round-trips, and flattening. Integration tests cover project pipelines,
template variable expansion in pipelines, mixed user-pipeline +
project-flat configs, pipeline serial ordering (marker file
verification), and pipeline failure propagation.
> _This was written by Claude Code on behalf of @max-sixty_
---------
Co-authored-by: Claude <noreply@anthropic.com>
|