Skip to content

fix(picker): abbreviate a PR's commits with git, not a fixed 8 - #3677

Merged
max-sixty merged 4 commits into
mainfrom
prs-short-hash
Aug 1, 2026
Merged

max-sixty merged 4 commits into
mainfrom
prs-short-hash

Conversation

@max-sixty

@max-sixty max-sixty commented Jul 31, 2026 •

Copy link
Copy Markdown
Owner

Follow-up to #3676, which made wt list's table render git's %h instead of &head[..8]. The --prs log tab was the last user-visible fixed-8 abbreviation.

The defect

wt switch --prs renders a row's log tab from the local git log when the PR head is in the object store, and from gh pr view <n> --json commits / glab api …/commits when it isn't. The forge path sliced each SHA to 8 characters, so on a repo where git abbreviates to 9 the same commit read abc1234 at 8 before a fetch and 9 after, and core.abbrev never reached the pane at all.

The fix

Repository::abbrev_len asks git how wide it abbreviates in this repo and caches the answer on RepoCache, so a picker with 30 PR rows probes once — clones share the Arc<RepoCache>, and once_cell::sync::OnceCell collapses the concurrent background rows onto one fork.

The pane wants the width rather than short_sha because of the branch it sits on: it runs precisely when the head isn't local, and git rev-parse --short takes a single revision — there is no batched form for a list of absent commits, and looping it would be one subprocess per commit per row. With no objects to disambiguate against, the width is git's whole answer for these SHAs.

Both forge arms now parse the full SHA and share one abbreviation step in render_commit_lines. GitLab's ready-made short_id goes unused: it's the server's abbreviation, blind to the reader's core.abbrev, and taking it printed an MR's commits at a different width from a PR's.

The audit

#8 claimed the --prs tab was the last one; that came from a grep. Sweeping .take(N), [..N], {:.N}, truncate(N), get(..N), and --abbrev/--short= across src/ turned up one more user-visible case: wt config state's CI cache table, which sliced cached.head to 8 and now truncates to abbrev_len.

One fixed-width slice over a SHA survives deliberately: collect/tasks.rs labels a tracing::debug! line with &sha[..8] as a fallback when a timed-out task has no branch name. It's a trace field rather than rendered output, and the timeout error path is the wrong place to spawn git.

Everything else the sweep found hashes something other than a git object — config::short_hash's base36 branch suffix, summary.rs's content digest — or is a column width rather than a truncation.

Cost

Both call sites are flat in the number of SHAs: one git rev-parse --short HEAD per repo per process.

The first cut of the CI table called short_sha per row instead, to keep git's extension of an ambiguous prefix. That's a fork apiece, and it cost more than the whole command was worth — measured on a repo with a cache entry per branch, wt config state get at 50 entries:

wt config state get, 50 cache entries, release build, hyperfine (warmup 20, 80+ runs, quiet machine):

mean ± σ vs. baseline
before this PR (.take(8)) 22.1 ± 0.8 ms —
abbrev_len (this PR) 26.1 ± 1.3 ms +4.0 ms, 1.18× ± 0.07
short_sha per row 266.7 ± 3.9 ms 12×

The +18% is one git rev-parse --short HEAD against a command whose entire
runtime is ~22 ms. For scale, a trace of the same command shows gh --version
at 15.3 ms and git config --list -z running three times for ~12 ms, both of
which predate this PR. The probe sits inside the non-empty branch, so a repo
with no CI cache pays nothing.

No Criterion target covers wt config state (time_to_first_output covers
remove/switch/list; picker_preview covers worktree rows, not --prs,
which needs a forge CLI), hence the standalone measurement. Worth noting against
the daily suite's own resolution: run-to-run spread on an unchanged bench
(worktree_scaling/warm/8, last 10 nightlies) is 18%, so a benchmark would not
have caught the +18% either — though it would have caught the 12×.

So the form here pays one probe and gives up disambiguation. Absent objects have nothing to collide with, and a live collision at 7–9 characters is rare enough for a display column to wear.

On the picker, abbrev_len lands on a background pool thread behind a gh/glab call that costs orders of magnitude more, and never on a first-paint path.

