feat(item): comma-separated state_group filter + default-branch edit guard

[getappz]

Summary

• item(list)'s state_group param now accepts a comma-separated list (e.g. "backlog,unstarted,started") instead of only a single exact value; /handoff's inbox grammar defaults to that open-states filter so completed/cancelled items don't show up unless the command explicitly says all.
• PreToolUse now hard-blocks Write/Edit/NotebookEdit/ctx_patch/ctx_edit whenever the repo is checked out on its default branch (resolved via origin/HEAD, else main/master, else whatever's actually checked out), redirecting to create a worktree first.
• Consolidated the git-shelling logic duplicated across mcp_server.rs, worktree.rs, hook_redirect.rs, gateway_integrations.rs, review.rs, and cli/review.rs into one src/git.rs module.

Test plan

• cargo test --bin agentflare — 453 passed
• cargo fmt --check
• cargo clippy --bin agentflare — no new warnings

Summary by CodeRabbit

• New Features
  • File-mutating tools are now blocked when running on the repository’s default branch; feature branches remain available for edits.
  • Item lists now accept multiple state_group values via comma-separated input.
  • Handoff inbox prompts now default to active items (while still honoring an explicit all option).
• Bug Fixes
  • Improved reliability of current branch, default branch, and repository-location detection, resulting in more consistent review diffs and Git behavior.
  • More robust detection of whether a Git remote references GitHub.

[coderabbitai]

📝 Walkthrough

Walkthrough

Changes

The PR centralizes Git command handling, updates existing Git consumers, adds default-branch protection for mutating tools, and extends MCP item filtering and inbox prompt defaults to support multiple state groups.

Git workflows and branch protection

Layer / File(s)	Summary
Shared Git primitives and validation src/git.rs, src/main.rs	Adds centralized Git execution, branch/default-branch resolution, repository-root discovery, diff generation, and unit tests.
Git helper adoption across workflows src/worktree.rs, src/review.rs, src/cli/review.rs, src/mcp_server.rs, src/gateway_integrations.rs	Routes existing Git operations through the shared helpers.
Default-branch mutation guard src/hook_redirect.rs	Blocks configured mutating tools on resolved default branches while retaining existing redirect behavior and adding tests.

MCP inbox state-group filtering

Layer / File(s)	Summary
Comma-separated state-group filtering src/mcp_server.rs, src/mcp_server/item.rs	Documents and implements comma-separated state-group filtering for item listing, with multi-group test coverage.
Inbox handoff filtering guidance src/mcp_prompts.rs	Defaults inbox prompts to open states and documents the explicit all behavior.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant PreToolUse
  participant redirect_decision
  participant git
  participant classify
  PreToolUse->>redirect_decision: tool invocation
  redirect_decision->>git: resolve current and default branches
  git-->>redirect_decision: branch context
  redirect_decision->>classify: tool name and branch context
  classify-->>PreToolUse: redirect or branch denial

Loading

Possibly related PRs

• getappz/agentflare#143: Changes related review CLI branch and diff resolution.
• getappz/agentflare#145: Uses the review diff and PR-resolution paths updated here.
• getappz/agentflare#170: Modifies the same hook redirect classification area.

Suggested labels: enhancement, rust

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main changes: comma-separated item filtering and a default-branch edit guard.
Description check	✅ Passed	The PR includes a solid Summary and Test plan; only the Notes for reviewers section is missing.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/item-list-state-group-filter

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/hook_redirect.rs (1)

109-119: 🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win

Branch resolution runs on every tool call, not just mutating ones.

current_branch() and default_branch() are computed unconditionally inside the timed closure before classify ever checks whether tool_name is in MUTATING_TOOLS. Per src/hook.rs's pre_tool_use, redirect_decision runs on every PreToolUse invocation — so every Read, Bash, Grep, etc. call now spawns up to ~5 git subprocesses (1 for current_branch, up to 4 inside resolve_default_branch's fallback chain) purely to gate a check that only applies to a small fixed set of mutating tools.

