Conversation
Extends structured-output coverage beyond list/switch/remove/merge to the
remaining commands where JSON is useful for scripting and Claude Code
integration: `step rebase`, `step push`, `step commit`, `step squash`,
`step relocate`, `step copy-ignored`, and `hook show`.
JSON shapes follow the existing pattern (additive on stdout; human prose
stays on stderr) and use stable snake_case `outcome` discriminators where
the result is one of several variants (rebased/fast_forwarded/up_to_date,
squashed/no_commits_ahead/already_single_commit/no_net_changes, etc.).
Notable refactors:
- `RebaseResult::Rebased` now carries `{target, fast_forward}`
- `SquashResult::Squashed` now carries `{sha, message, stage_mode}`
- `CommitOutcome` returned from `commit_staged_changes` and
`CommitOptions::commit` carries `{sha, message, stage_mode}` (resolved
mode, not the raw CLI flag)
- `handle_push` / `handle_no_ff_merge` return `PushResult` with target,
commit_count, and outcome (FastForwarded/UpToDate/MergeCommit{merge_sha})
- `relocate::GatherResult` carries `template_error_branches` so JSON
surfaces template-expansion failures as `skipped` entries with
`reason: "template_error"` (text mode already reported these; JSON now
matches)
`--show-prompt` is rejected when combined with `--format=json` for
`step commit` / `step squash` — show-prompt emits raw LLM-prompt text
that would corrupt a JSON consumer's stdout.
Co-Authored-By: Claude <noreply@anthropic.com>
worktrunk-bot
left a comment
There was a problem hiding this comment.
A few observations, nothing blocking.
Display abbreviation regression in commit_hash. The two display sites in src/commands/commit.rs and src/commands/step_commands.rs switched from git rev-parse --short HEAD to a fixed 7-char slice of the full SHA. --short honors core.abbrev and auto-extends for ambiguous prefixes; the slice does neither. Default-config users see no change, but anyone with a custom core.abbrev or a repo with 7-char collisions will see a quieter, possibly-ambiguous hash in Committed changes @ … / Squashed @ …. Cheapest preserve-behavior fix is keeping a separate git rev-parse --short HEAD call alongside the full one. Snapshots redact the hash, so the change isn't caught by tests.
--format flag descriptions are thinner than the existing pattern. wt switch/wt remove/wt merge each include a one-liner describing what the JSON contains (e.g. "JSON prints structured result to stdout after removal completes"). The new commands all use Output format (text, json) only. Worth a brief sentence per command so the shape isn't only discoverable via the PR body.
Missing test for step push --no-ff --format=json. fast_forwarded and up_to_date are covered, but the merge_commit outcome (with merge_sha) — the new field that was previously discarded — isn't asserted anywhere. That's the one variant most worth pinning down.
Heads-up on overlap with #2557. Both PRs touch src/cli/step.rs and src/commands/commit.rs, and #2557 hides --show-prompt while this PR adds a runtime guard against --show-prompt --format=json. The two are compatible, but landing them in either order will need a small rebase — worth noting which goes first.
worktrunk-bot
left a comment
There was a problem hiding this comment.
CI fails to compile across all platforms (lint, feature-check, code-coverage, test linux/macos/windows, affected tests). Single error, one site:
error[E0308]: mismatched types
--> src/commands/hook_commands.rs:413:13
413 | project_config.as_ref(),
| ^^^^^^^^^^^^^^^^^^^^^^^ expected `Option<&ProjectConfig>`, found `Option<&&ProjectConfig>`
LoadedConfigs::project is already Option<&ProjectConfig> (see src/config/loaded.rs:48), so .as_ref() adds an unwanted second &. Drop it. Happy to push the one-character fix if you'd like — otherwise the inline suggestion below applies.
CI failed compiling `hook_commands.rs:413` with E0308 — `project_config.as_ref()` was inferred as `Option<&&ProjectConfig>` against the expected `Option<&ProjectConfig>`. The same source compiles cleanly locally on the same toolchain (rustc 1.94.0). Replacing the destructure-binding with an explicit `let project_config: Option<&ProjectConfig> = configs.project.as_ref();` fixes the inference path so both environments agree. Co-Authored-By: Claude <noreply@anthropic.com>
Merge main into the branch, resolving conflicts in `step_commands.rs` and `main.rs` from the new `step commit --dry-run` and `step squash --dry-run` features (#2557 area). The JSON `--show-prompt` guard now also rejects `--dry-run + --format=json` for the same reason — both emit raw text. Main also changed `LoadedConfigs` to hold borrowed references (PR #2556), which is what caused the prior CI compile failure that was invisible locally. Drop the now-unnecessary `.as_ref()` calls in `handle_hook_show` (`configs.project` is already `Option<&ProjectConfig>`). Address tend-bot review feedback on the JSON additions: - **Display abbreviation**: `commit_staged_changes` and `handle_squash` now call `git rev-parse --short HEAD` for the success-line hash (honors `core.abbrev` and auto-extends ambiguous prefixes), keeping the full 40-char SHA only for the JSON payload. Previously a fixed 7-char slice silently lost both behaviors. - **`--format` flag descriptions**: each new `--format` flag now includes a one-liner naming the JSON shape, matching the pattern set by `wt switch`/`wt remove`/`wt merge`. - **Missing test**: add `test_step_push_no_ff_json` exercising the `merge_commit` outcome and the new `merge_sha` field. Co-Authored-By: Claude <noreply@anthropic.com>
worktrunk-bot
left a comment
There was a problem hiding this comment.
Display-abbreviation regression flagged previously is fixed in commit.rs and step_commands.rs, and test_step_push_no_ff_json pins down the new merge_sha field.
One same-pattern follow-up outside the diff but in the same file the refactor touches: src/commands/worktree/push.rs:444 still uses &merge_sha[..merge_sha.len().min(7)] for the Merged to @ <hash> line on --no-ff — same core.abbrev / ambiguous-prefix issue as the two sites just fixed. Pre-existing, so not a regression introduced here; happy to push a one-line fix as a follow-up if you'd like.
Lint is failing on a lychee 502 to github.com/max-sixty/worktrunk/tree/main/dev/opencode-plugin.ts — transient GitHub-side flake, should clear on rerun.
Codecov flagged uncovered diff lines in main.rs for the JSON branches that weren't exercised by existing tests. Add two integration tests: - step squash with multiple commits → emits the Squashed variant (full SHA, message, stage_mode) - step rebase against an advanced target → emits 'rebased' (true rebase), not 'fast_forwarded' Co-Authored-By: Claude <noreply@anthropic.com>
|
CI is finished — required tests all pass. Two non-required checks need a note:
This pattern holds across all the lines codecov flags in
Approval stands — the underlying coverage is fine. Worth a rerun if you want green checks before merging. |
Drop schema enumerations from the new --format help text — the existing flags in src/cli/mod.rs just say "JSON prints structured result to stdout"; match that brevity. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- step squash --format=json with NoNetChanges - step relocate --format=json with successful execution - step copy-ignored --format=json with same-worktree and no-matches - hook show --expanded --format=json with both user and project hooks (covers both filter-mismatch continues) Closes the codecov/patch gap that the false-positive theory missed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…gic (#2576) Every site that abbreviated a commit SHA was either slicing `&sha[..7]` or running its own ad-hoc `git rev-parse --short` call. 7-char prefixes regularly collide in repos with many commits, and none of the slicing sites honored `core.abbrev`. Cut over to a single canonical helper. ## Single helper, every display site `Repository::short_sha(&str) -> Result<String>` wraps `git rev-parse --short`. Routes display through git's own abbreviation logic so `core.abbrev` is honored and prefixes auto-extend on collision. Used by: - `step commit` / `step squash` success lines - `step push --no-ff` `Merged to @ <hash>` line — the original bug. Flagged on #2560 as a pre-existing third instance of the same pattern fixed there in `commit.rs` and `step_commands.rs`. - `{{ short_commit }}` template var in hook contexts (`command_executor.rs`, `template_vars.rs`) - post-remove hook context for the removed worktree - safety-backup ref display (`create_safety_backup`) - `(detached <sha>)` label in the orphan-check loop All seven sites previously sliced `commit[..7]` or called their own `rev-parse`. Now they route through one helper. ## Batched form for `wt list --format=json` The JSON list path emits one `short_sha` per worktree row. Looping `short_sha` would fork a subprocess per row, so the short SHA is folded into the existing `commit_details_many` batch instead — `%h` is added to the `git log --no-walk --format=...` call that already fetches timestamp and subject. One subprocess for the whole list, same `core.abbrev` behavior as every other site. `CommitDetails` gains a `short_sha: String` field with the same provenance as the timestamp and subject. The JSON schema is unchanged (`commit.short_sha` was already a field) — only its length now varies by `core.abbrev` instead of being hard-coded to 7. ## API change `TemplateVars::with_active_commit(commit, short_commit)` now takes both forms. Previously it sliced `commit.get(..7)` internally. The sole caller (`worktree/finish.rs`) resolves the short form via `Repository::short_sha` and passes both. ## Docs `{{ short_commit }}` is no longer documented as "(7 chars)" — `src/cli/mod.rs` and `src/config/project.rs` now describe `core.abbrev` behavior. Doc-sync regenerated `docs/content/hook.md` and the skill mirror. ## Tests Full suite passes (3463/3463). `commit_details_many` tests updated for the new tuple shape; `CommitDetails` fixtures in `layout.rs` get a `short_sha` field; `template_vars` tests pass both forms explicitly. Two integration tests previously asserting `short_commit` was exactly 7 chars (`post_start_commands.rs`, `user_hooks.rs`) still pass — fresh test repos default to `core.abbrev = 7`. > _This was written by Claude Code on behalf of @max-sixty_ --------- Co-authored-by: Claude <noreply@anthropic.com>
## Summary Release v0.48.0. See [CHANGELOG.md](https://github.com/max-sixty/worktrunk/blob/release/CHANGELOG.md) for the full notes. Highlights: - `--format=json` extends to seven step + hook commands (#2560) - `wt step commit` / `wt step squash` gain `--dry-run` (#2557) - New `dirname` / `basename` template filters (#2592, #2605) - New `[remove] delete-branch` config option (#2589) - `wt-perf timeline` subcommand (#2558) - Faster `wt list` on dirty worktrees (#2602) and faster alias dispatch (#2556, #2573) - Short-SHA display honors `core.abbrev` (#2576) - Cleaner `wt config show` shell-integration section for new users ## Test plan - [x] `cargo run -- hook pre-merge --yes` (3497 tests, lints clean) - [x] `cargo semver-checks check-release -p worktrunk` consulted; minor bump confirmed - [ ] CI green
Extends structured-output coverage beyond
list/switch/remove/mergeto the remaining commands where JSON is useful for scripting and Claude Code integration:step rebase,step push,step commit,step squash,step relocate,step copy-ignored, andhook show.JSON shapes follow the existing pattern (additive on stdout; human prose stays on stderr) and use stable snake_case
outcomediscriminators where the result is one of several variants.JSON shapes
step rebase{target, outcome: "rebased"|"fast_forwarded"|"up_to_date"}step push{target, outcome: "fast_forwarded"|"up_to_date"|"merge_commit", commits, merge_sha?}step commit{commit, message, stage_mode}(resolved mode, not raw flag)step squash{outcome: "squashed"|"no_commits_ahead"|"already_single_commit"|"no_net_changes", commit?, message?, stage_mode?, target?}step relocate{dry_run, entries: [{branch, from, to}], skipped: [{branch, reason}]}step copy-ignored{outcome, dry_run, from, to, entries: [{path, kind}], files, bytes}hook show[{type, source, name, template, needs_approval, expanded?}]Notable refactors
RebaseResult::Rebasednow carries{target, fast_forward}.SquashResult::Squashednow carries{sha, message, stage_mode}.CommitOutcomereturned fromcommit_staged_changesandCommitOptions::commitcarries{sha, message, stage_mode}— the resolved mode that was actually applied (CLI flag merged with config defaults), not the rawOption<StageMode>from clap. Without this,--format=jsonwould emitnullwhenever the user didn't pass--stage.handle_push/handle_no_ff_mergereturnPushResult { target, commit_count, outcome }with outcome variantsFastForwarded/UpToDate/MergeCommit { merge_sha }. The merge-commit SHA was previously discarded.relocate::GatherResultcarriestemplate_error_branches(was: opaque count). Text mode already showed these in stderr; JSON now surfaces them asskippedentries withreason: "template_error"so automation can detect a brokenworktree-pathconfig rather than readingentries: []as success.Behavioral guards
--show-promptis rejected when combined with--format=jsonforstep commit/step squash— show-prompt emits raw LLM-prompt text that would corrupt a JSON consumer's stdout.Reviewed by Codex
Two rounds. Round 1 caught the relocate template-error case (fixed in this PR). Round 2 reported no remaining correctness issues.
Tests
13 new integration tests covering the JSON shapes (parsed into
serde_json::Valueand asserted against concrete keys). Full suite: 1604 / 1604 passing locally.