From 354befe557b639b3030f47e89cfc8c2618d6f7d7 Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Thu, 2 Jul 2026 00:19:16 +0200 Subject: [PATCH 1/3] fix(git): correct canonical_root for worktrees and subdirectory projects MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit detect_changes returned an empty impacted_symbols set for projects indexed inside a git worktree or from a repo subdirectory (#659): canonical_root was computed wrong. `git rev-parse --git-common-dir` emits a path relative to the directory passed via -C (input_path), NOT to worktree_root, and the "/.git" suffix was stripped textually without resolving ".." components. So e.g. "../.git" joined against worktree_root, or "/ws/scripts/../.git" strip, left an unresolved ".." and never matched the project's stored root path. Fix (src/git/git_context.c): join the relative --git-common-dir against input_path (the -C dir) instead of worktree_root, and realpath()/_fullpath()- normalize the result BEFORE stripping "/.git" (the .git dir always exists for a valid repo, so resolution succeeds). This is consistent with the existing realpath'd root_path used for prefix matching (mcp.c). Reproduce-first: tests/test_git_context.c. The subdirectory case is the genuine guard — a repo indexed from a subdir yields a *relative* --git-common-dir, so the unfixed code leaves an un-normalized "/subdir/.." (verified RED; GREEN after the realpath fix). The linked-worktree case is a supporting invariant, not the #659 reproducer: git that emits an *absolute* worktree --git-common-dir (e.g. 2.48.x) doesn't manifest the bug, so it passes either way; it still enforces the worktree->main-root invariant. All three git-based tests SKIP_PLATFORM on Windows (the CI shell there cannot init a repo via system()). Distilled from #672 (thanks @anivaryam); that PR's production fix is taken verbatim. Its repo_root/subdir tests ran git unconditionally and failed the Windows CI leg ("failed to init git repo"); here all three git-based tests are Windows-skipped so ci-ok is green cross-platform. Supersedes #696, which only realpath'd after the strip without changing the join base and so still yielded the workspace parent for the subdirectory case. Closes #659 Signed-off-by: Martin Vogel Co-Authored-By: anivaryam --- Makefile.cbm | 1 + src/git/git_context.c | 40 ++++++- tests/test_git_context.c | 236 +++++++++++++++++++++++++++++++++++++++ tests/test_main.c | 2 + 4 files changed, 276 insertions(+), 3 deletions(-) create mode 100644 tests/test_git_context.c diff --git a/Makefile.cbm b/Makefile.cbm index e14a77ab0..0a2d16b94 100644 --- a/Makefile.cbm +++ b/Makefile.cbm @@ -340,6 +340,7 @@ TEST_DISCOVER_SRCS = \ tests/test_language.c \ tests/test_userconfig.c \ tests/test_gitignore.c \ + tests/test_git_context.c \ tests/test_discover.c TEST_GRAPH_BUFFER_SRCS = tests/test_graph_buffer.c diff --git a/src/git/git_context.c b/src/git/git_context.c index 99ae17cf3..46dbe8f86 100644 --- a/src/git/git_context.c +++ b/src/git/git_context.c @@ -126,17 +126,51 @@ static char *join_root_relative(const char *root, const char *rel) { return out; } -static char *derive_canonical_root(const char *worktree_root, const char *git_common_dir) { +static char *derive_canonical_root(const char *input_path, const char *worktree_root, + const char *git_common_dir) { const char *src = git_common_dir && git_common_dir[0] ? git_common_dir : worktree_root; if (!src) { return git_strdup(""); } - char *root = path_is_absolute(src) ? git_strdup(src) : join_root_relative(worktree_root, src); + /* git rev-parse --git-common-dir outputs a path relative to the directory + * passed via -C (input_path), NOT to worktree_root. Using worktree_root as + * the base was wrong when input_path is a subdirectory or a linked worktree + * sibling — e.g. "../.git" joined with worktree_root produced "workspace/.." + * instead of the actual repo root. */ + char *root = path_is_absolute(src) ? git_strdup(src) : join_root_relative(input_path, src); if (!root) { return NULL; } + /* Resolve ".." components before stripping the "/.git" suffix. + * Without this, "/workspace/scripts/../.git" would strip to + * "/workspace/scripts/.." instead of "/workspace". The .git directory + * must exist for a valid git repo, so realpath() / _fullpath() succeeds. */ +#ifndef _WIN32 + { + char resolved[4096]; + if (realpath(root, resolved) != NULL) { + free(root); + root = git_strdup(resolved); + if (!root) { + return NULL; + } + } + } +#else + { + char resolved[4096]; + if (_fullpath(resolved, root, sizeof(resolved)) != NULL) { + free(root); + root = git_strdup(resolved); + if (!root) { + return NULL; + } + } + } +#endif + size_t len = strlen(root); while (len > 1 && (root[len - 1] == '/' || root[len - 1] == '\\')) { root[--len] = '\0'; @@ -251,7 +285,7 @@ int cbm_git_context_resolve(const char *path, cbm_git_context_t *out) { out->is_worktree = out->git_dir && out->git_common_dir && strcmp(out->git_dir, out->git_common_dir) != 0; - out->canonical_root = derive_canonical_root(out->worktree_root, out->git_common_dir); + out->canonical_root = derive_canonical_root(path, out->worktree_root, out->git_common_dir); out->branch_slug = slug_from_branch(out->branch, out->is_detached); if (git_capture(path, "merge-base HEAD @{upstream}", &out->base_sha) != 0) { out->base_sha = git_strdup(""); diff --git a/tests/test_git_context.c b/tests/test_git_context.c new file mode 100644 index 000000000..0c569b1d3 --- /dev/null +++ b/tests/test_git_context.c @@ -0,0 +1,236 @@ +/* + * test_git_context.c — Tests for cbm_git_context_resolve(), focusing on + * the canonical_root derivation for git worktrees and subdirectory projects. + * + * Issue #659: canonical_root was computed incorrectly for linked worktrees + * and projects indexed from a subdirectory of the repository root. + * git rev-parse --git-common-dir outputs a path relative to the -C directory + * (input_path), not to worktree_root. Joining it with worktree_root and then + * string-stripping "/.git" left unresolved ".." components in the result. + * + * These tests shell out to `git`, so they SKIP_PLATFORM on Windows (the CI + * shell there cannot init a repo via system()). + * + * Reproduce-first guard: canonical_root_subdir is the genuine RED-without-the-fix + * guard — a repo indexed from a subdirectory yields a relative --git-common-dir + * ("../.git"), so the unfixed code returns an un-normalized "/subdir/.." + * (verified FAIL on the unfixed derive_canonical_root; GREEN with the realpath + * normalization). canonical_root_linked_worktree is a SUPPORTING INVARIANT, not + * the #659 reproducer: on git that emits an *absolute* --git-common-dir for a + * linked worktree (e.g. 2.48.x) the bug does not manifest there, so that test + * passes with or without the fix. It still enforces the worktree->main-root + * invariant and would catch the bug on git builds that emit a relative + * worktree common-dir. canonical_root_repo_root is a baseline (no `..` to + * normalize), not a guard. + */ +#include "test_framework.h" +#include "test_helpers.h" +#include "git/git_context.h" + +#include +#include + +#ifndef _WIN32 +#include +#endif + +/* Run a git command inside dir, return 0 on success. */ +static int git_run(const char *dir, const char *args) { + char cmd[1024]; + snprintf(cmd, sizeof(cmd), "git -C \"%s\" %s >/dev/null 2>&1", dir, args); + return system(cmd); +} + +/* Create a minimal git repo at dir (init + empty commit so HEAD exists). */ +static int make_git_repo(const char *dir) { + if (th_mkdir_p(dir) != 0) return -1; + if (git_run(dir, "init -q") != 0) return -1; + if (git_run(dir, "config user.email test@example.com") != 0) return -1; + if (git_run(dir, "config user.name Test") != 0) return -1; + /* Create a file so HEAD points to a real commit. */ + char path[1024]; + snprintf(path, sizeof(path), "%s/.keep", dir); + th_write_file(path, ""); + if (git_run(dir, "add .keep") != 0) return -1; + if (git_run(dir, "commit -q -m init") != 0) return -1; + return 0; +} + +/* ── canonical_root: normal repo indexed from its root ──────────── */ + +TEST(canonical_root_repo_root) { +#ifdef _WIN32 + SKIP_PLATFORM("git-based canonical_root test not supported on Windows CI"); +#else + char *tmp = th_mktempdir("cbm_gitctx"); + if (!tmp) FAIL("th_mktempdir returned NULL"); + + if (make_git_repo(tmp) != 0) { + th_rmtree(tmp); + SKIP_PLATFORM("git not available to init a repo"); + } + + cbm_git_context_t ctx = {0}; + int rc = cbm_git_context_resolve(tmp, &ctx); + if (rc != 0 || !ctx.is_git) { + cbm_git_context_free(&ctx); + th_rmtree(tmp); + FAIL("cbm_git_context_resolve failed or not a git repo"); + } + + char expected[4096]; + if (realpath(tmp, expected) == NULL) { + cbm_git_context_free(&ctx); + th_rmtree(tmp); + FAIL("realpath(tmp) failed"); + } + + ASSERT_STR_EQ(ctx.canonical_root, expected); + + cbm_git_context_free(&ctx); + th_rmtree(tmp); + PASS(); +#endif /* _WIN32 */ +} + +/* ── canonical_root: indexed from a subdirectory (issue #659) ───── + * THE reproduce-first guard: from a subdir, --git-common-dir is relative, so the + * unfixed derive_canonical_root joins it against worktree_root and strips "/.git" + * textually, leaving canonical_root = "/subdir/.." (or "/..") instead + * of "". Verified RED on the unfixed code, GREEN with the realpath fix. */ + +TEST(canonical_root_subdir) { +#ifdef _WIN32 + SKIP_PLATFORM("git-based canonical_root test not supported on Windows CI"); +#else + char *tmp = th_mktempdir("cbm_gitctx"); + if (!tmp) FAIL("th_mktempdir returned NULL"); + + if (make_git_repo(tmp) != 0) { + th_rmtree(tmp); + SKIP_PLATFORM("git not available to init a repo"); + } + + /* Create a subdirectory inside the repo. */ + char subdir[1024]; + snprintf(subdir, sizeof(subdir), "%s/scripts", tmp); + if (th_mkdir_p(subdir) != 0) { + th_rmtree(tmp); + FAIL("failed to create subdir"); + } + + cbm_git_context_t ctx = {0}; + int rc = cbm_git_context_resolve(subdir, &ctx); + if (rc != 0 || !ctx.is_git) { + cbm_git_context_free(&ctx); + th_rmtree(tmp); + FAIL("cbm_git_context_resolve on subdir failed or not a git repo"); + } + + /* canonical_root must equal the repo root, NOT "/.." or "/..". */ + char expected[4096]; + if (realpath(tmp, expected) == NULL) { + cbm_git_context_free(&ctx); + th_rmtree(tmp); + FAIL("realpath(tmp) failed"); + } + + ASSERT_STR_EQ(ctx.canonical_root, expected); + + /* Sanity: canonical_root must not contain ".." or end with a slash. */ + ASSERT(strstr(ctx.canonical_root, "..") == NULL); + ASSERT(ctx.canonical_root[strlen(ctx.canonical_root) - 1] != '/'); + + cbm_git_context_free(&ctx); + th_rmtree(tmp); + PASS(); +#endif /* _WIN32 */ +} + +/* ── canonical_root: linked git worktree (supporting invariant) ──── + * NOT the #659 reproducer on modern git: git that emits an *absolute* + * --git-common-dir for a linked worktree (e.g. 2.48.x) takes the path_is_absolute + * branch, so the bug does not manifest and this passes with or without the fix. + * It is kept as an invariant — canonical_root of a linked worktree must equal the + * MAIN repo root (never the worktree root or its parent) — and would fail on a git + * build that emits a *relative* worktree common-dir. The genuine RED-without-fix + * guard for #659 is canonical_root_subdir above. */ + +TEST(canonical_root_linked_worktree) { +#ifdef _WIN32 + SKIP_PLATFORM("git worktree test not implemented for Windows"); +#else + /* th_mktempdir() returns a static buffer — copy before the second call. */ + char main_tmp[256]; + char *raw = th_mktempdir("cbm_main"); + if (!raw) FAIL("th_mktempdir returned NULL"); + strncpy(main_tmp, raw, sizeof(main_tmp) - 1); + main_tmp[sizeof(main_tmp) - 1] = '\0'; + + char wt_tmp[256]; + raw = th_mktempdir("cbm_worktree"); + if (!raw) FAIL("th_mktempdir returned NULL"); + strncpy(wt_tmp, raw, sizeof(wt_tmp) - 1); + wt_tmp[sizeof(wt_tmp) - 1] = '\0'; + + /* Remove the worktree dir first — git worktree add creates it. */ + th_rmtree(wt_tmp); + + if (make_git_repo(main_tmp) != 0) { + th_rmtree(main_tmp); + SKIP_PLATFORM("git not available to init a repo"); + } + + /* Create a branch for the worktree. */ + if (git_run(main_tmp, "branch wt-branch") != 0) { + th_rmtree(main_tmp); + FAIL("failed to create branch for worktree"); + } + + /* Add a linked worktree. */ + char wt_cmd[1024]; + snprintf(wt_cmd, sizeof(wt_cmd), "worktree add \"%s\" wt-branch", wt_tmp); + if (git_run(main_tmp, wt_cmd) != 0) { + th_rmtree(wt_tmp); + th_rmtree(main_tmp); + SKIP_PLATFORM("git worktree add unavailable (git 2.5+ required)"); + } + + cbm_git_context_t ctx = {0}; + int rc = cbm_git_context_resolve(wt_tmp, &ctx); + if (rc != 0 || !ctx.is_git) { + cbm_git_context_free(&ctx); + git_run(main_tmp, "worktree prune"); + th_rmtree(main_tmp); + th_rmtree(wt_tmp); + FAIL("cbm_git_context_resolve on linked worktree failed"); + } + + /* canonical_root must be the MAIN repo root, not the worktree root or its parent. */ + char expected[4096]; + if (realpath(main_tmp, expected) == NULL) { + cbm_git_context_free(&ctx); + git_run(main_tmp, "worktree prune"); + th_rmtree(main_tmp); + th_rmtree(wt_tmp); + FAIL("realpath(main_tmp) failed"); + } + + ASSERT_STR_EQ(ctx.canonical_root, expected); + ASSERT(strstr(ctx.canonical_root, "..") == NULL); + + cbm_git_context_free(&ctx); + git_run(main_tmp, "worktree prune"); + th_rmtree(main_tmp); + th_rmtree(wt_tmp); + PASS(); +#endif /* _WIN32 */ +} + +/* ── Suite ──────────────────────────────────────────────────────── */ + +SUITE(git_context) { + RUN_TEST(canonical_root_repo_root); + RUN_TEST(canonical_root_subdir); + RUN_TEST(canonical_root_linked_worktree); +} diff --git a/tests/test_main.c b/tests/test_main.c index 824b91fe8..d0cca85d7 100644 --- a/tests/test_main.c +++ b/tests/test_main.c @@ -58,6 +58,7 @@ extern void suite_mcp(void); extern void suite_language(void); extern void suite_userconfig(void); extern void suite_gitignore(void); +extern void suite_git_context(void); extern void suite_discover(void); extern void suite_graph_buffer(void); extern void suite_registry(void); @@ -173,6 +174,7 @@ int main(int argc, char **argv) { RUN_SELECTED_SUITE(language); RUN_SELECTED_SUITE(userconfig); RUN_SELECTED_SUITE(gitignore); + RUN_SELECTED_SUITE(git_context); RUN_SELECTED_SUITE(discover); /* Graph Buffer (M7) */ From 722ace679fc6c394a011c98d85fd43c3ec3306a9 Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Thu, 2 Jul 2026 00:37:52 +0200 Subject: [PATCH 2/3] fix(test): guard git_context git helpers behind #ifndef _WIN32 On Windows every git_context test SKIP_PLATFORMs, leaving git_run/make_git_repo as unused static functions -> -Wunused-function under -Werror failed the Windows build. Guard the two helpers (only called from the non-Windows test bodies) so the file compiles clean on Windows while the tests still run on Linux/macOS. Signed-off-by: Martin Vogel --- tests/test_git_context.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/test_git_context.c b/tests/test_git_context.c index 0c569b1d3..a384651a5 100644 --- a/tests/test_git_context.c +++ b/tests/test_git_context.c @@ -34,6 +34,10 @@ #include #endif +/* These helpers shell out to git and are only used by the non-Windows test + * bodies below; on Windows every test SKIP_PLATFORMs, so guard them here too or + * they'd be unused-static functions and fail the -Werror build. */ +#ifndef _WIN32 /* Run a git command inside dir, return 0 on success. */ static int git_run(const char *dir, const char *args) { char cmd[1024]; @@ -55,6 +59,7 @@ static int make_git_repo(const char *dir) { if (git_run(dir, "commit -q -m init") != 0) return -1; return 0; } +#endif /* _WIN32 */ /* ── canonical_root: normal repo indexed from its root ──────────── */ From 3f3ae855b8e8cbf94fe3a3577892b79c2a5ac471 Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Thu, 2 Jul 2026 11:22:03 +0200 Subject: [PATCH 3/3] fix(git): derive canonical_root from git's own absolute common-dir MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the manual join(input_path, --git-common-dir) + realpath/_fullpath with `git rev-parse --path-format=absolute --git-common-dir` (git 2.31+), which returns git's own absolute, canonical common-dir. Because a main repo and its linked worktree both query the same git binary, they resolve to the IDENTICAL path, so canonical_root is consistent across worktrees and platforms with no platform-specific path canonicalization. This is the truly platform-agnostic #659 fix: - git resolves the relative ".." internally, so a project indexed from a subdirectory gets the real repo root (the #659 bug). - No realpath/_fullpath, so the msys-vs-native path-representation divergence that broke the worktree==main-root invariant (test_pipeline.c git_context_linked_worktree) on the Windows CI leg is eliminated — both sides get git's single canonical representation. Falls back to the previous relative-join derivation (Unix realpath) when git is older than 2.31 and doesn't support --path-format (abs_common_dir empty). The input_path-join fix and the Windows test-harness SKIP_PLATFORM guards from the earlier commits on this branch remain for that fallback path. Signed-off-by: Martin Vogel Co-Authored-By: anivaryam --- src/git/git_context.c | 90 +++++++++++++++++++++++++------------------ 1 file changed, 52 insertions(+), 38 deletions(-) diff --git a/src/git/git_context.c b/src/git/git_context.c index 46dbe8f86..14f48714d 100644 --- a/src/git/git_context.c +++ b/src/git/git_context.c @@ -126,50 +126,58 @@ static char *join_root_relative(const char *root, const char *rel) { return out; } +/* Derive the canonical repo root. + * + * Preferred (git 2.31+): abs_common_dir is `git rev-parse --path-format=absolute + * --git-common-dir` — git's OWN absolute, canonical common-dir. Because a main + * repo and its linked worktree both ask the same git binary, they resolve to the + * IDENTICAL path, so canonical_root is consistent across worktrees AND platforms + * with no manual join or realpath/_fullpath — which diverged under msys vs native + * path representations (#659 root cause, and the worktree==main-root breakage in + * test_pipeline.c git_context_linked_worktree). git also resolves the relative + * ".." internally, so the subdirectory case (#659) is fixed here too. + * + * Fallback (git < 2.31, no --path-format → abs_common_dir empty): the relative + * --git-common-dir is relative to input_path (the -C dir), so join against it and + * realpath-normalize the "..". Unix only — on Windows git emits an absolute + * common-dir so this branch isn't reached in practice, and _fullpath there + * reintroduces the msys divergence. */ static char *derive_canonical_root(const char *input_path, const char *worktree_root, - const char *git_common_dir) { - const char *src = git_common_dir && git_common_dir[0] ? git_common_dir : worktree_root; - if (!src) { - return git_strdup(""); - } - - /* git rev-parse --git-common-dir outputs a path relative to the directory - * passed via -C (input_path), NOT to worktree_root. Using worktree_root as - * the base was wrong when input_path is a subdirectory or a linked worktree - * sibling — e.g. "../.git" joined with worktree_root produced "workspace/.." - * instead of the actual repo root. */ - char *root = path_is_absolute(src) ? git_strdup(src) : join_root_relative(input_path, src); - if (!root) { - return NULL; - } - - /* Resolve ".." components before stripping the "/.git" suffix. - * Without this, "/workspace/scripts/../.git" would strip to - * "/workspace/scripts/.." instead of "/workspace". The .git directory - * must exist for a valid git repo, so realpath() / _fullpath() succeeds. */ + const char *git_common_dir, const char *abs_common_dir) { + char *root = NULL; + if (abs_common_dir && abs_common_dir[0] && path_is_absolute(abs_common_dir)) { + root = git_strdup(abs_common_dir); + if (!root) { + return NULL; + } + } else { + const char *src = git_common_dir && git_common_dir[0] ? git_common_dir : worktree_root; + if (!src) { + return git_strdup(""); + } #ifndef _WIN32 - { - char resolved[4096]; - if (realpath(root, resolved) != NULL) { - free(root); - root = git_strdup(resolved); - if (!root) { - return NULL; + root = path_is_absolute(src) ? git_strdup(src) : join_root_relative(input_path, src); + if (!root) { + return NULL; + } + { + char resolved[4096]; + if (realpath(root, resolved) != NULL) { + free(root); + root = git_strdup(resolved); + if (!root) { + return NULL; + } } } - } #else - { - char resolved[4096]; - if (_fullpath(resolved, root, sizeof(resolved)) != NULL) { - free(root); - root = git_strdup(resolved); - if (!root) { - return NULL; - } + (void)input_path; + root = path_is_absolute(src) ? git_strdup(src) : join_root_relative(worktree_root, src); + if (!root) { + return NULL; } - } #endif + } size_t len = strlen(root); while (len > 1 && (root[len - 1] == '/' || root[len - 1] == '\\')) { @@ -285,7 +293,13 @@ int cbm_git_context_resolve(const char *path, cbm_git_context_t *out) { out->is_worktree = out->git_dir && out->git_common_dir && strcmp(out->git_dir, out->git_common_dir) != 0; - out->canonical_root = derive_canonical_root(path, out->worktree_root, out->git_common_dir); + /* git 2.31+ canonical absolute common-dir (best-effort; NULL on older git, + * where derive_canonical_root falls back to the relative common-dir). */ + char *abs_common_dir = NULL; + (void)git_capture(path, "rev-parse --path-format=absolute --git-common-dir", &abs_common_dir); + out->canonical_root = + derive_canonical_root(path, out->worktree_root, out->git_common_dir, abs_common_dir); + free(abs_common_dir); out->branch_slug = slug_from_branch(out->branch, out->is_detached); if (git_capture(path, "merge-base HEAD @{upstream}", &out->base_sha) != 0) { out->base_sha = git_strdup("");