Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/).
- **`huly issue create` human output printed the raw UUID** instead of
`TSK-1`. Now re-fetches the issue to display the assigned identifier, and
falls back to the UUID if the local server hasn't assigned one. (#1.10)
- **`huly issue label remove` rejected labels that WERE attached to the
issue** with `label <name> not found`. The implementation first looked up
the label in the workspace-level `tags:class:TagElement` catalog and
bailed out if it could not be resolved there — but a label can be
attached to an issue even when no matching catalog entry exists. Now
looks up `tags:class:TagReference` directly by
`attachedTo = issue._id` + `title = <name>`, mirroring how the labels are
actually stored. (#48)

### Changed

Expand Down
130 changes: 130 additions & 0 deletions packages/cli/src/resources/issue.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
seedDefaultPriorities,
validateRelationType,
previewDelete,
removeIssueLabel,
} from './issue.js'
import { CliError, ExitCode } from '../output/errors.js'
import { fakePlatformClient } from '../__tests__/fakePlatformClient.js'
Expand Down Expand Up @@ -379,3 +380,132 @@ describe('previewDelete', () => {
expect(parsed[0].relations).toBe(1)
})
})

describe('removeIssueLabel', () => {
beforeEach(() => {
mockClient.current = fakePlatformClient()
vi.spyOn(console, 'log').mockImplementation(() => {})
vi.spyOn(console, 'error').mockImplementation(() => {})
})

it('removes a label that is attached to the issue, even if the workspace TagElement catalog is empty (issue #48)', async () => {
mockClient.current!.state.docs.push({
_id: 'HULY-4',
_class: 'tracker:class:Issue',
space: 'p-1',
title: 'demo',
} as never)
mockClient.current!.state.docs.push({
_id: 'tag-ref-1',
_class: 'tags:class:TagReference',
space: 'p-1',
attachedTo: 'HULY-4',
attachedToClass: 'tracker:class:Issue',
collection: 'labels',
tag: 'tag-el-1',
title: 'publication',
color: 0,
} as never)
mockClient.current!.state.docs.push({
_id: 'tag-ref-2',
_class: 'tags:class:TagReference',
space: 'p-1',
attachedTo: 'HULY-4',
attachedToClass: 'tracker:class:Issue',
collection: 'labels',
tag: 'tag-el-2',
title: 'security',
color: 0,
} as never)
await removeIssueLabel('HULY-4', 'publication', { json: true })
expect(mockClient.current!.state.collectionRemoves).toEqual([
expect.objectContaining({
id: 'tag-ref-1',
collection: 'labels',
parent: 'HULY-4',
}),
])
const remaining = mockClient.current!.state.docs.filter(
(d) => d._class === 'tags:class:TagReference' && (d as Record<string, unknown>).attachedTo === 'HULY-4',
)
expect(remaining.map((d) => (d as Record<string, unknown>).title)).toEqual(['security'])
})

it('still removes the label even when no matching TagElement exists in the workspace catalog', async () => {
mockClient.current!.state.docs.push({
_id: 'HULY-5',
_class: 'tracker:class:Issue',
space: 'p-1',
} as never)
mockClient.current!.state.docs.push({
_id: 'tag-ref-9',
_class: 'tags:class:TagReference',
space: 'p-1',
attachedTo: 'HULY-5',
attachedToClass: 'tracker:class:Issue',
collection: 'labels',
tag: 'ghost-tag-id',
title: 'orphan-label',
color: 0,
} as never)
await removeIssueLabel('HULY-5', 'orphan-label', { json: true })
expect(mockClient.current!.state.collectionRemoves).toHaveLength(1)
expect(mockClient.current!.state.collectionRemoves[0].id).toBe('tag-ref-9')
})

it('throws NotFound with a clear message when the label is not attached to the issue', async () => {
mockClient.current!.state.docs.push({
_id: 'HULY-6',
_class: 'tracker:class:Issue',
space: 'p-1',
} as never)
await expect(removeIssueLabel('HULY-6', 'ghost', { json: true })).rejects.toMatchObject({
code: ExitCode.NotFound,
message: /label ghost not on issue HULY-6/,
})
expect(mockClient.current!.state.collectionRemoves).toHaveLength(0)
})

it('throws NotFound when the issue itself does not exist', async () => {
await expect(removeIssueLabel('HULY-999', 'whatever', { json: true })).rejects.toMatchObject({
code: ExitCode.NotFound,
message: /issue HULY-999 not found/,
})
})

it('only removes references from the labels collection, not other collections sharing the same title', async () => {
mockClient.current!.state.docs.push({
_id: 'HULY-7',
_class: 'tracker:class:Issue',
space: 'p-1',
} as never)
mockClient.current!.state.docs.push({
_id: 'tag-ref-labels',
_class: 'tags:class:TagReference',
space: 'p-1',
attachedTo: 'HULY-7',
attachedToClass: 'tracker:class:Issue',
collection: 'labels',
tag: 'tag-1',
title: 'shared-title',
color: 0,
} as never)
mockClient.current!.state.docs.push({
_id: 'tag-ref-components',
_class: 'tags:class:TagReference',
space: 'p-1',
attachedTo: 'HULY-7',
attachedToClass: 'tracker:class:Issue',
collection: 'components',
tag: 'tag-2',
title: 'shared-title',
color: 0,
} as never)
await removeIssueLabel('HULY-7', 'shared-title', { json: true })
expect(mockClient.current!.state.collectionRemoves).toEqual([
expect.objectContaining({ id: 'tag-ref-labels', collection: 'labels' }),
])
const componentsRef = mockClient.current!.state.docs.find((d) => d._id === 'tag-ref-components')
expect(componentsRef).toBeDefined()
})
})
15 changes: 6 additions & 9 deletions packages/cli/src/resources/issue.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1373,19 +1373,16 @@ export async function removeIssueLabel(
})
const issue = await client.findOne(CLASS.Issue as Ref<Class<Issue>>, { _id: issueId as Ref<Issue> })
if (!issue) throw new CliError(ExitCode.NotFound, `issue ${ref} not found`)
const tagClass = 'tags:class:TagElement' as Ref<Class<Doc>>
const tag = (await client.findAll(tagClass, { title: labelName }))[0] as
| (Doc & { _id: Ref<Doc> })
| undefined
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, {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

attachedTo: issue._id,
tag: tag._id,
})) as Doc[]
collection: 'labels',
title: labelName,
})) as Array<Doc & { _id: Ref<Doc> }>
if (refs.length === 0) throw new CliError(ExitCode.NotFound, `label ${labelName} not on issue ${ref}`)
for (const r of refs) {
await client.removeCollection(
'tags:class:TagReference' as Ref<Class<Doc>>,
tagRefClass,
issue.space as Ref<Space>,
r._id,
issue._id,
Expand Down
Loading