fix(cli): let worktree set --comment "" clear the comment (#37515)

`worktree set` read --comment through getOptionalStringFlag, which turns
an empty value into undefined, so `--comment ""` and `--comment=` sent no
comment field and the stored comment survived while the command reported
ok:true. Keep an explicit empty value for this flag only; the shared
helper and the RPC schema are unchanged.

Fixes #37464
This commit is contained in:
Max Hsu
2026-11-02 20:49:15 +00:00
committed by GitHub
parent 52d65e984f
commit 4eaa12fef2
5 changed files with 78 additions and 1 deletions
+10
View File
@@ -41,6 +41,16 @@ export function getOptionalStringFlag(
return typeof value === 'string' && value.length > 0 ? value : undefined
}
// Clearing flags (e.g. `worktree set --comment ""`) must keep the empty value instead of dropping it.
export function getOptionalStringFlagAllowingEmpty(
flags: Map<string, string | boolean>,
name: string
): string | undefined {
const value = flags.get(name)
rejectValuelessFlag(value, name)
return typeof value === 'string' ? value : undefined
}
export function getPresentStringFlag(
flags: Map<string, string | boolean>,
name: string
+3 -1
View File
@@ -18,6 +18,7 @@ import { RuntimeClientError } from '../runtime-client'
import {
getOptionalPositiveIntegerFlag,
getOptionalStringFlag,
getOptionalStringFlagAllowingEmpty,
getPresentStringFlag,
getRequiredStringFlag
} from '../flags'
@@ -243,7 +244,8 @@ export const WORKTREE_HANDLERS: Record<string, CommandHandler> = {
displayName: getOptionalStringFlag(flags, 'display-name'),
...reviewLinks,
...linearIssueLink,
comment: getOptionalStringFlag(flags, 'comment'),
// An explicit empty value clears the stored comment.
comment: getOptionalStringFlagAllowingEmpty(flags, 'comment'),
workspaceStatus: getOptionalStringFlag(flags, 'workspace-status'),
isUnread,
parentWorktree: await withWorktreeSelectorRecovery(
+23
View File
@@ -559,6 +559,29 @@ describe('orca cli worktree awareness', () => {
)
})
it.each([[['--comment', '']], [['--comment=']]])(
'sends an explicit empty comment for %j so worktree.set clears it',
async (flags) => {
queueFixtures(
callMock,
okFixture('req_clear_comment', {
worktree: buildWorktree('/tmp/repo/child', 'feature/child')
})
)
vi.spyOn(console, 'log').mockImplementation(() => {})
await main(
['worktree', 'set', '--worktree', 'id:repo::/tmp/repo/child', ...flags, '--json'],
'/tmp/repo'
)
expect(callMock).toHaveBeenCalledExactlyOnceWith(
'worktree.set',
expect.objectContaining({ worktree: 'id:repo::/tmp/repo/child', comment: '' })
)
}
)
it.each([
[['--unread', '--read'], 'Choose either --unread or --read'],
[['--read', '--unread'], 'Choose either --unread or --read'],
+2
View File
@@ -24,6 +24,7 @@ export const WORKTREE_SET_COMMAND_SPEC: CommandSpec = {
],
notes: [
'Workspace status ids match the board columns (defaults: todo, in-progress, in-review, completed); custom statuses use their configured id.',
'Pass --comment "" (or --comment=) to clear the comment.',
'Pass --linear-issue null to clear the Linear issue link.',
'Pass --pr null, --gitlab-issue null, or --gitlab-mr null to clear only the matching link.',
'Each link flag updates its own field and leaves unrelated links unchanged. GitLab URLs must match the stored source project or remote; they cannot select a foreign project.'
@@ -31,6 +32,7 @@ export const WORKTREE_SET_COMMAND_SPEC: CommandSpec = {
examples: [
'orca worktree set --worktree active --linear-issue STA-335 --json',
'orca worktree set --worktree active --linear-issue null --json',
'orca worktree set --worktree active --comment "" --json',
'orca worktree set --worktree active --workspace-status in-review --unread --json'
]
}
@@ -2,6 +2,7 @@ import { describe, expect, it, vi } from 'vitest'
import type { WorktreeMeta } from '../../shared/worktree/meta-types'
import { hostWorktreeTarget } from '../../shared/worktree/host-qualified-identity'
import type { Worktree } from '../../shared/worktree/types'
import { mergeWorktreeMetaForWrite } from '../persistence/loading-store/worktree-meta-write-normalization'
import { updateRuntimeManagedWorktreeMetadata } from './runtime-managed-worktree-metadata'
import type { ResolvedWorktree } from './runtime-worktree-path-identity'
import type { RuntimeStore } from './runtime-store-contract'
@@ -43,6 +44,45 @@ describe('updateRuntimeManagedWorktreeMetadata', () => {
)
})
it('persists an empty comment so a set comment can be cleared (#37464)', async () => {
const worktree = {
id: 'repo-1::/workspace/app',
repoId: 'repo-1',
hostId: 'local',
path: '/workspace/app',
instanceId: 'instance-1'
} as unknown as ResolvedWorktree
let row: WorktreeMeta | undefined
// oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the update only reads these store members.
const store = {
getRepos: () => [],
setWorktreeMeta: (_target: unknown, meta: Partial<WorktreeMeta>) => {
row = mergeWorktreeMetaForWrite(row, meta)
}
} as unknown as RuntimeStore
const ports = {
resolveWorktree: vi.fn(async () => worktree),
validateParent: vi.fn(),
invalidateResolved: vi.fn(),
invalidateScan: vi.fn(),
notifyChanged: vi.fn(),
showWorktree: vi.fn(async () => worktree as unknown as Worktree)
}
const update = (comment: string) =>
updateRuntimeManagedWorktreeMetadata({
selector: `id:${worktree.id}`,
updates: { comment },
store,
ports
})
await update('waiting on review')
expect(row?.comment).toBe('waiting on review')
await update('')
expect(row?.comment).toBe('')
})
describe('a folder workspace removed while its resolution was in flight (#22712)', () => {
const folderRepo = { id: 'repo-1', path: '/workspace/app', displayName: 'app', kind: 'folder' }
// oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the update only reads id, repoId, hostId and instanceId.