fix(issue): removeIssueLabel rejects labels attached to the issue (#48) - #52
Conversation
`huly issue label remove` was looking up the label in the workspace-level `tags:class:TagElement` catalog first and throwing `label <name> not found` when no catalog entry existed, even when the label was attached to the issue (e.g. labels imported via another client or seeded outside the catalog). Query `tags:class:TagReference` directly by `attachedTo = issue._id` + `title = <name>`, which mirrors how labels are actually stored on the issue. Added 4 unit tests covering the original repro, orphan-label removal, and the not-attached / not-found error paths.
|
Warning Review limit reached
Next review available in: 6 minutes Limit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?Wait for the limit to reset, then comment An organization admin can change what happens after included review limits in Billing. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📜 Recent review details⏰ Context from checks skipped due to timeout. (1)
🔇 Additional comments (4)
📝 WalkthroughWalkthroughThe CLI now removes issue labels through issue-specific ChangesIssue label removal
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This localized fix removes attached labels without requiring a catalog entry while preserving the not-attached error behavior; no actionable merge-blocking risk remains after normal checks and review. Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| if (!tag) throw new CliError(ExitCode.NotFound, `label ${labelName} not found`) | ||
| const refs = (await client.findAll('tags:class:TagReference' as Ref<Class<Doc>>, { | ||
| const tagRefClass = 'tags:class:TagReference' as Ref<Class<Doc>> | ||
| const refs = (await client.findAll(tagRefClass, { |
There was a problem hiding this comment.
SUGGESTION: The new findAll query no longer filters by collection: 'labels'. The old code filtered by tag: tag._id (TagElement ID), which would only match references to that specific TagElement. The new code filters by title: labelName, which could match TagReferences in other collections (e.g., 'components') attached to the same issue. If such a colliding reference exists, the subsequent removeCollection call (which targets 'labels') would fail or remove the wrong reference, and the loop would abort partway through. Consider adding collection: 'labels' to the query filter so orphan-label removal stays scoped to the label collection.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Overview
The previous review's SUGGESTION (missing Files Reviewed (2 files)
Previous Review Summary (commit 0fd0d33)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 0fd0d33)Status: 1 Issue Found | Recommendation: Address before merge Overview
Issue Details (click to expand)SUGGESTION
Files Reviewed (3 files)
Reviewed by minimax-m3 · Input: 23.2K · Output: 2.2K · Cached: 131.3K |
Address PR #52 review feedback. The previous fix queried `tags:class:TagReference` by `attachedTo` + `title` alone, which could match references in other collections (`'components'`, etc.) attached to the same issue if they shared a title. The subsequent `removeCollection` call targeting `'labels'` would then fail or remove the wrong reference, aborting the loop partway through. Add `collection: 'labels'` to the `findAll` filter so orphan-label removal stays scoped to the label collection. Add a regression test that verifies a non-label collection reference with the same title is left untouched.
|
Addressed review feedback: added
|
1 participant
Fixes #48.
Bug
`huly issue label remove` rejected labels that were actually attached to the issue with `label not found`. The implementation first looked up the label in the workspace-level `tags:class:TagElement` catalog and bailed out if no catalog entry existed — but a label can be attached to an issue even when no matching `TagElement` row is in the catalog (e.g. imported from another client, seeded outside the catalog, or written via raw tx).
Repro (before fix)
```
$ huly issue label add HULY-4 --label security --label publication --label telemetry
$ huly issue get HULY-4 --json | jq .labels
["security","publication","telemetry"]
$ huly issue label remove HULY-4 --label publication
✗ error [2] label publication not found
```
Fix
Query `tags:class:TagReference` directly by `attachedTo = issue._id` + `title = `, which mirrors how labels are actually stored on the issue. The `TagReference` already carries `title`, so the catalog lookup is unnecessary. The "label not on issue" error message is preserved for the genuine not-attached case.
Tests
Added 4 unit tests in `packages/cli/src/resources/issue.test.ts`:
Verification
Acceptance criteria