feat(labels): MCP list/update/delete + project-scope enforcement on attach - #205
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds backend label scope validation, per-scope automatic sort ordering, and MCP label create/list/update/delete operations. Tests cover ordering, CRUD roundtrips, missing identifiers, and cross-project or workspace rejection. ChangesLabel management
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant MCPClient
participant mcp_server
participant LabelBackend
participant Database
MCPClient->>mcp_server: Submit label action
mcp_server->>LabelBackend: Validate scope and execute operation
LabelBackend->>Database: Read or persist label state
Database-->>LabelBackend: Return operation result
LabelBackend-->>mcp_server: Return label or validation error
mcp_server-->>MCPClient: Return MCP response
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: 5
🤖 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 @.claude/skills/pm/reference/rubric.md:
- Around line 11-12: Update the effort definition in the rubric so the value
used by the RICE calculation is 1 for tiny work and 5 for large work, either by
reversing the documented scale or by converting existing estimates before
division. Preserve the UNESTIMATED behavior for unknown effort.
In `@crates/agentflare-backend/src/label.rs`:
- Around line 68-90: Update create and next_sort_order so automatic sort-order
calculation and label insertion occur within a single SQLite write transaction,
preventing concurrent creates from observing the same maximum; preserve
caller-supplied sort_order behavior and commit the transaction before returning
the created Label. Add a regression test that performs concurrent label creation
and verifies automatic sort orders are uniquely and deterministically allocated.
In `@scripts/pm-fixtures.sh`:
- Around line 20-29: Update the fixture updates in the bd function and its
callers so they target exact seeded item IDs or an explicit fixture marker
rather than mutable names, while retaining the project constraint and adding
deleted_at IS NULL. Ensure each intended FIX fixture is updated without
modifying real or soft-deleted items.
- Around line 25-29: Update the fixture setup around the bd function to validate
that the expected active rows exist before applying updates, and fail the script
when any required fixture row is missing. Ensure zero-row updates cannot produce
a successful “backdated” result, while preserving the existing date assignments
for FIX-08, FIX-09, FIX-05, FIX-06, and FIX-03.
In `@src/mcp_server.rs`:
- Around line 2419-2432: The label update and delete branches in the MCP request
handler must enforce the resolved project scope before mutating by ID. Load each
label by the supplied ID, verify its project matches the resolved project used
by create/list, and only then call `agentflare_backend::label::update` or
`delete`; return the existing not-found or invalid-parameter error for
mismatches. Add tests covering cross-project update and delete attempts.
🪄 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: f61cf783-3dce-4077-ba5b-e53cbb754a21
📒 Files selected for processing (8)
.claude/skills/pm/SKILL.md.claude/skills/pm/reference/read-recipe.md.claude/skills/pm/reference/rubric.mdcrates/agentflare-backend/src/error.rscrates/agentflare-backend/src/item.rscrates/agentflare-backend/src/label.rsscripts/pm-fixtures.shsrc/mcp_server.rs
Comment on lines
+11
to
+12
| - Effort — size. From a `size:S|M|L` label or an estimate in the body. | ||
| 1 = large/expensive … 5 = tiny. If unknown, mark UNESTIMATED (see below). |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Invert the effort scale or formula.
RICE divides by Effort, but the stated scale gives large work 1 and tiny work 5; this ranks larger work above otherwise identical tiny work. Define effort as 1=tiny through 5=large, or convert the value before division.
🤖 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 @.claude/skills/pm/reference/rubric.md around lines 11 - 12, Update the
effort definition in the rubric so the value used by the RICE calculation is 1
for tiny work and 5 for large work, either by reversing the documented scale or
by converting existing estimates before division. Preserve the UNESTIMATED
behavior for unknown effort.
Comment on lines
+20
to
+29
| # Guard: refuse if any FIX-* row lives outside this project (name collision). | ||
| STRAY=$(sqlite3 "$DB" "SELECT count(*) FROM items WHERE name LIKE 'FIX-%' AND project_id<>'$PID';") | ||
| [ "$STRAY" = "0" ] || { echo "refusing: $STRAY FIX-* items outside project $PID" >&2; exit 1; } | ||
|
|
||
| now=$(date +%s); day=86400 | ||
| bd() { sqlite3 "$DB" "UPDATE items SET updated_at=$1 WHERE project_id='$PID' AND name IN ($2);"; } | ||
| bd $((now-25*day)) "'FIX-08','FIX-09'" # stale backlog >14d | ||
| bd $((now-3*day)) "'FIX-05'" # completed this week | ||
| bd $((now-10*day)) "'FIX-06'" # completed prior week | ||
| bd $((now-20*day)) "'FIX-03'" # started but stuck |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Do not target fixture rows by mutable names alone.
Line 25 updates every matching item in the project; the schema does not enforce unique item names, and the predicate also omits deleted_at IS NULL. A real item named FIX-05 or a soft-deleted fixture can therefore be modified, contradicting the script’s safety guarantee. Use exact seeded item IDs or an explicit fixture marker, and at minimum exclude deleted rows.
🤖 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 `@scripts/pm-fixtures.sh` around lines 20 - 29, Update the fixture updates in
the bd function and its callers so they target exact seeded item IDs or an
explicit fixture marker rather than mutable names, while retaining the project
constraint and adding deleted_at IS NULL. Ensure each intended FIX fixture is
updated without modifying real or soft-deleted items.
Comment on lines
+25
to
+29
| bd() { sqlite3 "$DB" "UPDATE items SET updated_at=$1 WHERE project_id='$PID' AND name IN ($2);"; } | ||
| bd $((now-25*day)) "'FIX-08','FIX-09'" # stale backlog >14d | ||
| bd $((now-3*day)) "'FIX-05'" # completed this week | ||
| bd $((now-10*day)) "'FIX-06'" # completed prior week | ||
| bd $((now-20*day)) "'FIX-03'" # started but stuck |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Fail when expected fixture rows are missing.
Each UPDATE can affect zero rows while the script still exits successfully and prints “backdated.” Preflight or validate the expected active-row counts so incomplete PM fixtures cannot be mistaken for a successful setup.
🤖 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 `@scripts/pm-fixtures.sh` around lines 25 - 29, Update the fixture setup around
the bd function to validate that the expected active rows exist before applying
updates, and fail the script when any required fixture row is missing. Ensure
zero-row updates cannot produce a successful “backdated” result, while
preserving the existing date assignments for FIX-08, FIX-09, FIX-05, FIX-06, and
FIX-03.
…ct scope on attach - label MCP tool: add list/update/delete actions; pass parent_id/sort_order on create - item::add_label: reject labels outside the item's project/workspace scope, guarding both the add_label action and item.create's label_ids path; new Error::Validation maps to MCP invalid_params - label::create: auto-append sort_order (max+10000 per scope) when unset, mirroring Plane
…cation Address CodeRabbit review on #205: - MCP label update/delete now verify the label belongs to the resolved project before mutating by ID (closes a cross-project mutation hole) - label::create allocates auto-append sort_order and inserts in one transaction so concurrent creates can't observe the same MAX
getappz
force-pushed
the
feat/label-mcp-actions
branch
from
July 16, 2026 08:12
3346608 to
53f9c9c
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
crates/agentflare-backend/src/item.rs (1)
791-831: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winTest rejection of a workspace-level label from another workspace.
This only exercises
project_id: Nonewhen workspace IDs match. Add a workspace-level label fromtest2, attach it to thetest1item, and assertValidationplus no persisted association.🤖 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 `@crates/agentflare-backend/src/item.rs` around lines 791 - 831, The test add_label_accepts_workspace_level_label_in_same_workspace only covers a valid same-workspace label. Extend it by creating a workspace-level label associated with test2, attempting to attach it to the existing test1 item, and asserting the operation returns Validation; then verify list_labels still contains no persisted association for that rejected label.
🤖 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/agentflare-backend/src/item.rs`:
- Around line 372-375: Update the validation error in the !in_scope branch of
the item label-scope check to use scope-neutral wording, such as reporting that
the label is outside the item’s scope, so both project and workspace mismatches
are accurately described.
---
Nitpick comments:
In `@crates/agentflare-backend/src/item.rs`:
- Around line 791-831: The test
add_label_accepts_workspace_level_label_in_same_workspace only covers a valid
same-workspace label. Extend it by creating a workspace-level label associated
with test2, attempting to attach it to the existing test1 item, and asserting
the operation returns Validation; then verify list_labels still contains no
persisted association for that rejected label.
🪄 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: 4c2430ee-390e-482b-8c4a-9c4d1ed3cd63
📒 Files selected for processing (4)
crates/agentflare-backend/src/error.rscrates/agentflare-backend/src/item.rscrates/agentflare-backend/src/label.rssrc/mcp_server.rs
🚧 Files skipped from review as they are similar to previous changes (3)
- crates/agentflare-backend/src/error.rs
- crates/agentflare-backend/src/label.rs
- src/mcp_server.rs
…tral error msg Address CodeRabbit re-review: make the add_label scope-mismatch message scope-neutral (fires for workspace mismatches too) and cover the workspace-level-label-from-another-workspace rejection path.
4 tasks
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.
What
Extends the label subsystem (Plane-modeled) in the task-management backend:
(a) MCP
labeltool — full CRUDlist,update,deleteactions (wascreate-only).createnow passesparent_id(nesting/groups) andsort_orderthrough — previously hardcodedNone, so those schema columns were unreachable via MCP.(b) Attach-scope enforcement
item::add_labelnow rejects a label that isn't in the item's scope: a project-scoped label must share the item's project; a workspace-level label (project_id NULL) must share the item's workspace.add_labelaction anditem.create'slabel_idspath (which routes throughadd_label).Error::Validationvariant → MCPinvalid_params.label_idcould be cross-attached).(c)
sort_orderauto-appendmax(sort_order) + 10000within their scope (65535 when empty), matching Plane'sLabel.save(). Explicitsort_orderis still respected.Tests
agentflare-backend: 67 lib tests pass (+3: cross-project reject, workspace-level accept, auto-append).clippy -D warningsclean onagentflare-backend.Note
Pre-existing
clippy -D warningsfailures inponytailandagentflare-caveman(newer clippy pedantic lints, untouched crates) are unrelated to this change.Summary by CodeRabbit
parent_id) and manual ordering (sort_order).