#3676 itself cost nothing at runtime, for the record: %h was already in the pre-skeleton git log --no-walk batch (%ct from the same fork drives row order), and the format string is byte-identical across that commit. It moved where the already-fetched string is copied onto the row and swapped a COMMIT_HASH_WIDTH = 8 constant for an O(N) width pass over the rendered SHAs. No fork added, no phase reordered — the pre-skeleton budget table in collect/mod.rs reads 23 commands on both sides.

Tests

  • abbrev_len_follows_core_abbrev_and_covers_absent_objects — the width matches %h, core.abbrev = 12 reaches it, and it equals what git gives for a SHA with no object behind it.
  • render_commits_abbreviates_to_the_given_width — the pane prints the width it's handed, at 7 and at 12.
  • parse_gitlab_commits_keeps_order_and_takes_the_full_id — id over short_id.
  • test_state_get_ci_head_uses_git_abbreviation — the CI table's Head cell equals git rev-parse --short. It can't be a snapshot: the width scales with the repo's object count and a test repo's SHA isn't fixed.

cargo run -- hook pre-merge --yes: 4494 passed.

This was written by Claude Code on behalf of max-sixty

The `--prs` `log` tab sliced a forge-supplied SHA to 8 characters, so the
same commit read one width before `wt switch pr:<n>` fetched it and
another after, and `core.abbrev` never reached the pane. `wt config
state`'s CI cache table sliced the same way.

`Repository::abbrev_len` asks git how wide it abbreviates here and caches
the answer per repo. The pane needs the width rather than `short_sha`
because it runs precisely when the PR's head isn't in the object store,
and `git rev-parse --short` takes a single revision, so a list of absent
commits has no batched form. The CI table's heads are local, so it calls
`short_sha` and keeps git's extension for an ambiguous prefix.

Both forge arms now parse the full SHA and share one abbreviation step;
GitLab's `short_id` is the server's width, not the reader's.

@worktrunk-bot worktrunk-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewing as a draft — flagging anything that looks worth a quick fix. Mark ready for a full review.

The change is clean and the audit is convincing — the split into parse_* + render_commit_lines reads well, and the tests pin the width behavior end to end. One small doc-accuracy nit inline; nothing blocking.

Comment thread src/git/repository/mod.rs Outdated
`wt config state`'s CI table called `short_sha` per row, which is a `git
rev-parse` fork apiece. Measured on a repo with a cache entry per branch:
38 ms → 285 ms at 50 entries.

`abbrev_len` is the same instrument the `--prs` log tab uses, and one
probe covers every row. What it gives up is git extending a prefix that
collides with another object — rare at this width, and a display column
can wear it.
`MINIMUM_ABBREV` is git's floor on `core.abbrev` and it is 4 —
`core.abbrev=4` yields four characters and 3 errors "out of range". The 7
is the auto default a small repo gets, which is what this falls back to.

Caught in review on #3677.
@max-sixty
max-sixty marked this pull request as ready for review August 1, 2026 07:47

@worktrunk-bot worktrunk-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The refactor is clean — splitting parse_* from render_commit_lines and probing the width once via abbrev_len reads well, the once_cell::sync::OnceCell collapse of concurrent picker rows onto one fork checks out, and the tests pin the width behavior (git's abbreviation, absent-object case, core.abbrev reach) end to end. The CI-cache-table find from the audit is a nice catch.

One non-blocking nit inline: the code comment's perf figures don't match the PR body's benchmark table for the same scenario.

Comment thread src/commands/config/state.rs Outdated
The comment carried the first, noisy 3-run debug timings; the PR body's
table is release/hyperfine over the same scenario. Reconciled to it.
@max-sixty
max-sixty merged commit bfbc2e2 into main Aug 1, 2026
37 checks passed
@max-sixty
max-sixty deleted the prs-short-hash branch August 1, 2026 08:21
social4hyq pushed a commit to social4hyq/homebrew-core that referenced this pull request Sep 20, 2026
worktrunk 0.72.0

Created-by: HarmonybrewBot
Commit-by: HarmonybrewBot
Merged-by: HarmonybrewBot
Description: Created by `brew bump`

---

Created with `brew bump-formula-pr`.<details>
  <summary>release notes</summary>
  <pre>## Release Notes

### Improved

- **A host carrying a forge's name anywhere resolves to that forge again**: 0.71.0 required `github`, `gitlab`, or `gitea` as a whole dot-separated label, which read as an ownership check but wasn't one — an attacker controls their own DNS — while shutting out self-hosters with hyphenated names. `github-enterprise.acme.com`, `mygithub.life-white.uk`, and the `github-personal` SSH alias classify again, so CI status, `wt switch --prs`, and `repo.provider` work with no config. ([#3673](max-sixty/worktrunk#3673))

- **One `[projects."…"]` entry can cover every repository on a host, and can set the forge**: A key containing `*` matches any run of characters, `/` included, so `[projects."git.company.example/*"]` covers a whole host; every matching entry applies, least- to most-specific. The table also gained `forge.platform` and `forge.hostname`, so a self-hosted host needs one entry here rather than a block in every repo. [Docs](https://worktrunk.dev/config/#user-project-specific-settings) ([#3701](max-sixty/worktrunk#3701), thanks @chrishas35 for the request and @witt-bit for the workspace-scoped case it partly serves)

- **`wt merge` and `wt step push` leave the target worktree's uncommitted changes in place**: The autostash that held a dirty target's changes restored them as unstaged; both now advance the target with a compare-and-swap `update-ref` and `read-tree -m -u`, which leaves uncommitted work untouched. (Breaking: the fast-forward path no longer runs `git push`, so `pre-push` and the receive-side hooks no longer fire, and a failed sync errors with the ref rolled back.) ([#3703](max-sixty/worktrunk#3703), [#3684](max-sixty/worktrunk#3684), [#3693](max-sixty/worktrunk#3693), thanks @gubasso for reporting)

- **Approval state and branch-removal outcomes are machine-readable**: `wt config approvals list --format=json` reports whether a non-interactive run would stop for approval. `wt remove` and `wt step prune` replace `branch_deleted` with `branch_outcome`: `deleted`, `deferred`, `not_attempted`, `retained_unmerged`, `retained_checked_out`, `retained_raced`, `retained_failed`. (Breaking.) ([#3710](max-sixty/worktrunk#3710), thanks @NathanaelRea for the requests)

- **Every commit hash worktrunk prints follows `core.abbrev`**: The `wt list` table, `wt switch --prs`'s `log` tab, and `wt config state`'s CI cache table sliced to 8 characters while `--format=json` carried git's `%h`. All now ask git how wide it abbreviates in this repo. ([#3676](max-sixty/worktrunk#3676), [#3677](max-sixty/worktrunk#3677))

- **`wt list --format=json` schema 2 has a published JSON Schema**: [worktrunk.dev/schema/list-v2.json](https://worktrunk.dev/schema/list-v2.json) holds the contract, and `wt list --print-schema` prints the same document. Four fields that were bare strings are now enumerated vocabularies; the emitted JSON is unchanged. ([#3747](max-sixty/worktrunk#3747))

- **A detached worktree is named by its commit, not `-`**: The Branch cell hardcoded `-`, which reads as missing data rather than a state; it now carries the row's abbreviated HEAD, and the picker and statusline name the worktree the same way. ([#3675](max-sixty/worktrunk#3675))

### Fixed

- **Piped output is plain and no longer panics**: `wt list | head -3` exited 101 with `failed printing to stdout: Broken pipe`, and `wt list` wrote ANSI to a pipe unconditionally. Every stdout surface now exits cleanly, and the human-read ones are plain when piped unless `CLICOLOR_FORCE=1`. ([#3746](max-sixty/worktrunk#3746), [#3766](max-sixty/worktrunk#3766))

- **A CI check that hasn't finished no longer reads as passed**: Each forge's status parser missed documented values, so a GitHub PR parked on an approval gate showed green, and GitLab's `canceling` and Azure DevOps's `postponed` read as no CI at all. ([#3741](max-sixty/worktrunk#3741), [#3740](max-sixty/worktrunk#3740))

- **The shell wrappers survive an `rm` alias and a failing `--execute`**: Aliases bake into the wrapper at parse time, so `alias rm='rm -v'` reached its cleanup — noise on zsh and bash, and on nushell an abort that leaked three temp files, as a failing `--execute` body also did. ([#3714](max-sixty/worktrunk#3714), thanks @Ar4l), ([#3732](max-sixty/worktrunk#3732), [#3734](max-sixty/worktrunk#3734))

- **An alias or hook wrapping `wt switch` or `wt remove` keeps your subdirectory**: The user's position came from the `wt` process's cwd, which inside an alias body is the worktree root, so an aliased `wt remove` from `feature/apps/gateway` landed at the primary worktree's root. Fixes [#3723](max-sixty/worktrunk#3723). ([#3724](max-sixty/worktrunk#3724), thanks @vivienm for reporting)

- **`wt merge` and `wt step push` refuse a target worktree parked mid-operation**: The two-tree sync refuses an unmerged index but not a stopped cherry-pick or rebase whose conflict was already staged, so the push range could land in a paused target and be committed by `--continue`. ([#3759](max-sixty/worktrunk#3759))

- **The Claude plugin's worktree-remove hook resolves against the worktree path**: The hook anchored at `CLAUDE_PROJECT_DIR`, which the `claude agents` view routinely leaves outside any repository, so `wt remove` died with `not a git repository` and the session became undeletable. Its guard now also requires a `.git` entry. ([#3754](max-sixty/worktrunk#3754), [#3767](max-sixty/worktrunk#3767), thanks @judewang for the fix and the report)

- **A Gitea API error is reported as one, not as a parse failure**: `tea api` exits 0 whatever the HTTP status, so both call sites guessed from the body's shape and blamed an API change for an API error. `--include` surfaces the status instead. ([#3713](max-sixty/worktrunk#3713), [#3600](max-sixty/worktrunk#3600))

- **`--print-schema` and the doc-generation help flags name the right command**: All three found the subcommand by scanning `argv` for a `/wt` suffix, which never matches `wt.exe` under a backslash path, so on Windows they read the binary's own path as the command. ([#3762](max-sixty/worktrunk#3762))

- **A multibyte shell name no longer panics**: `extract_filename_from_path` sliced at `len() - 4` to test for `.exe` with no char-boundary check: `SHELL=/bin/日本語 wt config show` panicked, and on macOS every process name goes through it during shell detection. ([#3727](max-sixty/worktrunk#3727))

- **`wt` installed under a dotted name generates shell integration for that name**: `binary_name` used `file_stem`, which cuts at the last dot, so `wt config shell init bash` under `wt.old` emitted a wrapper for `wt`. It now strips only the executable suffix. ([#3719](max-sixty/worktrunk#3719))

- **Concurrent `wt step prune` removals no longer race the worktree registry**: `git worktree remove` reads every sibling under `.git/worktrees/`, so two overlapping removals could have one read a sibling mid-teardown. Registry-mutating removals now serialize behind a second lock. ([#3692](max-sixty/worktrunk#3692))

- **The `wt switch` first-run offer previews the legacy files it removes**: Accepting "Install shell integration?" could delete a deprecated worktrunk-managed wrapper the prompt never named. What gets removed is unchanged. ([#3656](max-sixty/worktrunk#3656))

- **`wt config create --project` writes a resolvable link**: The comment it writes into `.config/wt.toml` carried a raw Zola target, because the link-conversion regex stopped at the first `]` — here the one closing a nested code span. ([#3731](max-sixty/worktrunk#3731))

- **`wt list --branches` counts a local branch containing `/` as local**: The summary tally classified branch-only rows by `branch.contains('/')`, so a local `feature/login` counted under "N remote branches". ([#3687](max-sixty/worktrunk#3687))

- **`wt step relocate`'s human summary counts template-error branches as skipped**: `--format=json` already folded them into `skipped`; the human tally undercounted by the number of branches whose `worktree-path` template failed to expand. ([#3688](max-sixty/worktrunk#3688))

### Documentation

- **SignPath attribution appears with the artifacts it describes**: The code-signing notice and a route to the policy now sit in the install section's Windows block on the README and the docs landing page, as SignPath Foundation's OSS program requires. ([#3709](max-sixty/worktrunk#3709))

- **`wt step copy-ignored`'s `--require-include` example renders as a terminal block**: It was the only `console` block in the command's long help missing the `$ ` prefix. ([#3706](max-sixty/worktrunk#3706))

### Internal

- **Library API rework** (Breaking library API): `cargo-semver-checks` fails five lints — `LegacyForgeAlias` and `Repository::legacy_forge_alias` removed with the forge-classification revert, `Repository::forge_platform_override` removed for one shared resolver, `stage_worktree_removal` gained two parameters, `UserProjectOverrides` gained a `forge` field, and the `real-repo-benches` feature was removed. ([#3673](max-sixty/worktrunk#3673), [#3694](htt

See merge request: Harmonybrew/homebrew-core!16275
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.

2 participants