From 9e2c7357da54566dcc454c6baf67df652ba3a46e Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Mon, 3 Aug 2026 20:15:08 -0700 Subject: [PATCH] fix(sidebar): respect one-issue-per-workspace rule conditionally Only clear displaced issue links when they actually existed, preventing unnecessary Linear keys in GitHub-only workspaces. Skip comment updates when unchanged to avoid workspace reordering. Add accessibility to displacement messages and improve folder workspace error handling. --- .../sidebar/WorktreeIssueLinkField.tsx | 4 ++ .../sidebar/WorktreeMetaDialog.test.tsx | 47 ++++++++++++++ .../components/sidebar/WorktreeMetaDialog.tsx | 6 ++ .../sidebar/worktree-issue-displacement.ts | 13 +++- .../sidebar/worktree-meta-updates.test.ts | 63 ++++++++++++------- .../sidebar/worktree-meta-updates.ts | 30 ++++++++- src/renderer/src/i18n/locales/en.json | 7 ++- .../src/store/slices/worktrees.test.ts | 31 +++++++++ src/renderer/src/store/slices/worktrees.ts | 25 +++++++- src/shared/issue-link-input.test.ts | 6 ++ 10 files changed, 202 insertions(+), 30 deletions(-) diff --git a/src/renderer/src/components/sidebar/WorktreeIssueLinkField.tsx b/src/renderer/src/components/sidebar/WorktreeIssueLinkField.tsx index 501ef1bc607..6a7306d0100 100644 --- a/src/renderer/src/components/sidebar/WorktreeIssueLinkField.tsx +++ b/src/renderer/src/components/sidebar/WorktreeIssueLinkField.tsx @@ -240,8 +240,12 @@ export function WorktreeIssueLinkField(props: WorktreeIssueLinkFieldProps): Reac {/* Why: two lines are reserved up front. The dialog is centered by transform, so a wrap-count change would shift the field being typed in. */} + {/* Why: the failure and displacement messages appear without focus moving + here, so only a live region announces them. */}

