Conversation
…tracked-repo scope flare-git-shim previously applied its full deny policy (protected-branch checkout/switch/delete/rename, trust-root push, plumbing block, worktree block, scope-check) to every git invocation inside an agentflare-tracked repo regardless of whether an agent CLI or a regular human was driving the shell. Only the narrower canonical-detach guard checked agent invocation; the main classify() path did not. - flare-git-shim/main.rs: bail out to real git immediately when classify::agent_invocation_detected() is false, before any classify/scope-check/audit work runs. - flare-git-core/classify.rs: agent_invocation_detected() now delegates to the agent-detector crate (already a dependency, used in provenance.rs) instead of a second hardcoded env-var list -- fixes two wrong var names the old list had (CODEX_CLI_SESSION, GEMINI_SESSION aren't real vars those tools set) and covers a much broader agent catalog. Kept the AGENTFLARE_AGENT OR-clause since agent-detector doesn't know about agentflare's own internal marker. Disabled agent-detector's process-tree feature: this shim runs from inside various agent CLIs by design, so ancestor-process matching would false-positive on a human's terminal merely because their editor (Cursor, VS Code) is an ancestor process. - shim_test.rs: shim() helper now sets CLAUDECODE=1 so existing deny-path tests stay deterministic instead of relying on ambient agent-marker leakage; added two new tests covering human passthrough on the two main deny paths (protected-branch checkout, default-branch push).
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe change centralizes agent detection using the ChangesAgent-aware Git dispatch
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant GitInvocation
participant GitShim
participant AgentDetector
participant RealGit
GitInvocation->>GitShim: Start git command
GitShim->>AgentDetector: Detect agent environment
AgentDetector-->>GitShim: Return agent status
alt Non-agent invocation
GitShim->>RealGit: Execute git immediately
else Agent invocation
GitShim->>GitShim: Run classification and policy checks
end
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/flare-git-shim/tests/shim_test.rs`:
- Around line 38-40: The human-path tests do not fully isolate agent-detection
environment variables. In crates/flare-git-shim/tests/shim_test.rs at lines
312-313, 368-375, and 404-411, add and use a shared helper that clears the child
environment and restores only required variables such as PATH and
AGENTFLARE_HOME_OVERRIDE; update the documentation at lines 38-40 and 341-346 to
describe this controlled environment and apply it consistently to all three
human-path test blocks.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: ac5cd6a0-4f13-4953-84d1-9e4c6c5a42ae
📒 Files selected for processing (4)
crates/flare-git-core/Cargo.tomlcrates/flare-git-core/src/classify.rscrates/flare-git-shim/src/main.rscrates/flare-git-shim/tests/shim_test.rs
getappz
changed the title
Human-path tests only stripped the old, narrow hardcoded agent-marker list before asserting passthrough, but agent_invocation_detected now delegates to the agent-detector crate's much wider catalog. Replace the manual env_remove chains with a shared human_shim helper that strips agent-detector's full env-var catalog (both its 'standard' tier -- AI_AGENT/AGENT -- and its ~26 tool-specific vars) plus AGENTFLARE_AGENT, so these tests actually prove human passthrough instead of only working by ambient coincidence. An env_clear-based approach was tried first but broke real-git resolution (which_in's path filtering needs more of the environment than PATH alone), so this keeps the full inherited environment and only removes known markers.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/flare-git-shim/tests/shim_test.rs`:
- Around line 366-368: Update the comment near the human_shim usage to
accurately state that human_shim inherits the child environment while removing
selected agent markers; do not describe it as clearing the whole environment.
Preserve the explanation for choosing human_shim over reliance on ambient marker
absence.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: c7753ce2-1f03-4a08-9fc9-f82bf4d3703c
📒 Files selected for processing (1)
crates/flare-git-shim/tests/shim_test.rs
Leftover phrasing from the earlier env_clear attempt said human_shim clears the whole environment; it now inherits it and strips known agent markers instead.
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
1 participant
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
main()applied its full deny policy (protected-branch checkout/switch/delete/rename, trust-root push, plumbing block, worktree block, scope-check) to every git invocation inside an agentflare-tracked repo, regardless of whether an agent CLI or a regular human was driving the shell. Only the narrower canonical-detach guard checked agent invocation; the main classify() path did not.main.rsnow bails out to real git immediately whenclassify::agent_invocation_detected()is false, before any classify/scope-check/audit work runs — a human interactively running git in a tracked repo now sees zero restrictions from this shim.agent_invocation_detected()now delegates to theagent-detectorcrate (already a dependency, used inprovenance.rs) instead of a second hardcoded env-var list — fixes two env var names that don't actually exist (CODEX_CLI_SESSION,GEMINI_SESSION) and covers a much broader agent catalog. Kept theAGENTFLARE_AGENTOR-clause sinceagent-detectordoesn't know about agentflare's own internal marker. Disabledagent-detector's process-tree feature, since this shim runs from inside various agent CLIs by design and ancestor-process matching would false-positive on a human's terminal merely because their editor (Cursor, VS Code) is an ancestor process.shim_test.rs:shim()helper now setsCLAUDECODE=1so existing deny-path tests stay deterministic instead of relying on ambient agent-marker leakage; added two new tests covering human passthrough on the two main deny paths (protected-branch checkout, default-branch push).Test plan
cargo build --workspacecargo test -p flare-git-core -p flare-git-shim(162 tests)cargo fmt --checkcargo clippy -p flare-git-core -p flare-git-shim --all-targets -- -A unsafe_code -A clippy::pedantic -D warningsSummary by CodeRabbit
New Features
Bug Fixes