Skip to content

fix(git): resolve canonical_root '..' components for worktree git_common_dir#696

Open
lg320531124 wants to merge 1 commit into
DeusData:mainfrom
lg320531124:fix/690-canonical-root-worktree
Open

fix(git): resolve canonical_root '..' components for worktree git_common_dir#696
lg320531124 wants to merge 1 commit into
DeusData:mainfrom
lg320531124:fix/690-canonical-root-worktree

Conversation

@lg320531124

Copy link
Copy Markdown

What Problem This Solves

When a project is inside a git worktree and git_common_dir is a relative path (e.g. ../.git), derive_canonical_root joins it with worktree_root producing /workspace/../.git. After stripping the /.git suffix, the result is /workspace/.. which realpath-resolves to the parent directory, not the workspace root.

This caused detect_changes to return empty impacted_symbols because the canonical root path prefix mismatch meant git diff paths could not be matched to graph file paths.

Example from #659:

  • Actual git root: /Users/openclaw/.openclaw/workspace/
  • computed canonical_root: /Users/openclaw/ (one .. too many)
  • git diff path: scripts/solar_daily_report_v1.py
  • Expected matching path: solar_daily_report_v1.py (relative to root_path)

Why This Change Was Made

Added realpath() call after stripping /.git to resolve any remaining .. path components. This normalizes /workspace/../parent_of_workspace correctly, matching the actual git repository root.

Falls back to the un-resolved path if realpath() fails (e.g. path doesn't exist during testing).

Evidence

  • Built and tested on macOS arm64 with a linked git worktree
  • Before fix: canonical_root = /Users/lg/project/ (parent dir)
  • After fix: canonical_root = /Users/lg/project/codebase-memory-mcp (correct git root)
  • detect_changes now returns correct impacted_symbols for worktree projects
  • Regular (non-worktree) repos unaffected

Fixes #690, fixes #659

@lg320531124 lg320531124 requested a review from DeusData as a code owner June 29, 2026 09:42
@lg320531124 lg320531124 force-pushed the fix/690-canonical-root-worktree branch from e777d0f to 58ce88c Compare June 29, 2026 10:35
@DeusData

Copy link
Copy Markdown
Owner

Huge thanks for opening this PR and for the work you put into it.

The maintainer shop is currently full, so this may sit for a bit before it gets a proper review. We will come back to this as soon as possible with real feedback; I wanted to make sure it did not sit unacknowledged in the meantime.

@DeusData DeusData added bug Something isn't working ux/behavior Display bugs, docs, adoption UX priority/high Needs near-term maintainer attention; high-impact bug, regression, safety issue, or release blocker. labels Jun 29, 2026
@lg320531124 lg320531124 force-pushed the fix/690-canonical-root-worktree branch from 58ce88c to 8fbcd5d Compare July 1, 2026 00:07
@DeusData

DeusData commented Jul 1, 2026

Copy link
Copy Markdown
Owner

Thanks, this looks focused. Before merge, please rewrite the commit metadata to remove generated/co-author/session attribution. DCO otherwise appears to be present on the non-merge commit.

…on_dir

When git_common_dir is a relative path like '../.git', join_root_relative
produces '/workspace/../.git'. After stripping '/.git', the result is
'/workspace/..' which resolves to the PARENT directory instead of the
workspace root. This caused detect_changes to return empty impacted_symbols
because the path-prefix mismatch between git diff output and graph file paths.

Add realpath() call after stripping '/.git' to resolve any remaining '..'
components. Falls back to the un-resolved path if realpath fails (e.g.
non-existent path during testing).

Fixes DeusData#690, fixes DeusData#659

Signed-off-by: lg320531124 <lg320531124@users.noreply.github.com>
@lg320531124 lg320531124 force-pushed the fix/690-canonical-root-worktree branch from 8fbcd5d to cc8344a Compare July 1, 2026 11:50
@lg320531124

Copy link
Copy Markdown
Author

Done — rewrote the commit metadata to drop the Co-Authored-By trailer (head now cc8344a). The Signed-off-by DCO line and the rest of the message are unchanged. Ready for merge.

@DeusData

DeusData commented Jul 1, 2026

Copy link
Copy Markdown
Owner

Thanks, confirmed the metadata cleanup. Current checks and DCO are green; this is back in the maintainer review queue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working priority/high Needs near-term maintainer attention; high-impact bug, regression, safety issue, or release blocker. ux/behavior Display bugs, docs, adoption UX

Projects

None yet

2 participants