Skip to content

worktree: follow-up polish from PR #4381 round 6 (observability nits, SHA-256 future-proofing, test coverage gaps) #4529

@LaZzyMan

Description

@LaZzyMan

Tracking the six round-6 review findings from #4381 that were declined as overthinking per the round-weighted bar but are real, low-priority polish items worth addressing later.

All six are on currently-correct code — none gate correctness or block #4381 from merging.

Observability

  • packages/cli/src/startup/worktreeStartup.ts:407writeWorktreeSessionMarker(...).catch(() => {}) swallows EPERM / ENOSPC silently. enter-worktree.ts already logs at warn level for the same operation. Mirror that pattern. (PR #4381 thread)
  • packages/core/src/services/gitWorktreeService.ts:1174getRegisteredWorktreeBranch returns null on lines.length < 4 with no debugLogger.debug breadcrumb, while every other rejection branch in the same function logs. (PR #4381 thread)

SHA-256 future-proofing

  • packages/core/src/services/gitWorktreeService.ts:392resolveRef validates with /^[0-9a-f]{40}$/ (SHA-1 only). In a SHA-256 git repo, --worktree=#123 fails with "FETCH_HEAD did not resolve to a commit SHA" even though the ref resolved fine. Relax to /^[0-9a-f]{40,64}$/. Asymmetric with getCurrentCommitHash which has no length check. (PR #4381 thread)

Test coverage

  • packages/cli/src/startup/worktreeStartup.test.ts — add a real-worktree-on-different-branch case to exercise the branch-mismatch guard at worktreeStartup.ts:219. Current test uses a plain directory, which short-circuits via getRegisteredWorktreeBranch() === null. (PR #4381 thread)
  • packages/cli/src/startup/worktreeStartup.test.ts — extend the re-attach test to commit between first/second setupStartupWorktree calls and assert originalHeadCommit equals the worktree's HEAD (not the launch repo's). (PR #4381 thread)
  • packages/core/src/services/gitWorktreeService.test.ts:570 — add a parsePRReference('https://github.com/o/r/pull/123/files') assertion. JSDoc documents /files / /commits / /checks sub-paths but no test exercises them. (PR #4381 thread)

Reviewer: @wenshao via qwen3.7-max /review.

Source: #4381 round 6 review batch (2026-05-25). PR is otherwise green and ready to merge.

Metadata

Metadata

Assignees

No one assigned

    Labels

    priority/P3Low - Minor, cosmetic, nice-to-fix issuesscope/gitGit integration featuresstatus/needs-triageIssue needs to be triaged and labeledtype/enhancementNon-bug improvement or optimization

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions