From 629a523edf802a40273e50d106a5803da400ebdf Mon Sep 17 00:00:00 2001 From: liuzhaochen03 Date: Mon, 21 Sep 2026 03:12:01 +0800 Subject: [PATCH 1/5] feat(desktop): add persistent review base branch picker Let each session select a comparison branch by canonical ref, with searchable localized options and persisted explicit choices. Keep implicit defaults dynamic, recover automatically when a saved branch disappears, and reset pagination when the comparison changes. Preserve branch options when diff computation fails and show truncated output when a large unified diff exceeds the process buffer. Add recovery and ref-resolution coverage, switch feedback, stories, and surface inventory documentation. Generated-by: pi (DeepSeek V4.1 Flash) Generated-by: OpenAI Codex --- CHANGELOG.md | 3 + .../main/__tests__/git-review-main.test.ts | 166 +++++++++++- .../session-review-base-branch.test.ts | 165 ++++++++++++ .../session-review-panel-recovery.test.ts | 245 ++++++++++++++++++ apps/desktop/src/main/git-review-main.ts | 131 +++++++--- .../src/renderer/features/workbar/testing.ts | 3 + .../session-review-base-branch-model.ts | 98 +++++++ .../session-review-base-branch-picker.tsx | 66 +++++ .../tools/review/session-review-panel.tsx | 135 +++++++++- .../src/renderer/locales/conversation-copy.ts | 8 +- .../src/renderer/styles/workbar/review.css | 36 +++ .../stories/session-workbar.stories.tsx | 10 +- apps/desktop/stories/workhub.stories.tsx | 1 + docs/astryx-surface-file-inventory.md | 3 +- docs/astryx-surface-file-inventory.paths | 1 + packages/core/src/git-review.ts | 17 +- packages/ui/src/astryx-copy.ts | 18 +- packages/ui/src/astryx-i18n.tsx | 2 + 18 files changed, 1049 insertions(+), 59 deletions(-) create mode 100644 apps/desktop/src/main/__tests__/session-review-base-branch.test.ts create mode 100644 apps/desktop/src/main/__tests__/session-review-panel-recovery.test.ts create mode 100644 apps/desktop/src/renderer/features/workbar/tools/review/session-review-base-branch-model.ts create mode 100644 apps/desktop/src/renderer/features/workbar/tools/review/session-review-base-branch-picker.tsx diff --git a/CHANGELOG.md b/CHANGELOG.md index 5199c4ae50..9bb0debcc9 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 b41b7c65b8..f565ff8ddf 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(), @@ -71,7 +71,7 @@ describe('Git Review snapshot authority', () => { ); 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,157 @@ 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, including legacy selections', 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'); + + for (const selection of ['refs/heads/release', 'release']) { + const result = await readGitReview(root, 'branch', undefined, selection); + 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 ambiguous legacy 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 +270,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__/session-review-base-branch.test.ts b/apps/desktop/src/main/__tests__/session-review-base-branch.test.ts new file mode 100644 index 0000000000..318dfe9aa7 --- /dev/null +++ b/apps/desktop/src/main/__tests__/session-review-base-branch.test.ts @@ -0,0 +1,165 @@ +/* + * 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, describe, it } from 'node:test'; +import { createElement } from 'react'; +import { renderToStaticMarkup } from 'react-dom/server'; +import { + persistSessionReviewBaseBranch, + readSessionReviewBaseBranch, + resolveAdoptedBaseBranch, + REVIEW_BASE_BRANCH_STORAGE_KEY, + reviewBaseBranchRequestValue, + SessionReviewBaseBranchPicker, +} from '../../renderer/features/workbar/testing.js'; + +function installMemoryLocalStorage(initial: Record = {}) { + const store = new Map(Object.entries(initial)); + const previous = Object.getOwnPropertyDescriptor(globalThis, 'localStorage'); + const memory: Storage = { + get length() { + return store.size; + }, + clear: () => store.clear(), + getItem: (key) => store.get(key) ?? null, + key: (index) => [...store.keys()][index] ?? null, + removeItem: (key) => store.delete(key), + setItem: (key, value) => store.set(key, String(value)), + }; + Object.defineProperty(globalThis, 'localStorage', { + configurable: true, + writable: true, + value: memory, + }); + return () => { + if (previous) Object.defineProperty(globalThis, 'localStorage', previous); + else Reflect.deleteProperty(globalThis, 'localStorage'); + }; +} + +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', () => { + const cleanups: Array<() => void> = []; + afterEach(() => { + while (cleanups.length > 0) cleanups.pop()?.(); + }); + + it('omits the request value until a branch is selected', () => { + assert.equal(reviewBaseBranchRequestValue(null), undefined); + assert.equal(reviewBaseBranchRequestValue('origin/develop'), 'origin/develop'); + }); + + it('adopts the resolved base branch only when the backend offers it', () => { + const options = [ + { label: 'main', value: 'refs/heads/main' }, + { label: 'origin/develop', value: 'refs/remotes/origin/develop' }, + ]; + assert.equal( + resolveAdoptedBaseBranch('refs/remotes/origin/develop', { + baseBranch: 'refs/heads/main', + baseBranchOptions: options, + }), + 'refs/remotes/origin/develop', + ); + assert.equal( + resolveAdoptedBaseBranch(null, { baseBranch: 'refs/heads/main', baseBranchOptions: options }), + null, + ); + // A resolved branch the backend would reject on the next read stays unpinned. + assert.equal( + resolveAdoptedBaseBranch('origin/gone', { + baseBranch: 'origin/gone', + baseBranchOptions: options, + }), + null, + ); + assert.equal( + resolveAdoptedBaseBranch(null, { baseBranch: null, baseBranchOptions: options }), + null, + ); + }); + + it('persists the canonical selection when the backend migrates a legacy name', () => { + cleanups.push(installMemoryLocalStorage()); + persistSessionReviewBaseBranch('legacy', 'main'); + const adopted = resolveAdoptedBaseBranch(readSessionReviewBaseBranch('legacy'), { + baseBranch: 'refs/heads/main', + baseBranchOptions: [{ label: 'main', value: 'refs/heads/main' }], + }); + assert.equal(adopted, 'refs/heads/main'); + persistSessionReviewBaseBranch('legacy', adopted); + assert.equal(readSessionReviewBaseBranch('legacy'), 'refs/heads/main'); + }); + + it('pins the branch per Session and survives corrupt storage', () => { + cleanups.push(installMemoryLocalStorage()); + persistSessionReviewBaseBranch('session-a', 'origin/develop'); + persistSessionReviewBaseBranch('session-b', 'main'); + assert.equal(readSessionReviewBaseBranch('session-a'), 'origin/develop'); + assert.equal(readSessionReviewBaseBranch('session-b'), 'main'); + + persistSessionReviewBaseBranch('session-a', null); + assert.equal(readSessionReviewBaseBranch('session-a'), null); + assert.equal(readSessionReviewBaseBranch('session-b'), 'main'); + + localStorage.setItem(REVIEW_BASE_BRANCH_STORAGE_KEY, '{ not json'); + assert.equal(readSessionReviewBaseBranch('session-b'), null); + localStorage.setItem( + REVIEW_BASE_BRANCH_STORAGE_KEY, + JSON.stringify({ 'session-c': 7, 'session-d': 'main' }), + ); + assert.equal(readSessionReviewBaseBranch('session-c'), null); + assert.equal(readSessionReviewBaseBranch('session-d'), 'main'); + }); + + 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 0000000000..6fd1614a64 --- /dev/null +++ b/apps/desktop/src/main/__tests__/session-review-panel-recovery.test.ts @@ -0,0 +1,245 @@ +/* + * 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, + persistSessionReviewBaseBranch, + readSessionReviewBaseBranch, +} 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 { + persistSessionReviewBaseBranch('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(); + } +}); + +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(readSessionReviewBaseBranch(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 { + persistSessionReviewBaseBranch(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(readSessionReviewBaseBranch(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 storage = new Map(); + 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, + localStorage: { + getItem: (key: string) => storage.get(key) ?? null, + setItem: (key: string, value: string) => storage.set(key, value), + }, + }; + 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 a75b209c13..a6c4af7a7d 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 + : resolveRequestedBaseBranch(requestedBaseBranch, baseBranchOptions); + 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,46 +397,83 @@ 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; } +// Old preferences contain display names. Match only enumerated branches and +// reject collisions between local and remote names rather than guessing. +function resolveRequestedBaseBranch( + requested: string, + options: readonly GitReviewBaseBranchOption[], +): GitReviewBaseBranchOption | undefined { + const exact = options.find((option) => option.value === requested); + if (exact) return exact; + const matches = options.filter((option) => option.label === requested); + return matches.length === 1 ? matches[0] : undefined; +} + 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/testing.ts b/apps/desktop/src/renderer/features/workbar/testing.ts index 7c67e20586..a9e31156c7 100644 --- a/apps/desktop/src/renderer/features/workbar/testing.ts +++ b/apps/desktop/src/renderer/features/workbar/testing.ts @@ -33,6 +33,9 @@ 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 * from './tools/review/session-review-base-branch-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, diff --git a/apps/desktop/src/renderer/features/workbar/tools/review/session-review-base-branch-model.ts b/apps/desktop/src/renderer/features/workbar/tools/review/session-review-base-branch-model.ts new file mode 100644 index 0000000000..60620f3339 --- /dev/null +++ b/apps/desktop/src/renderer/features/workbar/tools/review/session-review-base-branch-model.ts @@ -0,0 +1,98 @@ +/* + * 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 type { GitReviewSnapshot } from '@maka/core/git-review'; + +export const REVIEW_BASE_BRANCH_STORAGE_KEY = 'maka-session-review-base-branch-v1'; + +// A local wrapper rather than @/browser-storage: the renderer architecture +// check forbids new feature-to-legacy imports, and a feature may use Web +// Storage directly. Storage can be unavailable in restricted renderer contexts. +function readStorage(key: string): string | null { + try { + return localStorage.getItem(key); + } catch { + return null; + } +} + +function writeStorage(key: string, value: string): void { + try { + localStorage.setItem(key, value); + } catch { + // Persistence is a preference, not a correctness requirement. + } +} + +function readStoredBaseBranches(): Record { + try { + const stored: unknown = JSON.parse(readStorage(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 {}; + } +} + +export function readSessionReviewBaseBranch(sessionId: string): string | null { + return readStoredBaseBranches()[sessionId] ?? null; +} + +/** `null` drops the Session's entry so the next read falls back to the resolved base. */ +export function persistSessionReviewBaseBranch( + sessionId: string, + branch: string | null, +): void { + const stored = readStoredBaseBranches(); + if (branch === null) delete stored[sessionId]; + else stored[sessionId] = branch; + writeStorage(REVIEW_BASE_BRANCH_STORAGE_KEY, JSON.stringify(stored)); +} + +/** The request omits `baseBranch` entirely while nothing is selected. */ +export function reviewBaseBranchRequestValue( + selection: string | null, +): string | undefined { + return selection ?? undefined; +} + +/** + * Canonicalizes an explicit selection, including legacy names. An implicit + * default stays unpinned so subsequent reads follow the repository default. + * The resolved branch must be one of the offered options: a value the backend + * would reject on the next read is worse than staying unresolved. + */ +export function resolveAdoptedBaseBranch( + selection: string | null, + snapshot: Pick, +): string | null { + if (selection === null) return null; + if (snapshot.baseBranchOptions.some((option) => option.value === selection)) { + return selection; + } + const resolved = snapshot.baseBranch; + if (resolved === null) return null; + return snapshot.baseBranchOptions.some((option) => option.value === resolved) + ? resolved + : null; +} 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 0000000000..8317e8d6c9 --- /dev/null +++ b/apps/desktop/src/renderer/features/workbar/tools/review/session-review-base-branch-picker.tsx @@ -0,0 +1,66 @@ +/* + * 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 } from '@astryxdesign/core/Selector'; + +/** + * 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. +
+ +
+ ); +} 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 6558cd42ab..b4e9951478 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,18 @@ 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 { + persistSessionReviewBaseBranch, + readSessionReviewBaseBranch, + resolveAdoptedBaseBranch, + reviewBaseBranchRequestValue, +} from './session-review-base-branch-model.js'; +import { SessionReviewBaseBranchPicker } from './session-review-base-branch-picker.js'; const REVIEW_FILE_PAGE_SIZE = 20; const REVIEW_DIFF_LINE_CAP = 500; @@ -76,21 +83,83 @@ export function SessionReviewPanel(props: { 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(() => + readSessionReviewBaseBranch(props.sessionId), + ); const revisionRef = useRef(0); + const displayedBaseBranchRef = useRef(undefined); + // Requests read the ref, not the state: adopting a resolved branch must not + // re-run the load effect, and a Session switch must not race a stale value. + const baseBranchRef = useRef(baseBranch); + + useEffect(() => { + displayedBaseBranchRef.current = undefined; + setVisibleFileCount(REVIEW_FILE_PAGE_SIZE); + setBranches(null); + setGitResult(null); + setSwitching(false); + const stored = readSessionReviewBaseBranch(props.sessionId); + baseBranchRef.current = stored; + setBaseBranch(stored); + }, [props.sessionId]); 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: reviewBaseBranchRequestValue(selection), }); + 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); + persistSessionReviewBaseBranch(props.sessionId, null); + nextGit = await readReview(null); + if (revision !== revisionRef.current) return; + } + const nextBranches = nextGit.ok ? nextGit.snapshot : nextGit.branches; + if (nextBranches) { + setBranches({ + currentBranch: nextBranches.currentBranch, + baseBranchOptions: nextBranches.baseBranchOptions, + }); + } + 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; + } + const adopted = resolveAdoptedBaseBranch( + baseBranchRef.current, + nextGit.snapshot, + ); + if (adopted !== baseBranchRef.current) { + baseBranchRef.current = adopted; + setBaseBranch(adopted); + persistSessionReviewBaseBranch(props.sessionId, adopted); + } + } setGitResult(nextGit); } catch (nextError) { if (revision === revisionRef.current) { @@ -99,10 +168,25 @@ 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]); + const selectBaseBranch = useCallback( + (branch: string) => { + if (branch === baseBranchRef.current) return; + baseBranchRef.current = branch; + setBaseBranch(branch); + persistSessionReviewBaseBranch(props.sessionId, branch); + setSwitching(true); + void load(); + }, + [load, props.sessionId], + ); + useEffect(() => { if (!props.active) return; let timer: number | undefined; @@ -154,9 +238,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 +250,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} 行未显示`, @@ -649,7 +649,7 @@ const COPY = { workspaceUnavailable: '目前任務目錄已不可用', unbornRepository: 'Git 倉庫還沒有可比較的提交', gitFailed: '無法讀取 Git 工作區變化', - invalidBaseBranch: '選擇的比較分支已不可用', + baseBranchLabel: '對比分支', truncated: '變化過多，僅顯示前一部分檔案', showMore: (remaining) => `再顯示 ${Math.min(20, remaining)} 個檔案`, hiddenLines: (count) => `另有 ${count} 行未顯示`, @@ -883,7 +883,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/styles/workbar/review.css b/apps/desktop/src/renderer/styles/workbar/review.css index f6d2f5787b..f267afc4a3 100644 --- a/apps/desktop/src/renderer/styles/workbar/review.css +++ b/apps/desktop/src/renderer/styles/workbar/review.css @@ -23,6 +23,42 @@ overflow-y: auto; } +/* The comparison header: ` -> `. The current branch + ellipsizes so a long name cannot push the picker past the panel edge. */ +.maka-session-review-branch-row { + justify-content: flex-start; +} + +/* 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 { + min-width: 0; + flex-shrink: 1; + 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: 180px; +} + +.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 9d4973ab45..c3d220ba51 100644 --- a/apps/desktop/stories/session-workbar.stories.tsx +++ b/apps/desktop/stories/session-workbar.stories.tsx @@ -251,8 +251,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), diff --git a/apps/desktop/stories/workhub.stories.tsx b/apps/desktop/stories/workhub.stories.tsx index 4ffbcf2c26..e57e7f7d32 100644 --- a/apps/desktop/stories/workhub.stories.tsx +++ b/apps/desktop/stories/workhub.stories.tsx @@ -548,6 +548,7 @@ export const FilterWorkConversations: Story = { await userEvent.keyboard('{Enter}'); await waitFor(() => expect(canvasElement.querySelectorAll('.maka-turn[data-turn-id]')).toHaveLength(2)); await userEvent.click(canvas.getByRole('button', { name: '显示全部对话' })); + await waitFor(() => expect(canvasElement.querySelectorAll('.maka-turn[data-turn-id]')).toHaveLength(4)); await userEvent.click(rail); await waitFor(() => expect(canvasElement.querySelector('[data-search-highlight="true"]')).toHaveTextContent('请检查发布检查清单。')); expect(canvasElement.querySelectorAll('.maka-turn[data-turn-id]')).toHaveLength(4); diff --git a/docs/astryx-surface-file-inventory.md b/docs/astryx-surface-file-inventory.md index 535284b5d5..f2a00b272f 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:** 291 files — blocker 0, reimplementation 0, polish 4, aligned 287. +**Totals:** 292 files — blocker 0, reimplementation 0, polish 4, aligned 288. ## Exclusions (explicit) @@ -115,6 +115,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 | aligned — uses Astryx (Selector) | 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 c4badcc459..3d45237942 100644 --- a/docs/astryx-surface-file-inventory.paths +++ b/docs/astryx-surface-file-inventory.paths @@ -86,6 +86,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 9ca9ec90ac..2d59d3e5c8 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 9a72ad5a77..d224e0452b 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 ae9820895a..d1cee51485 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, From f9ebc8f1458c8701999a8e591c10b88c1faf9e76 Mon Sep 17 00:00:00 2001 From: liuzhaochen03 Date: Mon, 21 Sep 2026 03:21:04 +0800 Subject: [PATCH 2/5] fix(desktop): handle long review branch names Generated-by: Codex --- .../review/session-review-base-branch-picker.tsx | 7 +++++++ .../desktop/src/renderer/styles/workbar/review.css | 14 +++++++++++--- 2 files changed, 18 insertions(+), 3 deletions(-) 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 index 8317e8d6c9..e0f45d2d67 100644 --- 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 @@ -20,6 +20,7 @@ import { useMemo } from 'react'; import type { GitReviewBaseBranchOption } from '@maka/core/git-review'; import { Selector } from '@astryxdesign/core/Selector'; +import { Text } from '@astryxdesign/core/Text'; /** * Picks the branch the review panel diffs against. Pure props: the panel owns @@ -52,12 +53,18 @@ export function SessionReviewBaseBranchPicker(props: { isLoading={props.isLoading} options={options} value={props.baseBranch ?? undefined} + renderValue={(option) => ( + + {option.label} + + )} 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/styles/workbar/review.css b/apps/desktop/src/renderer/styles/workbar/review.css index f267afc4a3..062106d984 100644 --- a/apps/desktop/src/renderer/styles/workbar/review.css +++ b/apps/desktop/src/renderer/styles/workbar/review.css @@ -23,12 +23,16 @@ overflow-y: auto; } -/* The comparison header: ` -> `. The current branch - ellipsizes so a long name cannot push the picker past the panel edge. */ +/* 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. */ @@ -37,9 +41,13 @@ pointer-events: none; } -.maka-session-review-current-branch { +.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); } From 54bcb5632ccea000fb4f0dae4f1ba4758c4c3085 Mon Sep 17 00:00:00 2001 From: liuzhaochen03 Date: Mon, 21 Sep 2026 03:35:20 +0800 Subject: [PATCH 3/5] docs(desktop): refresh Astryx surface inventory Regenerate the file-level inventory after 0f11ae7b9 added a Text usage to the review base branch picker without regenerating. The markdown row had drifted from the generator; the .paths list was already in sync. Keeps the astryx:surface-inventory coverage gate green. Generated-by: Maka --- docs/astryx-surface-file-inventory.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/astryx-surface-file-inventory.md b/docs/astryx-surface-file-inventory.md index f2a00b272f..45c0611425 100644 --- a/docs/astryx-surface-file-inventory.md +++ b/docs/astryx-surface-file-inventory.md @@ -115,7 +115,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 | aligned — uses Astryx (Selector) | aligned | +| `apps/desktop/src/renderer/features/workbar/tools/review/session-review-base-branch-picker.tsx` | shell-chrome-or-panel | Selector, Text | aligned — uses Astryx (Selector, 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 | From 45eb36bb064cf4f0d7d35b18484afc068a824ff0 Mon Sep 17 00:00:00 2001 From: liuzhaochen03 Date: Mon, 21 Sep 2026 19:57:56 +0800 Subject: [PATCH 4/5] style: fix --- .../review/session-review-base-branch-picker.tsx | 15 ++++++++++++--- .../src/renderer/styles/workbar/review.css | 2 +- 2 files changed, 13 insertions(+), 4 deletions(-) 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 index e0f45d2d67..1825728210 100644 --- 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 @@ -19,7 +19,7 @@ import { useMemo } from 'react'; import type { GitReviewBaseBranchOption } from '@maka/core/git-review'; -import { Selector } from '@astryxdesign/core/Selector'; +import { Selector, SelectorOption } from '@astryxdesign/core/Selector'; import { Text } from '@astryxdesign/core/Text'; /** @@ -54,10 +54,19 @@ export function SessionReviewBaseBranchPicker(props: { options={options} value={props.baseBranch ?? undefined} renderValue={(option) => ( - - {option.label} + + {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 diff --git a/apps/desktop/src/renderer/styles/workbar/review.css b/apps/desktop/src/renderer/styles/workbar/review.css index 062106d984..c451ad54cb 100644 --- a/apps/desktop/src/renderer/styles/workbar/review.css +++ b/apps/desktop/src/renderer/styles/workbar/review.css @@ -56,7 +56,7 @@ 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: 180px; + max-width: 280px; } .maka-session-review-base-branch [role='listbox'] { From f6ffba1ace733c766b8e7e5c99fb5f6431faffdc Mon Sep 17 00:00:00 2001 From: liuzhaochen03 Date: Mon, 21 Sep 2026 19:59:51 +0800 Subject: [PATCH 5/5] docs(desktop): reconcile surface inventory after rebase --- docs/astryx-surface-file-inventory.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/astryx-surface-file-inventory.md b/docs/astryx-surface-file-inventory.md index 45c0611425..6bf1c03e93 100644 --- a/docs/astryx-surface-file-inventory.md +++ b/docs/astryx-surface-file-inventory.md @@ -115,7 +115,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, Text | aligned — uses Astryx (Selector, Text) | 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 |