mirror of
https://github.com/stablyai/orca.git
synced 2026-10-20 08:03:28 +00:00
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.
This commit is contained in:
@@ -240,8 +240,12 @@ export function WorktreeIssueLinkField(props: WorktreeIssueLinkFieldProps): Reac
|
||||
</div>
|
||||
{/* 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. */}
|
||||
<p
|
||||
id={helperId}
|
||||
role="status"
|
||||
aria-live="polite"
|
||||
className={cn(
|
||||
'min-h-[28px] text-[10px] leading-[14px]',
|
||||
(openIssueFailed || displacedLinkLabels?.length) && !isInvalid && !isReadOnly
|
||||
|
||||
@@ -237,6 +237,53 @@ describe('WorktreeMetaDialog issue link row', () => {
|
||||
).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 () => {
|
||||
|
||||
@@ -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)
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -20,6 +20,7 @@ function makeDraft(overrides: Partial<WorktreeMetaDraft> = {}): WorktreeMetaDraf
|
||||
function makeSnapshot(overrides: Partial<WorktreeMetaSnapshot> = {}): 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('')
|
||||
})
|
||||
})
|
||||
|
||||
@@ -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<WorktreeMeta> {
|
||||
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<WorktreeMeta> = 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<WorktreeMeta> {
|
||||
const trimmed = draft.prInput.trim()
|
||||
if (trimmed === '') {
|
||||
@@ -152,7 +176,7 @@ export function buildWorktreeMetaUpdates(
|
||||
current: WorktreeMetaSnapshot
|
||||
): Partial<WorktreeMeta> {
|
||||
return {
|
||||
comment: draft.commentInput.trim(),
|
||||
...buildCommentUpdate(draft, current),
|
||||
...buildDisplayNameUpdate(draft, current),
|
||||
...buildIssueLinkUpdates(draft, current),
|
||||
...buildPrLinkUpdate(draft)
|
||||
|
||||
@@ -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": {
|
||||
|
||||
@@ -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<AppState>)
|
||||
const updateFolderWorkspace = vi.fn().mockResolvedValue(false)
|
||||
store.setState({ updateFolderWorkspace } as Partial<AppState>)
|
||||
|
||||
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<AppState>)
|
||||
const updateFolderWorkspace = vi.fn().mockRejectedValue(new Error('Runtime is offline'))
|
||||
store.setState({ updateFolderWorkspace } as Partial<AppState>)
|
||||
|
||||
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({
|
||||
|
||||
@@ -4515,10 +4515,29 @@ export const createWorktreeSlice: StateCreator<AppState, [], [], WorktreeSlice>
|
||||
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)
|
||||
|
||||
@@ -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', () => {
|
||||
|
||||
Reference in New Issue
Block a user