fix(git): make Git subprocess and byte-exact fixture boundaries portable on Windows - #953
Conversation
fs::canonicalize returns the \\?\ verbatim form on Windows. Win32 file
APIs and Command::current_dir accept it, but Git for Windows rewrites a
path *argument* to //?/D:/... and then fails ("could not create leading
directories" from git worktree add). Add git_path_argument, which drops
only the verbatim prefix at the Git argument boundary and passes every
other component through unchanged, and use it for the -C repository
selector and the PR-autotrack worktree add/remove arguments. Callers keep
the canonical path as the root identity they key on.
Refs #923
Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
The host-admission fixtures build under the canonical temp root, which on Windows is verbatim, and then handed that spelling to git worktree add and wrote it into a gitfile. Route both through the Git argument spelling while the fixture keeps the verbatim root as the identity in the payload. Refs #923 Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
The 32-level nested-tree fixture exceeds MAX_PATH on Windows and git commit failed with 'Filename too long'. Pass core.longpaths=true to the fixture's Git so the tree-depth condition under test is unchanged. Refs #923 Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
real_rebase_reconcile_matches_clean_scan compares an incremental reconcile, which reads the worktree files git rebase rewrote, against a clean scan that captures HEAD's blobs. Under the Windows runner's global core.autocrlf=true the rewritten files came back CRLF, so the corpora differed at src/feature.rs and src/main_only.rs. Set core.autocrlf=false in the fixture repository so both read the same bytes. Refs #923 Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
branch_add_response only compiles the activation journey under cfg(unix) and answers code_index_scheduler_unavailable elsewhere, so the end-to-end CLI journey cannot pass on Windows by construction. Refs #923 Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
|
|
This branch no longer merges cleanly with |
128fa8f deleted suspend/restore_immutability but left their #[hotpath::measure] attributes above the tests module. The attribute is inert without the feature, so only the hotpath slice lane saw the macro reject a module ('expected fn') and fail to compile tracedecay-global-db. Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
# Conflicts: # crates/tracedecay-runtime-core/src/git.rs # crates/tracedecay-runtime-core/tests/git_repository_authority.rs
ScriptedAlchemy
left a comment
ScriptedAlchemy
left a comment
There was a problem hiding this comment.
Review — keep the Git argument boundary; HOLD for coverage and conflicts
Reviewed ea537d2584eae6bf0f0cb871881dcff2da6722f1; source review only. GitHub reports conflicts; no PR workflow run was returned for this exact head.
The narrow git_path_argument helper is the correct direction: respell only when crossing into Git argv, retaining the admitted native path for identity and authorization. Preserve OsStr/non-UTF-8 data, component boundaries and unchanged handling of device/volume paths. Require native Windows drive and UNC paths with spaces/non-ASCII/long components, plus a filesystem check that the argv spelling still addresses the same directory. Do not use this helper as a project-identity normalizer.
P1 coverage: scoping the positive branch_add_seals... journey to Unix is only honest if the Windows contract is explicitly unsupported. Add a Windows test of the named unsupported response and prove no branch/store artifacts are created. Record the platform limitation in the support/acceptance inventory. Removing the sole Windows test is not a completed cross-platform feature fix; enabling that feature is separate work, not necessary in this PR.
Pinning core.autocrlf=false is appropriate for the byte-exact fixture. Keep a separate ordinary Windows/CRLF behavior test where the product claims it, so fixture normalization does not mask a production source-binding defect. Keep the checked-in evaluator pack/labels unchanged.
Resolve overlap with #959's path producers and #949's identical orphaned-hotpath deletion once. Run affected Git/host-admission/rebase tests on the resolved Windows merge tree before integration into #707. No timeout or freshness relaxation, and no #707 state change.
Performance Comparison
|
|
Resolved against the current redesign base and pushed as Conflict decisions:
Verification on the integrated tree: the rebase-vs-clean-scan test passed both normally and with simulated global |
…sor/windows-git-boundaries-d09f
320173d
into
codex/tracedecay-total-redesign-plan-reopened
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
2 participants
Summary
tracedecay_runtime_core::git::git_path_argument, a spelling boundary that drops only the\\?\verbatim prefix when a native path is handed to Git as an argument (\\?\D:\x→D:\x,\\?\UNC\s\v\x→\\s\v\x), leaving every other component (long, spaced, non-ASCII) byte-identical. Used at the-Crepository selector and the PR-autotrackworktree add/removearguments. Callers keep the canonical path as the root identity they key on.worktree addand gitfile spelling,core.longpathsfor the 32-level nested-tree fixture, andcore.autocrlf=falsefor the byte-exact scheduler corpus.branch addend-to-end journey test to Unix, matching the daemon's#[cfg(not(unix))]arm inbranch_add_response.slice-tests (hotpath on)lane:128fa8fa2deleted two functions inglobal-dbtriggers.rsbut left their#[hotpath::measure]attributes abovemod tests, which only fails to compile with thehotpathfeature on.Motivation
Fixes #923 (Git-boundary child of #903). Baseline run 34039753429, head
9d17537b4. This is a Git adapter / fixture construction fix, not a semantic freshness workaround.Root causes confirmed from the archived junit reports:
host_admission_tests::add_branch_at_replay_rejects_stale_branch_after_switch,..._stale_root_after_adversarial_replace,..._restart_replay_rejects_common_dir_driftgit worktree add "\\?\D:\a\_temp\tmp\.tmpX-admission-wt"→fatal: could not create leading directories of '//?/D:/...': Invalid argumentlinked_worktree_onand the gitfile write now use the Git spelling while the verbatimPathBufstays the payload/assertion identitygit_repository_authority::authority_bounds_nested_tree_inventory_before_rename_diffgit commit:Filename too longatlevel-0/.../level-25-c core.longpaths=trueon the fixture's Git; tree depth unchangedcode_index_scheduler::tests::real_rebase_reconcile_matches_clean_scansnapshot_content_identitydigests differ (bb77f439…vs707d16e0…); log showsLF will be replaced by CRLFforsrc/lib.rs,src/feature.rs,src/main_only.rsgit rebaserewrote as CRLF under the runner's globalcore.autocrlf=true, while the clean scan captures HEAD's blobs (find_blob) which are LF. Pinnedcore.autocrlf=falsein the fixture repo (inherited by template copies). Rebase reconciliation is unchanged. Reproduced on Linux: withGIT_CONFIG_GLOBALset tocore.autocrlf=true, the unpinned fixture fails with exactly the same two digests as the Windows job; with the pin it passes.cli_non_interactive_test::branch_add_seals_the_single_store_branch_and_remove_retires_its_exact_artifactscode_index_scheduler_unavailable: code-index scheduler authority is unavailable for branch activation#[cfg(not(unix))]arm inbranch_add.rs, not a Git boundary: the manual branch activation journey is not compiled on Windows. The test is gated#[cfg(unix)]with a comment naming that arm. Enabling the journey on Windows is a separate feature.slice-tests (hotpath on)lane:tracedecay-global-dberror: expected fnattriggers.rs:1718 mod tests#[hotpath::measure]attributes overmod testsleft by128fa8fa2Changes
crates/tracedecay-runtime-core/src/git.rs—git_path_argument(Windows impl viaComponent/Prefix; refuses to respell paths containing literal./..or\\?\Volume{...}prefixes), applied ingit_command_at; unit tests including a Windows-only spelling table and a cross-platform "spelled path canonicalizes back to the same directory" check.crates/tracedecay/src/daemon/pr_autotrack.rs— worktree add/remove arguments go through the boundary.crates/tracedecay/src/mcp/server/writer_test_support.rs,host_admission_tests.rs—git_path_argumentwrapper; worktree add arg and common-dir gitfile pointer.crates/tracedecay-runtime-core/tests/git_repository_authority.rs—core.longpaths=true.crates/tracedecay-code-index-runtime/src/code_index_scheduler/tests.rs—core.autocrlf=falseinGitFixture::build_fresh.crates/tracedecay-cli/tests/core_cli_suite/cli_non_interactive_test.rs—#[cfg(unix)]on the branch-add journey test and its only-here import.crates/tracedecay-global-db/src/schema_contract/invariants/triggers.rs— drop the orphanedhotpath::measureattributes.Not changed: traversal/authentication checks, rebase reconciliation, the checked-in evaluator object pack, labels or historical commits.
Test plan
cargo test -p tracedecay-runtime-core --lib git::— 13 passed (incl. the new spelling tests)cargo test -p tracedecay-runtime-core --test git_repository_authority— 15 passedcargo test -p tracedecay-code-index-runtime --lib code_index_scheduler::tests::real_rebase_reconcile_matches_clean_scan— passes with default config and withGIT_CONFIG_GLOBAL→core.autocrlf=true; fails without the pin under that config (negative control)cargo test -p tracedecay --lib mcp::server::host_admission_tests— 24 passedcargo test -p tracedecay --lib daemon::pr_autotrack— 24 passedcargo test -p tracedecay-cli --test core_cli_suite branch_add_seals— passes on Linuxslice-tests (hotpath on)command verbatim:HOTPATH_METRICS_SERVER_OFF=1 cargo test -p tracedecay-rusqlite-runtime -p tracedecay-global-db --locked --features tracedecay-rusqlite-runtime/hotpath,tracedecay-global-db/hotpath,tracedecay-global-db/test-helpers— compiles and passescargo clippy --testsontracedecay-runtime-core,tracedecay-code-index-runtime,tracedecay,tracedecay-cli— no warningscargo fmt --all -- --check,npm run lint:commit— cleanChecklist
CHANGELOG.mdupdated (test/CI-only fix plus an internal helper; no user-facing change).envfiles includedTargets
codex/tracedecay-total-redesign-plan-reopened(#707). #707 stays draft; nothing here undrafts or merges it.