Conversation
FLARE_GIT_SHIM_DEPTH counted total process nesting rather than actual self-recursion, since the incremented value was inherited unchanged into the real git child (and anything it spawns). A long-lived process like cargo test --workspace accumulates depth across unrelated git invocations until the guard trips on legitimate usage -- reproduced identically on origin/master (8 failing tests), passing fine in CI where no shim is installed. Reset the counter immediately before handing off to the real binary so each real-git subtree starts its own fresh budget, while still keeping the guard active for this shim's own internal git shell-outs (branch/ classify/snapshot) that happen before that handoff. Fixes #379.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe git shim adds an ChangesReal git delegation
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
FLARE_GIT_SHIM_DEPTHwas meant to guard against the shim resolvinggitback to itself, but it counted total process nesting instead — the incremented value was inherited unchanged into the real git child process (and anything it spawns), so it kept accumulating across unrelated git invocations within a single long-lived process tree.cargo test --workspacelocally fails 8 tests that shell out to git, tripping the guard on legitimate usage; reproduces identically onorigin/master, and passes fine in CI (no shim installed there).exec_real()clearsRECURSION_ENVimmediately before handing off to the real git binary, so each real-git subtree gets a fresh budget. The guard still applies to this shim's own internalgitshell-outs (branch/classify/snapshot) made before that handoff, so genuine self-recursion is still caught.Closes agentflare item #379.
Test plan
cargo test --workspace— 866/866 passed in the main binary (previously 8 failing), no "recursion guard tripped" anywhere in the runcargo clippy --workspace --all-targets --all-features -- -D warnings -A unsafe_code -A clippy::pedantic— cleancargo fmt --check— cleanSummary by CodeRabbit