feat(git-shim): path-scope enforcement for claims (QuorumGit adoption) - #303
Conversation
Adopt QuorumGit's claim-scope classification for flare-git-shim, closing the top deferred guard gap: all existing branch-protection layers key off host cwd, not changed file paths, which is why opencode has twice edited the canonical checkout instead of its claim's worktree. - claims can now declare path-glob write scopes (`--scope`/MCP `scope` param on claim acquire); unscoped claims (the back-compat default) never deny another agent's unrelated work - flare-git-core::scope::classify_scopes classifies commit/push against live claim scopes: CLEAR/RELATED pass, OVERLAPPING (touches another live claim's declared scope) and OUT_OF_TREE (own claim held but committing from the canonical checkout) deny - flare-git-shim shells out to the new hidden `agentflare git scope-check` CLI command for commit/push (the shim itself has no DB access) and is deliberately fail-closed on any scope-resolution error, unlike this crate's usual fail-open default; existing bypass envs remain the escape hatch - claim acquire warns (non-blocking) when a new scope overlaps another live claim's declared scope -- v1 enforcement is at mutation time only Every denial is audited to ~/.agentflare/audit/git.jsonl, same sink as the rest of the shim's decisions. Not yet wired: the opencode tool.execute.before plugin (hook_redirect.rs) still only checks branch protection, not claim scopes -- left as a fast-follow since it's a softer, best-effort layer and the native shim/hook boundary is where real enforcement lives. Agentflare-Agent: claude-code_2-1-217_agent Agentflare-Branch: item-234-path-scope-claims
|
Warning Review limit reached
Next review available in: 11 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughAdds claim write scopes with SQLite persistence, glob-based overlap classification, acquisition warnings, and enforcement for agent-driven ChangesClaim scope enforcement
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Agent
participant GitShim
participant ScopeCheck
participant ClaimsLedger
participant Git
Agent->>GitShim: git commit or git push
GitShim->>ScopeCheck: request scope-check
ScopeCheck->>ClaimsLedger: load live claims
ScopeCheck->>Git: resolve changed paths
ScopeCheck-->>GitShim: allow or deny JSON
alt denied
GitShim-->>Agent: print reason and exit failure
else allowed
GitShim->>Git: execute operation
end
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/claims.rs (1)
79-101: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winRe-acquiring without
--scopesilently clears a previously declared scope.
acquire_persists_and_overwrites_scopeconfirms this is intentional ("mirroring git_commit's always-overwrite behavior"), but it means any caller that re-acquires an already-held claim (e.g. a heartbeat-style refresh done viaacquireinstead ofheartbeat) without re-supplying--scope/scopesilently disables path-scope enforcement for that claim, with no warning surfaced anywhere. Given this is a security-relevant control, consider at least emitting a warning (similar toscope_overlap_warning) when an existing non-empty scope is being cleared, so agents don't lose enforcement unknowingly.🤖 Prompt for 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. In `@src/claims.rs` around lines 79 - 101, Update claims::acquire so re-acquiring a claim with scope=None detects an existing non-empty scope and emits a warning, using the established scope_overlap_warning behavior or logging path. Preserve the current overwrite semantics for git_commit and scope while ensuring the warning is produced before the existing scope is cleared.
🤖 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 `@src/cli/git.rs`:
- Around line 587-607: Update changed_paths for the "commit" subcommand to
include both staged changes and unstaged working-tree changes, so git commit
-a/--all paths are included in the result passed to run_scope_check. Preserve
the existing cached-diff behavior and combine the two name-only path sets
without omitting either source.
---
Nitpick comments:
In `@src/claims.rs`:
- Around line 79-101: Update claims::acquire so re-acquiring a claim with
scope=None detects an existing non-empty scope and emits a warning, using the
established scope_overlap_warning behavior or logging path. Preserve the current
overwrite semantics for git_commit and scope while ensuring the warning is
produced before the existing scope is cleared.
🪄 Autofix (Beta)
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: f8052b22-7e6b-4416-905c-46e82cb73dfe
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (9)
crates/flare-git-core/src/lib.rscrates/flare-git-core/src/scope.rscrates/flare-git-shim/Cargo.tomlcrates/flare-git-shim/src/main.rssrc/claims.rssrc/cli/claim.rssrc/cli/git.rssrc/mcp_server/claim.rssrc/mcp_server/types.rs
…clear Two CodeRabbit findings on PR #303: - changed_paths() for "commit" only checked `git diff --cached`, so `git commit -a`/`--all` (which implicitly stages+commits tracked modifications without a prior `git add`) bypassed path-scope enforcement entirely. Union staged + working-tree diffs instead. - claims::acquire() always overwrites the scope column (matching git_commit's existing always-overwrite behavior), so re-acquiring an already-held claim without re-supplying --scope silently disabled enforcement with no signal anywhere. Added scope_clear_warning(), wired into both the CLI and MCP acquire handlers alongside the existing overlap warning. Agentflare-Agent: claude-code_2-1-217_agent Agentflare-Branch: item-234-path-scope-claims
1 participant
Summary
Adopts QuorumGit's claim-scope classification for
flare-git-shim, closing the top deferred guard gap: all existing branch-protection layers key off host cwd, not changed file paths, which is why opencode has twice edited the canonical checkout instead of its claim's worktree (agentflare item #234).claim acquire --scope/ MCPscopeparam). Unscoped claims (the back-compat default) never deny another agent's unrelated work — many claims normally coexist per repo, one per target.flare_git_core::scope::classify_scopesclassifies acommit/push's changed paths against live claim scopes:Clear/Relatedpass,Overlapping(touches another live claim's declared scope) andOutOfTree(own claim held but committing from the canonical checkout, not its worktree) deny.flare-git-shimshells out to a new hiddenagentflare git scope-checkCLI command forcommit/push(the shim itself has no DB access to live claims) and is deliberately fail-closed on any scope-resolution error — a documented exception to this crate's usual fail-open default, matching the spec. Existing bypass envs (AGENTFLARE_GIT_BYPASSand friends) remain the escape hatch.claim acquireprints a non-blocking warning when a new scope overlaps another live claim's declared scope — v1 enforcement is at mutation time only, per spec non-goals (no claim-time hard refusal, no server-side pre-receive hook).~/.agentflare/audit/git.jsonl, same sink as the rest of the shim's decisions.Not included (fast-follow): the opencode
tool.execute.beforeplugin (src/hook_redirect.rs) still only checks branch protection, not claim scopes. Acceptance criteria didn't require it, and it's the softer best-effort layer — the shim/hook is where real enforcement lives.Test plan
cargo test --workspace— 732 passed, 0 failedcargo test -p flare-git-core— 82/82 (incl. 10 newscopetests)cargo test -p flare-git-shim— 13/13 integration testscargo fmt --checkclean on touched cratescargo clippy --all-features -- -D warningsclean on touched crates (2 remaining errors indaemon_autostart.rsare pre-existing and untouched by this diff, confirmed viagit diff --stat origin/master)agentflare git scope-check --subcommand commitagainst the real claim DB from inside this PR's own worktree (claim on item fix(flare-output): don't fail RealLlm on a benign stdin broken-pipe #234) — correctly passedSummary by CodeRabbit
New Features
git commitandgit pushoperations.Compatibility