Skip to content

fix: align all pre-* hooks to FailFast — no exceptions - #1708

Merged
max-sixty merged 3 commits into
mainfrom
worktree-agent-a39c6140
Mar 24, 2026
Merged

max-sixty merged 3 commits into
mainfrom
worktree-agent-a39c6140

Conversation

@worktrunk-bot

@worktrunk-bot worktrunk-bot commented Mar 24, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

  • Audited all hook failure strategies — the only inconsistency was pre-start using Warn instead of FailFast
  • Aligned pre-start with the symmetric pre- (blocking, fail-fast) / post- (background, warn) pattern from feat!: rationalize hooks — rename post-create, add post-commit, background post-merge #1679
  • Removed the documented exception ("except pre-start, which warns and continues") from CLI help and docs
  • Updated execute_pre_start_commands and wt hook pre-start to both use FailFast
  • Updated tests and snapshots for the new behavior

Test plan

  • All 67 user hook tests pass
  • Help snapshots updated
  • Doc sync passes
  • CI passes

Closes #1703

🤖 Generated with Claude Code

…avior

`wt hook pre-start` was grouped with other `pre-*` hooks using FailFast
strategy, but `execute_pre_start_commands` (automatic execution during
worktree creation) uses Warn. The docs explicitly say pre-start warns and
continues on failure. This aligns the manual invocation.

Closes #1703

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@max-sixty

Copy link
Copy Markdown
Owner

can we do a full audit: there should be no "exceptions" etc given our new consolidated approach to pre vs post hooks

is our code consistent with that?

Full audit of hook failure strategies confirms the only inconsistency
was pre-start using Warn while all other pre-* hooks use FailFast.
Align pre-start with the symmetric pre (blocking, fail-fast) / post
(background, warn) pattern established in the hook rationalization.

Changes:
- hook_commands.rs: merge PreStart back into the FailFast match arm
- worktree/hooks.rs: execute_pre_start_commands uses FailFast
- cli/mod.rs + docs: remove "(except pre-start)" exception text
- tests: update snapshot and assertions for FailFast behavior

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@worktrunk-bot

Copy link
Copy Markdown
Collaborator Author

Done — full audit completed and pre-start aligned with all other pre-hooks.

Audit results: The only inconsistency was PreStart using Warn (in both hook_commands.rs and worktree/hooks.rs) while all other pre-hooks use FailFast. Every other hook correctly follows the symmetric pattern from #1679:

Strategy Hooks
FailFast PreSwitch, PreStart, PreCommit, PreMerge, PreRemove
Warn (background) PostStart, PostSwitch, PostCommit, PostMerge, PostRemove

Changes in a9503ed:

  • Merged PreStart back into the FailFast match arm in hook_commands.rs
  • Changed execute_pre_start_commands in worktree/hooks.rs to FailFast
  • Removed the "(except pre-start)" exception from CLI help and docs
  • Updated test + snapshot to assert non-zero exit on hook failure

Note: since pre-start runs after worktree creation (it was renamed from post-create), a failing pre-start hook leaves the worktree in place but the command exits non-zero. This matches the behavior of other pre-hooks that fail after partial work.

@worktrunk-bot worktrunk-bot changed the title fix: use Warn strategy for wt hook pre-start to match automatic behavior fix: align all pre-* hooks to FailFast — no exceptions Mar 24, 2026

@worktrunk-bot worktrunk-bot left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CI failure: test_post_create_failing_command in tests/integration_tests/post_start_commands.rs has a stale snapshot. This test also exercises a failing pre-start hook (via the deprecated post-create config key) but its snapshot wasn't updated alongside the one in user_hooks.rs.

The snapshot at tests/snapshots/integration__integration_tests__post_start_commands__post_create_failing_command.snap still expects success: true / exit_code: 0 with the old warn-and-continue output. It needs updating to match the new FailFast behavior (success: false, exit_code: 1, new error message).

The test comment on line 111 ("Should show warning but continue") also needs updating to reflect the new behavior.

Fix: run cargo insta test --accept -- --test integration test_post_create_failing_command and update the test comment.

Comment thread tests/integration_tests/user_hooks.rs
Co-Authored-By: Claude <noreply@anthropic.com>
@max-sixty
max-sixty merged commit f8b80af into main Mar 24, 2026
22 of 23 checks passed
@max-sixty
max-sixty deleted the worktree-agent-a39c6140 branch March 24, 2026 14:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: wt hook pre-start uses FailFast instead of Warn

2 participants