Files
orca/tests/e2e/diff-note-layout.spec.ts
Brennan BensonandClaude Opus 5.5 e59b843336 Keep review note changes on the workspace copy they came from (#37204)
* Keep review note changes on their captured workspace owner

* Keep review note actions on the copy whose notes are shown

Review notes now read their owner and their notes together, so an action
always changes the copy the user is looking at:

- A paired SSH copy saves through its server even when its row does not
  name the server, instead of saving on this computer.
- The Markdown preview shows and edits the selected copy's notes, not the
  first copy listed with the same workspace id.
- While the selected copy is not listed yet, no other copy's notes are
  shown; a note started then saves to that copy once it lists.
- A listing that stamps the same copy's host no longer looks like a new
  copy: a pending Send stays held and unsaved note edits are kept.
- Clear all / Clear file confirmations stay on the copy they opened on
  when the selected host changes.
- A copy removed since the user acted reports the existing save or clear
  failure once instead of failing silently.
- Mounted note views skip ownership work on store updates that cannot
  change it, and do not re-render for another copy's changes.

* Keep review notes on the exact workspace copy each surface shows

Notes, their owner and every note action now come from one copy identity.
Previews and editors of a server's copy read and save that copy, paired
SSH copies keep working through their server, an SSH draft never lands on
a local row, and controls stay disabled instead of claiming "All notes
sent" while a copy's notes are unknown. Open Clear confirmations share the
cached read, so unrelated store writes do no row work.

* Resolve review-note copies through the shared workspace ownership rules

Every listed row now gets its copy from the same rules the app routes
writes with, including server provenance from detected listings and the
SSH place behind a paired server for older rows without host stamps.
Previews, editors and diffs read notes from their own tab's provenance
instead of the selected copy, folders honor their source's host, and a
note composer's shortcut captures the copy its surface shows when it
opens.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* Keep folder review notes on the folder copy their source names

A folder's notes now come from the same source rules as worktree notes,
so a file from this app never shows or saves to a server's folder with
the same id. Queued folder saves and rollbacks check the captured folder
copy the same way reads do, and resolving copies no longer rescans the
project list for every row.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* Read review-note copies through the shared id indexes

Folder and worktree note views now read only their own workspace's
catalog entries from the shared indexes, and readers of one folder
share its qualification, so listing updates cost work proportional to
what is shown rather than catalog size times readers. Also fixes the
type errors and the end-to-end note seeds, which now pass the owner of
the worktree they seed.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* Share review-note copy resolution across rows and readers

Rows of one workspace on the same host now share a single ownership
answer per catalog snapshot, so duplicate listings of an SSH worktree
no longer make every note view rescan its peers. Folders that are not
listed return one shared answer instead of leaving cache entries behind.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* Name the catalog snapshots review-note ownership results are shared under

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* Save renamed-file drafts under the new path and encode restored SSH targets

A diff note draft opened before its file was renamed in place now saves
under the file's current path, still on the copy it was opened on. A
restored SSH file whose target id needs encoding now shows and saves its
own copy's notes.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* Update the diff note path ref after render instead of during it

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
2026-11-02 10:32:22 -08:00

137 lines
5.1 KiB
TypeScript

import type { Page, TestInfo } from '@stablyai/playwright-test'
import { expect, test } from './helpers/orca-app'
import { waitForActiveWorktree, waitForSessionReady } from './helpers/store'
const NOTE_LINE = 6
const INITIAL_ZONE_HEIGHT = 88
const FOLLOWING_LINE = 'export const line07 = "following-line-marker"'
const NOTE_BODY =
'This saved note is intentionally one long paragraph so it wraps across several visual lines in narrow and wide diff layouts without adding newline characters to the initial zone estimate.'
async function assertCardClearsFollowingLine(page: Page): Promise<void> {
const card = page.locator('.orca-diff-comment-card').first()
const followingLine = page
.locator('.modified-in-monaco-diff-editor .view-lines .view-line')
.filter({ hasText: FOLLOWING_LINE })
.first()
await expect(card).toBeVisible({ timeout: 15_000 })
await expect(followingLine).toBeVisible({ timeout: 15_000 })
await expect
.poll(async () => (await card.boundingBox())?.height ?? 0)
.toBeGreaterThan(INITIAL_ZONE_HEIGHT)
await expect
.poll(
async () => {
const [cardBox, lineBox] = await Promise.all([
card.boundingBox(),
followingLine.boundingBox()
])
return cardBox && lineBox ? lineBox.y - (cardBox.y + cardBox.height) : -1
},
{ message: 'saved note overlaps the following diff line' }
)
.toBeGreaterThanOrEqual(0)
}
async function attachDiffScreenshot(page: Page, testInfo: TestInfo, name: string): Promise<void> {
const diff = page.locator('.monaco-diff-editor').first()
const screenshotPath = testInfo.outputPath(`${name}.png`)
await diff.screenshot({ path: screenshotPath })
await testInfo.attach(name, { path: screenshotPath, contentType: 'image/png' })
}
test.describe('Diff note layout', () => {
test.beforeEach(async ({ orcaPage }) => {
await waitForSessionReady(orcaPage)
await waitForActiveWorktree(orcaPage)
})
test('saved notes reserve their rendered height in both diff layouts', async ({
orcaPage
}, testInfo) => {
await orcaPage.setViewportSize({ width: 1200, height: 800 })
const worktreeId = await waitForActiveWorktree(orcaPage)
const relativePath = await orcaPage.evaluate(async (wId) => {
const store = window.__store
if (!store) {
throw new Error('window.__store is not available')
}
const state = store.getState()
const worktree = Object.values(state.worktreesByRepo)
.flat()
.find((entry) => entry.id === wId)
if (!worktree) {
throw new Error('active worktree not found')
}
const separator = worktree.path.includes('\\') ? '\\' : '/'
const relative = `src${separator}diff-note-layout.ts`
const lines = Array.from({ length: 14 }, (_, index) => {
const number = String(index + 1).padStart(2, '0')
const value = index + 1 === 7 ? 'following-line-marker' : `value-${number}`
return `export const line${number} = "${value}"`
})
await window.api.fs.writeFile({
filePath: `${worktree.path}${separator}${relative}`,
content: `${lines.join('\n')}\n`
})
await state.updateSettings({ diffDefaultView: 'side-by-side' })
return relative
}, worktreeId)
const added = await orcaPage.evaluate(
({ wId, filePath, lineNumber, body }) =>
// The seeded worktree is this app's own (local) copy.
window.__store?.getState().addDiffComment(
{
kind: 'worktree',
worktreeId: wId,
authority: { endpoint: { kind: 'self' }, at: 'local' }
},
{
worktreeId: wId,
filePath,
source: 'diff',
lineNumber,
body,
side: 'modified'
}
),
{ wId: worktreeId, filePath: relativePath, lineNumber: NOTE_LINE, body: NOTE_BODY }
)
expect(added, 'addDiffComment returned null').not.toBeNull()
await orcaPage.evaluate(
({ wId, filePath }) => {
const state = window.__store?.getState()
const worktree = Object.values(state?.worktreesByRepo ?? {})
.flat()
.find((entry) => entry.id === wId)
if (!state || !worktree) {
throw new Error('active worktree not found')
}
const separator = worktree.path.includes('\\') ? '\\' : '/'
state.openDiff(
wId,
`${worktree.path}${separator}${filePath}`,
filePath,
'typescript',
false
)
},
{ wId: worktreeId, filePath: relativePath }
)
await expect(orcaPage.locator('button:has(svg.lucide-rows-2)')).toBeVisible()
await assertCardClearsFollowingLine(orcaPage)
await attachDiffScreenshot(orcaPage, testInfo, 'side-by-side-diff-note-layout')
await orcaPage.evaluate(() =>
window.__store?.getState().updateSettings({ diffDefaultView: 'inline' })
)
await expect(orcaPage.locator('button:has(svg.lucide-columns-2)')).toBeVisible()
await assertCardClearsFollowingLine(orcaPage)
await attachDiffScreenshot(orcaPage, testInfo, 'inline-diff-note-layout')
})
})