diff --git a/src/main/github/pull-requests/refresh/pr-refresh-budget-fairness.test.ts b/src/main/github/pull-requests/refresh/pr-refresh-budget-fairness.test.ts new file mode 100644 index 00000000000..3f3880ab775 --- /dev/null +++ b/src/main/github/pull-requests/refresh/pr-refresh-budget-fairness.test.ts @@ -0,0 +1,73 @@ +import { expect, it } from 'vitest' +import { PRWatchJobRunner } from './pr-refresh-watch-jobs' +import { PRRefreshRetryState } from './pr-refresh-retry-state' +import { PRRefreshPacing } from './pr-refresh-pacing' +import { PRRefreshQueue, type PRRefreshQueueEntry } from './pr-refresh-queue' +import { makeCandidate } from './pr-refresh-coordinator-test-harness' + +it('yields a selected list read to another ready review before restoring its priority', () => { + const pacing = new PRRefreshPacing() + const queue = new PRRefreshQueue(() => {}) + const entry = (key: string, selected: boolean, priority: number): PRRefreshQueueEntry => ({ + key, + candidate: makeCandidate({ cacheKey: key, isSelected: selected }), + aliases: new Map(), + reason: 'visible', + priority, + dueAt: 0, + queuedAt: 0 + }) + const selected = entry('selected', true, 80) + const other = entry('other', false, 40) + queue.set(selected.key, selected) + queue.set(other.key, other) + const next = () => queue.ordered((a, b) => pacing.order(a, b))[0].key + expect(next()).toBe('selected') + pacing.noteBackgroundStart(selected) + expect(next()).toBe('other') + pacing.noteBackgroundStart(other) + expect(next()).toBe('selected') + pacing.noteBackgroundStart(selected) + queue.delete(other.key) + expect(next()).toBe('selected') +}) + +it('preserves a visible list turn when selected reads share the budget with aged watches', () => { + const pacing = new PRRefreshPacing() + const queue = new PRRefreshQueue(() => {}) + const watches = new PRWatchJobRunner(queue, new PRRefreshRetryState(), { schedule() {} }) + const entry = (key: string, selected: boolean): PRRefreshQueueEntry => ({ + key, + candidate: makeCandidate({ isSelected: selected }), + aliases: new Map(), + reason: 'visible', + priority: selected ? 80 : 40, + dueAt: 0, + queuedAt: 0 + }) + const selected = entry('selected', true) + const other = entry('other', false) + const watch: PRRefreshQueueEntry = { + ...entry('watch', false), + reason: 'watch', + watch: { prKey: 'pr' } + } + for (const item of [selected, other, watch]) { + queue.set(item.key, item) + } + const next = () => { + watches.age(queue.values(), 120_000) + return queue.ordered((a, b) => pacing.order(a, b))[0].key + } + const started = (item: PRRefreshQueueEntry) => { + pacing.noteBackgroundStart(item) + watches.noteBudgetedStart(item) + } + expect(next()).toBe('selected') + started(selected) + expect(next()).toBe('watch') + started(watch) + expect(next()).toBe('selected') + started(selected) + expect(next()).toBe('other') +}) diff --git a/src/main/github/pull-requests/refresh/pr-refresh-candidate-policy.ts b/src/main/github/pull-requests/refresh/pr-refresh-candidate-policy.ts index c846f032ada..de532e3d8ad 100644 --- a/src/main/github/pull-requests/refresh/pr-refresh-candidate-policy.ts +++ b/src/main/github/pull-requests/refresh/pr-refresh-candidate-policy.ts @@ -167,6 +167,19 @@ export function visibleCandidateAfterOutcome( } } +const SELECTED_VISIBLE_PRIORITY = 80 +const LIST_VISIBLE_PRIORITY = 40 + +/** The selected workspace's follow-up wins the next list-budget slot; a deselected one rejoins list order. */ +export function visibleFollowUpPriority( + candidate: GitHubPRRefreshCandidate, + priority: number +): number { + return candidate.isSelected === true + ? Math.max(priority, SELECTED_VISIBLE_PRIORITY) + : Math.min(priority, LIST_VISIBLE_PRIORITY) +} + export function hasStaleHead(candidate: GitHubPRRefreshCandidate): boolean { return ( candidate.currentHeadOid != null && diff --git a/src/main/github/pull-requests/refresh/pr-refresh-pacing.ts b/src/main/github/pull-requests/refresh/pr-refresh-pacing.ts index 1644d9c7f16..2013a5e05ae 100644 --- a/src/main/github/pull-requests/refresh/pr-refresh-pacing.ts +++ b/src/main/github/pull-requests/refresh/pr-refresh-pacing.ts @@ -19,6 +19,7 @@ export class PRRefreshPacing { private readonly backgroundStarts: number[] = [] private readonly activeStartsByScope = new Map() private lastBackgroundStartAt = 0 + private lastBudgetedStartWasSelected = false clearActiveBurstWindow(windowId: number): void { const windowPrefix = `${windowId}::` @@ -29,7 +30,9 @@ export class PRRefreshPacing { } } - noteBackgroundStart(): void { + noteBackgroundStart(entry: PRRefreshQueueEntry): void { + this.lastBudgetedStartWasSelected = + entry.reason === 'visible' && entry.candidate.isSelected === true const now = Date.now() this.lastBackgroundStartAt = now this.backgroundStarts.push(now) @@ -50,7 +53,11 @@ export class PRRefreshPacing { return Math.max(spacingDelay, windowDelay) } - activeOrder(a: PRRefreshQueueEntry, b: PRRefreshQueueEntry): number { + order(a: PRRefreshQueueEntry, b: PRRefreshQueueEntry): number { + const priority = this.priority(b) - this.priority(a) + if (priority !== 0) { + return priority + } if (!usesActiveRefreshPacing(a) || !usesActiveRefreshPacing(b)) { return 0 } @@ -60,6 +67,16 @@ export class PRRefreshPacing { return b.queuedAt - a.queuedAt } + private priority(entry: PRRefreshQueueEntry): number { + // Reserve the next budgeted slot for other ready reviews after a selected list read. + return this.lastBudgetedStartWasSelected && + entry.reason === 'visible' && + entry.candidate.isSelected === true && + entry.bypassBackgroundBudget !== true + ? 0 + : entry.priority + } + entryDelay(entry: PRRefreshQueueEntry): number { const activeDelay = usesActiveRefreshPacing(entry) ? this.nextActiveBurstDelay(entry) : 0 if (activeDelay > 0) { diff --git a/src/main/github/pull-requests/refresh/pr-refresh-queue-aliases.ts b/src/main/github/pull-requests/refresh/pr-refresh-queue-aliases.ts new file mode 100644 index 00000000000..52641c52914 --- /dev/null +++ b/src/main/github/pull-requests/refresh/pr-refresh-queue-aliases.ts @@ -0,0 +1,47 @@ +import type { GitHubPRRefreshAlias } from '../../../../shared/github/pull-requests/refresh/pull-request-refresh-types' + +/** Retire old-branch aliases when a linked-PR key survives worktree branch changes. */ +export function setLiveAlias( + aliases: Map, + alias: GitHubPRRefreshAlias +): void { + if (alias.worktreeId) { + for (const [cacheKey, existing] of aliases) { + if (cacheKey !== alias.cacheKey && existing.worktreeId === alias.worktreeId) { + aliases.delete(cacheKey) + } + } + } + aliases.set(alias.cacheKey, alias) +} + +/** Queued aliases are newer than request-time follow-ups; preserve them with their matching candidate. */ +export function mergeFollowUpAlias( + aliases: Map, + alias: GitHubPRRefreshAlias +): GitHubPRRefreshAlias | undefined { + if (alias.worktreeId) { + for (const existing of aliases.values()) { + if (existing.worktreeId === alias.worktreeId) { + return existing + } + } + } + setLiveAlias(aliases, alias) + return undefined +} + +/** Manual refresh writes copied maps back; keep the latest branch per worktree on re-entry too. */ +export function dropSupersededWorktreeAliases(aliases: Map): void { + const liveCacheKeyByWorktree = new Map() + for (const [cacheKey, alias] of aliases) { + if (!alias.worktreeId) { + continue + } + const superseded = liveCacheKeyByWorktree.get(alias.worktreeId) + if (superseded !== undefined) { + aliases.delete(superseded) + } + liveCacheKeyByWorktree.set(alias.worktreeId, cacheKey) + } +} diff --git a/src/main/github/pull-requests/refresh/pr-refresh-queue-drainer.ts b/src/main/github/pull-requests/refresh/pr-refresh-queue-drainer.ts index 9a4273da40b..17d3712655f 100644 --- a/src/main/github/pull-requests/refresh/pr-refresh-queue-drainer.ts +++ b/src/main/github/pull-requests/refresh/pr-refresh-queue-drainer.ts @@ -13,7 +13,8 @@ import { isMergeabilityPendingOutcome, sameAliasRequestIdentity, validateCandidate, - visibleCandidateAfterOutcome + visibleCandidateAfterOutcome, + visibleFollowUpPriority } from './pr-refresh-candidate-policy' import type { PRRefreshEventPublisher } from './pr-refresh-event-publisher' import { type PRRefreshPacing, usesActiveRefreshPacing } from './pr-refresh-pacing' @@ -33,7 +34,6 @@ export type PRRefreshRegistrationCheck = (candidate: GitHubPRRefreshCandidate) = export class PRRefreshQueueDrainer { private draining = false private timer: ReturnType | null = null - // Why: queued work can outlive its project; re-check on every removal path before Git runs. private registrationCheck: PRRefreshRegistrationCheck | null = null readonly watchJobs: PRWatchJobRunner @@ -52,9 +52,7 @@ export class PRRefreshQueueDrainer { } schedule(delay = 0): void { - if (this.timer) { - clearTimeout(this.timer) - } + clearTimeout(this.timer ?? undefined) this.timer = setTimeout(() => { this.timer = null // The drain serves every queued repo; a scheduling call inside a WSL user act must not lend it that act. @@ -125,7 +123,7 @@ export class PRRefreshQueueDrainer { candidate: followUpCandidate, aliases: new Map(aliases.map((alias) => [alias.cacheKey, alias])), reason: 'visible', - priority, + priority: visibleFollowUpPriority(followUpCandidate, priority), dueAt, queuedAt: this.queue.nextOrder(), bypassBackgroundBudget: pendingDueAt !== null, @@ -137,7 +135,7 @@ export class PRRefreshQueueDrainer { private ordered(): PRRefreshQueueEntry[] { this.watchJobs.age(this.queue.values(), Date.now()) - return this.queue.ordered((a, b) => this.pacing.activeOrder(a, b)) + return this.queue.ordered((a, b) => this.pacing.order(a, b)) } private nextQueuedWakeDelay(excludedKey: string): number | null { @@ -237,7 +235,7 @@ export class PRRefreshQueueDrainer { continue } if (next.bypassBackgroundBudget !== true && isBudgetedBackground(next.reason)) { - this.pacing.noteBackgroundStart() + this.pacing.noteBackgroundStart(next) this.watchJobs.noteBudgetedStart(next) } if (usesActiveRefreshPacing(next)) { diff --git a/src/main/github/pull-requests/refresh/pr-refresh-queue.ts b/src/main/github/pull-requests/refresh/pr-refresh-queue.ts index 2c315cc9d00..00caf6e68ae 100644 --- a/src/main/github/pull-requests/refresh/pr-refresh-queue.ts +++ b/src/main/github/pull-requests/refresh/pr-refresh-queue.ts @@ -12,7 +12,8 @@ import { refreshKey, sameAliasRequestIdentity, refreshIntervalForCandidate, - shouldSkipFresh + shouldSkipFresh, + visibleFollowUpPriority } from './pr-refresh-candidate-policy' export type PRRefreshQueueEntry = { @@ -40,51 +41,11 @@ export type PRRefreshEnqueue = { type PRRefreshRequestState = { until: number; requestSequence?: number } -/** Retire old-branch aliases when a linked-PR key survives worktree branch changes. */ -function setLiveAlias( - aliases: Map, - alias: GitHubPRRefreshAlias -): void { - if (alias.worktreeId) { - for (const [cacheKey, existing] of aliases) { - if (cacheKey !== alias.cacheKey && existing.worktreeId === alias.worktreeId) { - aliases.delete(cacheKey) - } - } - } - aliases.set(alias.cacheKey, alias) -} - -/** Queued aliases are newer than request-time follow-ups; preserve them with their matching candidate. */ -function mergeFollowUpAlias( - aliases: Map, - alias: GitHubPRRefreshAlias -): GitHubPRRefreshAlias | undefined { - if (alias.worktreeId) { - for (const existing of aliases.values()) { - if (existing.worktreeId === alias.worktreeId) { - return existing - } - } - } - setLiveAlias(aliases, alias) - return undefined -} - -/** Manual refresh writes copied maps back; keep the latest branch per worktree on re-entry too. */ -function dropSupersededWorktreeAliases(aliases: Map): void { - const liveCacheKeyByWorktree = new Map() - for (const [cacheKey, alias] of aliases) { - if (!alias.worktreeId) { - continue - } - const superseded = liveCacheKeyByWorktree.get(alias.worktreeId) - if (superseded !== undefined) { - aliases.delete(superseded) - } - liveCacheKeyByWorktree.set(alias.worktreeId, cacheKey) - } -} +import { + setLiveAlias, + mergeFollowUpAlias, + dropSupersededWorktreeAliases +} from './pr-refresh-queue-aliases' export class PRRefreshQueue { private readonly entries = new Map() @@ -155,6 +116,7 @@ export class PRRefreshQueue { continue } entry.candidate = candidateFor(entry.key, entry.candidate) + entry.priority = visibleFollowUpPriority(entry.candidate, entry.priority) entry.dueAt = Math.max( freshRetryAt(entry.candidate) ?? Date.now(), this.backgroundNotBefore.get(entry.key)?.until ?? 0 @@ -313,14 +275,14 @@ export class PRRefreshQueue { } ordered( - activeOrder: (a: PRRefreshQueueEntry, b: PRRefreshQueueEntry) => number + pacingOrder: (a: PRRefreshQueueEntry, b: PRRefreshQueueEntry) => number ): PRRefreshQueueEntry[] { const now = Date.now() return Array.from(this.entries.values()).sort((a, b) => { const aReady = a.dueAt <= now const bReady = b.dueAt <= now if (aReady && bReady) { - return b.priority - a.priority || activeOrder(a, b) || a.dueAt - b.dueAt + return pacingOrder(a, b) || b.priority - a.priority || a.dueAt - b.dueAt } if (aReady !== bReady) { return aReady ? -1 : 1 diff --git a/src/main/github/pull-requests/refresh/pr-refresh-watch-jobs.ts b/src/main/github/pull-requests/refresh/pr-refresh-watch-jobs.ts index f51c5f4ea79..9551d0e28a0 100644 --- a/src/main/github/pull-requests/refresh/pr-refresh-watch-jobs.ts +++ b/src/main/github/pull-requests/refresh/pr-refresh-watch-jobs.ts @@ -103,7 +103,10 @@ export class PRWatchJobRunner { } noteBudgetedStart(entry: PRRefreshQueueEntry): void { - this.lastBudgetedStartWasWatch = entry.watch !== undefined + // Selected list reads have their own reserved turn; preserve the watch/list alternation. + if (entry.reason !== 'visible' || entry.candidate.isSelected !== true) { + this.lastBudgetedStartWasWatch = entry.watch !== undefined + } } /** Re-ranks watch entries before each pick, so aging needs no change to the queue's order. */ diff --git a/src/renderer/src/components/right-sidebar/source-control/panel/merged-review-workspace-notice.fixture.tsx b/src/renderer/src/components/right-sidebar/source-control/panel/merged-review-workspace-notice.fixture.tsx new file mode 100644 index 00000000000..d401170403e --- /dev/null +++ b/src/renderer/src/components/right-sidebar/source-control/panel/merged-review-workspace-notice.fixture.tsx @@ -0,0 +1,66 @@ +import { defineFixture } from '../../../../component-fixtures/component-fixture' +import type { HostedReviewInfo } from '../../../../../../shared/source-control/hosted-review/hosted-review-contracts' +import { makeWorktree } from '../../../worktree-jump/worktree-jump-palette-test-fixtures' +import { SourceControlMergedReviewNotice } from './merged-review-workspace-notice' + +const review: HostedReviewInfo = { + provider: 'github', + number: 412, + title: 'Add retry to sync', + state: 'merged', + url: 'https://github.com/acme/app/pull/412', + status: 'success', + updatedAt: '2026-11-10T00:00:00Z', + mergeable: 'UNKNOWN', + headSha: 'a1b2c3d' +} + +const worktree = makeWorktree('repo-1::/work/app-sync', 'app-sync', { + branch: 'refs/heads/feature/sync-retry' +}) + +function Frame({ children }: { children: React.ReactNode }) { + return
{children}
+} + +export const safeToDelete = defineFixture({ + width: 300, + render: () => ( + + + + ) +}) + +export const uncommittedChanges = defineFixture({ + width: 300, + render: () => ( + + + + ) +}) + +export const headMovedPastReview = defineFixture({ + width: 300, + render: () => ( + + + + ) +}) diff --git a/src/renderer/src/components/right-sidebar/source-control/panel/merged-review-workspace-notice.test.tsx b/src/renderer/src/components/right-sidebar/source-control/panel/merged-review-workspace-notice.test.tsx new file mode 100644 index 00000000000..84de080ec37 --- /dev/null +++ b/src/renderer/src/components/right-sidebar/source-control/panel/merged-review-workspace-notice.test.tsx @@ -0,0 +1,82 @@ +// @vitest-environment happy-dom +import { act } from 'react' +import { createRoot } from 'react-dom/client' +import { expect, it, vi } from 'vitest' +import { mkdtemp, rm, writeFile } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import path from 'node:path' +import { runProcess } from '@orca/process-host' + +const mocks = vi.hoisted(() => ({ runWorktreeDelete: vi.fn() })) +vi.mock('../../../sidebar/workspace-actions/deletion/delete-worktree-flow', () => mocks) +vi.mock('@/store/index', () => ({ + useAppStore: (selector: (state: { deleteStateByWorktreeId: object }) => unknown) => + selector({ deleteStateByWorktreeId: {} }) +})) + +import { SourceControlMergedReviewNotice } from './merged-review-workspace-notice' +import { makeWorktree } from '../../../worktree-jump/worktree-jump-palette-test-fixtures' + +it('requires the ignored-file confirmation for an ignored-only merged checkout', async () => { + const directory = await mkdtemp(path.join(tmpdir(), 'merged-notice-')) + const container = document.createElement('div') + const root = createRoot(container) + const git = async (args: string[]) => { + const result = await runProcess({ program: 'git', args, cwd: directory }) + if (result.code !== 0) { + throw new Error(result.stderr) + } + return result.stdout.trim() + } + try { + await git(['init', '--quiet']) + await writeFile(path.join(directory, '.gitignore'), '.env\n') + await git(['add', '.gitignore']) + await git([ + '-c', + 'user.name=Test', + '-c', + 'user.email=test@example.com', + 'commit', + '-m', + 'initial' + ]) + await writeFile(path.join(directory, '.env'), 'LOCAL_ONLY=value\n') + const head = await git(['rev-parse', 'HEAD']) + const status = await git(['status', '--porcelain']) + expect(status).toBe('') + const worktree = makeWorktree('ignored-only', 'Ignored only', { + path: directory, + instanceId: 'original' + }) + await act(async () => + root.render( + + ) + ) + await act(async () => container.querySelector('button')?.click()) + expect(mocks.runWorktreeDelete).toHaveBeenCalledWith('ignored-only', { + expectedHostId: undefined, + expectedInstanceId: 'original', + forceConfirm: true + }) + } finally { + await act(async () => root.unmount()) + await rm(directory, { recursive: true, force: true }) + } +}) diff --git a/src/renderer/src/components/right-sidebar/source-control/panel/merged-review-workspace-notice.tsx b/src/renderer/src/components/right-sidebar/source-control/panel/merged-review-workspace-notice.tsx new file mode 100644 index 00000000000..938a8dbdccf --- /dev/null +++ b/src/renderer/src/components/right-sidebar/source-control/panel/merged-review-workspace-notice.tsx @@ -0,0 +1,119 @@ +import React from 'react' +import { GitMerge, LoaderCircle, Trash2 } from 'lucide-react' +import { Button } from '@/components/ui/button' +import { useAppStore } from '@/store/index' +import { useTranslate } from '@/i18n/use-translate' +import { + localizedHostedReviewCopy, + resolveSupportedHostedReviewCopyProvider +} from '@/i18n/hosted-review-localized-copy' +import type { HostedReviewInfo } from '../../../../../../shared/source-control/hosted-review/hosted-review-contracts' +import type { Worktree } from '../../../../../../shared/worktree/types' +import { resolveMergedReviewWorkspaceReadiness } from '../../../../../../shared/source-control/decisions/merged-review-workspace-readiness' +import { runWorktreeDelete } from '../../../sidebar/workspace-actions/deletion/delete-worktree-flow' +import { getDeleteStateForWorktreeHost } from '../../../sidebar/workspace-actions/deletion/worktree-delete-state-host-match' + +/** Says whether a merged workspace still holds work; the branch compare alone keeps squash-merged commits listed. */ +export function SourceControlMergedReviewNotice({ + review, + worktree, + statusHeadOid, + uncommittedCount +}: { + review: HostedReviewInfo | null + worktree: Worktree + /** HEAD from the latest successful git status read; null until one completes. */ + statusHeadOid: string | null + uncommittedCount: number +}): React.JSX.Element | null { + const t = useTranslate() + const isDeleting = useAppStore( + (s) => getDeleteStateForWorktreeHost(worktree, s.deleteStateByWorktreeId)?.isDeleting ?? false + ) + const readiness = resolveMergedReviewWorkspaceReadiness({ + review, + headOid: statusHeadOid, + uncommittedCount + }) + // Without a status read the change count is unknown, so say nothing rather than guess. + if (!review || !readiness || !statusHeadOid) { + return null + } + const label = `${ + localizedHostedReviewCopy(resolveSupportedHostedReviewCopyProvider(review.provider), t) + .shortLabel + } #${review.number}` + const canDelete = readiness.kind === 'safe-to-delete' && !worktree.isMainWorktree + + let message: string + if (readiness.kind === 'uncommitted-changes') { + message = t( + 'auto.components.right.sidebar.SourceControl.mergedReviewUncommitted', + '{{review}} is merged, but this workspace has {{count}} uncommitted changes that are not in it.', + { + review: label, + count: readiness.count, + defaultValue_one: + '{{review}} is merged, but this workspace has {{count}} uncommitted change that is not in it.' + } + ) + } else if (readiness.kind === 'head-unverified') { + message = t( + 'auto.components.right.sidebar.SourceControl.mergedReviewHeadMoved', + '{{review}} is merged, but Orca could not confirm that it includes this branch’s current commits.', + { review: label } + ) + } else if (worktree.isMainWorktree) { + message = t( + 'auto.components.right.sidebar.SourceControl.mergedReviewNothingLeft', + '{{review}} is merged and includes this branch’s committed changes.', + { review: label } + ) + } else { + message = t( + 'auto.components.right.sidebar.SourceControl.mergedReviewSafeToDelete', + '{{review}} is merged and includes this branch’s committed changes. Ignored files may still be lost when deleting this workspace.', + { review: label } + ) + } + + return ( +
+
+ ) +} diff --git a/src/renderer/src/components/right-sidebar/source-control/panel/panel-content.tsx b/src/renderer/src/components/right-sidebar/source-control/panel/panel-content.tsx index 122388ceffe..aef82b2e029 100644 --- a/src/renderer/src/components/right-sidebar/source-control/panel/panel-content.tsx +++ b/src/renderer/src/components/right-sidebar/source-control/panel/panel-content.tsx @@ -8,6 +8,7 @@ import { SourceControlUncommittedSections } from '../listing/uncommitted-section import { CompareUnavailable } from '../sync/compare-summary' import { SourceControlCommitSurface } from './commit-surface' import { SourceControlForkPushNotice } from './fork-push-notice' +import { SourceControlMergedReviewNotice } from './merged-review-workspace-notice' import type { SourceControlPanelReadyProps } from './panel-props' import { useSourceControlScope } from './worktree-scope' import { openGitGraph } from '@/lib/source-control/graph/git-graph-tab' @@ -135,6 +136,19 @@ export function SourceControlPanelContent(props: SourceControlPanelReadyProps) { {/* Keep CommitArea through clean/mid-commit states (#1448); only active conflict operations replace the action surface. */} + {model.isFolder ? null : ( + + )} + {shouldRenderCommitArea( unresolvedConflicts.length, conflictOperation, diff --git a/src/renderer/src/components/sidebar/workspace-actions/deletion/delete-worktree-flow.test.ts b/src/renderer/src/components/sidebar/workspace-actions/deletion/delete-worktree-flow.test.ts index 179ab483b53..dc4e51ebbbb 100644 --- a/src/renderer/src/components/sidebar/workspace-actions/deletion/delete-worktree-flow.test.ts +++ b/src/renderer/src/components/sidebar/workspace-actions/deletion/delete-worktree-flow.test.ts @@ -108,6 +108,8 @@ vi.mock('./delete-worktree-failure-toast', () => ({ import { toast } from 'sonner' import { showDeleteWorktreeFailureToast } from './delete-worktree-failure-toast' +import { runWorktreeDelete } from './delete-worktree-flow' + import { runWorktreeDeletesInParallel } from './worktree-delete-execution' function setWorktrees( @@ -159,6 +161,18 @@ describe('delete worktree flow', () => { setWorktrees([]) }) + it('keeps the ignored-file confirmation available when a caller requires it despite skip-confirm', () => { + setWorktrees([{ id: 'ignored-only' }]) + mocks.state.settings.skipDeleteWorktreeConfirm = true + runWorktreeDelete('ignored-only', { forceConfirm: true }) + expect(mocks.state.openModal).toHaveBeenCalledWith('delete-worktree', { + worktreeId: 'ignored-only', + worktreeDeleteIdentities: [{ id: 'ignored-only', instanceId: 'ignored-only-instance' }], + allowSkipConfirm: false + }) + expect(mocks.state.removeWorktree).not.toHaveBeenCalled() + }) + it('revalidates each queued instance immediately before execution', async () => { // Model a host relying on renderer serialization for same-repo branch cleanup. setWorktrees([ diff --git a/src/renderer/src/components/sidebar/workspace-actions/deletion/delete-worktree-flow.ts b/src/renderer/src/components/sidebar/workspace-actions/deletion/delete-worktree-flow.ts index fffde41c774..df5298c931d 100644 --- a/src/renderer/src/components/sidebar/workspace-actions/deletion/delete-worktree-flow.ts +++ b/src/renderer/src/components/sidebar/workspace-actions/deletion/delete-worktree-flow.ts @@ -96,6 +96,7 @@ export function runWorktreeDelete(worktreeId: string, options: WorktreeDeleteOpt const hasLineageChildren = deleteLineage.descendants.length > 0 // Skip-confirm preferences never authorize discarding unsaved editor text. const skipConfirm = + options.forceConfirm !== true && (state.settings?.skipDeleteWorktreeConfirm ?? false) && collectRemovalUnsavedFiles(useAppStore.getState, [target]).length === 0 if (skipConfirm && !hasLineageChildren) { @@ -114,7 +115,7 @@ export function runWorktreeDelete(worktreeId: string, options: WorktreeDeleteOpt ]) } : {}), - ...(hasLineageChildren ? { allowSkipConfirm: false } : {}) + ...(hasLineageChildren || options.forceConfirm ? { allowSkipConfirm: false } : {}) }) } diff --git a/src/renderer/src/components/sidebar/workspace-actions/deletion/worktree-delete-request.ts b/src/renderer/src/components/sidebar/workspace-actions/deletion/worktree-delete-request.ts index 8a8da65caaf..04e926e6cc4 100644 --- a/src/renderer/src/components/sidebar/workspace-actions/deletion/worktree-delete-request.ts +++ b/src/renderer/src/components/sidebar/workspace-actions/deletion/worktree-delete-request.ts @@ -15,6 +15,7 @@ export type WorktreeBatchDeleteOptions = { export type WorktreeDeleteIdentity = Pick export type WorktreeDeleteOptions = { + forceConfirm?: boolean expectedInstanceId?: string /** Pin deletion to the confirmed host because workspace IDs repeat across hosts. */ expectedHostId?: ExecutionHostId diff --git a/src/renderer/src/components/sidebar/workspace-card/surface/WorktreeCardStatusSlot.tsx b/src/renderer/src/components/sidebar/workspace-card/surface/WorktreeCardStatusSlot.tsx index 5d2f1b671ca..a32b5015990 100644 --- a/src/renderer/src/components/sidebar/workspace-card/surface/WorktreeCardStatusSlot.tsx +++ b/src/renderer/src/components/sidebar/workspace-card/surface/WorktreeCardStatusSlot.tsx @@ -114,6 +114,9 @@ function getReviewStatusLabel(review: WorktreeCardPrDisplay, t: Translate = tran if (outcome === 'success') { return t('workspace.reviewStatus.checksPassing', '{{review}} checks: Passing', values) } + if (!review.state && !outcome) { + return getReviewLabel(review) + } return t('workspace.reviewStatus.open', '{{review}}: Open', values) } @@ -148,14 +151,17 @@ export function WorktreeCardStatusSlot({ const vmState = vmRuntime ? getEphemeralVmHostState(vmRuntime.status, vmWaking) : null const vmFailure = vmRuntime?.failure ?? null const statusLabel = getWorktreeStatusLabel(status) || status - // Sleeping remains distinct from quiet done/branch/review states; attention keeps its own glyph. - const canShowSleepingStatus = - newCardStyle && showStatus && isSleeping && QUIET_REVIEW_REPLACEABLE_STATUSES.has(status) const canShowReviewStatus = newCardStyle && showStatus && (prDisplay !== null || Boolean(collectionChecks)) && - !canShowSleepingStatus && + QUIET_REVIEW_REPLACEABLE_STATUSES.has(status) + // Review outranks sleep: the dimmed row already shows sleep, only the glyph can show the PR. + const canShowSleepingStatus = + newCardStyle && + showStatus && + isSleeping && + !canShowReviewStatus && QUIET_REVIEW_REPLACEABLE_STATUSES.has(status) const canShowBranchStatus = newCardStyle && @@ -178,10 +184,14 @@ export function WorktreeCardStatusSlot({ isReviewStale && canShowReviewStatus && (collectionChecks || prDisplay) ? `${passiveStatusLabel} · ${t('workspace.links.lastKnownStatus', 'Last known status')}` : passiveStatusLabel + const laneLabel = + canShowReviewStatus && isSleeping + ? `${reviewLabel} · ${getSleepingStatusLabel(t)}` + : reviewLabel const passiveStatusAnnouncement = newCardStyle && isUnread - ? t('workspace.activity.unreadLabel', '{{status}} · Unread', { status: reviewLabel }) - : reviewLabel + ? t('workspace.activity.unreadLabel', '{{status}} · Unread', { status: laneLabel }) + : laneLabel // Working/permission own the lane; retain unread in accessible copy and restore its dot afterward. const showNewCardUnreadAlert = newCardStyle && isUnread && showStatus && status !== 'working' && status !== 'permission' diff --git a/src/renderer/src/i18n/en-runtime-required.json b/src/renderer/src/i18n/en-runtime-required.json index 1ddc84b29ec..de2a463f586 100644 --- a/src/renderer/src/i18n/en-runtime-required.json +++ b/src/renderer/src/i18n/en-runtime-required.json @@ -429,6 +429,8 @@ "discardFailedSampleMore_other": "{{paths}}, +{{count}} more", "discardFailed_one": "Failed to discard {{count}} file", "discardFailed_other": "Failed to discard {{count}} files", + "mergedReviewUncommitted_one": "{{review}} is merged, but this workspace has {{count}} uncommitted change that is not in it.", + "mergedReviewUncommitted_other": "{{review}} is merged, but this workspace has {{count}} uncommitted changes that are not in it.", "noteCount_one": "{{count}} note", "noteCount_other": "{{count}} notes" }, diff --git a/src/renderer/src/i18n/locales/en.json b/src/renderer/src/i18n/locales/en.json index 7bcaf318aee..dc1abe90ef6 100644 --- a/src/renderer/src/i18n/locales/en.json +++ b/src/renderer/src/i18n/locales/en.json @@ -14798,6 +14798,11 @@ "locallyResolvedConflictOfKind": "Conflict resolved locally ({{kind}})", "locallyResolvedConflict": "Conflict resolved locally", "pushesToForkTarget": "Pushes to fork {{target}}", + "mergedReviewUncommitted_one": "{{review}} is merged, but this workspace has {{count}} uncommitted change that is not in it.", + "mergedReviewUncommitted_other": "{{review}} is merged, but this workspace has {{count}} uncommitted changes that are not in it.", + "mergedReviewHeadMoved": "{{review}} is merged, but Orca could not confirm that it includes this branch’s current commits.", + "mergedReviewNothingLeft": "{{review}} is merged and includes this branch’s committed changes.", + "mergedReviewSafeToDelete": "{{review}} is merged and includes this branch’s committed changes. Ignored files may still be lost when deleting this workspace.", "abortRebaseConfirmTitle": "Abort rebase?", "abortMergeConfirmTitle": "Abort merge?", "abortRebaseConfirmDescription": "This cancels the rebase in progress and can discard conflict resolutions made during this rebase.", diff --git a/src/shared/source-control/decisions/merged-review-workspace-readiness.test.ts b/src/shared/source-control/decisions/merged-review-workspace-readiness.test.ts new file mode 100644 index 00000000000..e0f4aaac528 --- /dev/null +++ b/src/shared/source-control/decisions/merged-review-workspace-readiness.test.ts @@ -0,0 +1,73 @@ +import { describe, expect, it } from 'vitest' +import { resolveMergedReviewWorkspaceReadiness } from './merged-review-workspace-readiness' + +const merged = { state: 'merged' as const, headSha: 'abc123' } + +describe('resolveMergedReviewWorkspaceReadiness', () => { + it('is safe to delete when HEAD is the merged head and the tree is clean', () => { + expect( + resolveMergedReviewWorkspaceReadiness({ + review: merged, + headOid: 'ABC123', + uncommittedCount: 0 + }) + ).toEqual({ kind: 'safe-to-delete' }) + }) + + it('is safe to delete when the provider confirmed HEAD is in the merged review', () => { + expect( + resolveMergedReviewWorkspaceReadiness({ + review: { ...merged, headSha: 'updated-on-github', confirmedContainedHeadOid: 'local' }, + headOid: 'local', + uncommittedCount: 0 + }) + ).toEqual({ kind: 'safe-to-delete' }) + }) + + it('reports uncommitted changes before HEAD identity', () => { + expect( + resolveMergedReviewWorkspaceReadiness({ + review: merged, + headOid: 'abc123', + uncommittedCount: 3 + }) + ).toEqual({ kind: 'uncommitted-changes', count: 3 }) + }) + + it('leaves a different checkout head unverified without assuming its commits are absent', () => { + expect( + resolveMergedReviewWorkspaceReadiness({ + review: merged, + headOid: 'def456', + uncommittedCount: 0 + }) + ).toEqual({ kind: 'head-unverified' }) + }) + + it('leaves missing checkout heads and missing review proof unverified', () => { + expect( + resolveMergedReviewWorkspaceReadiness({ + review: merged, + headOid: null, + uncommittedCount: 0 + }) + ).toEqual({ kind: 'head-unverified' }) + expect( + resolveMergedReviewWorkspaceReadiness({ + review: { state: 'merged' }, + headOid: 'abc123', + uncommittedCount: 0 + }) + ).toEqual({ kind: 'head-unverified' }) + }) + + it('says nothing for reviews that are not merged', () => { + expect( + resolveMergedReviewWorkspaceReadiness({ + review: { ...merged, state: 'open' }, + headOid: 'abc123', + uncommittedCount: 0 + }) + ).toBeNull() + }) +}) diff --git a/src/shared/source-control/decisions/merged-review-workspace-readiness.ts b/src/shared/source-control/decisions/merged-review-workspace-readiness.ts new file mode 100644 index 00000000000..41f93f7dda0 --- /dev/null +++ b/src/shared/source-control/decisions/merged-review-workspace-readiness.ts @@ -0,0 +1,33 @@ +import type { HostedReviewInfo } from '../hosted-review/hosted-review-contracts' + +export type MergedReviewWorkspaceReadiness = + /** HEAD is the merged review's head (or GitHub confirmed it is in the review) and nothing is uncommitted. */ + | { kind: 'safe-to-delete' } + | { kind: 'uncommitted-changes'; count: number } + /** Merged, but this checkout's HEAD cannot be matched to the review, so its commits may be elsewhere. */ + | { kind: 'head-unverified' } + +/** + * Whether a workspace whose review merged still holds work. A squash or rebase merge copies commits onto + * the base with new IDs, so the branch compare keeps listing them; only HEAD identity proves they landed. + */ +export function resolveMergedReviewWorkspaceReadiness(input: { + review: Pick | null + headOid: string | null | undefined + uncommittedCount: number +}): MergedReviewWorkspaceReadiness | null { + const { review, uncommittedCount } = input + if (review?.state !== 'merged') { + return null + } + if (uncommittedCount > 0) { + return { kind: 'uncommitted-changes', count: uncommittedCount } + } + const head = input.headOid?.trim().toLowerCase() ?? '' + const reviewHeads = [review.headSha, review.confirmedContainedHeadOid].map( + (oid) => oid?.trim().toLowerCase() ?? '' + ) + return head.length > 0 && reviewHeads.includes(head) + ? { kind: 'safe-to-delete' } + : { kind: 'head-unverified' } +}