{ ).toBeTruthy() }) + // The warning above only promises the displacement — this asserts the payload + // that carries it out, which is where the one-issue-per-workspace rule lives. + it('clears the displaced GitHub link when a Linear value is saved', async () => { + openDialog({ worktree: { linkedIssue: 42 } }) + + fireEvent.click(screen.getByRole('menuitemradio', { name: 'Linear' })) + fireEvent.change(issueInput(), { target: { value: 'STA-335' } }) + await act(async () => { + fireEvent.click(saveButton()) + }) + + await waitFor(() => expect(updateWorktreeMeta).toHaveBeenCalledTimes(1)) + const updates = updateWorktreeMeta.mock.calls[0]?.[1] ?? {} + expect(updates.linkedLinearIssue).toBe('STA-335') + expect(updates.linkedIssue).toBeNull() + }) + + // A GitHub-only save must carry no Linear keys: persistence gates the remote + // Linear capability on key presence, so a synthetic clear fails the save. + it('sends no Linear keys when the workspace has no Linear link', async () => { + openDialog({ worktree: { linkedIssue: 42 } }) + + fireEvent.change(issueInput(), { target: { value: '99' } }) + await act(async () => { + fireEvent.click(saveButton()) + }) + + await waitFor(() => expect(updateWorktreeMeta).toHaveBeenCalledTimes(1)) + const updates = updateWorktreeMeta.mock.calls[0]?.[1] ?? {} + expect(updates.linkedIssue).toBe(99) + expect(updates).not.toHaveProperty('linkedLinearIssue') + }) + + // updateWorktreeMeta stamps lastActivityAt on any comment write, which would + // reorder the workspace under the time-decay sidebar sort. + it('sends no comment when only the issue link changed', async () => { + openDialog({ worktree: { linkedLinearIssue: 'STA-335' } }) + + fireEvent.change(issueInput(), { target: { value: 'STA-999' } }) + await act(async () => { + fireEvent.click(saveButton()) + }) + + await waitFor(() => expect(updateWorktreeMeta).toHaveBeenCalledTimes(1)) + expect(updateWorktreeMeta.mock.calls[0]?.[1] ?? {}).not.toHaveProperty('comment') + }) + // A failed save refetches and reverts the optimistic write, so closing here // would report success for an edit that silently undid itself. it('keeps the dialog open and reports why when the save fails', async () => { diff --git a/src/renderer/src/components/sidebar/WorktreeMetaDialog.tsx b/src/renderer/src/components/sidebar/WorktreeMetaDialog.tsx index b5c1d059c57..61e2bb1151c 100644 --- a/src/renderer/src/components/sidebar/WorktreeMetaDialog.tsx +++ b/src/renderer/src/components/sidebar/WorktreeMetaDialog.tsx @@ -41,6 +41,7 @@ function resizeCommentTextarea(textarea: HTMLTextAreaElement): void { /** Only read before the first open, when nothing can be saved yet. */ const EMPTY_SNAPSHOT: WorktreeMetaSnapshot = { displayName: '', + comment: '', issueInput: '', issueProvider: 'github' } @@ -152,8 +153,10 @@ const WorktreeMetaDialog = React.memo(function WorktreeMetaDialog() { // the next comment-only save would write the stale seed back over the new link. setSnapshot({ displayName: currentDisplayName, + comment: currentComment, issueInput: currentIssue, issueProvider: currentProvider, + linkedLinearIssue, linkedWorkItemProvider: worktree?.linkedWorkItem?.provider ?? null, linkedWorkItemType: worktree?.linkedWorkItem?.type ?? null }) @@ -246,6 +249,9 @@ const WorktreeMetaDialog = React.memo(function WorktreeMetaDialog() { return } setSaving(true) + // Why: a stale failure from the previous attempt must not sit under the + // spinner for the whole in-flight save. + setSaveError(null) try { const updates = buildWorktreeMetaUpdates(draft, snapshot) diff --git a/src/renderer/src/components/sidebar/worktree-issue-displacement.ts b/src/renderer/src/components/sidebar/worktree-issue-displacement.ts index a056ca7f45e..bfc74e7162e 100644 --- a/src/renderer/src/components/sidebar/worktree-issue-displacement.ts +++ b/src/renderer/src/components/sidebar/worktree-issue-displacement.ts @@ -1,3 +1,4 @@ +import { translate } from '@/i18n/i18n' import type { IssueLinkProvider } from '../../../../shared/issue-link-input' import { isIssueFieldDirty, @@ -6,7 +7,17 @@ import { } from './worktree-meta-updates' function formatLinkLabel(provider: IssueLinkProvider, value: string): string { - return provider === 'linear' ? `Linear ${value}` : `GitHub #${value}` + return provider === 'linear' + ? translate( + 'auto.components.sidebar.worktreeIssueDisplacement.3f61c0a8d2', + 'Linear {{value}}', + { value } + ) + : translate( + 'auto.components.sidebar.worktreeIssueDisplacement.9c4b7e1f60', + 'GitHub #{{value}}', + { value } + ) } /** Names the persisted links a save would drop. A workspace tracks one issue, so diff --git a/src/renderer/src/components/sidebar/worktree-meta-updates.test.ts b/src/renderer/src/components/sidebar/worktree-meta-updates.test.ts index d963d58eccb..c7ffc4a6524 100644 --- a/src/renderer/src/components/sidebar/worktree-meta-updates.test.ts +++ b/src/renderer/src/components/sidebar/worktree-meta-updates.test.ts @@ -20,6 +20,7 @@ function makeDraft(overrides: Partial = {}): WorktreeMetaDraf function makeSnapshot(overrides: Partial = {}): WorktreeMetaSnapshot { return { displayName: 'Workspace', + comment: '', issueInput: '', issueProvider: 'github', ...overrides @@ -64,8 +65,7 @@ describe('buildWorktreeMetaUpdates', () => { }) it('writes a GitHub issue number and clears the Linear slots', () => { - expect(buildUpdates({ issueInput: '12' })).toEqual({ - comment: '', + expect(buildUpdates({ issueInput: '12' }, { linkedLinearIssue: 'STA-335' })).toEqual({ linkedIssue: 12, linkedLinearIssue: null, linkedLinearIssueWorkspaceId: null, @@ -74,9 +74,19 @@ describe('buildWorktreeMetaUpdates', () => { }) }) + // Persistence gates the remote Linear capability on key presence, so a clear + // for a link that never existed fails a GitHub-only save on an older runtime. + it('emits no Linear clear when the workspace holds no Linear link', () => { + const updates = buildUpdates({ issueInput: '12' }) + + expect(updates).toEqual({ linkedIssue: 12, linkedPR: null }) + for (const key of LINEAR_LINK_KEYS) { + expect(updates).not.toHaveProperty(key) + } + }) + it('writes a bare Linear identifier and clears the stored organization key', () => { expect(buildUpdates({ issueInput: 'sta-335', issueProvider: 'linear' })).toEqual({ - comment: '', linkedIssue: null, linkedLinearIssue: 'STA-335', linkedLinearIssueWorkspaceId: null, @@ -92,7 +102,6 @@ describe('buildWorktreeMetaUpdates', () => { issueProvider: 'linear' }) ).toEqual({ - comment: '', linkedIssue: null, linkedLinearIssue: 'STA-335', linkedLinearIssueWorkspaceId: null, @@ -102,8 +111,9 @@ describe('buildWorktreeMetaUpdates', () => { }) it('clears every provider slot when the issue field is emptied', () => { - expect(buildUpdates({ issueInput: ' ' }, { issueInput: '42' })).toEqual({ - comment: '', + expect( + buildUpdates({ issueInput: ' ' }, { issueInput: '42', linkedLinearIssue: 'STA-335' }) + ).toEqual({ linkedIssue: null, linkedLinearIssue: null, linkedLinearIssueWorkspaceId: null, @@ -144,6 +154,8 @@ describe('buildWorktreeMetaUpdates', () => { expect(updates).not.toHaveProperty('linkedTaskSourceContext') }) + // The row cannot render a GitLab or Jira issue, so clearing one would destroy a + // link the user was never shown — and neither has another editor to restore it. it('leaves work items owned by other providers alone', () => { for (const provider of ['jira', 'gitlab'] as const) { const updates = buildUpdates( @@ -156,15 +168,6 @@ describe('buildWorktreeMetaUpdates', () => { } }) - // The row cannot render a GitLab or Jira issue, so clearing one would destroy a - // link the user was never shown — and neither has another editor to restore it. - it('leaves a GitLab issue attached', () => { - expect(buildUpdates({ issueInput: '12' })).not.toHaveProperty('linkedGitLabIssue') - expect( - buildUpdates({ issueInput: '12' }, { linkedWorkItemProvider: 'gitlab' }) - ).not.toHaveProperty('linkedGitLabIssue') - }) - it('ignores a provider switch on an empty field', () => { const updates = buildUpdates( { issueInput: '', issueProvider: 'linear' }, @@ -204,21 +207,22 @@ describe('buildWorktreeMetaUpdates', () => { }) it('rejects issue URLs in the PR input', () => { - expect(buildUpdates({ prInput: 'https://github.com/stablyai/orca/issues/6933' })).toEqual({ - comment: '' - }) + expect(buildUpdates({ prInput: 'https://github.com/stablyai/orca/issues/6933' })).toEqual({}) }) it('accepts PR URLs in the PR input', () => { expect(buildUpdates({ prInput: 'https://github.com/stablyai/orca/pull/6934' })).toEqual({ - comment: '', linkedPR: 6934 }) }) it('accepts issue URLs in the issue input', () => { - expect(buildUpdates({ issueInput: 'https://github.com/stablyai/orca/issues/6933' })).toEqual({ - comment: '', + expect( + buildUpdates( + { issueInput: 'https://github.com/stablyai/orca/issues/6933' }, + { linkedLinearIssue: 'STA-335' } + ) + ).toEqual({ linkedIssue: 6933, linkedLinearIssue: null, linkedLinearIssueWorkspaceId: null, @@ -229,8 +233,23 @@ describe('buildWorktreeMetaUpdates', () => { it('rejects PR URLs in the issue input', () => { expect(buildUpdates({ issueInput: 'https://github.com/stablyai/orca/pull/6934' })).toEqual({ - comment: '', linkedPR: null }) }) + + // Persistence stamps lastActivityAt on any comment write, so re-emitting an + // unchanged note reorders the workspace under the time-decay sidebar sort. + it('emits no comment when the note is unchanged', () => { + const updates = buildUpdates( + { issueInput: '12', commentInput: 'shipping today' }, + { comment: 'shipping today' } + ) + + expect(updates).not.toHaveProperty('comment') + expect(updates.linkedIssue).toBe(12) + }) + + it('clears a comment with empty string, never a present-undefined key', () => { + expect(buildUpdates({ commentInput: ' ' }, { comment: 'old note' }).comment).toBe('') + }) }) diff --git a/src/renderer/src/components/sidebar/worktree-meta-updates.ts b/src/renderer/src/components/sidebar/worktree-meta-updates.ts index 60d51ec8aa4..9cc41b3de2f 100644 --- a/src/renderer/src/components/sidebar/worktree-meta-updates.ts +++ b/src/renderer/src/components/sidebar/worktree-meta-updates.ts @@ -26,8 +26,11 @@ export type WorktreeMetaDraft = { * background write move the baseline and make an untouched field "dirty". */ export type WorktreeMetaSnapshot = { displayName: string + comment: string issueInput: string issueProvider: IssueLinkProvider + /** The stored Linear identifier, which decides whether a clear is real. */ + linkedLinearIssue?: string | null linkedWorkItemProvider?: WorkspaceSourceProvider | null /** `linkedWorkItem` also describes PRs and MRs, which this row does not own. */ linkedWorkItemType?: WorkspaceLinkedItem['type'] | null @@ -69,6 +72,17 @@ function buildDisplayNameUpdate( return trimmed === current.displayName ? {} : { displayName: trimmed } } +// Why: persistence bumps lastActivityAt whenever a `comment` key is present, so +// re-emitting an unchanged note reorders the workspace under the time-decay +// sidebar sort on a save that only touched the issue link. +function buildCommentUpdate( + draft: WorktreeMetaDraft, + current: WorktreeMetaSnapshot +): Partial { + const trimmed = draft.commentInput.trim() + return trimmed === current.comment ? {} : { comment: trimmed } +} + export function isIssueFieldDirty( draft: WorktreeMetaDraft, current: WorktreeMetaSnapshot @@ -107,10 +121,17 @@ function buildIssueLinkUpdates( ? { linkedWorkItem: null, linkedTaskSourceContext: null } : {} + // Why: persistence gates the remote Linear capability on key presence, not + // value. A synthetic clear on a workspace that never held a Linear link would + // fail a GitHub-only save against an older runtime, citing Linear. + const displacedLinear: Partial = current.linkedLinearIssue + ? LINEAR_ISSUE_LINK_CLEARED + : {} + if (trimmed === '') { return { linkedIssue: null, - ...LINEAR_ISSUE_LINK_CLEARED, + ...displacedLinear, ...displacedWorkItem } } @@ -125,7 +146,7 @@ function buildIssueLinkUpdates( if (parsed.provider === 'github') { return { linkedIssue: parsed.number, - ...LINEAR_ISSUE_LINK_CLEARED, + ...displacedLinear, ...displacedWorkItem } } @@ -134,6 +155,9 @@ function buildIssueLinkUpdates( return linearUpdates ? { linkedIssue: null, ...linearUpdates, ...displacedWorkItem } : {} } +// Requires the dialog to seed `prInput` from the persisted `linkedPR`: the blank +// input is written through as a clear, so an unseeded field drops the link on an +// untouched save. function buildPrLinkUpdate(draft: WorktreeMetaDraft): Partial { const trimmed = draft.prInput.trim() if (trimmed === '') { @@ -152,7 +176,7 @@ export function buildWorktreeMetaUpdates( current: WorktreeMetaSnapshot ): Partial { return { - comment: draft.commentInput.trim(), + ...buildCommentUpdate(draft, current), ...buildDisplayNameUpdate(draft, current), ...buildIssueLinkUpdates(draft, current), ...buildPrLinkUpdate(draft) diff --git a/src/renderer/src/i18n/locales/en.json b/src/renderer/src/i18n/locales/en.json index 9bff9276748..8de9d267c9a 100644 --- a/src/renderer/src/i18n/locales/en.json +++ b/src/renderer/src/i18n/locales/en.json @@ -280,7 +280,8 @@ "3b57982bf6": "Git could not safely delete branch \"{{value0}}\" after deleting workspace \"{{value1}}\", so Orca kept it to avoid losing local commits.", "81f13f48d2": "Git could not safely delete branch \"{{value0}}\" after deleting worktree \"{{value1}}\", so Orca kept it to avoid losing local commits.", "runtimeScopeForbiddenTitle": "This connection has limited (mobile) access", - "runtimeScopeForbiddenDescription": "Workspaces are unavailable on a mobile-scope pairing. Reconnect using the browser access link from Settings → Runtime Environments → Share this Orca server." + "runtimeScopeForbiddenDescription": "Workspaces are unavailable on a mobile-scope pairing. Reconnect using the browser access link from Settings → Runtime Environments → Share this Orca server.", + "a17f4d2e93": "Could not update this workspace." }, "repos": { "2975400634": "Update Orca server to open non-Git folders on this runtime.", @@ -5142,6 +5143,10 @@ "ad78f9bee2": "Issue", "662ae142f8": "Issue #, or a GitHub or Linear URL", "929c98d05a": "Issue provider" + }, + "worktreeIssueDisplacement": { + "3f61c0a8d2": "Linear {{value}}", + "9c4b7e1f60": "GitHub #{{value}}" } }, "shared": { diff --git a/src/renderer/src/store/slices/worktrees.test.ts b/src/renderer/src/store/slices/worktrees.test.ts index d4d944db201..1d2b822dffd 100644 --- a/src/renderer/src/store/slices/worktrees.test.ts +++ b/src/renderer/src/store/slices/worktrees.test.ts @@ -7024,6 +7024,37 @@ describe('worktree remote runtime mutations', () => { }) }) + // A rejected folder update reconciles the optimistic write away, so reporting + // ok would show the dialog a save that silently undid itself. + it('reports a rejected folder workspace update to the caller', async () => { + const store = createTestStore() + const folderWorkspace = makeFolderWorkspace({ id: 'folder-rejected' }) + store.setState({ folderWorkspaces: [folderWorkspace] } as Partial) + const updateFolderWorkspace = vi.fn().mockResolvedValue(false) + store.setState({ updateFolderWorkspace } as Partial) + + const result = await store + .getState() + .updateWorktreeMeta(folderWorkspaceKey(folderWorkspace.id), { comment: 'note' }) + + expect(updateFolderWorkspace).toHaveBeenCalledTimes(1) + expect(result).toEqual({ ok: false, error: expect.any(String) }) + }) + + it('reports a thrown folder workspace update instead of rejecting', async () => { + const store = createTestStore() + const folderWorkspace = makeFolderWorkspace({ id: 'folder-throwing' }) + store.setState({ folderWorkspaces: [folderWorkspace] } as Partial) + const updateFolderWorkspace = vi.fn().mockRejectedValue(new Error('Runtime is offline')) + store.setState({ updateFolderWorkspace } as Partial) + + const result = await store + .getState() + .updateWorktreeMeta(folderWorkspaceKey(folderWorkspace.id), { comment: 'note' }) + + expect(result).toEqual({ ok: false, error: 'Runtime is offline' }) + }) + it('persists activity for hidden detected worktrees', async () => { const store = createTestStore() const hidden = makeWorktree({ diff --git a/src/renderer/src/store/slices/worktrees.ts b/src/renderer/src/store/slices/worktrees.ts index 09248d3b3bd..7e218715253 100644 --- a/src/renderer/src/store/slices/worktrees.ts +++ b/src/renderer/src/store/slices/worktrees.ts @@ -4515,10 +4515,29 @@ export const createWorktreeSlice: StateCreator const workspaceScope = parseWorkspaceKey(worktreeId) if (workspaceScope?.type === 'folder') { const folderUpdates = getFolderWorkspaceMetaUpdates(updates) - if (Object.keys(folderUpdates).length > 0) { - await get().updateFolderWorkspace(workspaceScope.folderWorkspaceId, folderUpdates) + if (Object.keys(folderUpdates).length === 0) { + return { ok: true } + } + try { + // Why: a rejected folder update reconciles the optimistic write away, so + // reporting ok would show the dialog a save that silently undid itself. + const updated = await get().updateFolderWorkspace( + workspaceScope.folderWorkspaceId, + folderUpdates + ) + return updated + ? { ok: true } + : { + ok: false, + error: translate( + 'auto.store.slices.worktrees.a17f4d2e93', + 'Could not update this workspace.' + ) + } + } catch (err) { + console.error('Failed to update folder workspace meta:', err) + return { ok: false, error: err instanceof Error ? err.message : String(err) } } - return { ok: true } } const normalizedUpdates = existingWorktree ? clearOlderHostedReviewLinksForReplacement(updates, existingWorktree) diff --git a/src/shared/issue-link-input.test.ts b/src/shared/issue-link-input.test.ts index 4e51ef0efeb..bf026edc43f 100644 --- a/src/shared/issue-link-input.test.ts +++ b/src/shared/issue-link-input.test.ts @@ -72,6 +72,12 @@ describe('parseIssueLinkInput', () => { it('rejects Linear identifiers', () => { expect(parseIssueLinkInput('STA-335', 'github')).toBeNull() }) + + // Past the safe-integer range every digit string parses to the same float, + // so an unbounded parse would link an arbitrary issue number. + it('rejects numbers beyond the safe-integer range', () => { + expect(parseIssueLinkInput('9'.repeat(400), 'github')).toBeNull() + }) }) describe('linear provider', () => {