⚡ Proposed fix — gate branch resolution behind the MUTATING_TOOLS check

     decide_with_timeout(GATING_TIMEOUT, move || {
-        let current = current_branch();
-        let default = default_branch();
+        let (current, default) = if MUTATING_TOOLS.contains(&tool_name.as_str()) {
+            (current_branch(), default_branch())
+        } else {
+            (None, None)
+        };
         let reason = classify(
             &tool_name,
             tool_input.as_ref(),
             (current.as_deref(), default.as_deref()),
         )?;

🤖 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/hook_redirect.rs` around lines 109 - 119, Update redirect_decision and
classify so tool mutability is checked before calling current_branch() or
default_branch(); return None immediately for tools outside MUTATING_TOOLS, and
resolve branch information only for mutating tools before continuing the
existing classification flow.

🤖 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/hook_redirect.rs`:
- Around line 64-73: Update branch_guard_reason_for to compare the branch only
with the resolved default branch. Remove the unconditional "main" and "master"
checks, while preserving the fallback behavior only when default is None if
resolution failure requires it; otherwise use the existing default value
directly.

---

Outside diff comments:
In `@src/hook_redirect.rs`:
- Around line 109-119: Update redirect_decision and classify so tool mutability
is checked before calling current_branch() or default_branch(); return None
immediately for tools outside MUTATING_TOOLS, and resolve branch information
only for mutating tools before continuing the existing classification flow.

🪄 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: 51797363-3068-4b7d-9593-62ac9789b3e0

📥 Commits

Reviewing files that changed from the base of the PR and between 5ece9c3 and da93ccc.

📒 Files selected for processing (9)

• src/cli/review.rs
• src/gateway_integrations.rs
• src/git.rs
• src/hook_redirect.rs
• src/main.rs
• src/mcp_prompts.rs
• src/mcp_server.rs
• src/review.rs
• src/worktree.rs

[coderabbitai]

🧹 Nitpick comments (1)

src/mcp_server/item.rs (1)

66-132: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Comma-separated state_group filter looks correct.

group.split(',').map(str::trim).collect() plus wanted.contains(...) correctly matches any of the listed groups and stays backward-compatible with single-value filters; verified against the new multi-group test in mcp_server.rs.

One optional gap: an unrecognized/misspelled group value (e.g. "backlgo") silently yields an empty result rather than an error, since unwrap_or(false) excludes non-matching items with no feedback. This mirrors the prior single-value behavior, so it's not a regression, but a small validation step against the documented set (backlog|unstarted|started|completed|cancelled|triage) would surface typos instead of a confusing empty list.

♻️ Optional validation

+const VALID_STATE_GROUPS: &[&str] = &["backlog", "unstarted", "started", "completed", "cancelled", "triage"];
+
 if let Some(group) = &req.state_group {
     let wanted: Vec<&str> = group.split(',').map(str::trim).collect();
+    if let Some(bad) = wanted.iter().find(|g| !VALID_STATE_GROUPS.contains(g)) {
+        return Err(ErrorData::invalid_params(
+            format!("unknown state_group '{bad}' — expected one of backlog|unstarted|started|completed|cancelled|triage"),
+            None,
+        ));
+    }
     items.retain(|i| {

🤖 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/mcp_server/item.rs` around lines 66 - 132, Optionally validate each
trimmed value in req.state_group against the documented groups backlog,
unstarted, started, completed, cancelled, and triage before filtering in
item_list. Return an invalid-params ErrorData for any unrecognized value;
preserve the existing comma-separated matching behavior for valid groups.

🤖 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.

Nitpick comments:
In `@src/mcp_server/item.rs`:
- Around line 66-132: Optionally validate each trimmed value in req.state_group
against the documented groups backlog, unstarted, started, completed, cancelled,
and triage before filtering in item_list. Return an invalid-params ErrorData for
any unrecognized value; preserve the existing comma-separated matching behavior
for valid groups.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: ab56b2b4-801d-48d6-8b18-b6629b6a319b

📥 Commits

Reviewing files that changed from the base of the PR and between fa83553 and 0086228.

📒 Files selected for processing (10)

• src/cli/review.rs
• src/gateway_integrations.rs
• src/git.rs
• src/hook_redirect.rs
• src/main.rs
• src/mcp_prompts.rs
• src/mcp_server.rs
• src/mcp_server/item.rs
• src/review.rs
• src/worktree.rs

🚧 Files skipped from review as they are similar to previous changes (8)

• src/mcp_prompts.rs
• src/main.rs
• src/cli/review.rs
• src/gateway_integrations.rs
• src/review.rs
• src/git.rs
• src/worktree.rs
• src/hook_redirect.rs