Skip to content

refactor(strategy): use canonical git metadata resolution (split part 4) - #2305

Open
MuskanPaliwal wants to merge 3 commits into
entireio:mainfrom
MuskanPaliwal:refactor-strategy-hook-metadata-resolution
Open

refactor(strategy): use canonical git metadata resolution (split part 4)#2305
MuskanPaliwal wants to merge 3 commits into
entireio:mainfrom
MuskanPaliwal:refactor-strategy-hook-metadata-resolution

Conversation

@MuskanPaliwal

@MuskanPaliwal MuskanPaliwal commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Strategy and hook paths still resolve Git metadata through separate helpers and subprocesses. This moves their physical directory and worktree-ID lookups to gitrepo.ResolveWorktreeMetadata, using explicit worktree roots while preserving rooted storage access and caller-owned failure policy.

Part 4 of #2190鈥檚 agreed split, following #2241 and #2254. Depends on #2285 (part 3). Keep this PR open until part 3 lands; then rebase onto refreshed main and rerun checks before requesting review.

Metadata resolution and storage

The canonical resolver supplies the common Git directory, per-worktree Git directory, and worktree ID for strategy storage, hook sequence detection, reset, cleanup, reconciliation, checkpoint-sync capture, and session routing. Shared session state and captured sync settings remain in the common directory; rebase and cherry-pick markers remain per-worktree.

The resolver stays context-free, option-free, cache-free, subprocess-free, and all-or-error. Strategy retains discovery and error policy. Reads, writes, and locks continue through the existing rooted/no-follow storage primitives. Git queries that validate repository membership or inspect configuration/history remain; explicitly targeted queries use the existing EnvWithoutRepoOverrides helper.

Consistent locking across aliases

Multi-store session operations resolve every directory identity before creating or acquiring locks. They deduplicate and order repositories by filesystem identity: device/inode on Unix and volume/file index on Windows, matching the identifiers used by os.SameFile.

Path sorting alone is insufficient on a case-insensitive filesystem. For example, callers using alpha and Alpha can order the same repository differently relative to Beta, then each block on the other caller鈥檚 first lock. Identity ordering makes both callers choose the same physical order. Rooted lock acquisition, cancellation behavior, and reverse-order release are preserved.

Scope of deletion

strategy.GetGitCommonDir is removed after migrating its callers. Shared session/gitdir resolver and cache APIs, paths.GetWorktreeID, and the old gitrepo composition APIs remain for the remaining consumers and final cleanup. Settings, status, Codex, trail, and unrelated consumers are outside this slice.

The shared migration ledger entries remain unchanged. Retired query entries now log diagnostics so the intermediate migration passes; unlisted queries and independent traversal still fail the guard. Final strict ledger cleanup belongs to part 5.

Verification

  • mise run check: passed in 279.46 seconds, including formatting, zero-issue lint, race-enabled unit/integration tests, Vogon 56/56, and the external deterministic canary 4/4.
  • A subsequent pre-push mise run lint passed.
  • mise run dup: completed with 908 repository-wide advisory findings; none involve the four files in the lock-ordering fix.
  • git diff --check: passed.
  • Windows amd64 strategy tests cross-compiled successfully; Windows runtime testing was not performed.
  • The case-alias ordering regression failed before the fix and passes afterward on macOS. The original temporary reproduction also passes. Other focused tests assert real lock contention and release, invalid identities before lock creation, lexical path preservation, shared-worktree storage, metadata damage/repair, linked-worktree reset, supported conflicting Git selectors, and no-follow refusal.
  • No paid real-agent E2E tests ran.

Independent Trace2 runs against parent 3dbdf8b83 and the migration measured complete strategy methods with fresh strategy instances and cleared discovery caches:

Operation Parent After
PrepareCommitMsg, no session 4 3
PrepareCommitMsg, active session 3 2
PostCommit, no session 2 2
PostCommit, active condensation 13 12

These count Git subprocesses within the strategy methods, not full Cobra startup, and do not establish latency improvements.

Separate existing limitations

The unchanged environment helper removes GIT_DIR, GIT_WORK_TREE, and GIT_INDEX_FILE, but not GIT_COMMON_DIR. That separate bug and its regression fix are excluded from this refactor. Multi-store lock acquisition also retains its existing unbounded wait behavior; cancellation while blocked is not newly made interruptible here.

Resolve strategy storage, hook markers, worktree IDs, and shallow metadata
through the explicit-root resolver while retaining rooted storage access.
Remove the unused strategy common-directory helper.

Deduplicate physical repository identities before acquiring multi-store
session locks, and preserve failure cleanup and cancellation behavior.
Keep semantic Git queries and isolate their explicit repository targets.

Add behavioral coverage and Trace2 measurements for complete hook methods.
Retain the shared traversal ledger for the remaining consumer migrations.

Entire-Checkpoint: 01M1VXRSXZ28KB6F9TQA8X3WGQ
Entire-Checkpoint: 01M1XAFRN8M65DVP35TSDCQMSM
@MuskanPaliwal
MuskanPaliwal marked this pull request as ready for review September 7, 2026 08:18
@MuskanPaliwal
MuskanPaliwal requested a review from a team as a code owner September 7, 2026 08:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant