Repository navigation
test: isolate benchmark Git fixtures from hook environment - #51
Conversation
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. |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: 6860ce3e-b959-441c-bdc9-eedf6b88b448) |
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Isolate fixture Git commands and check repository integrity tests/test_bench_integrity.py |
Shared helpers filter inherited Git variables for fixture commands and patch application. A regression test sets Git overrides and checks that the sentinel repository remains unchanged. |
Priority:
Estimated code review effort: 2 (Simple) | ~8 minutes
Change: Other
Merge Risk: 
51457
-
No merge-blocking issue is identified in the fixture isolation change. Merge after normal checks pass.
Pre-merge checks: 
-
Check name Status Explanation Resolution Docstring Coverage Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat… Write docstrings for the functions missing them to satisfy the coverage threshold. -
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly and concisely describes the main change: isolating benchmark Git fixtures from inherited hook environment variables. Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 3 functions across 1 files.
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
-
Finishing Touches: 💡 1
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 51457a2de2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| import importlib.util | ||
| import io | ||
| import json | ||
| import os |
There was a problem hiding this comment.
Commit f2f6629 records both its author and committer as Codex <codex@openai.com>. This directly violates the root AGENTS.md convention prohibiting AI-agent credit in Git authorship metadata. Submit the change with appropriate human authorship and keep agent activity in internal receipts, handoffs, or review notes instead. This finding concerns the commit metadata, not the highlighted import.
AGENTS.md reference: AGENTS.md:L47-L47
Useful? React with 👍 / 👎.
When this suite runs from another repository's Git hook, inherited Git environment variables redirect temporary fixture commands into the invoking repository. This caused
git initfailures and altered the caller's Git configuration and index during the agents pre-commit gate.Isolate every Git command in the benchmark integrity fixture from inherited
GIT_*variables, including binary patch application. Add a sentinel-repository regression that injects hook-style Git directory, worktree, index, common-directory, and configuration variables, then verifies the sentinel configuration, index, and files are preserved.Validation: all 503 stdlib tests pass (45 skipped), all 15 benchmark integrity tests pass, and targeted Ruff plus diff checks pass. Runtime agent behavior is unchanged. Independent review and CI remain pending. The agents branch refresh remains held until this prerequisite can be used safely.
Note
Low Risk
Test-only changes to subprocess Git invocation; no production runtime or auth/data paths affected.
Overview
Benchmark integrity tests that spin up temporary Git repos were picking up
GIT_*variables from the invoking environment (e.g. pre-commit hooks), which could redirectgit init/index operations into the real repository and break or corrupt the caller’s Git state.This PR adds
fixture_git_env()andfixture_git()to run fixture Git commands (andgit apply --binary) with inheritedGIT_*variables stripped, refactors the existing index-preservation test to use them, and addstest_git_fixture_ignores_inherited_hook_context, which injects hook-style Git directory/worktree/index/config env vars and asserts a sentinel repo’s config, index, and tree stay unchanged while the full index fixture test runs.Reviewed by Cursor Bugbot for commit 51457a2. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit