refactor(mcp): split item_inner's dispatch arms into src/mcp_server/item.rs

[getappz]

Summary

• item_inner was a single 411-line function (the file's largest, and its top complexity hotspot per both ast-grep line-span scanning and lean-ctx's own health scoring) dispatching all 13 item actions inline.
• Moved each arm's body verbatim into its own item_<action> method in a new src/mcp_server/item.rs submodule; item_inner is now just the match dispatch. No behavior change.

Test plan

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

Summary by CodeRabbit

• Bug Fixes

  • Improved validation for item requests, including required fields and pagination values.
  • Improved reliability of item claiming, completion, and release workflows to prevent race conditions.
  • Standardized error handling and response formatting across item actions.

• New Features

  • Added support for attaching and removing labels from items, with clear operation results.

[coderabbitai]

📝 Walkthrough

Walkthrough

Item MCP handling is extracted from item_inner into src/mcp_server/item.rs. Dedicated methods now cover CRUD, claims, completion, cancellation, and label operations while preserving validation, backend access, worktree workflows, and JSON responses.

Changes

Item MCP handlers

Layer / File(s)	Summary
Core item handlers src/mcp_server.rs, src/mcp_server/item.rs	The server declares the item submodule, delegates ItemRequest actions to dedicated methods, and implements CRUD validation, backend operations, pagination, state updates, deletion, and JSON serialization.
Claim and completion workflows src/mcp_server/item.rs	Claim, heartbeat, release, and done handlers coordinate backend locking with worktree creation and pull-request publication.
Cancellation and label actions src/mcp_server/item.rs	Cancellation updates item state and releases the caller’s claim, while label handlers add or remove associations and return operation status.

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

Possibly related PRs

• getappz/agentflare#162: Adds related MCP work-item and label handlers in the same server surface.
• getappz/agentflare#177: Adjusts the backend database closure type in the claim handling logic.
• getappz/agentflare#178: Changes overlapping item.list handling and adds the item.cancel branch.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the refactor of moving item_inner dispatch arms into src/mcp_server/item.rs.
Description check	✅ Passed	The description includes the required Summary and Test plan sections and is mostly complete, with only the Notes for reviewers section omitted.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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 refactor/split-item-inner-dispatch

---

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

[coderabbitai]

🧹 Nitpick comments (3)

src/mcp_server/item.rs (3)

188-245: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Progress-sender fallback duplicated with item_done.

The PROGRESS_SENDER.try_with(|ps| f(item, &repo_root, target, ps.as_ref())).unwrap_or_else(|_| f(item, &repo_root, target, None)) fallback (lines 218-226) is repeated almost verbatim in item_done below (lines 313-321), only swapping create_worktree for push_and_open_pr. Worth factoring into a shared helper that takes the worktree fn as a parameter.

🤖 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 188 - 245, Factor the repeated
PROGRESS_SENDER.try_with fallback pattern from item_claim and item_done into a
shared helper that accepts the worktree operation as a parameter. Update
item_claim’s create_worktree call and item_done’s push_and_open_pr call to use
the helper while preserving their existing arguments and fallback behavior.

---

278-348: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Mirrors the same progress-sender fallback in item_claim.

Lines 313-321 duplicate the PROGRESS_SENDER.try_with(...).unwrap_or_else(...) fallback already flagged in item_claim above, just calling push_and_open_pr instead of create_worktree. Same extraction opportunity applies here.

🤖 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 278 - 348, Extract the duplicated
PROGRESS_SENDER.try_with(...).unwrap_or_else(...) fallback in item_done into the
shared helper introduced for item_claim, preserving the existing
push_and_open_pr invocation and fallback behavior. Update item_done to call that
helper while keeping the surrounding PR URL handling unchanged.

---

10-51: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consider a shared "required field" helper.

The ok_or_else(...)? + .trim().is_empty() pattern here (name, lines 11-16) repeats near-identically for every required field across all 13 handlers in this file (id in item_get/item_delete/item_claim/etc., label_id in item_add_label/item_remove_label, state_id in item_update_state). A small generic helper would cut this boilerplate significantly.

♻️ Example helper

fn require_field(value: Option<String>, field: &str, action: &str) -> Result<String, ErrorData> {
    let v = value
        .ok_or_else(|| ErrorData::invalid_params(format!("{field} is required for {action}"), None))?;
    if v.trim().is_empty() {
        return Err(ErrorData::invalid_params(format!("{field} is required"), None));
    }
    Ok(v)
}

🤖 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 10 - 51, Introduce a shared
required-field helper near the item handlers that validates an optional string
for both presence and non-blank content, using the field and action to produce
the existing error messages. Replace the repeated validation patterns across
handlers including item_create, item_get/item_delete/item_claim,
item_add_label/item_remove_label, and item_update_state, while preserving each
handler’s current required-field semantics and error behavior.

🤖 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 188-245: Factor the repeated PROGRESS_SENDER.try_with fallback
pattern from item_claim and item_done into a shared helper that accepts the
worktree operation as a parameter. Update item_claim’s create_worktree call and
item_done’s push_and_open_pr call to use the helper while preserving their
existing arguments and fallback behavior.
- Around line 278-348: Extract the duplicated
PROGRESS_SENDER.try_with(...).unwrap_or_else(...) fallback in item_done into the
shared helper introduced for item_claim, preserving the existing
push_and_open_pr invocation and fallback behavior. Update item_done to call that
helper while keeping the surrounding PR URL handling unchanged.
- Around line 10-51: Introduce a shared required-field helper near the item
handlers that validates an optional string for both presence and non-blank
content, using the field and action to produce the existing error messages.
Replace the repeated validation patterns across handlers including item_create,
item_get/item_delete/item_claim, item_add_label/item_remove_label, and
item_update_state, while preserving each handler’s current required-field
semantics and error behavior.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: f18c1ea7-b508-4109-bd4c-cfbf30bf25e1

📥 Commits

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

📒 Files selected for processing (2)

• src/mcp_server.rs
• src/mcp_server/item.rs