diff --git a/CHANGELOG.md b/CHANGELOG.md index 2adde427ff7..298d272dbe1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,6 +32,9 @@ offer an action to open the most recently imported task or create another independent import. An uncertain result remains visible as a warning while still allowing a user-initiated import again in both Desktop and TUI. +- The Git changes panel can pick which branch it compares against — searchable, + per Session — instead of only the branch the backend resolves. The choice + persists, and falls back to the resolved branch when the pinned one disappears. - Added `/transcript` to browse long TUI sessions without depending on terminal scrollback, with line, page, and first/last navigation. diff --git a/apps/desktop/src/main/__tests__/git-review-main.test.ts b/apps/desktop/src/main/__tests__/git-review-main.test.ts index b41b7c65b81..c820ada0d7c 100644 --- a/apps/desktop/src/main/__tests__/git-review-main.test.ts +++ b/apps/desktop/src/main/__tests__/git-review-main.test.ts @@ -51,11 +51,11 @@ describe('Git Review snapshot authority', () => { const branch = await readGitReview(root, 'branch'); assert.equal(branch.ok, true); if (!branch.ok) return; - assert.equal(branch.snapshot.baseBranch, 'main'); + assert.equal(branch.snapshot.baseBranch, 'refs/heads/main'); assert.equal(branch.snapshot.currentBranch, 'feature/review'); assert.deepEqual(branch.snapshot.baseBranchOptions, [ - 'feature/review', - 'main', + { label: 'main', value: 'refs/heads/main' }, + { label: 'feature/review', value: 'refs/heads/feature/review' }, ]); assert.deepEqual( branch.snapshot.files.map((file) => file.path).sort(), @@ -67,11 +67,11 @@ describe('Git Review snapshot authority', () => { root, 'branch', undefined, - 'feature/review', + 'refs/heads/feature/review', ); assert.equal(currentBranchOnly.ok, true); if (currentBranchOnly.ok) { - assert.equal(currentBranchOnly.snapshot.baseBranch, 'feature/review'); + assert.equal(currentBranchOnly.snapshot.baseBranch, 'refs/heads/feature/review'); assert.equal( currentBranchOnly.snapshot.files.some((file) => file.path === 'feature.txt'), false, @@ -79,7 +79,10 @@ describe('Git Review snapshot authority', () => { } assert.deepEqual( await readGitReview(root, 'branch', undefined, 'missing-branch'), - { ok: false, reason: 'invalid_base_branch' }, + { ok: false, reason: 'invalid_base_branch', branches: { + currentBranch: branch.snapshot.currentBranch, + baseBranchOptions: branch.snapshot.baseBranchOptions, + } }, ); const unstaged = await readGitReview(root, 'unstaged'); @@ -99,6 +102,155 @@ describe('Git Review snapshot authority', () => { ); }); + it('lists the remote default branch before the branches it resolves from', async () => { + const origin = await repository(); + await git(origin, 'branch', 'release/0.1'); + const root = await temporaryRoot(); + await git(root, 'clone', origin, '.'); + + const result = await readGitReview(root, 'branch'); + assert.equal(result.ok, true); + if (!result.ok) return; + assert.equal(result.snapshot.baseBranch, 'refs/remotes/origin/main'); + assert.deepEqual(result.snapshot.baseBranchOptions, [ + { label: 'origin/HEAD', value: 'refs/remotes/origin/HEAD' }, + { label: 'origin/main', value: 'refs/remotes/origin/main' }, + { label: 'main', value: 'refs/heads/main' }, + { label: 'origin/release/0.1', value: 'refs/remotes/origin/release/0.1' }, + ]); + }); + + it('compares the branch rather than a same-named tag, using a qualified ref', async () => { + const root = await repository(); + await git(root, 'tag', 'release'); + await git(root, 'checkout', '-b', 'release'); + await writeFile(join(root, 'release.txt'), 'release\n', 'utf8'); + await git(root, 'add', '.'); + await git(root, 'commit', '-m', 'release'); + await git(root, 'checkout', '-b', 'feature'); + await writeFile(join(root, 'feature.txt'), 'feature\n', 'utf8'); + await git(root, 'add', '.'); + await git(root, 'commit', '-m', 'feature'); + + const result = await readGitReview(root, 'branch', undefined, 'refs/heads/release'); + assert.equal(result.ok, true); + if (!result.ok) return; + assert.equal(result.snapshot.baseBranch, 'refs/heads/release'); + assert.deepEqual(result.snapshot.files.map((file) => file.path), ['feature.txt']); + assert.ok(result.snapshot.baseBranchOptions.every((option) => + option.value.startsWith('refs/heads/') || option.value.startsWith('refs/remotes/'))); + assert.deepEqual(await readGitReview(root, 'branch', undefined, 'refs/tags/release'), + { ok: false, reason: 'invalid_base_branch', branches: { currentBranch: 'feature', baseBranchOptions: [ + { label: 'main', value: 'refs/heads/main' }, + { label: 'feature', value: 'refs/heads/feature' }, + { label: 'release', value: 'refs/heads/release' }, + ] } }); + await git(root, 'tag', 'tag-only'); + assert.deepEqual(await readGitReview(root, 'branch', undefined, 'tag-only'), + { ok: false, reason: 'invalid_base_branch', branches: { currentBranch: 'feature', baseBranchOptions: [ + { label: 'main', value: 'refs/heads/main' }, + { label: 'feature', value: 'refs/heads/feature' }, + { label: 'release', value: 'refs/heads/release' }, + ] } }); + }); + + it('keeps local and remote refs with the same label distinct and rejects unqualified names', async () => { + const root = await repository(); + await git(root, 'update-ref', 'refs/remotes/origin/release', 'HEAD'); + await git(root, 'checkout', '-b', 'origin/release'); + await writeFile(join(root, 'local.txt'), 'local\n', 'utf8'); + await git(root, 'add', '.'); + await git(root, 'commit', '-m', 'local'); + const local = await readGitReview(root, 'branch', undefined, 'refs/heads/origin/release'); + const remote = await readGitReview(root, 'branch', undefined, 'refs/remotes/origin/release'); + assert.equal(local.ok, true); + assert.equal(remote.ok, true); + if (!local.ok || !remote.ok) return; + assert.equal(local.snapshot.files.length, 0); + assert.deepEqual(remote.snapshot.files.map((file) => file.path), ['local.txt']); + assert.equal(local.snapshot.baseBranchOptions.filter((option) => option.label === 'origin/release').length, 2); + assert.deepEqual(await readGitReview(root, 'branch', undefined, 'origin/release'), + { ok: false, reason: 'invalid_base_branch', branches: { + currentBranch: local.snapshot.currentBranch, + baseBranchOptions: local.snapshot.baseBranchOptions, + } }); + }); + + it('resolves the default branch without following a same-named tag', async () => { + const root = await repository(); + await git(root, 'tag', 'main'); + await writeFile(join(root, 'main.txt'), 'main\n', 'utf8'); + await git(root, 'add', '.'); + await git(root, 'commit', '-m', 'advance main'); + await git(root, 'checkout', '-b', 'feature'); + const result = await readGitReview(root, 'branch'); + assert.equal(result.ok, true); + if (!result.ok) return; + assert.equal(result.snapshot.baseBranch, 'refs/heads/main'); + assert.equal(result.snapshot.files.length, 0); + }); + + it('returns branch choices when unrelated history prevents a diff, including on repeated reads', async () => { + const root = await repository(); + await git(root, 'checkout', '--orphan', 'gh-pages'); + await git(root, 'rm', '-rf', '.'); + await writeFile(join(root, 'index.html'), 'site\n', 'utf8'); + await git(root, 'add', '.'); + await git(root, 'commit', '-m', 'independent site history'); + await git(root, 'checkout', 'main'); + + for (let attempt = 0; attempt < 2; attempt += 1) { + const result = await readGitReview(root, 'branch', undefined, 'refs/heads/gh-pages'); + assert.equal(result.ok, false); + if (result.ok) return; + assert.equal(result.reason, 'git_failed'); + assert.deepEqual(result.branches, { + currentBranch: 'main', + baseBranchOptions: [ + { label: 'main', value: 'refs/heads/main' }, + { label: 'gh-pages', value: 'refs/heads/gh-pages' }, + ], + }); + } + const recovered = await readGitReview(root, 'branch', undefined, 'refs/heads/main'); + assert.equal(recovered.ok, true); + if (recovered.ok) assert.equal(recovered.snapshot.files.length, 0); + }); + + it('degrades a diff that overflows the git buffer to a truncated review', async () => { + const root = await repository(); + await git(root, 'checkout', '-b', 'feature'); + await writeFile(join(root, 'feature.txt'), 'feature\n', 'utf8'); + await git(root, 'add', '.'); + await git(root, 'commit', '-m', 'feature'); + + const result = await readGitReview(root, 'branch', async (gitRoot, args) => { + if (args.includes('--binary')) { + // Node rejects an over-limit child buffer, handing back what it read. + throw Object.assign(new Error('stdout maxBuffer length exceeded'), { + code: 'ERR_CHILD_PROCESS_STDIO_MAXBUFFER', + stdout: [ + 'diff --git a/feature.txt b/feature.txt', + 'new file mode 100644', + '--- /dev/null', + '+++ b/feature.txt', + '@@ -0,0 +1 @@', + '+feature', + '', + ].join('\n'), + }); + } + const { stdout } = await execFileAsync('git', ['-C', gitRoot, ...args], { + encoding: 'utf8', + }); + return stdout; + }); + assert.equal(result.ok, true); + if (!result.ok) return; + assert.equal(result.snapshot.truncated, true); + assert.deepEqual(result.snapshot.files.map((file) => file.path), ['feature.txt']); + }); + it('returns an explicit non-repository outcome', async () => { const root = await temporaryRoot(); assert.deepEqual(await readGitReview(root, 'branch'), { @@ -116,7 +268,7 @@ describe('Git Review snapshot authority', () => { const result = await readGitReview(root, 'branch'); assert.equal(result.ok, true); if (!result.ok) return; - assert.equal(result.snapshot.baseBranch, null); + assert.equal(result.snapshot.baseBranch, 'refs/heads/main'); assert.deepEqual( result.snapshot.files.map((file) => file.path).sort(), ['base.txt', 'staged.txt'], diff --git a/apps/desktop/src/main/__tests__/review-base-branch-preferences.test.ts b/apps/desktop/src/main/__tests__/review-base-branch-preferences.test.ts new file mode 100644 index 00000000000..e3decda7fc9 --- /dev/null +++ b/apps/desktop/src/main/__tests__/review-base-branch-preferences.test.ts @@ -0,0 +1,101 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import assert from 'node:assert/strict'; +import { afterEach, beforeEach, describe, it } from 'node:test'; +import { + createReviewBaseBranchPreferences, + REVIEW_BASE_BRANCH_STORAGE_KEY, +} from '../../renderer/platform/desktop/review-base-branch-preferences.js'; + +function memoryStorage() { + const items = new Map(); + return { + getItem: (key: string) => items.get(key) ?? null, + setItem: (key: string, value: string) => { items.set(key, value); }, + }; +} + +describe('Desktop review base branch preferences', () => { + let storage: ReturnType; + let originalStorage: PropertyDescriptor | undefined; + + beforeEach(() => { + originalStorage = Object.getOwnPropertyDescriptor(globalThis, 'localStorage'); + storage = memoryStorage(); + Object.defineProperty(globalThis, 'localStorage', { configurable: true, value: storage }); + }); + + afterEach(() => { + if (originalStorage) Object.defineProperty(globalThis, 'localStorage', originalStorage); + else Reflect.deleteProperty(globalThis, 'localStorage'); + }); + + it('persists explicit refs across adapter instances without pinning new Sessions', () => { + const preferences = createReviewBaseBranchPreferences(); + preferences.write('session-a', 'refs/remotes/origin/develop'); + preferences.write('session-b', 'refs/heads/main'); + const reopened = createReviewBaseBranchPreferences(); + assert.equal(reopened.read('session-a'), 'refs/remotes/origin/develop'); + assert.equal(reopened.read('session-b'), 'refs/heads/main'); + assert.equal(reopened.read('new-session'), null); + reopened.write('session-a', null); + assert.equal(preferences.read('session-a'), null); + assert.equal(preferences.read('session-b'), 'refs/heads/main'); + }); + + it('ignores malformed records and recovers on the next explicit choice', () => { + const preferences = createReviewBaseBranchPreferences(); + for (const invalid of ['{ not json', 'null', '[]', '3']) { + storage.setItem(REVIEW_BASE_BRANCH_STORAGE_KEY, invalid); + assert.equal(preferences.read('session'), null); + preferences.write('session', 'refs/heads/main'); + assert.equal(preferences.read('session'), 'refs/heads/main'); + } + storage.setItem(REVIEW_BASE_BRANCH_STORAGE_KEY, JSON.stringify({ + empty: '', invalid: 7, valid: 'refs/heads/main', + })); + assert.equal(preferences.read('empty'), null); + assert.equal(preferences.read('invalid'), null); + assert.equal(preferences.read('valid'), 'refs/heads/main'); + preferences.write('another', 'refs/heads/develop'); + assert.deepEqual(JSON.parse(storage.getItem(REVIEW_BASE_BRANCH_STORAGE_KEY)!), { + valid: 'refs/heads/main', another: 'refs/heads/develop', + }); + }); + + it('handles a restricted localStorage getter lazily', () => { + Object.defineProperty(globalThis, 'localStorage', { + configurable: true, + get() { throw new Error('Storage access denied'); }, + }); + const preferences = createReviewBaseBranchPreferences(); + assert.equal(preferences.read('session'), null); + assert.doesNotThrow(() => preferences.write('session', 'refs/heads/main')); + assert.doesNotThrow(() => preferences.write('session', null)); + }); + + it('handles read failures and quota errors without breaking comparisons', () => { + storage.getItem = () => { throw new Error('Read denied'); }; + storage.setItem = () => { throw new Error('Quota exceeded'); }; + const preferences = createReviewBaseBranchPreferences(); + assert.equal(preferences.read('session'), null); + assert.doesNotThrow(() => preferences.write('session', 'refs/heads/main')); + }); +}); diff --git a/apps/desktop/src/main/__tests__/session-review-base-branch.test.ts b/apps/desktop/src/main/__tests__/session-review-base-branch.test.ts new file mode 100644 index 00000000000..82fd8ddbec8 --- /dev/null +++ b/apps/desktop/src/main/__tests__/session-review-base-branch.test.ts @@ -0,0 +1,63 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import assert from 'node:assert/strict'; +import { describe, it } from 'node:test'; +import { createElement } from 'react'; +import { renderToStaticMarkup } from 'react-dom/server'; +import { + SessionReviewBaseBranchPicker, +} from '../../renderer/features/workbar/testing.js'; + +const AUTO_SENTINEL = 'AUTO_SENTINEL'; + +function renderPicker(baseBranch: string | null) { + return renderToStaticMarkup( + createElement(SessionReviewBaseBranchPicker, { + baseBranch, + baseBranchOptions: [ + { label: 'main', value: 'refs/heads/main' }, + { label: 'origin/develop', value: 'refs/remotes/origin/develop' }, + ], + label: AUTO_SENTINEL, + onSelect: () => undefined, + }), + ); +} + +/** The visible trigger only: `label` is required by Selector and always lands + * in the markup as a visually hidden element, sentinel and all. */ +function renderTrigger(baseBranch: string | null) { + const markup = renderPicker(baseBranch); + const start = markup.indexOf('', start)); +} + +describe('session review base branch', () => { + it('shows the compared branch instead of an auto pseudo-entry', () => { + const trigger = renderTrigger('refs/remotes/origin/develop'); + assert.match(trigger, />origin\/develop { + assert.match(renderTrigger(null), new RegExp(AUTO_SENTINEL)); + }); +}); diff --git a/apps/desktop/src/main/__tests__/session-review-panel-recovery.test.ts b/apps/desktop/src/main/__tests__/session-review-panel-recovery.test.ts new file mode 100644 index 00000000000..c5637efca04 --- /dev/null +++ b/apps/desktop/src/main/__tests__/session-review-panel-recovery.test.ts @@ -0,0 +1,275 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import assert from 'node:assert/strict'; +import { test } from 'node:test'; +import { parseHTML } from 'linkedom'; +import { act, createElement } from 'react'; +import { createRoot } from 'react-dom/client'; +import { LocaleProvider } from '@maka/ui'; +import { + type WorkbarServices, + createFakeWorkbarServices, + WorkbarServicesProvider, + SessionReviewPanel, +} from '../../renderer/features/workbar/testing.js'; + +test('a saved failing comparison keeps the picker available and can recover', async () => { + const { document, restore } = installDom(); + const container = document.querySelector('#root'); + assert.ok(container); + const root = createRoot(container); + const requests: Array = []; + const branches = { + currentBranch: 'feature', + baseBranchOptions: [ + { label: 'main', value: 'refs/heads/main' }, + { label: 'gh-pages', value: 'refs/heads/gh-pages' }, + ], + }; + const review: WorkbarServices['review'] = { + read: async ({ baseBranch }) => { + requests.push(baseBranch); + if (baseBranch === 'refs/heads/gh-pages') { + return { ok: false, reason: 'git_failed', branches }; + } + return { + ok: true, + snapshot: { + ...branches, source: 'branch', repositoryRoot: '/repo', + baseBranch: 'refs/heads/main', revision: 'recovered', + files: [], additions: 0, deletions: 0, truncated: false, + }, + }; + }, + subscribeSessionEvents: () => () => undefined, + }; + const services = createFakeWorkbarServices({ review }); + try { + services.reviewBaseBranchPreference.write('saved-session', 'refs/heads/gh-pages'); + await act(async () => { + root.render(createElement(LocaleProvider, { + locale: 'en', + children: createElement(WorkbarServicesProvider, { services }, + createElement(SessionReviewPanel, { sessionId: 'saved-session', active: true })), + })); + }); + assert.match(container.textContent ?? '', /Could not read Git workspace changes/); + const trigger = container.querySelector('.maka-session-review-base-branch button'); + assert.ok(trigger, 'failed initial read must preserve a comparison picker'); + await act(async () => { trigger.click(); }); + const main = Array.from(document.querySelectorAll('[role="option"]')).find( + (option) => option.textContent === 'main'); + assert.ok(main, 'main remains selectable after the diff fails'); + await act(async () => { main.click(); }); + assert.deepEqual(requests, ['refs/heads/gh-pages', 'refs/heads/main']); + assert.doesNotMatch(container.textContent ?? '', /Could not read Git workspace changes/); + assert.ok(container.querySelector('.maka-session-review-base-branch button')); + } finally { + await act(async () => { root.unmount(); }); + restore(); + } +}); + +for (const reason of ['not_git_repository', 'workspace_unavailable', 'git_failed'] as const) { + test(`a refresh clears stale branch options after ${reason}`, async () => { + const { document, restore } = installDom(); + const container = document.querySelector('#root'); + assert.ok(container); + const root = createRoot(container); + let unavailable = false; + const services = createFakeWorkbarServices({ review: { + read: async () => unavailable + ? { ok: false, reason } + : { ok: true, snapshot: { + source: 'branch', repositoryRoot: '/repo', currentBranch: 'feature', + baseBranch: 'refs/heads/main', + baseBranchOptions: [{ label: 'main', value: 'refs/heads/main' }], + revision: 'initial', files: [], additions: 0, deletions: 0, truncated: false, + } }, + subscribeSessionEvents: () => () => undefined, + } }); + const render = (active: boolean) => root.render(createElement(LocaleProvider, { + locale: 'en', + children: createElement(WorkbarServicesProvider, { services }, + createElement(SessionReviewPanel, { sessionId: 'vanished-repo', active })), + })); + try { + await act(async () => { render(true); }); + assert.ok(container.querySelector('.maka-session-review-base-branch')); + await act(async () => { render(false); }); + unavailable = true; + await act(async () => { render(true); }); + assert.equal(container.querySelector('.maka-session-review-base-branch'), null); + } finally { + await act(async () => { root.unmount(); }); + restore(); + } + }); +} + +test('a disappeared saved branch clears the pin and retries with the dynamic default', async () => { + const { document, restore } = installDom(); + const container = document.querySelector('#root'); + assert.ok(container); + const root = createRoot(container); + const sessionId = 'disappeared-branch-session'; + const requests: Array = []; + const review: WorkbarServices['review'] = { + read: async ({ baseBranch }) => { + requests.push(baseBranch); + if (requests.length === 1) { + return { ok: false, reason: 'invalid_base_branch', branches: { + currentBranch: 'feature', + baseBranchOptions: [{ label: 'main', value: 'refs/heads/main' }], + } }; + } + assert.equal(services.reviewBaseBranchPreference.read(sessionId), null, 'clear storage before retrying'); + return { + ok: true, + snapshot: { + currentBranch: 'feature', + baseBranchOptions: [{ label: 'main', value: 'refs/heads/main' }], + source: 'branch', repositoryRoot: '/repo', + baseBranch: 'refs/heads/main', revision: 'recovered', + files: [], additions: 0, deletions: 0, truncated: false, + }, + }; + }, + subscribeSessionEvents: () => () => undefined, + }; + const services = createFakeWorkbarServices({ review }); + try { + services.reviewBaseBranchPreference.write(sessionId, 'refs/heads/gh-pages'); + await act(async () => { + root.render(createElement(LocaleProvider, { + locale: 'en', + children: createElement(WorkbarServicesProvider, { services }, + createElement(SessionReviewPanel, { sessionId, active: true })), + })); + }); + assert.deepEqual(requests, ['refs/heads/gh-pages', undefined]); + assert.equal(services.reviewBaseBranchPreference.read(sessionId), null, 'the resolved default must remain unpinned'); + const trigger = container.querySelector('.maka-session-review-base-branch button'); + assert.ok(trigger, 'automatic recovery restores the comparison picker'); + assert.match(trigger.textContent ?? '', /main/); + await act(async () => { trigger.click(); }); + const selected = document.querySelector('[role="option"][aria-selected="true"]'); + assert.equal(selected?.textContent, 'main', 'the resolved default is selected in the picker'); + assert.equal(container.querySelector('[role="alert"]'), null, 'recovery leaves no error banner'); + assert.doesNotMatch(container.textContent ?? '', /Could not read Git workspace changes/); + assert.equal(container.querySelector('[aria-busy="true"]'), null); + } finally { + await act(async () => { root.unmount(); }); + restore(); + } +}); + +test('a comparison switch spins the picker and dims the stale diff until it lands', async () => { + const { document, restore } = installDom(); + const container = document.querySelector('#root'); + assert.ok(container); + const root = createRoot(container); + const branches = { + currentBranch: 'feature', + baseBranchOptions: [ + { label: 'main', value: 'refs/heads/main' }, + { label: 'gh-pages', value: 'refs/heads/gh-pages' }, + ], + }; + let landSlowRead: (() => void) | undefined; + const review: WorkbarServices['review'] = { + read: async ({ baseBranch }) => { + if (baseBranch === 'refs/heads/gh-pages') { + await new Promise((resolve) => { landSlowRead = resolve; }); + } + return { + ok: true, + snapshot: { + ...branches, source: 'branch', repositoryRoot: '/repo', + baseBranch: baseBranch ?? 'refs/heads/main', revision: baseBranch ?? 'initial', + files: [], additions: 0, deletions: 0, truncated: false, + }, + }; + }, + subscribeSessionEvents: () => () => undefined, + }; + const services = createFakeWorkbarServices({ review }); + try { + await act(async () => { + root.render(createElement(LocaleProvider, { + locale: 'en', + children: createElement(WorkbarServicesProvider, { services }, + createElement(SessionReviewPanel, { sessionId: 'switch-session', active: true })), + })); + }); + assert.equal(container.querySelector('.maka-session-review-switching'), null); + const trigger = container.querySelector('.maka-session-review-base-branch button'); + assert.ok(trigger); + await act(async () => { trigger.click(); }); + const ghPages = Array.from(document.querySelectorAll('[role="option"]')).find( + (option) => option.textContent === 'gh-pages'); + assert.ok(ghPages); + await act(async () => { ghPages.click(); }); + assert.ok(container.querySelector('.maka-session-review-switching'), 'the panel reports the pending switch'); + assert.ok(container.querySelector('.maka-session-review-base-branch [aria-busy="true"]'), 'the picker spins while the read is in flight'); + await act(async () => { landSlowRead?.(); }); + assert.equal(container.querySelector('.maka-session-review-switching'), null); + assert.equal(container.querySelector('.maka-session-review-base-branch [aria-busy="true"]'), null); + } finally { + await act(async () => { root.unmount(); }); + restore(); + } +}); + +function installDom() { + const { document, window } = parseHTML('
'); + const globals = { + document, window, + matchMedia: (media: string) => ({ + matches: false, + media, + onchange: null, + addListener() {}, + removeListener() {}, + addEventListener() {}, + removeEventListener() {}, + dispatchEvent: () => true, + }), + HTMLElement: window.HTMLElement, + HTMLIFrameElement: window.HTMLIFrameElement ?? class HTMLIFrameElement {}, + requestAnimationFrame: () => 1, + cancelAnimationFrame: () => undefined, + IS_REACT_ACT_ENVIRONMENT: true, + }; + const previous = new Map(Object.keys(globals).map((key) => + [key, Object.getOwnPropertyDescriptor(globalThis, key)])); + for (const [key, value] of Object.entries(globals)) { + Object.defineProperty(globalThis, key, { configurable: true, writable: true, value }); + } + return { + document, + restore: () => { + for (const [key, descriptor] of previous) { + if (descriptor) Object.defineProperty(globalThis, key, descriptor); + else Reflect.deleteProperty(globalThis, key); + } + }, + }; +} diff --git a/apps/desktop/src/main/git-review-main.ts b/apps/desktop/src/main/git-review-main.ts index a75b209c13e..1c39775f197 100644 --- a/apps/desktop/src/main/git-review-main.ts +++ b/apps/desktop/src/main/git-review-main.ts @@ -24,6 +24,8 @@ import { isAbsolute, relative, resolve } from 'node:path'; import { promisify } from 'node:util'; import { countDiffLineStats } from '@maka/core/unified-diff'; import { + type GitReviewBaseBranchOption, + type GitReviewBranchContext, type GitReviewFile, type GitReviewFileStatus, type GitReviewReadResult, @@ -47,6 +49,7 @@ export async function readGitReview( runGit: GitReviewCommandRunner = runGitCommand, requestedBaseBranch?: string, ): Promise { + let branches: GitReviewBranchContext | undefined; try { const repositoryRoot = await resolveProjectRoot([cwd]); if (!(await resolveProjectGitInfo(repositoryRoot)).isGitRepo) { @@ -61,17 +64,17 @@ export async function readGitReview( source === 'branch' && hasHead ? await listBaseBranches(repositoryRoot, runGit) : []; - if ( - source === 'branch' && - requestedBaseBranch != null && - !baseBranchOptions.includes(requestedBaseBranch) - ) { - return { ok: false, reason: 'invalid_base_branch' }; + if (source === 'branch') branches = { currentBranch, baseBranchOptions }; + const requestedOption = requestedBaseBranch == null + ? undefined + : baseBranchOptions.find((option) => option.value === requestedBaseBranch); + if (source === 'branch' && requestedBaseBranch != null && !requestedOption) { + return { ok: false, reason: 'invalid_base_branch', branches }; } const baseBranch = source === 'branch' && hasHead - ? requestedBaseBranch ?? - (await resolveBaseBranch(repositoryRoot, currentBranch, runGit)) + ? requestedOption?.value ?? + (await resolveBaseBranch(repositoryRoot, baseBranchOptions, runGit)) : null; const branchComparison = source === 'branch' && baseBranch @@ -139,9 +142,9 @@ export async function readGitReview( }; } catch (error) { if (isUnbornRepositoryError(error)) { - return { ok: false, reason: 'unborn_repository' }; + return { ok: false, reason: 'unborn_repository', ...(branches ? { branches } : {}) }; } - return { ok: false, reason: 'git_failed' }; + return { ok: false, reason: 'git_failed', ...(branches ? { branches } : {}) }; } } @@ -168,7 +171,7 @@ async function readTrackedChanges(input: { let truncated = false; for (const comparison of comparisons) { - const [nameStatus, unified] = await Promise.all([ + const [nameStatus, unifiedResult] = await Promise.all([ input.runGit(input.repositoryRoot, [ 'diff', '--name-status', @@ -176,7 +179,7 @@ async function readTrackedChanges(input: { '--find-renames', ...comparison, ]), - input.runGit(input.repositoryRoot, [ + runDiffAllowTruncated(input.runGit, input.repositoryRoot, [ 'diff', '--no-ext-diff', '--no-color', @@ -187,7 +190,7 @@ async function readTrackedChanges(input: { ]), ]); const entries = parseNameStatus(nameStatus); - const chunks = splitUnifiedDiff(unified); + const chunks = splitUnifiedDiff(unifiedResult.stdout); for (let index = 0; index < entries.length; index += 1) { const entry = entries[index]!; const diff = chunks[index] ?? ''; @@ -203,11 +206,44 @@ async function readTrackedChanges(input: { diffChars += diff.length; } if (truncated) break; + // The diff itself was cut off at the buffer limit: keep what we read and + // say so, rather than failing the whole review on a huge branch diff. + if (unifiedResult.truncated) { + truncated = true; + break; + } } return { files: dedupeReviewFiles(files), diffChars, truncated }; } +/** + * A branch diff can exceed the child process buffer on far-diverged branches. + * Node hands back the bytes it managed to read, which the caller caps at + * REVIEW_MAX_DIFF_CHARS anyway, so overflow degrades to a truncated diff. + */ +async function runDiffAllowTruncated( + runGit: GitReviewCommandRunner, + root: string, + args: readonly string[], +): Promise<{ stdout: string; truncated: boolean }> { + try { + return { stdout: await runGit(root, args), truncated: false }; + } catch (error) { + const partial = maxBufferStdout(error); + if (partial === null) throw error; + return { stdout: partial, truncated: true }; + } +} + +function maxBufferStdout(error: unknown): string | null { + if (!(error instanceof Error)) return null; + const { code, stdout } = error as { code?: unknown; stdout?: unknown }; + return code === 'ERR_CHILD_PROCESS_STDIO_MAXBUFFER' && typeof stdout === 'string' + ? stdout + : null; +} + async function readUntrackedChanges( repositoryRoot: string, runGit: GitReviewCommandRunner, @@ -361,29 +397,38 @@ function dedupeReviewFiles(files: readonly GitReviewFile[]): GitReviewFile[] { return [...byPath.values()]; } +// The branches a review is most likely to compare against, in the order a +// reader expects them. `origin/HEAD` stays selectable: it is the remote's +// declared default, and the one name that survives a rename of that branch. +const BASE_BRANCH_PRIORITY = [ + 'refs/remotes/origin/HEAD', + 'refs/remotes/origin/main', + 'refs/remotes/origin/master', + 'refs/heads/main', + 'refs/heads/master', +]; + +// The branch a review falls back to when the caller names none. The remote's +// resolved default comes first so the panel shows a concrete name, then the +// shared priority order covers repositories whose origin/HEAD is unset. async function resolveBaseBranch( repositoryRoot: string, - currentBranch: string | null, + options: readonly GitReviewBaseBranchOption[], runGit: GitReviewCommandRunner, ): Promise { const remoteHead = cleanLine( await runGit(repositoryRoot, [ 'symbolic-ref', '--quiet', - '--short', 'refs/remotes/origin/HEAD', ]).catch(() => ''), ); - const candidates = [ - remoteHead, - 'origin/main', - 'origin/master', - 'main', - 'master', - ].filter((candidate): candidate is string => Boolean(candidate)); + const candidates = [...new Set([remoteHead, ...BASE_BRANCH_PRIORITY])].filter( + (candidate): candidate is string => Boolean(candidate), + ); for (const candidate of candidates) { - if (candidate === currentBranch) continue; - if (await gitRefExists(repositoryRoot, candidate, runGit)) return candidate; + if (options.some((option) => option.value === candidate) && + await gitRefExists(repositoryRoot, candidate, runGit)) return candidate; } return null; } @@ -391,16 +436,32 @@ async function resolveBaseBranch( async function listBaseBranches( repositoryRoot: string, runGit: GitReviewCommandRunner, -): Promise { +): Promise { const output = await runGit(repositoryRoot, [ 'for-each-ref', - '--format=%(refname:short)', + '--format=%(refname)', 'refs/heads', 'refs/remotes', ]); - return [...new Set(output.split('\n').map((line) => line.trim()).filter(Boolean))] - .filter((branch) => !branch.endsWith('/HEAD')) - .sort((left, right) => left.localeCompare(right)); + const branches: GitReviewBaseBranchOption[] = []; + for (const value of new Set(output.split('\n').map((line) => line.trim()))) { + if (value.startsWith('refs/heads/')) { + branches.push({ label: value.slice('refs/heads/'.length), value }); + } else if (value.startsWith('refs/remotes/') && + (value === 'refs/remotes/origin/HEAD' || !value.endsWith('/HEAD'))) { + branches.push({ label: value.slice('refs/remotes/'.length), value }); + } + } + return branches.sort((left, right) => { + const leftRank = BASE_BRANCH_PRIORITY.indexOf(left.value); + const rightRank = BASE_BRANCH_PRIORITY.indexOf(right.value); + if (leftRank !== rightRank) { + if (leftRank === -1) return 1; + if (rightRank === -1) return -1; + return leftRank - rightRank; + } + return left.label.localeCompare(right.label); + }); } async function gitRefExists( diff --git a/apps/desktop/src/renderer/features/workbar/README.md b/apps/desktop/src/renderer/features/workbar/README.md index f49cbdd94a0..214870b953b 100644 --- a/apps/desktop/src/renderer/features/workbar/README.md +++ b/apps/desktop/src/renderer/features/workbar/README.md @@ -50,6 +50,17 @@ remounted when the active session changes. - `selectors.rightCollapsed` drives the titlebar restore affordance and `selectors.hiddenSessionIds` filters ephemeral companion forks from the rail. +## Review comparison preference + +`WorkbarServices.reviewBaseBranchPreference` is the feature-owned persistence +port. It stores only an explicit qualified ref for one Session; `null` leaves +the comparison unpinned so reads follow the repository default. New Sessions +start unpinned, and existing choices survive panel remounts and app restarts. +The panel owns selection and invalid-branch recovery. The Desktop adapter owns +the localStorage key, JSON format, validation and best-effort error handling. +Tests and Storybook use an isolated in-memory implementation through the same +port; they do not persist review choices into the browser profile. + ## Lifecycle invariants - Review, Work Board, Browser, Files and Inspector tabs are persisted globally. diff --git a/apps/desktop/src/renderer/features/workbar/ports.ts b/apps/desktop/src/renderer/features/workbar/ports.ts index a7c1e6bc523..41e5e4b9bd5 100644 --- a/apps/desktop/src/renderer/features/workbar/ports.ts +++ b/apps/desktop/src/renderer/features/workbar/ports.ts @@ -284,6 +284,15 @@ export interface SideChatSessionPort { export interface WorkbarServices { popupMenu(input: import('../../../shared/native-menu.js').NativeMenuRequest): Promise; readonly review: WorkbarReviewService; + /** + * Explicit qualified refs persist per Session across panel/app restarts. + * `null` means unpinned: new Sessions follow their repository's current default. + * Unavailable persistence reads as null; writes are best-effort and never throw. + */ + readonly reviewBaseBranchPreference: { + read(sessionId: string): string | null; + write(sessionId: string, branch: string | null): void; + }; readonly terminal: WorkbarTerminalService; readonly browser: WorkbarBrowserService; readonly artifacts: WorkbarArtifactsService; diff --git a/apps/desktop/src/renderer/features/workbar/testing.ts b/apps/desktop/src/renderer/features/workbar/testing.ts index 7c67e205867..114fd9513b3 100644 --- a/apps/desktop/src/renderer/features/workbar/testing.ts +++ b/apps/desktop/src/renderer/features/workbar/testing.ts @@ -33,6 +33,8 @@ export * from './model/workbar-tool-definitions.js'; export * from './tools/artifacts/artifact-list-keyboard.js'; export * from './tools/artifacts/artifact-visibility.js'; export * from '../../application/contracts/session-inspector/session-inspector-panel-model.js'; +export { SessionReviewPanel } from './tools/review/session-review-panel.js'; +export { SessionReviewBaseBranchPicker } from './tools/review/session-review-base-branch-picker.js'; export { compactNumberFormatter, InspectorCompositionSection, @@ -67,7 +69,15 @@ const noopSubscription = (): (() => void) => () => undefined; export function createFakeWorkbarServices( overrides: Partial = {}, ): WorkbarServices { + const reviewBaseBranches = new Map(); return { + reviewBaseBranchPreference: { + read: (sessionId) => reviewBaseBranches.get(sessionId) ?? null, + write: (sessionId, branch) => { + if (branch === null) reviewBaseBranches.delete(sessionId); + else reviewBaseBranches.set(sessionId, branch); + }, + }, popupMenu: async () => null, review: { read: async () => { diff --git a/apps/desktop/src/renderer/features/workbar/tools/review/session-review-base-branch-picker.tsx b/apps/desktop/src/renderer/features/workbar/tools/review/session-review-base-branch-picker.tsx new file mode 100644 index 00000000000..18257282105 --- /dev/null +++ b/apps/desktop/src/renderer/features/workbar/tools/review/session-review-base-branch-picker.tsx @@ -0,0 +1,82 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { useMemo } from 'react'; +import type { GitReviewBaseBranchOption } from '@maka/core/git-review'; +import { Selector, SelectorOption } from '@astryxdesign/core/Selector'; +import { Text } from '@astryxdesign/core/Text'; + +/** + * Picks the branch the review panel diffs against. Pure props: the panel owns + * the selection, its persistence, and the reload. + * + * The trigger and every option read a real branch name — never an "auto" + * pseudo-entry — so the panel always says what it is comparing to. The search + * box comes from `Selector` itself; its placeholder and empty copy are already + * localized by the Astryx catalog at the renderer root. + */ +export function SessionReviewBaseBranchPicker(props: { + baseBranch: string | null; + baseBranchOptions: readonly GitReviewBaseBranchOption[]; + /** A comparison is being re-read: the field spins and the panel dims behind it. */ + isLoading?: boolean; + label: string; + onSelect: (branch: string) => void; +}) { + const options = useMemo(() => [...props.baseBranchOptions], [props.baseBranchOptions]); + return ( + // The wrapper is what lets review.css cap the panel: Selector portals its + // listbox next to the field, not inside the trigger it styles. +
+ ( + + {option.label ?? option.value} + + )} + renderOption={(option) => ( + + {option.label ?? option.value} + + } + /> + )} + onChange={props.onSelect} + placeholder={props.label} + // The trigger names a branch, it is not a call to action: read like + // the current branch it sits beside, not like a primary button. + style={{ + color: 'var(--color-text-secondary)', + fontSize: 'var(--text-supporting-size)', + fontWeight: 'var(--font-weight-normal)', + }} + /> +
+ ); +} diff --git a/apps/desktop/src/renderer/features/workbar/tools/review/session-review-panel.tsx b/apps/desktop/src/renderer/features/workbar/tools/review/session-review-panel.tsx index 6558cd42ab5..2e5947b656d 100644 --- a/apps/desktop/src/renderer/features/workbar/tools/review/session-review-panel.tsx +++ b/apps/desktop/src/renderer/features/workbar/tools/review/session-review-panel.tsx @@ -28,11 +28,12 @@ import { Skeleton } from '@astryxdesign/core/Skeleton'; import { Text } from '@astryxdesign/core/Text'; import { redactSecrets as displayRedactSecrets } from '@maka/core/display-redaction'; import { generalizedErrorMessageForLocale } from '@maka/core/redaction'; -import { type GitReviewReadResult } from '@maka/core/git-review'; +import { type GitReviewBranchContext, type GitReviewReadResult } from '@maka/core/git-review'; import { DiffCodePreview, useUiLocale } from '@maka/ui'; -import { ICON_SIZE, GitBranch } from '@maka/ui/icons'; -import { getDesktopConversationCopy } from '../../../../locales/conversation-copy'; +import { ICON_SIZE, ArrowRight, GitBranch } from '@maka/ui/icons'; +import { getDesktopConversationCopy } from '../../../../locales/conversation-copy.js'; import { useWorkbarServices } from '../../services-context.js'; +import { SessionReviewBaseBranchPicker } from './session-review-base-branch-picker.js'; const REVIEW_FILE_PAGE_SIZE = 20; const REVIEW_DIFF_LINE_CAP = 500; @@ -72,25 +73,62 @@ export function SessionReviewPanel(props: { sessionId: string; active: boolean; }) { - const { review } = useWorkbarServices(); + const { review, reviewBaseBranchPreference } = useWorkbarServices(); const locale = useUiLocale(); const copy = getDesktopConversationCopy(locale).reviewPanel; const [gitResult, setGitResult] = useState(null); + const [branches, setBranches] = useState(null); const [loading, setLoading] = useState(false); + // Distinct from `loading`: a background refresh must not flash the switch + // feedback, so only a user's pick drives it. + const [switching, setSwitching] = useState(false); const [visibleFileCount, setVisibleFileCount] = useState(REVIEW_FILE_PAGE_SIZE); const [error, setError] = useState(null); + const [baseBranch, setBaseBranch] = useState(() => + reviewBaseBranchPreference.read(props.sessionId), + ); const revisionRef = useRef(0); + const displayedBaseBranchRef = useRef(undefined); + // Read the latest selection without restarting the subscription effect. + // WorkbarSurface keys this panel by sessionId, so each Session initializes afresh. + const baseBranchRef = useRef(baseBranch); const load = useCallback(async () => { const revision = ++revisionRef.current; setLoading(true); setError(null); - try { - const nextGit = await review.read({ + const readReview = (selection: string | null) => + review.read({ sessionId: props.sessionId, source: 'branch', + baseBranch: selection ?? undefined, }); + try { + let nextGit = await readReview(baseBranchRef.current); if (revision !== revisionRef.current) return; + if ( + !nextGit.ok && + nextGit.reason === 'invalid_base_branch' && + baseBranchRef.current !== null + ) { + // The pinned branch is gone. Drop it and re-read once: the retry has no + // selection left to reject, so this cannot loop. + if (nextGit.branches) setBranches(nextGit.branches); + baseBranchRef.current = null; + setBaseBranch(null); + reviewBaseBranchPreference.write(props.sessionId, null); + nextGit = await readReview(null); + if (revision !== revisionRef.current) return; + } + const nextBranches = nextGit.ok ? nextGit.snapshot : nextGit.branches; + setBranches(nextBranches ?? null); + if (nextGit.ok) { + // Preserve expansion on refresh, but start each comparison at page one. + if (displayedBaseBranchRef.current !== nextGit.snapshot.baseBranch) { + setVisibleFileCount(REVIEW_FILE_PAGE_SIZE); + displayedBaseBranchRef.current = nextGit.snapshot.baseBranch; + } + } setGitResult(nextGit); } catch (nextError) { if (revision === revisionRef.current) { @@ -99,9 +137,24 @@ export function SessionReviewPanel(props: { ); } } finally { - if (revision === revisionRef.current) setLoading(false); + if (revision === revisionRef.current) { + setLoading(false); + setSwitching(false); + } } - }, [copy.loadFailed, locale, props.sessionId, review]); + }, [copy.loadFailed, locale, props.sessionId, review, reviewBaseBranchPreference]); + + const selectBaseBranch = useCallback( + (branch: string) => { + if (branch === baseBranchRef.current) return; + baseBranchRef.current = branch; + setBaseBranch(branch); + reviewBaseBranchPreference.write(props.sessionId, branch); + setSwitching(true); + void load(); + }, + [load, props.sessionId, reviewBaseBranchPreference], + ); useEffect(() => { if (!props.active) return; @@ -154,9 +207,7 @@ export function SessionReviewPanel(props: { ? copy.workspaceUnavailable : gitResult.reason === 'unborn_repository' ? copy.unbornRepository - : gitResult.reason === 'invalid_base_branch' - ? copy.invalidBaseBranch - : copy.gitFailed; + : copy.gitFailed; const empty = !loading && !error && !sourceError && gitFiles.length === 0; return ( @@ -168,7 +219,40 @@ export function SessionReviewPanel(props: { aria-label={copy.ariaLabel} aria-busy={loading || undefined} > - + + {/* Keep branch selection available when computing the diff fails. */} + {branches && branches.baseBranchOptions.length > 0 ? ( + + {branches.currentBranch ? ( + <> + + {branches.currentBranch} + + + + ) : null} + + + ) : null} {loading && gitResult === null ? ( `再显示 ${Math.min(20, remaining)} 个文件`, hiddenLines: (count) => `另有 ${count} 行未显示`, @@ -651,7 +651,7 @@ const COPY = { workspaceUnavailable: '目前任務目錄已不可用', unbornRepository: 'Git 倉庫還沒有可比較的提交', gitFailed: '無法讀取 Git 工作區變化', - invalidBaseBranch: '選擇的比較分支已不可用', + baseBranchLabel: '對比分支', truncated: '變化過多,僅顯示前一部分檔案', showMore: (remaining) => `再顯示 ${Math.min(20, remaining)} 個檔案`, hiddenLines: (count) => `另有 ${count} 行未顯示`, @@ -885,7 +885,7 @@ const COPY = { workspaceUnavailable: 'This task directory is unavailable', unbornRepository: 'This Git repository has no commit to compare yet', gitFailed: 'Could not read Git workspace changes', - invalidBaseBranch: 'The selected comparison branch is unavailable', + baseBranchLabel: 'Compare against', truncated: 'Too many changes; showing the first files only', showMore: (remaining) => `Show ${Math.min(20, remaining)} more file${Math.min(20, remaining) === 1 ? '' : 's'}`, diff --git a/apps/desktop/src/renderer/platform/desktop/browser-storage.ts b/apps/desktop/src/renderer/platform/desktop/browser-storage.ts new file mode 100644 index 00000000000..474f6065d7d --- /dev/null +++ b/apps/desktop/src/renderer/platform/desktop/browser-storage.ts @@ -0,0 +1,34 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +export function safeLocalStorageGet(key: string): string | null { + try { + return localStorage.getItem(key); + } catch { + return null; + } +} + +export function safeLocalStorageSet(key: string, value: string): void { + try { + localStorage.setItem(key, value); + } catch { + // Storage may be unavailable in restricted or test renderer contexts. + } +} diff --git a/apps/desktop/src/renderer/platform/desktop/create-workbar-services.ts b/apps/desktop/src/renderer/platform/desktop/create-workbar-services.ts index 8622be3fc47..71c678686b3 100644 --- a/apps/desktop/src/renderer/platform/desktop/create-workbar-services.ts +++ b/apps/desktop/src/renderer/platform/desktop/create-workbar-services.ts @@ -24,6 +24,7 @@ import { isTerminalShellRunStatus, } from '@maka/core/shell-run'; import type { WorkbarServices } from '../../features/workbar'; +import { createReviewBaseBranchPreferences } from './review-base-branch-preferences.js'; import { readSettledMessagesFrom } from './session-message-settlement.js'; import { expectSessionUpdate } from './create-session-settings-services.js'; @@ -105,6 +106,7 @@ export function createDesktopWorkbarServices( }; return { + reviewBaseBranchPreference: createReviewBaseBranchPreferences(), popupMenu: (input) => bridge.appWindow.popupMenu(input), review: { read: (input) => bridge.gitReview.read(input), diff --git a/apps/desktop/src/renderer/platform/desktop/review-base-branch-preferences.ts b/apps/desktop/src/renderer/platform/desktop/review-base-branch-preferences.ts new file mode 100644 index 00000000000..48b67622204 --- /dev/null +++ b/apps/desktop/src/renderer/platform/desktop/review-base-branch-preferences.ts @@ -0,0 +1,50 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { safeLocalStorageGet, safeLocalStorageSet } from './browser-storage.js'; +import type { WorkbarServices } from '../../features/workbar/index.js'; + +export const REVIEW_BASE_BRANCH_STORAGE_KEY = 'maka-session-review-base-branch-v1'; + +/** Desktop persistence for explicit per-Session comparison choices. */ +export function createReviewBaseBranchPreferences(): WorkbarServices['reviewBaseBranchPreference'] { + function readStoredBranches(): Record { + try { + const stored: unknown = JSON.parse(safeLocalStorageGet(REVIEW_BASE_BRANCH_STORAGE_KEY) ?? '{}'); + if (!stored || typeof stored !== 'object' || Array.isArray(stored)) return {}; + return Object.fromEntries( + Object.entries(stored).filter( + ([, value]) => typeof value === 'string' && value.length > 0, + ), + ); + } catch { + return {}; + } + } + + return { + read: (sessionId) => readStoredBranches()[sessionId] ?? null, + write(sessionId, branch) { + const stored = readStoredBranches(); + if (branch === null) delete stored[sessionId]; + else stored[sessionId] = branch; + safeLocalStorageSet(REVIEW_BASE_BRANCH_STORAGE_KEY, JSON.stringify(stored)); + }, + }; +} diff --git a/apps/desktop/src/renderer/styles/workbar/review.css b/apps/desktop/src/renderer/styles/workbar/review.css index f6d2f5787be..c451ad54cb0 100644 --- a/apps/desktop/src/renderer/styles/workbar/review.css +++ b/apps/desktop/src/renderer/styles/workbar/review.css @@ -23,6 +23,50 @@ overflow-y: auto; } +/* The comparison header: ` -> `. Both branches can + shrink for long names while the arrow keeps its size. */ +.maka-session-review-branch-row { + justify-content: flex-start; +} + +.maka-session-review-branch-row > svg { + flex-shrink: 0; +} + +/* A switch re-reads the whole branch diff and can take a while on a diverged + branch. The picker spins and everything below it stays readable but visibly + stale until the new snapshot lands. */ +.maka-session-review-switching > *:not(.maka-session-review-branch-row) { + opacity: 0.5; + pointer-events: none; +} + +.maka-session-review-current-branch, +.maka-session-review-base-branch { + min-width: 0; + flex-shrink: 1; +} + +.maka-session-review-current-branch { + margin-right: var(--space-3); +} + +/* Astryx gives Selector no panel-width prop, and an uncapped panel grows to the + longest branch name. The panel's layer stays inside this wrapper, so the cap + lands on the surface; the option's `Item` needs `min-width: 0` to let its + label ellipsize instead of pushing the row wider. */ +.maka-session-review-base-branch .astryx-popover-surface { + max-width: 280px; +} + +.maka-session-review-base-branch [role='listbox'] { + max-height: 288px; +} + +.maka-session-review-base-branch [role='option'] .astryx-item { + min-width: 0; +} + .maka-session-review-file-stats { flex-shrink: 0; white-space: nowrap; diff --git a/apps/desktop/stories/session-workbar.stories.tsx b/apps/desktop/stories/session-workbar.stories.tsx index 6b5b0439ae5..6cf97f8e4c5 100644 --- a/apps/desktop/stories/session-workbar.stories.tsx +++ b/apps/desktop/stories/session-workbar.stories.tsx @@ -354,8 +354,14 @@ const gitReviewSnapshot: GitReviewSnapshot = { source: 'branch', repositoryRoot: '/Users/reviewer/maka-agent', currentBranch: 'feat/git-authoritative-changes', - baseBranch: 'main', - baseBranchOptions: ['main', 'release/0.1'], + baseBranch: 'refs/remotes/origin/main', + baseBranchOptions: [ + { label: 'origin/HEAD', value: 'refs/remotes/origin/HEAD' }, + { label: 'origin/main', value: 'refs/remotes/origin/main' }, + { label: 'main', value: 'refs/heads/main' }, + { label: 'release/0.1', value: 'refs/heads/release/0.1' }, + { label: 'origin/feature/payments-migration-2026', value: 'refs/remotes/origin/feature/payments-migration-2026' }, + ], revision: 'storybook-git-review', additions: gitReviewFiles.reduce((total, file) => total + file.additions, 0), deletions: gitReviewFiles.reduce((total, file) => total + file.deletions, 0), @@ -1269,6 +1275,45 @@ export const Changes: Story = { render: () => , }; +// Real path: 任务工作栏 → 变更 → open the base branch picker in a narrow +// workbar, search a long branch name, select it, then reopen the comparison menu. +export const ChangesBaseBranchPicker: Story = { + decorators: [bridge()], + render: () => , + play: async ({ canvasElement }) => { + const picker = await waitFor(() => { + const element = canvasElement.querySelector('.maka-session-review-base-branch'); + expect(element).not.toBeNull(); + return element!; + }); + const trigger = within(picker).getByRole('button'); + await userEvent.click(trigger); + const body = within(canvasElement.ownerDocument.body); + const listbox = await body.findByRole('listbox'); + const longName = 'origin/feature/payments-migration-2026'; + const option = within(listbox).getByRole('option', { name: longName }); + await waitFor(() => { + const surface = picker.querySelector('.astryx-popover-surface'); + expect(surface).not.toBeNull(); + expect(surface!.getBoundingClientRect().width).toBeGreaterThan(0); + expect(surface!.getBoundingClientRect().width).toBeLessThanOrEqual(280); + expect(listbox.getBoundingClientRect().height).toBeLessThanOrEqual(288); + const text = option.querySelector('.astryx-text'); + expect(text).not.toBeNull(); + expect(text!.scrollWidth).toBeGreaterThan(text!.clientWidth); + expect(getComputedStyle(text!).textOverflow).toBe('ellipsis'); + }); + const search = within(picker).getByRole('combobox'); + await userEvent.type(search, 'payments'); + await waitFor(() => expect(within(listbox).getAllByRole('option')).toHaveLength(1)); + await userEvent.click(within(listbox).getByRole('option', { name: longName })); + await waitFor(() => expect(trigger).toHaveTextContent(longName)); + await userEvent.click(trigger); + const reopened = await body.findByRole('listbox'); + expect(within(reopened).getByRole('option', { name: longName })).toHaveAttribute('aria-selected', 'true'); + }, +}; + // Real path: 变更 open, then 浏览器 and 生成文件 opened from [+]. Faces are added to // the right of the strip and never reordered, so this is what three of them // look like — one selected, two not, which is the only arrangement where the diff --git a/docs/astryx-surface-file-inventory.md b/docs/astryx-surface-file-inventory.md index d2d49812607..5a0c13b506d 100644 --- a/docs/astryx-surface-file-inventory.md +++ b/docs/astryx-surface-file-inventory.md @@ -6,7 +6,7 @@ Generated against `@astryxdesign/core@0.6.2` (195 component exports). Wiki bar: Design Conventions · API Use-the-System · Theming · Container Padding. -**Totals:** 299 files — blocker 0, reimplementation 0, polish 4, aligned 295. +**Totals:** 300 files — blocker 0, reimplementation 0, polish 4, aligned 296. ## Exclusions (explicit) @@ -121,6 +121,7 @@ Wiki bar: Design Conventions · API Use-the-System · Theming · Container Paddi | `apps/desktop/src/renderer/features/workbar/tools/artifacts/artifact-preview.tsx` | shell-chrome-or-panel | Banner, Button, CodeBlock, Spinner | aligned — uses Astryx (Banner, Button, CodeBlock, Spinner) | aligned | | `apps/desktop/src/renderer/features/workbar/tools/browser/browser-panel.tsx` | shell-chrome-or-panel | EmptyState, IconButton, TextInput, Toolbar, Tooltip | aligned — uses Astryx (EmptyState, IconButton, TextInput, Toolbar, Tooltip) | aligned | | `apps/desktop/src/renderer/features/workbar/tools/inspector/live-context-usage-probe.tsx` | shell-chrome-or-panel | none | aligned — no raw controls; no Astryx JSX usage | aligned | +| `apps/desktop/src/renderer/features/workbar/tools/review/session-review-base-branch-picker.tsx` | shell-chrome-or-panel | Selector, SelectorOption, Text | aligned — uses Astryx (Selector, SelectorOption, Text) | aligned | | `apps/desktop/src/renderer/features/workbar/tools/review/session-review-panel.tsx` | shell-chrome-or-panel | Banner, Button, Collapsible, CollapsibleGroup, EmptyState, HStack, Section, Skeleton, Text, VStack | aligned — uses Astryx (Banner, Button, Collapsible, CollapsibleGroup, EmptyState, HStack, Section, Skeleton) | aligned | | `apps/desktop/src/renderer/features/workbar/tools/side-chat/quote-companion-panel.tsx` | shell-chrome-or-panel | Banner | aligned — uses Astryx (Banner) | aligned | | `apps/desktop/src/renderer/features/workbar/tools/terminal/session-terminal-panel.tsx` | shell-chrome-or-panel | Banner, EmptyState | aligned — uses Astryx (Banner, EmptyState) | aligned | diff --git a/docs/astryx-surface-file-inventory.paths b/docs/astryx-surface-file-inventory.paths index a6c4762bb1f..17d5ec8906d 100644 --- a/docs/astryx-surface-file-inventory.paths +++ b/docs/astryx-surface-file-inventory.paths @@ -91,6 +91,7 @@ apps/desktop/src/renderer/features/workbar/tools/artifacts/artifact-preview-regi apps/desktop/src/renderer/features/workbar/tools/artifacts/artifact-preview.tsx apps/desktop/src/renderer/features/workbar/tools/browser/browser-panel.tsx apps/desktop/src/renderer/features/workbar/tools/inspector/live-context-usage-probe.tsx +apps/desktop/src/renderer/features/workbar/tools/review/session-review-base-branch-picker.tsx apps/desktop/src/renderer/features/workbar/tools/review/session-review-panel.tsx apps/desktop/src/renderer/features/workbar/tools/side-chat/quote-companion-panel.tsx apps/desktop/src/renderer/features/workbar/tools/terminal/session-terminal-panel.tsx diff --git a/packages/core/src/git-review.ts b/packages/core/src/git-review.ts index 9ca9ec90ace..2d59d3e5c82 100644 --- a/packages/core/src/git-review.ts +++ b/packages/core/src/git-review.ts @@ -37,12 +37,21 @@ export interface GitReviewFile { deletions: number; } -export interface GitReviewSnapshot { +export interface GitReviewBaseBranchOption { + label: string; + /** Fully qualified branch ref, never a tag or an ambiguous revision. */ + value: string; +} + +export interface GitReviewBranchContext { + currentBranch: string | null; + baseBranchOptions: GitReviewBaseBranchOption[]; +} + +export interface GitReviewSnapshot extends GitReviewBranchContext { source: GitReviewSource; repositoryRoot: string; - currentBranch: string | null; baseBranch: string | null; - baseBranchOptions: string[]; revision: string; files: GitReviewFile[]; additions: number; @@ -54,6 +63,8 @@ export type GitReviewReadResult = | { ok: true; snapshot: GitReviewSnapshot } | { ok: false; + /** Available even when computing the selected branch diff fails. */ + branches?: GitReviewBranchContext; reason: | 'workspace_unavailable' | 'not_git_repository' diff --git a/packages/ui/src/astryx-copy.ts b/packages/ui/src/astryx-copy.ts index 9a72ad5a779..d224e0452b4 100644 --- a/packages/ui/src/astryx-copy.ts +++ b/packages/ui/src/astryx-copy.ts @@ -88,8 +88,8 @@ export interface AstryxCopy { lightbox: { mediaViewer: string; previous: string; next: string }; menus: { dropdown: string; more: string }; multiSelector: { clearAll: string; selectAll: string }; - /** Selector and MultiSelector render the same two search affordances. */ - search: { options: string; placeholder: string }; + /** Selector and MultiSelector share search labels; result feedback is Selector-only. */ + search: { emptySearch: string; options: string; placeholder: string; resultCount: string }; sideNav: { label: string; resizeSidebar: string; @@ -161,7 +161,12 @@ export const ASTRYX_COPY_ZH: AstryxCopy = { lightbox: { mediaViewer: '媒体查看器', previous: '上一张', next: '下一张' }, menus: { dropdown: '菜单', more: '更多选项' }, multiSelector: { clearAll: '清除全部{label}', selectAll: '全选' }, - search: { options: '搜索选项', placeholder: '搜索…' }, + search: { + options: '搜索选项', + placeholder: '搜索…', + emptySearch: '没有匹配的选项', + resultCount: '{count, number} 个选项', + }, sideNav: { label: '侧边导航', resizeSidebar: '调整侧边栏宽度', @@ -233,7 +238,12 @@ export const ASTRYX_COPY_ZH_TW: AstryxCopy = { lightbox: { mediaViewer: '媒體檢視器', previous: '上一張', next: '下一張' }, menus: { dropdown: '選單', more: '更多選項' }, multiSelector: { clearAll: '清除全部{label}', selectAll: '全選' }, - search: { options: '搜尋選項', placeholder: '搜尋…' }, + search: { + options: '搜尋選項', + placeholder: '搜尋…', + emptySearch: '沒有符合的選項', + resultCount: '{count, number} 個選項', + }, sideNav: { label: '側邊導航', resizeSidebar: '調整側邊欄寬度', diff --git a/packages/ui/src/astryx-i18n.tsx b/packages/ui/src/astryx-i18n.tsx index ae9820895a2..d1cee514857 100644 --- a/packages/ui/src/astryx-i18n.tsx +++ b/packages/ui/src/astryx-i18n.tsx @@ -191,6 +191,8 @@ function chineseOverrides(locale: 'zh-CN' | 'zh-TW', astryx: typeof ASTRYX_COPY_ '@astryx.moreMenu.label': astryx.menus.more, '@astryx.selector.searchOptions': astryx.search.options, '@astryx.selector.searchPlaceholder': astryx.search.placeholder, + '@astryx.selector.emptySearchResults': astryx.search.emptySearch, + '@astryx.selector.resultCount': astryx.search.resultCount, '@astryx.multiSelector.searchOptions': astryx.search.options, '@astryx.multiSelector.searchPlaceholder': astryx.search.placeholder, '@astryx.multiSelector.selectPlaceholder': form.selectPlaceholder,