Commit Graph

5 Commits

Author SHA1 Message Date
Maximilian Roos 11129d3811 fix(plugin): fail worktree hooks before side effects; document path args (#3060)
Hardens the plugin's worktree-lifecycle hooks against malformed
payloads, and documents two things they rely on.

**Hooks** (`plugins/worktrunk/hooks/hooks.json`): in the pipeline form,
`jq -r .name | xargs … wt switch --create {} …` runs `wt` with whatever
jq printed — so a payload missing `.name` minted a real branch named
`null`, and `set -o pipefail` could only report the failure after the
side effect (verified under `/bin/sh`). Both hooks now validate the
field in a command substitution before `wt` runs (`name=$(jq -er .name)
|| exit 1; …`), which fails with nothing created and stays
whitespace-safe via quoted variables. The `bash -c` wrapper remains —
hook commands must parse under fish/zsh/bash and fish rejects
`name=$(…)` — but `set -o pipefail` is gone: the only remaining pipe
ends in `jq -er .path`, whose exit is the pipeline's. Exercised under
`sh -c` against the shipped JSON: missing field → exit 1, no branch;
fresh create → path on stdout, exit 0; existing branch → wt's real
error, nonzero; remove by path → removed.

**Docs**: `wt remove`'s positional also accepts worktree paths
(`resolve_worktree_arg` tries branches first, then paths) and the
`WorktreeRemove` hook passes a path — the help line now reads "Branch
name or worktree path". The plugin README lists `jq` as a hook
dependency, and the skill's branch-naming step asks for names consistent
with the repo's existing worktrees.

> _This was written by Claude Code on behalf of max_

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
2026-06-12 00:33:58 -07:00
Maximilian Roos fd3ae5bff4 feat(plugin): optional branch name and single-route flow for /wt-switch-create (#3058)
The `/wt-switch-create` branch argument becomes optional (a name is
picked from the task when omitted), and the skill's workflow is rebuilt
from a guard-heavy three-route flow into one route plus one error-driven
fallback.

**The new flow**: pick a branch name if none was given, create the
worktree with `wt -C <repo> switch --create <branch> --no-cd
--format=json` in Bash (rerunning without `--create` when the user named
an existing branch), re-root the session with `EnterWorktree({path})`,
and on a graceful rejection (different repo, pinned cwd, nested session)
work in the worktree via absolute paths instead.

**Why the rewrite**: the old flow's guards encoded hypotheses about
Claude Code that turned out wrong or untested. The load-bearing
discoveries, each verified by binary inspection, official docs, or live
tests and documented in the new `skills/wt-switch-create/rationale.md`:
`EnterWorktree({path})` accepts worktrunk's sibling layout on first
entry; every rejection it can raise is side-effect-free, so
try-then-fallback replaces prediction; the previous
`EnterWorktree({name})`/hook route silently auto-removes a clean
worktree at session exit, which is wrong for durable worktrunk worktrees
(path-entered worktrees persist); and the "pinned cwd" the old guards
defended against was a misdiagnosis of the harness resetting `cd` that
leaves the session's working directories.

**Hook fix**: both `WorktreeCreate` and `WorktreeRemove` pipelines are
wrapped in `bash -c 'set -o pipefail; ...'`. Without it the trailing
`jq` exits 0 on empty input, so a `wt` failure surfaced as a
"successful" hook with an empty path. The wrapper is explicit `bash`
because hooks spawn via `/bin/sh -c`, where dash rejects `set -o
pipefail` fatally. `xargs -I{}` also fixes worktree paths containing
spaces. Tested end-to-end in both directions (success exits 0 with the
path; failure exits 1 with empty stdout).

Docs, README, and plugin CLAUDE.md are aligned with the new mechanism.
The agent-isolation use of the `WorktreeCreate` hook is unchanged.

Review trail: the design survived an evidence audit (every rationale
claim independently re-verified), a code review (bare directory-named
tokens no longer parse as the repo; mid-session moves now stash
uncommitted work across), and a level check that considered and rejected
fixing this in `wt` itself (an idempotent `--create` cannot encode who
chose the branch name, which determines the correct recovery).

> _This was written by Claude Code on behalf of max_

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
2026-06-11 23:01:13 -07:00
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>
2026-05-21 16:03:03 +00:00
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)
2026-05-20 19:31:50 -07:00
Maximilian Roos d22917702e refactor(plugins): consolidate Claude + Codex into one payload dir (#2789)
The Claude plugin lived at the repo root (`.claude-plugin/`) while the
Codex plugin lived in `plugins/worktrunk/` — two homes for one logical
plugin, with the description string duplicated across both and drifting
independently. This collapses them into a single payload directory.

**What moved.** `git mv
.claude-plugin/{plugin.json,hooks/,CLAUDE.md,README.md} →
plugins/worktrunk/` (history-preserving renames). The repo root keeps
only the two marketplace pointers — Claude and Codex each hardcode their
marketplace path with no fallback, so two pointer files is the
irreducible floor. Both pointers' `source` now resolves to
`./plugins/worktrunk`. `.claude-plugin/` is now a single 2-line file.

**The load-bearing constraint** (verified live against claude-cli
2.1.x): for a *subdirectory* `source`, Claude expects `plugin.json` at
the plugin root **without** a `.claude-plugin/` wrapper — that wrapper
is marketplace-root-only. (First attempt with the wrapper failed `Plugin
not found`; the corrected layout installs cleanly.) Codex keeps its own
required `.codex-plugin/` wrapper. So inside `plugins/worktrunk/`:
`plugin.json` is Claude's, `.codex-plugin/plugin.json` is Codex's,
`hooks/` is Claude's, `skills → ../../skills` is shared.

**Path edits inside moved files:** `plugin.json` `hooks` →
`./hooks/hooks.json`; `hooks.json`
`${CLAUDE_PLUGIN_ROOT}/.claude-plugin/hooks/wt.sh` →
`${CLAUDE_PLUGIN_ROOT}/hooks/wt.sh`.

**Reviewer navigation:** `git log` shows the moves as renames.
`plugins/worktrunk/CLAUDE.md` was rewritten to document the unified
layout + per-tool path resolution; the repo `CLAUDE.md` "Plugin Layout"
section likewise. `tests/integration_tests/config_show.rs` adds
`test_plugin_layout_is_consolidated`, which locks the layout invariants
(`.claude-plugin/` is marketplace-only, Claude manifest at plugin root,
hooks relative to it) **and** asserts the duplicated Claude description
stays byte-identical across the marketplace pointer and the manifest —
that was the standing follow-up; JSON can't `include!`, so a drift test
is the right tool, not a generator.

**Verification:** Live end-to-end on both real CLIs, twice — once with
copied skill dirs, once mirroring the real tree *including the
`plugins/worktrunk/skills` symlink* (Claude's `skills` array resolves
through it). Full pre-merge gate green: 3720 passed, 0 skipped, all
pre-commit lints, no snapshot churn. No user-facing docs/help text
changed (the consolidation is repo-internal), so `test_docs_are_in_sync`
needed no resync.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-17 16:35:25 -07:00