Skip to content

docs: [vent] item(action=\"doctor\", reclaim=true, force=true) with no dry-run/previe… - #625

Merged
getappz merged 1 commit into
masterfrom
task/480-vent-item-action-doctor-reclaim-true-for
Aug 27, 2026
Merged

docs: [vent] item(action=\"doctor\", reclaim=true, force=true) with no dry-run/previe…#625
getappz merged 1 commit into
masterfrom
task/480-vent-item-action-doctor-reclaim-true-for

Conversation

@getappz

@getappz getappz commented Aug 27, 2026

Copy link
Copy Markdown
Owner

Status:

Done. Default is now safe — unscoped force=true reclaim refused unless explicit intent.

  • src/mcp_server/item_doctor.rs: guard — reclaim && force && worktree.is_none() && !repo_wideErr(invalid_params) before any deletion.
  • src/mcp_server/types.rs: new repo_wide field + updated force/worktree descriptions.
  • src/mcp_server.rs: item tool description updated.
  • src/mcp_server/tests/item_doctor_tests.rs: new regression test (refused unscoped force leaves lanes intact; repo_wide=true sweeps all).

Tests: cargo test item_doctor → 4/4 pass (incl. new test); cargo test mcp_server → 247 passed, 0 failed. cargo fmt --check clean. Clippy: no new warnings in changed files.

Concerns:

  • CLI agentflare git doctor --force --reclaim left unscoped — human-in-loop explicit flag, so deliberate. Could add same guard later if desired.
  • Didn't run full workspace test/clippy gate (heavy); change is localized to mcp_server.
  • Not committed (per rules — awaiting your go).

Opened by opencode on flared:c997d745ae66 for item #480 via agentflare.

Summary by CodeRabbit

  • New Features

    • Added an explicit confirmation option for repository-wide forced reclamation.
    • Clarified that targeted worktree cleanup is the preferred scoped operation.
  • Bug Fixes

    • Unscoped forced reclamation is now rejected unless repository-wide scope is explicitly confirmed.
    • Prevented dirty worktrees from being removed when an unsafe request is refused.

**Done.** Default is now safe — unscoped `force=true` reclaim refused unless explicit intent.

- `src/mcp_server/item_doctor.rs`: guard — `reclaim && force && worktree.is_none() && !repo_wide` → `Err(invalid_params)` before any deletion.
- `src/mcp_server/types.rs`: new `repo_wide` field + updated `force`/`worktree` descriptions.
- `src/mcp_server.rs`: `item` tool description updated.
- `src/mcp_server/tests/item_doctor_tests.rs`: new regression test (refused unscoped force leaves lanes intact; `repo_wide=true` sweeps all).

**Tests:** `cargo test item_doctor` → 4/4 pass (incl. new test); `cargo test mcp_server` → 247 passed, 0 failed. `cargo fmt --check` clean. Clippy: no new warnings in changed files.

**Concerns:**
- CLI `agentflare git doctor --force --reclaim` left unscoped — human-in-loop explicit flag, so deliberate. Could add same guard later if desired.
- Didn't run full workspace test/clippy gate (heavy); change is localized to mcp_server.
- Not committed (per rules — awaiting your go).
@coderabbitai

coderabbitai Bot commented Aug 27, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 2e5beb58-687b-4d44-9336-172037accb1b

📥 Commits

Reviewing files that changed from the base of the PR and between 5107f4a and 7ec9b0a.

📒 Files selected for processing (4)
  • src/mcp_server.rs
  • src/mcp_server/item_doctor.rs
  • src/mcp_server/tests/item_doctor_tests.rs
  • src/mcp_server/types.rs

Included review availability: 0 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 1 review per hour.


📝 Walkthrough

Walkthrough

The change adds explicit repo_wide=true confirmation for unscoped forced reclamation. Scoped reclamation continues to use the resolved force flag. Tests verify that refused requests preserve dirty worktrees and confirmed repo-wide requests reclaim them.

Changes

Force-reclaim safety

Layer / File(s) Summary
Request contract and tool documentation
src/mcp_server/types.rs, src/mcp_server.rs
ItemRequest adds the optional repo_wide field and documents the required confirmation. The item tool description matches this behavior.
Force-reclaim validation and regression coverage
src/mcp_server/item_doctor.rs, src/mcp_server/tests/item_doctor_tests.rs
Unscoped reclaim=true with force=true now fails unless repo_wide=true. Tests verify that refused requests preserve dirty worktrees and confirmed requests reclaim them.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 7ec9b

The change narrows unsafe unscoped forced reclaim behavior and adds regression coverage; no actionable merge-blocking risk remains beyond normal checks and review.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 4 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title identifies the affected item doctor operation and the unsafe unscoped force-reclaim scenario. It is related to the main change, although it is truncated and uses a documentation-oriented pre…
Description check ✅ Passed The description explains the change, motivation, implementation, regression coverage, test results, risks, and the limited validation scope. It does not follow the template headings or checkbox format…
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.
Full details: Title check

Explanation

The title identifies the affected item doctor operation and the unsafe unscoped force-reclaim scenario. It is related to the main change, although it is truncated and uses a documentation-oriented prefix for a behavior change.

Full details: Description check

Explanation

The description explains the change, motivation, implementation, regression coverage, test results, risks, and the limited validation scope. It does not follow the template headings or checkbox format exactly, but it provides the required information in substance.

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch task/480-vent-item-action-doctor-reclaim-true-for

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

@getappz
getappz merged commit 00b4d2c into master Aug 27, 2026
18 checks passed
@getappz
getappz deleted the task/480-vent-item-action-doctor-reclaim-true-for branch August 27, 2026 17:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant