diff --git a/src/main/runtime/rpc/methods/mobile-web-source-control-compare-projection.ts b/src/main/runtime/rpc/methods/mobile-web-source-control-compare-projection.ts new file mode 100644 index 00000000000..c0cb8aa4364 --- /dev/null +++ b/src/main/runtime/rpc/methods/mobile-web-source-control-compare-projection.ts @@ -0,0 +1,104 @@ +import type { + GitBranchChangeEntry, + GitBranchCompareResult, + GitCommitCompareResult +} from '../../../../shared/git-diff-compare-types' +import { + MOBILE_WEB_SOURCE_CONTROL_COMPARE_MAX_ENTRIES, + MOBILE_WEB_SOURCE_CONTROL_COMPARE_RESPONSE_MAX_BYTES, + MobileWebSourceControlBranchCompareResultSchema, + MobileWebSourceControlCommitCompareResultSchema, + type MobileWebSourceControlCompareEntry +} from '../../../../shared/mobile-web/source-control-history-contract' +import { MobileWebRelativePathSchema } from '../../../../shared/mobile-web/bridge-operation-contract' +import { + boundedText, + encodedByteLength, + gitObjectId, + RESPONSE_BUDGET_RESERVE_BYTES +} from './mobile-web-source-control-projection-bounds' + +export function projectMobileWebBranchCompare(result: GitBranchCompareResult, baseRef: string) { + const page = compareEntryPage(result.entries) + const changedFiles = Math.max(result.entries.length, result.summary.changedFiles) + const { workspaceId: _workspaceId, ...projected } = + MobileWebSourceControlBranchCompareResultSchema.parse({ + workspaceId: 'page', + baseRef, + compareRef: boundedText(result.summary.compareRef, 240) ?? 'HEAD', + baseOid: gitObjectId(result.summary.baseOid), + headOid: gitObjectId(result.summary.headOid), + mergeBase: gitObjectId(result.summary.mergeBase), + changedFiles, + ...(result.summary.commitsAhead === undefined + ? {} + : { commitsAhead: result.summary.commitsAhead }), + status: result.summary.status === 'loading' ? 'error' : result.summary.status, + entries: page.entries, + truncated: page.truncated || changedFiles > page.entries.length + }) + return projected +} + +export function projectMobileWebCommitCompare(result: GitCommitCompareResult, commitId: string) { + const page = compareEntryPage(result.entries) + const changedFiles = Math.max(result.entries.length, result.summary.changedFiles) + const { workspaceId: _workspaceId, ...projected } = + MobileWebSourceControlCommitCompareResultSchema.parse({ + workspaceId: 'page', + commitId, + commitOid: gitObjectId(result.summary.commitOid), + parentOid: gitObjectId(result.summary.parentOid), + compareRef: boundedText(result.summary.compareRef, 240) ?? commitId.slice(0, 12), + baseRef: boundedText(result.summary.baseRef, 240) ?? 'parent', + changedFiles, + status: result.summary.status, + entries: page.entries, + truncated: page.truncated || changedFiles > page.entries.length + }) + return projected +} + +/** One response carries the whole compare, so the entry list is bounded by both the entry cap and + * the bridge response budget rather than by a resumable offset the page would have to drive. */ +function compareEntryPage(candidates: readonly GitBranchChangeEntry[]): { + entries: MobileWebSourceControlCompareEntry[] + truncated: boolean +} { + const entries: MobileWebSourceControlCompareEntry[] = [] + let retainedBytes = 0 + let droppedByBudget = false + for (const candidate of candidates.slice(0, MOBILE_WEB_SOURCE_CONTROL_COMPARE_MAX_ENTRIES)) { + const entry = compareEntry(candidate) + if (!entry) { + continue + } + const nextBytes = encodedByteLength(entry) + 1 + if ( + retainedBytes + nextBytes > + MOBILE_WEB_SOURCE_CONTROL_COMPARE_RESPONSE_MAX_BYTES - RESPONSE_BUDGET_RESERVE_BYTES + ) { + droppedByBudget = true + break + } + retainedBytes += nextBytes + entries.push(entry) + } + return { entries, truncated: droppedByBudget || entries.length < candidates.length } +} + +/** A changed file the page cannot address as a workspace-relative path leaves the list. */ +function compareEntry(entry: GitBranchChangeEntry): MobileWebSourceControlCompareEntry | null { + const relativePath = MobileWebRelativePathSchema.safeParse(entry.path) + if (!relativePath.success) { + return null + } + const oldRelativePath = MobileWebRelativePathSchema.safeParse(entry.oldPath) + return { + relativePath: relativePath.data, + ...(oldRelativePath.success ? { oldRelativePath: oldRelativePath.data } : {}), + status: entry.status, + ...(entry.added === undefined ? {} : { added: entry.added }), + ...(entry.removed === undefined ? {} : { removed: entry.removed }) + } +} diff --git a/src/main/runtime/rpc/methods/mobile-web-source-control-compare.ts b/src/main/runtime/rpc/methods/mobile-web-source-control-compare.ts index 74b59e95654..a20a1adea8e 100644 --- a/src/main/runtime/rpc/methods/mobile-web-source-control-compare.ts +++ b/src/main/runtime/rpc/methods/mobile-web-source-control-compare.ts @@ -1,9 +1,5 @@ import { defineMethod } from '../core' -import { - MOBILE_WEB_PAGE_IDENTITY, - MobileWebWorktreeScope, - sourceControlHostMethod -} from './mobile-web-source-control-host-method' +import { MobileWebWorktreeScope } from './mobile-web-source-control-host-method' import { MobileWebGitObjectIdSchema, MobileWebGitRefNameSchema @@ -11,41 +7,25 @@ import { import { projectMobileWebBranchCompare, projectMobileWebCommitCompare -} from '../../../../shared/mobile-web/source-control-history-presentation' -import { withoutMobileWebWorkspaceId } from './mobile-web-source-control-workspace-id' - -const branchCompare = sourceControlHostMethod('git.branchCompare') -const commitCompare = sourceControlHostMethod('git.commitCompare') +} from './mobile-web-source-control-compare-projection' export const MOBILE_WEB_SOURCE_CONTROL_COMPARE_METHODS = [ defineMethod({ name: 'mobileWeb.sourceControl.branchCompare', params: MobileWebWorktreeScope.extend({ baseRef: MobileWebGitRefNameSchema }), - handler: async (params, context) => - withoutMobileWebWorkspaceId( - projectMobileWebBranchCompare( - await branchCompare.handler( - { worktree: params.worktree, baseRef: params.baseRef }, - context - ), - MOBILE_WEB_PAGE_IDENTITY, - params.baseRef - ) + handler: async (params, { runtime }) => + projectMobileWebBranchCompare( + await runtime.getRuntimeGitBranchCompare(params.worktree, params.baseRef), + params.baseRef ) }), defineMethod({ name: 'mobileWeb.sourceControl.commitCompare', params: MobileWebWorktreeScope.extend({ commitId: MobileWebGitObjectIdSchema }), - handler: async (params, context) => - withoutMobileWebWorkspaceId( - projectMobileWebCommitCompare( - await commitCompare.handler( - { worktree: params.worktree, commitId: params.commitId }, - context - ), - MOBILE_WEB_PAGE_IDENTITY, - params.commitId - ) + handler: async (params, { runtime }) => + projectMobileWebCommitCompare( + await runtime.getRuntimeGitCommitCompare(params.worktree, params.commitId), + params.commitId ) }) ] diff --git a/src/main/runtime/rpc/methods/mobile-web-source-control-history-projection.ts b/src/main/runtime/rpc/methods/mobile-web-source-control-history-projection.ts new file mode 100644 index 00000000000..5e680fa22b6 --- /dev/null +++ b/src/main/runtime/rpc/methods/mobile-web-source-control-history-projection.ts @@ -0,0 +1,136 @@ +import type { + GitHistoryItem, + GitHistoryItemRef, + GitHistoryResult +} from '../../../../shared/git-history-types' +import type { RuntimeGitLocalBranches } from '../../../../shared/runtime-worktree-contracts' +import { + MOBILE_WEB_SOURCE_CONTROL_BRANCH_LIMIT, + MOBILE_WEB_SOURCE_CONTROL_HISTORY_PARENT_LIMIT, + MOBILE_WEB_SOURCE_CONTROL_HISTORY_REFERENCE_LIMIT, + MOBILE_WEB_SOURCE_CONTROL_HISTORY_RESPONSE_MAX_BYTES, + MobileWebSourceControlBranchesResultSchema, + MobileWebSourceControlHistoryResultSchema, + type MobileWebSourceControlHistoryItem, + type MobileWebSourceControlHistoryRef +} from '../../../../shared/mobile-web/source-control-history-contract' +import { + boundedText, + gitObjectId, + gitRefName, + RESPONSE_BUDGET_RESERVE_BYTES, + encodedByteLength +} from './mobile-web-source-control-projection-bounds' + +const MESSAGE_MAX_CHARACTERS = 8 * 1024 +const SUBJECT_MAX_CHARACTERS = 512 + +export function projectMobileWebBranches(result: RuntimeGitLocalBranches) { + const branches = result.branches + .slice(0, MOBILE_WEB_SOURCE_CONTROL_BRANCH_LIMIT) + .flatMap((branch) => gitRefName(branch) ?? []) + const { workspaceId: _workspaceId, ...projected } = + MobileWebSourceControlBranchesResultSchema.parse({ + workspaceId: 'page', + current: gitRefName(result.current) ?? null, + branches, + totalCount: result.branches.length, + truncated: branches.length < result.branches.length + }) + return projected +} + +export function projectMobileWebHistory(result: GitHistoryResult, limit: number) { + const items: MobileWebSourceControlHistoryItem[] = [] + let retainedBytes = 0 + let droppedByBudget = false + for (const candidate of result.items.slice(0, limit)) { + const item = projectHistoryItem(candidate) + if (!item) { + continue + } + const nextBytes = encodedByteLength(item) + 1 + if ( + retainedBytes + nextBytes > + MOBILE_WEB_SOURCE_CONTROL_HISTORY_RESPONSE_MAX_BYTES - RESPONSE_BUDGET_RESERVE_BYTES + ) { + droppedByBudget = true + break + } + retainedBytes += nextBytes + items.push(item) + } + const currentRef = projectHistoryRef(result.currentRef) + const remoteRef = projectHistoryRef(result.remoteRef) + const baseRef = projectHistoryRef(result.baseRef) + const mergeBase = gitObjectId(result.mergeBase) + const { workspaceId: _workspaceId, ...projected } = + MobileWebSourceControlHistoryResultSchema.parse({ + workspaceId: 'page', + items, + ...(currentRef ? { currentRef } : {}), + ...(remoteRef ? { remoteRef } : {}), + ...(baseRef ? { baseRef } : {}), + ...(mergeBase ? { mergeBase } : {}), + hasIncomingChanges: result.hasIncomingChanges, + hasOutgoingChanges: result.hasOutgoingChanges, + hasMore: result.hasMore || droppedByBudget || items.length < result.items.length, + limit + }) + return projected +} + +/** A commit whose id is not an object id cannot be addressed by the page contract at all, so it + * leaves the page rather than arriving unaddressable. */ +function projectHistoryItem(item: GitHistoryItem): MobileWebSourceControlHistoryItem | null { + const id = gitObjectId(item.id) + if (!id) { + return null + } + const message = boundedText(item.message, MESSAGE_MAX_CHARACTERS) ?? '' + return { + id, + parentIds: item.parentIds + .slice(0, MOBILE_WEB_SOURCE_CONTROL_HISTORY_PARENT_LIMIT) + .flatMap((parent) => gitObjectId(parent) ?? []), + displayId: id.slice(0, 12), + subject: + boundedText(item.subject, SUBJECT_MAX_CHARACTERS) ?? + boundedText(message.split(/\r?\n/, 1)[0], SUBJECT_MAX_CHARACTERS) ?? + '(no commit message)', + message, + ...(boundedText(item.author, 256) === undefined ? {} : { author: item.author!.slice(0, 256) }), + ...(safeTimestamp(item.timestamp) === undefined ? {} : { timestamp: item.timestamp }), + references: (item.references ?? []) + .slice(0, MOBILE_WEB_SOURCE_CONTROL_HISTORY_REFERENCE_LIMIT) + .flatMap((reference) => projectHistoryRef(reference) ?? []) + } +} + +function projectHistoryRef( + reference: GitHistoryItemRef | undefined +): MobileWebSourceControlHistoryRef | null { + const id = reference && boundedText(reference.id, 320) + const name = reference && boundedText(reference.name, 240) + if (!reference || !id || !name) { + return null + } + const revision = gitObjectId(reference.revision) + const description = boundedText(reference.description, 512) + return { + id, + name, + ...(revision ? { revision } : {}), + ...(reference.category ? { category: reference.category } : {}), + ...(description === undefined ? {} : { description }) + } +} + +function safeTimestamp(value: number | undefined): number | undefined { + return value !== undefined && + Number.isSafeInteger(value) && + value >= -8_640_000_000_000_000 && + value <= 8_640_000_000_000_000 + ? value + : undefined +} diff --git a/src/main/runtime/rpc/methods/mobile-web-source-control-history.test.ts b/src/main/runtime/rpc/methods/mobile-web-source-control-history.test.ts index 996f6430f59..f0bfe565a1e 100644 --- a/src/main/runtime/rpc/methods/mobile-web-source-control-history.test.ts +++ b/src/main/runtime/rpc/methods/mobile-web-source-control-history.test.ts @@ -1,32 +1,40 @@ -import { afterEach, describe, expect, it, vi } from 'vitest' +import { describe, expect, it, vi } from 'vitest' import type { RpcContext } from '../core' -import { GIT_METHODS } from './git' import { MOBILE_WEB_SOURCE_CONTROL_COMPARE_METHODS } from './mobile-web-source-control-compare' import { MOBILE_WEB_SOURCE_CONTROL_HISTORY_METHODS } from './mobile-web-source-control-history' -const context = { signal: new AbortController().signal } as RpcContext const worktree = 'id:private-host-workspace' const OID = 'a'.repeat(40) +const METHODS = [ + ...MOBILE_WEB_SOURCE_CONTROL_HISTORY_METHODS, + ...MOBILE_WEB_SOURCE_CONTROL_COMPARE_METHODS +] -function fixture(name: string, sourceMethod: string, raw: unknown) { - const source = GIT_METHODS.find((method) => method.name === sourceMethod)! - const handler = vi.spyOn(source, 'handler').mockResolvedValue(raw) - const method = [ - ...MOBILE_WEB_SOURCE_CONTROL_HISTORY_METHODS, - ...MOBILE_WEB_SOURCE_CONTROL_COMPARE_METHODS - ].find((entry) => entry.name === name)! +function fixture(name: string, runtimeMethod: string, result: unknown) { + const call = vi.fn().mockResolvedValue(result) + const method = METHODS.find((entry) => entry.name === name)! return { - handler, + call, run: async (params: Record = {}) => - method.handler(method.params!.parse({ worktree, ...params }), context) + method.handler(method.params!.parse({ worktree, ...params }), { + runtime: { [runtimeMethod]: call } + } as unknown as RpcContext) } } -afterEach(() => vi.restoreAllMocks()) +function historyItem(index: number, message: string) { + return { + id: index.toString(16).padStart(40, '0'), + parentIds: [], + subject: 'subject', + message, + references: [] + } +} describe('bounded host Source Control history reads', () => { it('caps a branch list the page cannot render and reports the true total', async () => { - const f = fixture('mobileWeb.sourceControl.branches', 'git.localBranches', { + const f = fixture('mobileWeb.sourceControl.branches', 'listRuntimeGitLocalBranches', { current: 'main', branches: Array.from({ length: 500 }, (_, index) => `branch-${index}`) }) @@ -34,24 +42,29 @@ describe('bounded host Source Control history reads', () => { expect(result.branches).toHaveLength(128) expect(result).toMatchObject({ totalCount: 500, truncated: true }) expect(JSON.stringify(result)).not.toContain('workspaceId') - expect(f.handler).toHaveBeenCalledWith({ worktree }, context) + expect(f.call).toHaveBeenCalledWith(worktree) + }) + + it('drops a branch name the page contract cannot address', async () => { + const f = fixture('mobileWeb.sourceControl.branches', 'listRuntimeGitLocalBranches', { + current: '--upload-pack=evil', + branches: ['main', '--upload-pack=evil'] + }) + await expect(f.run()).resolves.toMatchObject({ + current: null, + branches: ['main'], + truncated: true + }) }) it('drops history items that would overrun the bridge budget and marks the page incomplete', async () => { - const raw = { - items: Array.from({ length: 100 }, (_, index) => ({ - id: index.toString(16).padStart(40, '0'), - parentIds: [], - subject: 'subject', - message: 'x'.repeat(16 * 1024), - references: [] - })), + const f = fixture('mobileWeb.sourceControl.history', 'getRuntimeGitHistory', { + items: Array.from({ length: 100 }, (_, index) => historyItem(index, 'x'.repeat(16 * 1024))), hasIncomingChanges: false, hasOutgoingChanges: false, hasMore: false, limit: 100 - } - const f = fixture('mobileWeb.sourceControl.history', 'git.history', raw) + }) const result = (await f.run({ limit: 100 })) as { items: { message: string }[] hasMore: boolean @@ -60,11 +73,31 @@ describe('bounded host Source Control history reads', () => { expect(result.hasMore).toBe(true) expect(result.items[0]!.message).toHaveLength(8 * 1024) expect(Buffer.byteLength(JSON.stringify(result))).toBeLessThan(192 * 1024) - expect(f.handler).toHaveBeenCalledWith({ worktree, limit: 100 }, context) + expect(f.call).toHaveBeenCalledWith(worktree, { limit: 100 }) + }) + + it('drops the desktop-only fields the page contract does not carry', async () => { + const f = fixture('mobileWeb.sourceControl.history', 'getRuntimeGitHistory', { + items: [ + { + ...historyItem(1, 'feat: ship'), + authorEmail: 'private@example.com', + statistics: { files: 1, insertions: 2, deletions: 3 }, + references: [{ id: 'ref', name: 'main', color: 'git-graph-ref' }] + } + ], + hasIncomingChanges: false, + hasOutgoingChanges: false, + hasMore: false, + limit: 50 + }) + const result = await f.run() + expect(JSON.stringify(result)).not.toMatch(/authorEmail|statistics|color/) + expect(result).toMatchObject({ items: [{ references: [{ id: 'ref', name: 'main' }] }] }) }) it('forwards the requested base ref and defaults the history limit', async () => { - const f = fixture('mobileWeb.sourceControl.history', 'git.history', { + const f = fixture('mobileWeb.sourceControl.history', 'getRuntimeGitHistory', { items: [], hasIncomingChanges: false, hasOutgoingChanges: false, @@ -72,14 +105,14 @@ describe('bounded host Source Control history reads', () => { limit: 50 }) await f.run({ baseRef: 'origin/main' }) - expect(f.handler).toHaveBeenCalledWith({ worktree, limit: 50, baseRef: 'origin/main' }, context) + expect(f.call).toHaveBeenCalledWith(worktree, { limit: 50, baseRef: 'origin/main' }) await expect(f.run({ baseRef: '--upload-pack=evil' })).rejects.toThrow() }) }) describe('bounded host Source Control compares', () => { it('clips a branch compare to the response budget and keeps the reported file count', async () => { - const raw = { + const f = fixture('mobileWeb.sourceControl.branchCompare', 'getRuntimeGitBranchCompare', { summary: { baseRef: 'main', baseOid: OID, @@ -93,8 +126,7 @@ describe('bounded host Source Control compares', () => { path: `src/${'deep/'.repeat(8)}file-${index}.ts`, status: 'modified' })) - } - const f = fixture('mobileWeb.sourceControl.branchCompare', 'git.branchCompare', raw) + }) const result = (await f.run({ baseRef: 'main' })) as { entries: unknown[] changedFiles: number @@ -103,11 +135,34 @@ describe('bounded host Source Control compares', () => { expect(result.entries.length).toBeLessThan(4_000) expect(result).toMatchObject({ changedFiles: 6_000, truncated: true }) expect(Buffer.byteLength(JSON.stringify(result))).toBeLessThan(192 * 1024) - expect(f.handler).toHaveBeenCalledWith({ worktree, baseRef: 'main' }, context) + expect(f.call).toHaveBeenCalledWith(worktree, 'main') + }) + + it('drops a changed file the page cannot address and reports a loading compare as an error', async () => { + const f = fixture('mobileWeb.sourceControl.branchCompare', 'getRuntimeGitBranchCompare', { + summary: { + baseRef: 'main', + baseOid: null, + compareRef: 'HEAD', + headOid: null, + mergeBase: null, + changedFiles: 2, + status: 'loading' + }, + entries: [ + { path: '../outside.ts', status: 'modified' }, + { path: 'src/app.ts', status: 'modified' } + ] + }) + await expect(f.run({ baseRef: 'main' })).resolves.toMatchObject({ + status: 'error', + entries: [{ relativePath: 'src/app.ts' }], + truncated: true + }) }) it('answers a commit compare in one page and refuses a short commit id', async () => { - const f = fixture('mobileWeb.sourceControl.commitCompare', 'git.commitCompare', { + const f = fixture('mobileWeb.sourceControl.commitCompare', 'getRuntimeGitCommitCompare', { summary: { commitOid: OID, parentOid: null, diff --git a/src/main/runtime/rpc/methods/mobile-web-source-control-history.ts b/src/main/runtime/rpc/methods/mobile-web-source-control-history.ts index 4fe88dffd67..4dc63339c12 100644 --- a/src/main/runtime/rpc/methods/mobile-web-source-control-history.ts +++ b/src/main/runtime/rpc/methods/mobile-web-source-control-history.ts @@ -1,10 +1,6 @@ import { z } from 'zod' import { defineMethod } from '../core' -import { - MOBILE_WEB_PAGE_IDENTITY, - MobileWebWorktreeScope, - sourceControlHostMethod -} from './mobile-web-source-control-host-method' +import { MobileWebWorktreeScope } from './mobile-web-source-control-host-method' import { MOBILE_WEB_SOURCE_CONTROL_HISTORY_DEFAULT_LIMIT, MOBILE_WEB_SOURCE_CONTROL_HISTORY_MAX_LIMIT, @@ -13,23 +9,14 @@ import { import { projectMobileWebBranches, projectMobileWebHistory -} from '../../../../shared/mobile-web/source-control-history-presentation' -import { withoutMobileWebWorkspaceId } from './mobile-web-source-control-workspace-id' - -const branches = sourceControlHostMethod('git.localBranches') -const history = sourceControlHostMethod('git.history') +} from './mobile-web-source-control-history-projection' export const MOBILE_WEB_SOURCE_CONTROL_HISTORY_METHODS = [ defineMethod({ name: 'mobileWeb.sourceControl.branches', params: MobileWebWorktreeScope, - handler: async (params, context) => - withoutMobileWebWorkspaceId( - projectMobileWebBranches( - await branches.handler({ worktree: params.worktree }, context), - MOBILE_WEB_PAGE_IDENTITY - ) - ) + handler: async (params, { runtime }) => + projectMobileWebBranches(await runtime.listRuntimeGitLocalBranches(params.worktree)) }), defineMethod({ name: 'mobileWeb.sourceControl.history', @@ -42,20 +29,13 @@ export const MOBILE_WEB_SOURCE_CONTROL_HISTORY_METHODS = [ .default(MOBILE_WEB_SOURCE_CONTROL_HISTORY_DEFAULT_LIMIT), baseRef: MobileWebGitRefNameSchema.optional() }), - handler: async (params, context) => - withoutMobileWebWorkspaceId( - projectMobileWebHistory( - await history.handler( - { - worktree: params.worktree, - limit: params.limit, - ...(params.baseRef === undefined ? {} : { baseRef: params.baseRef }) - }, - context - ), - MOBILE_WEB_PAGE_IDENTITY, - params.limit - ) + handler: async (params, { runtime }) => + projectMobileWebHistory( + await runtime.getRuntimeGitHistory(params.worktree, { + limit: params.limit, + ...(params.baseRef === undefined ? {} : { baseRef: params.baseRef }) + }), + params.limit ) }) ] diff --git a/src/main/runtime/rpc/methods/mobile-web-source-control-projection-bounds.ts b/src/main/runtime/rpc/methods/mobile-web-source-control-projection-bounds.ts new file mode 100644 index 00000000000..c8203a4bfda --- /dev/null +++ b/src/main/runtime/rpc/methods/mobile-web-source-control-projection-bounds.ts @@ -0,0 +1,27 @@ +import { + MobileWebGitObjectIdSchema, + MobileWebGitRefNameSchema +} from '../../../../shared/mobile-web/source-control-history-contract' + +/** Headroom for the envelope the projected page result still has to fit inside. */ +export const RESPONSE_BUDGET_RESERVE_BYTES = 8 * 1024 + +/** A Git string the page contract accepts only in a narrower form. Desktop types guarantee a + * string, never that it is an object id or a ref name the page schema admits. */ +export function gitObjectId(value: string | null | undefined): string | null { + const parsed = MobileWebGitObjectIdSchema.safeParse(value) + return parsed.success ? parsed.data : null +} + +export function gitRefName(value: string | null | undefined): string | null { + const parsed = MobileWebGitRefNameSchema.safeParse(value) + return parsed.success ? parsed.data : null +} + +export function boundedText(value: string | undefined, limit: number): string | undefined { + return value !== undefined && value.length > 0 ? value.slice(0, limit) : undefined +} + +export function encodedByteLength(value: unknown): number { + return Buffer.byteLength(JSON.stringify(value)) +} diff --git a/src/main/runtime/rpc/methods/mobile-web-source-control-repository.ts b/src/main/runtime/rpc/methods/mobile-web-source-control-repository.ts index 78c910ae95b..8a39218e20c 100644 --- a/src/main/runtime/rpc/methods/mobile-web-source-control-repository.ts +++ b/src/main/runtime/rpc/methods/mobile-web-source-control-repository.ts @@ -1,53 +1,49 @@ -import { defineMethod, type RpcContext } from '../core' -import { - MOBILE_WEB_PAGE_IDENTITY, - MobileWebWorktreeScope, - sourceControlHostMethod -} from './mobile-web-source-control-host-method' -import { projectMobileWebRepositoryState } from '../../../../shared/mobile-web/source-control-repository-presentation' -import { withoutMobileWebWorkspaceId } from './mobile-web-source-control-workspace-id' - -const status = sourceControlHostMethod('git.status') -const upstream = sourceControlHostMethod('git.upstreamStatus') -const worktreeShow = sourceControlHostMethod('worktree.show') -const repoBaseRefDefault = sourceControlHostMethod('repo.baseRefDefault') +import { defineMethod } from '../core' +import type { OrcaRuntimeService } from '../../orca-runtime' +import { MobileWebWorktreeScope } from './mobile-web-source-control-host-method' +import { MobileWebSourceControlRepositoryStateSchema } from '../../../../shared/mobile-web/source-control-sync-contract' +import { gitObjectId, gitRefName } from './mobile-web-source-control-projection-bounds' export const MOBILE_WEB_SOURCE_CONTROL_REPOSITORY_METHODS = [ defineMethod({ name: 'mobileWeb.sourceControl.repositoryState', params: MobileWebWorktreeScope, - handler: async (params, context) => { - const [statusResult, upstreamResult, baseRef] = await Promise.all([ - status.handler({ worktree: params.worktree }, context), - upstream.handler({ worktree: params.worktree }, context), - resolveBaseRef(params.worktree, context) + handler: async (params, { runtime }) => { + const [status, upstream, baseRef] = await Promise.all([ + runtime.getRuntimeGitStatus(params.worktree, { admissionTier: 'status' }), + runtime.getRuntimeGitUpstreamStatus(params.worktree), + resolveBaseRef(runtime, params.worktree) ]) - return withoutMobileWebWorkspaceId( - projectMobileWebRepositoryState({ - status: statusResult, - upstream: upstreamResult, - baseRef, - workspaceId: MOBILE_WEB_PAGE_IDENTITY + const { workspaceId: _workspaceId, ...projected } = + MobileWebSourceControlRepositoryStateSchema.parse({ + workspaceId: 'page', + head: gitObjectId(status.head), + branch: gitRefName(status.branch), + conflictOperation: status.conflictOperation, + baseRef: gitRefName(baseRef), + upstream: { + hasUpstream: upstream.hasUpstream, + ...(upstream.upstreamName ? { upstreamName: upstream.upstreamName.slice(0, 240) } : {}), + ahead: upstream.ahead, + behind: upstream.behind, + hasConfiguredPushTarget: upstream.hasConfiguredPushTarget === true, + behindCommitsArePatchEquivalent: upstream.behindCommitsArePatchEquivalent === true + } }) - ) + return projected } }) ] /** The workspace ref wins; the project default only fills in a workspace that never pinned one. */ -async function resolveBaseRef(worktree: string, context: RpcContext): Promise { - const shown = await worktreeShow.handler({ worktree }, context) - const record = isRecord(shown) && isRecord(shown.worktree) ? shown.worktree : undefined - if (typeof record?.baseRef === 'string' && record.baseRef.length > 0) { +async function resolveBaseRef( + runtime: OrcaRuntimeService, + worktree: string +): Promise { + const record = await runtime.showManagedWorktree(worktree) + if (record.baseRef) { return record.baseRef } - if (typeof record?.repoId !== 'string' || record.repoId.length === 0) { - return null - } - const fallback = await repoBaseRefDefault.handler({ repo: `id:${record.repoId}` }, context) - return isRecord(fallback) ? fallback.defaultBaseRef : null -} - -function isRecord(value: unknown): value is Record { - return typeof value === 'object' && value !== null && !Array.isArray(value) + const fallback = await runtime.getRepoBaseRefDefault(`id:${record.repoId}`) + return fallback.defaultBaseRef } diff --git a/src/main/runtime/rpc/methods/mobile-web-source-control-review-link.ts b/src/main/runtime/rpc/methods/mobile-web-source-control-review-link.ts index 33c0459c676..a790c401baa 100644 --- a/src/main/runtime/rpc/methods/mobile-web-source-control-review-link.ts +++ b/src/main/runtime/rpc/methods/mobile-web-source-control-review-link.ts @@ -1,22 +1,10 @@ -import { z } from 'zod' -import { defineMethod, type RpcContext } from '../core' -import { - MOBILE_WEB_PAGE_IDENTITY, - MobileWebWorktreeScope, - sourceControlHostMethod -} from './mobile-web-source-control-host-method' -import { - MobileWebSourceControlReviewLinkUpdatePayloadSchema, - type MobileWebSourceControlReviewLinkResult -} from '../../../../shared/mobile-web/source-control-review-contract' +import { defineMethod } from '../core' +import { MobileWebWorktreeScope } from './mobile-web-source-control-host-method' +import { MobileWebSourceControlReviewLinkUpdatePayloadSchema } from '../../../../shared/mobile-web/source-control-review-contract' import { mobileWebReviewLinkWorktreeField, projectMobileWebReviewLink -} from '../../../../shared/mobile-web/source-control-review-presentation' -import { withoutMobileWebWorkspaceId } from './mobile-web-source-control-workspace-id' - -const worktreeShow = sourceControlHostMethod('worktree.show') -const worktreeSet = sourceControlHostMethod('worktree.set') +} from './mobile-web-source-control-review-projection' const UpdateParams = MobileWebWorktreeScope.extend( MobileWebSourceControlReviewLinkUpdatePayloadSchema.omit({ workspaceId: true }).shape @@ -26,31 +14,18 @@ export const MOBILE_WEB_SOURCE_CONTROL_REVIEW_LINK_METHODS = [ defineMethod({ name: 'mobileWeb.sourceControl.reviewLink', params: MobileWebWorktreeScope, - handler: async (params, context) => - withoutMobileWebWorkspaceId(await readReviewLink(params.worktree, context)) + handler: async (params, { runtime }) => + projectMobileWebReviewLink(await runtime.showManagedWorktree(params.worktree)) }), defineMethod({ name: 'mobileWeb.sourceControl.reviewLinkUpdate', params: UpdateParams, - handler: async (params, context) => { - await worktreeSet.handler( - { - worktree: params.worktree, - ...mobileWebReviewLinkWorktreeField(params.provider, params.number), - ...(params.baseRef ? { baseRef: params.baseRef } : {}) - }, - context - ) - return withoutMobileWebWorkspaceId(await readReviewLink(params.worktree, context)) + handler: async (params, { runtime }) => { + await runtime.updateManagedWorktreeMeta(params.worktree, { + ...mobileWebReviewLinkWorktreeField(params.provider, params.number), + ...(params.baseRef ? { baseRef: params.baseRef } : {}) + }) + return projectMobileWebReviewLink(await runtime.showManagedWorktree(params.worktree)) } }) ] - -async function readReviewLink( - worktree: string, - context: RpcContext -): Promise { - const shown = await worktreeShow.handler({ worktree }, context) - const record = z.object({ worktree: z.unknown() }).parse(shown).worktree - return projectMobileWebReviewLink(record, MOBILE_WEB_PAGE_IDENTITY) -} diff --git a/src/main/runtime/rpc/methods/mobile-web-source-control-review-metadata.ts b/src/main/runtime/rpc/methods/mobile-web-source-control-review-metadata.ts index d738e0afefb..89362dfb4e4 100644 --- a/src/main/runtime/rpc/methods/mobile-web-source-control-review-metadata.ts +++ b/src/main/runtime/rpc/methods/mobile-web-source-control-review-metadata.ts @@ -1,23 +1,13 @@ -import { z } from 'zod' -import { defineMethod, type RpcContext } from '../core' -import { - MOBILE_WEB_PAGE_IDENTITY, - MobileWebWorktreeScope, - sourceControlHostMethod -} from './mobile-web-source-control-host-method' +import { defineMethod } from '../core' +import { MobileWebWorktreeScope } from './mobile-web-source-control-host-method' import { MobileWebSourceControlReviewMetadataUpdateShape, - rejectDuplicateReviewMetadataKeys, - type MobileWebSourceControlReviewMetadataResult + rejectDuplicateReviewMetadataKeys } from '../../../../shared/mobile-web/source-control-review-contract' import { mobileWebReviewMetadataWorktreeFields, projectMobileWebReviewMetadata -} from '../../../../shared/mobile-web/source-control-review-presentation' -import { withoutMobileWebWorkspaceId } from './mobile-web-source-control-workspace-id' - -const worktreeShow = sourceControlHostMethod('worktree.show') -const worktreeSet = sourceControlHostMethod('worktree.set') +} from './mobile-web-source-control-review-projection' const UpdateParams = MobileWebWorktreeScope.extend(MobileWebSourceControlReviewMetadataUpdateShape) .strict() @@ -27,43 +17,33 @@ export const MOBILE_WEB_SOURCE_CONTROL_REVIEW_METADATA_METHODS = [ defineMethod({ name: 'mobileWeb.sourceControl.reviewMetadata', params: MobileWebWorktreeScope, - handler: async (params, context) => - withoutMobileWebWorkspaceId(await readReviewMetadata(params.worktree, context)) + handler: async (params, { runtime }) => + projectMobileWebReviewMetadata(await runtime.showManagedWorktree(params.worktree)) }), defineMethod({ name: 'mobileWeb.sourceControl.reviewMetadataUpdate', params: UpdateParams, - handler: async (params, context) => { - const current = await readReviewMetadata(params.worktree, context) + handler: async (params, { runtime }) => { + const current = projectMobileWebReviewMetadata( + await runtime.showManagedWorktree(params.worktree) + ) if (current.revision !== params.expectedRevision) { throw new Error('conflict') } - // worktree.set has no compare-and-set, so another writer can still win after this read. - await worktreeSet.handler( - { - worktree: params.worktree, - ...mobileWebReviewMetadataWorktreeFields({ - worktreeId: worktreeIdFromSelector(params.worktree), - comments: params.comments, - reviewState: params.reviewState - }) - }, - context + // The workspace record has no compare-and-set, so another writer can still win after this read. + await runtime.updateManagedWorktreeMeta( + params.worktree, + mobileWebReviewMetadataWorktreeFields({ + worktreeId: worktreeIdFromSelector(params.worktree), + comments: params.comments, + reviewState: params.reviewState + }) ) - return withoutMobileWebWorkspaceId(await readReviewMetadata(params.worktree, context)) + return projectMobileWebReviewMetadata(await runtime.showManagedWorktree(params.worktree)) } }) ] -async function readReviewMetadata( - worktree: string, - context: RpcContext -): Promise { - const shown = await worktreeShow.handler({ worktree }, context) - const record = z.object({ worktree: z.unknown() }).parse(shown).worktree - return projectMobileWebReviewMetadata(record, MOBILE_WEB_PAGE_IDENTITY) -} - function worktreeIdFromSelector(worktree: string): string { return worktree.startsWith('id:') ? worktree.slice('id:'.length) : worktree } diff --git a/src/main/runtime/rpc/methods/mobile-web-source-control-review-projection.ts b/src/main/runtime/rpc/methods/mobile-web-source-control-review-projection.ts new file mode 100644 index 00000000000..dbde5ecbb8f --- /dev/null +++ b/src/main/runtime/rpc/methods/mobile-web-source-control-review-projection.ts @@ -0,0 +1,168 @@ +import { sha256 } from '../../../../shared/sha256' +import type { DiffComment, MobileDiffReviewState } from '../../../../shared/diff-comment-types' +import type { RuntimeWorktreeRecord } from '../../../../shared/runtime-worktree-contracts' +import { + MOBILE_WEB_REVIEW_COMMENT_LIMIT, + MOBILE_WEB_REVIEW_COMMENT_MAX_CHARACTERS, + MOBILE_WEB_REVIEW_FILE_STATE_LIMIT, + MobileWebSourceControlReviewLinkResultSchema, + MobileWebSourceControlReviewMetadataResultSchema, + type MobileWebSourceControlReviewComment, + type MobileWebSourceControlReviewState +} from '../../../../shared/mobile-web/source-control-review-contract' + +export function projectMobileWebReviewMetadata(worktree: RuntimeWorktreeRecord) { + const rawComments = worktree.diffComments ?? [] + const rawFiles = Object.values(worktree.mobileDiffReview?.files ?? {}) + if ( + rawComments.length > MOBILE_WEB_REVIEW_COMMENT_LIMIT || + rawFiles.length > MOBILE_WEB_REVIEW_FILE_STATE_LIMIT + ) { + throw new Error('too_large') + } + const comments = rawComments.map(projectComment) + const reviewState: MobileWebSourceControlReviewState = { + version: 1, + ...(worktree.mobileDiffReview?.updatedAt === undefined + ? {} + : { updatedAt: worktree.mobileDiffReview.updatedAt }), + ...(worktree.mobileDiffReview?.completedAt === undefined + ? {} + : { completedAt: worktree.mobileDiffReview.completedAt }), + files: rawFiles.map((file) => ({ + key: file.key, + relativePath: file.filePath, + ...(file.oldPath ? { oldRelativePath: file.oldPath } : {}), + scope: file.scope, + ...(file.lastOpenedAt === undefined ? {} : { lastOpenedAt: file.lastOpenedAt }), + ...(file.lastSeenDiffIdentity ? { lastSeenDiffIdentity: file.lastSeenDiffIdentity } : {}), + ...(file.reviewedAt === undefined ? {} : { reviewedAt: file.reviewedAt }), + ...(file.reviewDiffIdentity ? { reviewDiffIdentity: file.reviewDiffIdentity } : {}) + })) + } + const { workspaceId: _workspaceId, ...projected } = + MobileWebSourceControlReviewMetadataResultSchema.parse({ + workspaceId: 'page', + revision: mobileWebReviewMetadataRevision({ comments, reviewState }), + comments, + reviewState + }) + return projected +} + +export function mobileWebReviewMetadataRevision(value: unknown): string { + return Array.from(sha256(new TextEncoder().encode(JSON.stringify(value))), (byte) => + byte.toString(16).padStart(2, '0') + ).join('') +} + +/** The only workspace fields a review write may touch. */ +export function mobileWebReviewMetadataWorktreeFields(args: { + worktreeId: string + comments: readonly MobileWebSourceControlReviewComment[] + reviewState: MobileWebSourceControlReviewState +}): { diffComments: DiffComment[]; mobileDiffReview: MobileDiffReviewState } { + return { + diffComments: args.comments.map((comment) => ({ + id: comment.id, + worktreeId: args.worktreeId, + filePath: comment.relativePath, + ...(comment.oldRelativePath ? { oldPath: comment.oldRelativePath } : {}), + ...(comment.source ? { source: comment.source } : {}), + ...(comment.selectedText === undefined ? {} : { selectedText: comment.selectedText }), + ...(comment.startLine === undefined ? {} : { startLine: comment.startLine }), + lineNumber: comment.lineNumber, + body: comment.body, + createdAt: comment.createdAt, + ...(comment.updatedAt === undefined ? {} : { updatedAt: comment.updatedAt }), + ...(comment.sentAt === undefined ? {} : { sentAt: comment.sentAt }), + ...(comment.scope ? { scope: comment.scope } : {}), + ...(comment.diffIdentity ? { diffIdentity: comment.diffIdentity } : {}), + side: 'modified' + })), + mobileDiffReview: { + version: 1, + ...(args.reviewState.updatedAt === undefined + ? {} + : { updatedAt: args.reviewState.updatedAt }), + ...(args.reviewState.completedAt === undefined + ? {} + : { completedAt: args.reviewState.completedAt }), + files: Object.fromEntries( + args.reviewState.files.map((file) => [ + file.key, + { + key: file.key, + filePath: file.relativePath, + ...(file.oldRelativePath ? { oldPath: file.oldRelativePath } : {}), + scope: file.scope, + ...(file.lastOpenedAt === undefined ? {} : { lastOpenedAt: file.lastOpenedAt }), + ...(file.lastSeenDiffIdentity + ? { lastSeenDiffIdentity: file.lastSeenDiffIdentity } + : {}), + ...(file.reviewedAt === undefined ? {} : { reviewedAt: file.reviewedAt }), + ...(file.reviewDiffIdentity ? { reviewDiffIdentity: file.reviewDiffIdentity } : {}) + } + ]) + ) + } + } +} + +export function projectMobileWebReviewLink(worktree: RuntimeWorktreeRecord) { + const { workspaceId: _workspaceId, ...projected } = + MobileWebSourceControlReviewLinkResultSchema.parse({ + workspaceId: 'page', + baseRef: worktree.baseRef ? worktree.baseRef.slice(0, 512) : null, + linkedGitHubPR: positiveInteger(worktree.linkedPR), + linkedGitLabMR: positiveInteger(worktree.linkedGitLabMR), + linkedBitbucketPR: positiveInteger(worktree.linkedBitbucketPR), + linkedAzureDevOpsPR: positiveInteger(worktree.linkedAzureDevOpsPR), + linkedGiteaPR: positiveInteger(worktree.linkedGiteaPR) + }) + return projected +} + +export function mobileWebReviewLinkWorktreeField( + provider: 'github' | 'gitlab' | 'bitbucket' | 'azure-devops' | 'gitea', + number: number | null +) { + if (provider === 'github') { + return { linkedPR: number } + } + if (provider === 'gitlab') { + return { linkedGitLabMR: number } + } + if (provider === 'bitbucket') { + return { linkedBitbucketPR: number } + } + if (provider === 'azure-devops') { + return { linkedAzureDevOpsPR: number } + } + return { linkedGiteaPR: number } +} + +function projectComment(comment: DiffComment): MobileWebSourceControlReviewComment { + return { + id: comment.id, + relativePath: comment.filePath, + ...(comment.oldPath ? { oldRelativePath: comment.oldPath } : {}), + ...(comment.source ? { source: comment.source } : {}), + ...(comment.selectedText === undefined + ? {} + : { selectedText: comment.selectedText.slice(0, MOBILE_WEB_REVIEW_COMMENT_MAX_CHARACTERS) }), + ...(comment.startLine === undefined ? {} : { startLine: comment.startLine }), + lineNumber: comment.lineNumber, + body: comment.body.slice(0, MOBILE_WEB_REVIEW_COMMENT_MAX_CHARACTERS), + createdAt: comment.createdAt, + ...(comment.updatedAt === undefined ? {} : { updatedAt: comment.updatedAt }), + ...(comment.sentAt === undefined ? {} : { sentAt: comment.sentAt }), + ...(comment.scope ? { scope: comment.scope } : {}), + ...(comment.diffIdentity ? { diffIdentity: comment.diffIdentity } : {}), + side: 'modified' + } +} + +function positiveInteger(value: number | null | undefined): number | null { + return typeof value === 'number' && Number.isSafeInteger(value) && value > 0 ? value : null +} diff --git a/src/main/runtime/rpc/methods/mobile-web-source-control-review.test.ts b/src/main/runtime/rpc/methods/mobile-web-source-control-review.test.ts index 1d33c34ae38..61e4b1a723c 100644 --- a/src/main/runtime/rpc/methods/mobile-web-source-control-review.test.ts +++ b/src/main/runtime/rpc/methods/mobile-web-source-control-review.test.ts @@ -1,15 +1,13 @@ import { afterEach, describe, expect, it, vi } from 'vitest' -import type { RpcAnyMethod, RpcContext, RpcMethod } from '../core' +import type { RpcContext, RpcMethod } from '../core' import { GIT_METHODS } from './git' -import { REPO_METHODS } from './repo' -import { WORKTREE_METHODS } from './worktree' import { TERMINAL_SEND_METHODS } from './terminal/terminal-send-method' import { MOBILE_WEB_SOURCE_CONTROL_REPOSITORY_METHODS } from './mobile-web-source-control-repository' import { MOBILE_WEB_SOURCE_CONTROL_REVIEW_METADATA_METHODS } from './mobile-web-source-control-review-metadata' import { MOBILE_WEB_SOURCE_CONTROL_REVIEW_LINK_METHODS } from './mobile-web-source-control-review-link' import { MOBILE_WEB_SOURCE_CONTROL_REVIEW_DIFF_METHODS } from './mobile-web-source-control-review-diff' import { MOBILE_WEB_SOURCE_CONTROL_REVIEW_TERMINAL_METHODS } from './mobile-web-source-control-review-terminal-send' -import { mobileWebReviewMetadataRevision } from '../../../../shared/mobile-web/source-control-review-presentation' +import { mobileWebReviewMetadataRevision } from './mobile-web-source-control-review-projection' const worktree = 'id:private-host-workspace' const OID = 'a'.repeat(40) @@ -21,26 +19,23 @@ const METHODS = [ ...MOBILE_WEB_SOURCE_CONTROL_REVIEW_TERMINAL_METHODS ] -function hostMethod(name: string): RpcAnyMethod { - return [...GIT_METHODS, ...WORKTREE_METHODS, ...REPO_METHODS, ...TERMINAL_SEND_METHODS].find( - (method) => method.name === name - )! -} - -function stub(name: string, result: unknown) { - return vi.spyOn(hostMethod(name) as RpcMethod, 'handler').mockResolvedValue(result) +function stubMethod(name: string, result: unknown) { + const method = [...GIT_METHODS, ...TERMINAL_SEND_METHODS].find((entry) => entry.name === name)! + return vi.spyOn(method as RpcMethod, 'handler').mockResolvedValue(result) } async function run( name: string, params: Record = {}, - context?: Partial + runtime: Record = {}, + context: Partial = {} ) { const method = METHODS.find((entry) => entry.name === name)! return method.handler(method.params!.parse({ worktree, ...params }), { signal: new AbortController().signal, + runtime, ...context - } as RpcContext) + } as unknown as RpcContext) } const comment = { @@ -52,14 +47,42 @@ const comment = { side: 'modified' as const } +const workspaceRecord = { + id: 'private-host-workspace', + repoId: 'repo-1', + path: '/private/repo', + setupScript: 'curl evil', + linkedPR: null, + linkedGitLabMR: null +} + afterEach(() => vi.restoreAllMocks()) describe('host repository state', () => { it('composes status, upstream and the workspace base ref into one page-shaped read', async () => { - stub('git.status', { head: OID, branch: 'main', conflictOperation: 'none' }) - stub('git.upstreamStatus', { hasUpstream: true, ahead: 2, behind: 0, upstreamName: 'origin/x' }) - stub('worktree.show', { worktree: { id: 'wt', repoId: 'repo-1', baseRef: 'origin/main' } }) - await expect(run('mobileWeb.sourceControl.repositoryState')).resolves.toEqual({ + const getRuntimeGitStatus = vi + .fn() + .mockResolvedValue({ entries: [], head: OID, branch: 'main', conflictOperation: 'unknown' }) + await expect( + run( + 'mobileWeb.sourceControl.repositoryState', + {}, + { + getRuntimeGitStatus, + getRuntimeGitUpstreamStatus: vi + .fn() + .mockResolvedValue({ + hasUpstream: true, + ahead: 2, + behind: 0, + upstreamName: 'origin/x' + }), + showManagedWorktree: vi + .fn() + .mockResolvedValue({ ...workspaceRecord, baseRef: 'origin/main' }) + } + ) + ).resolves.toEqual({ head: OID, branch: 'main', conflictOperation: 'unknown', @@ -73,68 +96,90 @@ describe('host repository state', () => { behindCommitsArePatchEquivalent: false } }) + expect(getRuntimeGitStatus).toHaveBeenCalledWith(worktree, { admissionTier: 'status' }) }) it('falls back to the project default when the workspace pinned no base ref', async () => { - stub('git.status', { head: null, branch: 'main', conflictOperation: 'rebase' }) - stub('git.upstreamStatus', { hasUpstream: false, ahead: 0, behind: 0 }) - stub('worktree.show', { worktree: { id: 'wt', repoId: 'repo-1', path: '/private/repo' } }) - const baseRefDefault = stub('repo.baseRefDefault', { defaultBaseRef: 'origin/trunk' }) - const result = await run('mobileWeb.sourceControl.repositoryState') - expect(result).toMatchObject({ baseRef: 'origin/trunk', conflictOperation: 'rebase' }) - expect(baseRefDefault.mock.calls[0]![0]).toEqual({ repo: 'id:repo-1' }) + const getRepoBaseRefDefault = vi + .fn() + .mockResolvedValue({ defaultBaseRef: 'origin/trunk', remoteCount: 1 }) + const result = await run( + 'mobileWeb.sourceControl.repositoryState', + {}, + { + getRuntimeGitStatus: vi + .fn() + .mockResolvedValue({ entries: [], branch: 'main', conflictOperation: 'rebase' }), + getRuntimeGitUpstreamStatus: vi + .fn() + .mockResolvedValue({ hasUpstream: false, ahead: 0, behind: 0 }), + showManagedWorktree: vi.fn().mockResolvedValue(workspaceRecord), + getRepoBaseRefDefault + } + ) + expect(result).toMatchObject({ + head: null, + baseRef: 'origin/trunk', + conflictOperation: 'rebase' + }) + expect(getRepoBaseRefDefault).toHaveBeenCalledWith('id:repo-1') expect(JSON.stringify(result)).not.toContain('private') }) }) describe('host review metadata', () => { it('projects only review fields off the workspace record', async () => { - stub('worktree.show', { - worktree: { - id: 'wt', - path: '/private/repo', - setupScript: 'curl evil', - diffComments: [{ ...comment, filePath: 'src/app.ts' }], - mobileDiffReview: { version: 1, files: {} } + const result = (await run( + 'mobileWeb.sourceControl.reviewMetadata', + {}, + { + showManagedWorktree: vi.fn().mockResolvedValue({ + ...workspaceRecord, + diffComments: [ + { + id: 'comment-1', + worktreeId: 'private-host-workspace', + filePath: 'src/app.ts', + lineNumber: 4, + body: 'needs a test', + createdAt: 1, + side: 'modified' + } + ], + mobileDiffReview: { version: 1, files: {} } + }) } - }) - const result = (await run('mobileWeb.sourceControl.reviewMetadata')) as { - comments: unknown[] - revision: string - } + )) as { comments: unknown[]; revision: string } expect(result.comments).toEqual([comment]) - expect(JSON.stringify(result)).not.toMatch(/private|setupScript|workspaceId/) + expect(JSON.stringify(result)).not.toMatch(/private|setupScript|workspaceId|worktreeId/) }) it('refuses a stale write and sends only review fields to the workspace record', async () => { - const record = { - id: 'wt', - path: '/private/repo', - diffComments: [], - mobileDiffReview: { version: 1, files: {} } - } - stub('worktree.show', { worktree: record }) - const set = stub('worktree.set', { worktree: record }) + const showManagedWorktree = vi + .fn() + .mockResolvedValue({ ...workspaceRecord, diffComments: [], mobileDiffReview: undefined }) + const updateManagedWorktreeMeta = vi.fn().mockResolvedValue(undefined) + const runtime = { showManagedWorktree, updateManagedWorktreeMeta } const reviewState = { version: 1 as const, files: [] } const revision = mobileWebReviewMetadataRevision({ comments: [], reviewState: { version: 1, files: [] } }) await expect( - run('mobileWeb.sourceControl.reviewMetadataUpdate', { - expectedRevision: 'b'.repeat(64), - comments: [], - reviewState - }) + run( + 'mobileWeb.sourceControl.reviewMetadataUpdate', + { expectedRevision: 'b'.repeat(64), comments: [], reviewState }, + runtime + ) ).rejects.toThrow('conflict') - expect(set).not.toHaveBeenCalled() - await run('mobileWeb.sourceControl.reviewMetadataUpdate', { - expectedRevision: revision, - comments: [comment], - reviewState - }) - expect(set.mock.calls[0]![0]).toEqual({ - worktree, + expect(updateManagedWorktreeMeta).not.toHaveBeenCalled() + + await run( + 'mobileWeb.sourceControl.reviewMetadataUpdate', + { expectedRevision: revision, comments: [comment], reviewState }, + runtime + ) + expect(updateManagedWorktreeMeta).toHaveBeenCalledWith(worktree, { diffComments: [ { id: 'comment-1', @@ -151,24 +196,31 @@ describe('host review metadata', () => { }) it('rejects a write that names another workspace', async () => { - stub('worktree.show', { worktree: { id: 'wt' } }) await expect( - run('mobileWeb.sourceControl.reviewMetadataUpdate', { - expectedRevision: 'b'.repeat(64), - comments: [], - reviewState: { version: 1, files: [] }, - workspaceId: 'other-workspace' - }) + run( + 'mobileWeb.sourceControl.reviewMetadataUpdate', + { + expectedRevision: 'b'.repeat(64), + comments: [], + reviewState: { version: 1, files: [] }, + workspaceId: 'other-workspace' + }, + { showManagedWorktree: vi.fn().mockResolvedValue(workspaceRecord) } + ) ).rejects.toThrow() }) }) describe('host review link', () => { it('reads the linked review numbers and writes one provider field', async () => { - const record = { id: 'wt', path: '/private/repo', baseRef: 'main', linkedGitLabMR: 7 } - stub('worktree.show', { worktree: record }) - const set = stub('worktree.set', { worktree: record }) - await expect(run('mobileWeb.sourceControl.reviewLink')).resolves.toEqual({ + const updateManagedWorktreeMeta = vi.fn().mockResolvedValue(undefined) + const runtime = { + showManagedWorktree: vi + .fn() + .mockResolvedValue({ ...workspaceRecord, baseRef: 'main', linkedGitLabMR: 7 }), + updateManagedWorktreeMeta + } + await expect(run('mobileWeb.sourceControl.reviewLink', {}, runtime)).resolves.toEqual({ baseRef: 'main', linkedGitHubPR: null, linkedGitLabMR: 7, @@ -176,14 +228,18 @@ describe('host review link', () => { linkedAzureDevOpsPR: null, linkedGiteaPR: null }) - await run('mobileWeb.sourceControl.reviewLinkUpdate', { provider: 'github', number: 12 }) - expect(set.mock.calls[0]![0]).toEqual({ worktree, linkedPR: 12 }) + await run( + 'mobileWeb.sourceControl.reviewLinkUpdate', + { provider: 'github', number: 12 }, + runtime + ) + expect(updateManagedWorktreeMeta).toHaveBeenCalledWith(worktree, { linkedPR: 12 }) }) }) describe('host review diff', () => { it('pages a staged diff and refuses a branch diff without compare identity', async () => { - stub('git.diff', { kind: 'text', originalContent: 'old\n', modifiedContent: 'new\n' }) + stubMethod('git.diff', { kind: 'text', originalContent: 'old\n', modifiedContent: 'new\n' }) await expect( run('mobileWeb.sourceControl.reviewDiff', { relativePath: 'src/app.ts', scope: 'staged' }) ).resolves.toMatchObject({ kind: 'text', scope: 'staged', relativePath: 'src/app.ts' }) @@ -194,17 +250,22 @@ describe('host review diff', () => { }) describe('host review terminal send', () => { - it('resolves the terminal from the requested workspace tab list', async () => { - const send = stub('terminal.send', { send: { accepted: true } }) - const listMobileSessionTabs = vi.fn().mockResolvedValue({ - worktree: 'private-host-workspace', - tabs: [{ id: 'tab-1', type: 'terminal', status: 'ready', terminal: 'terminal-1' }] + const tabs = (worktreeId: string, tabId: string) => ({ + listMobileSessionTabs: vi.fn().mockResolvedValue({ + worktree: worktreeId, + tabs: [{ id: tabId, type: 'terminal', status: 'ready', terminal: 'terminal-1' }] }) + }) + + it('resolves the terminal from the requested workspace tab list', async () => { + const send = stubMethod('terminal.send', { send: { accepted: true } }) await expect( - run('mobileWeb.sourceControl.reviewTerminalSend', { tabId: 'tab-1', text: 'review this' }, { - clientId: 'device-1', - runtime: { listMobileSessionTabs } - } as unknown as RpcContext) + run( + 'mobileWeb.sourceControl.reviewTerminalSend', + { tabId: 'tab-1', text: 'review this' }, + tabs('private-host-workspace', 'tab-1'), + { clientId: 'device-1' } + ) ).resolves.toEqual({ accepted: true }) expect(send.mock.calls[0]![0]).toMatchObject({ terminal: 'terminal-1', @@ -214,32 +275,18 @@ describe('host review terminal send', () => { }) }) - it('refuses a tab that belongs to another workspace', async () => { - const send = stub('terminal.send', { send: { accepted: true } }) - const listMobileSessionTabs = vi.fn().mockResolvedValue({ - worktree: 'other-workspace', - tabs: [{ id: 'tab-1', type: 'terminal', status: 'ready', terminal: 'terminal-1' }] - }) + it.each([ + ['another workspace', 'other-workspace', 'tab-1'], + ['a tab id the workspace does not list', 'private-host-workspace', 'tab-2'] + ])('refuses %s', async (_label, worktreeId, tabId) => { + const send = stubMethod('terminal.send', { send: { accepted: true } }) await expect( - run('mobileWeb.sourceControl.reviewTerminalSend', { tabId: 'tab-1', text: 'review this' }, { - clientId: 'device-1', - runtime: { listMobileSessionTabs } - } as unknown as RpcContext) - ).rejects.toThrow('selector_not_found') - expect(send).not.toHaveBeenCalled() - }) - - it('refuses a tab id the requested workspace does not list', async () => { - const send = stub('terminal.send', { send: { accepted: true } }) - const listMobileSessionTabs = vi.fn().mockResolvedValue({ - worktree: 'private-host-workspace', - tabs: [{ id: 'tab-2', type: 'terminal', status: 'ready', terminal: 'terminal-2' }] - }) - await expect( - run('mobileWeb.sourceControl.reviewTerminalSend', { tabId: 'tab-1', text: 'review this' }, { - clientId: 'device-1', - runtime: { listMobileSessionTabs } - } as unknown as RpcContext) + run( + 'mobileWeb.sourceControl.reviewTerminalSend', + { tabId: 'tab-1', text: 'review this' }, + tabs(worktreeId, tabId), + { clientId: 'device-1' } + ) ).rejects.toThrow('selector_not_found') expect(send).not.toHaveBeenCalled() }) diff --git a/src/mobile-web/src/mobile-web-source-control-host-client.ts b/src/mobile-web/src/mobile-web-source-control-host-client.ts index ad8e5e723e0..c599cbe95c9 100644 --- a/src/mobile-web/src/mobile-web-source-control-host-client.ts +++ b/src/mobile-web/src/mobile-web-source-control-host-client.ts @@ -5,6 +5,11 @@ import type { MobileWebOneShotRequestClient } from './mobile-web-one-shot-reques type PayloadSchema = { safeParse: (value: unknown) => { success: boolean } } +/** A Git write can outlast a read: a push over a slow link, a pull with a large fetch, a commit + * behind a slow hook. This raises the page's own wait; the shell still applies its request + * deadline until the host-request payload carries one. */ +export const MOBILE_WEB_SOURCE_CONTROL_WRITE_TIMEOUT_MS = 60_000 + /** Every Source Control call is one Desktop method addressed by the page's workspace handle. The * payload contract is checked here so a malformed request never reaches the bridge. */ export class MobileWebSourceControlHostClient { @@ -22,4 +27,17 @@ export class MobileWebSourceControlHostClient { } return requestMobileWebHost(this.requests, method, payload.workspaceId, params, options) } + + protected hostWrite( + schema: PayloadSchema, + payload: { workspaceId: string }, + method: string, + params: Record, + options?: MobileWebBridgeRequestOptions + ): Promise { + return this.host(schema, payload, method, params, { + ...options, + timeoutMs: options?.timeoutMs ?? MOBILE_WEB_SOURCE_CONTROL_WRITE_TIMEOUT_MS + }) + } } diff --git a/src/mobile-web/src/mobile-web-source-control-request-client.test.ts b/src/mobile-web/src/mobile-web-source-control-request-client.test.ts index cca833cd2df..260ec6ee394 100644 --- a/src/mobile-web/src/mobile-web-source-control-request-client.test.ts +++ b/src/mobile-web/src/mobile-web-source-control-request-client.test.ts @@ -143,6 +143,7 @@ describe('page Source Control writes over the host lane', () => { expect(f.request.mock.calls[0]!.slice(0, 3)).toEqual( hostRequest('git.commit', { message: 'feat: mobile' }) ) + expect(f.request.mock.calls[0]!.at(-1)).toMatchObject({ timeoutMs: 60_000 }) }) it('refuses a blank commit message before it reaches the bridge', async () => { diff --git a/src/mobile-web/src/mobile-web-source-control-request-client.ts b/src/mobile-web/src/mobile-web-source-control-request-client.ts index becfab10618..b9b55a99161 100644 --- a/src/mobile-web/src/mobile-web-source-control-request-client.ts +++ b/src/mobile-web/src/mobile-web-source-control-request-client.ts @@ -145,7 +145,7 @@ export class MobileWebSourceControlRequestClient extends MobileWebSourceControlR payload: MobileWebSourceControlCommitPayload, options?: MobileWebBridgeRequestOptions ): Promise { - return this.host( + return this.hostWrite( MobileWebSourceControlCommitPayloadSchema, payload, 'git.commit', diff --git a/src/mobile-web/src/mobile-web-source-control-sync-request-client.test.ts b/src/mobile-web/src/mobile-web-source-control-sync-request-client.test.ts index 17a7a494f56..6a2a9dc8f2f 100644 --- a/src/mobile-web/src/mobile-web-source-control-sync-request-client.test.ts +++ b/src/mobile-web/src/mobile-web-source-control-sync-request-client.test.ts @@ -87,6 +87,8 @@ describe('page repository writes', () => { for (const [index, [run, method, params]] of cases.entries()) { await expect(run()).resolves.toBeUndefined() expect(f.request.mock.calls[index]!.slice(0, 3), method).toEqual(hostRequest(method, params)) + // A Git write waits past the read timeout the page applies to itself. + expect(f.request.mock.calls[index]!.at(-1), method).toMatchObject({ timeoutMs: 60_000 }) } }) diff --git a/src/mobile-web/src/mobile-web-source-control-sync-request-client.ts b/src/mobile-web/src/mobile-web-source-control-sync-request-client.ts index 5b58e77625c..baf45dbe039 100644 --- a/src/mobile-web/src/mobile-web-source-control-sync-request-client.ts +++ b/src/mobile-web/src/mobile-web-source-control-sync-request-client.ts @@ -118,6 +118,6 @@ export class MobileWebSourceControlSyncRequestClient extends MobileWebSourceCont params: Record, options?: MobileWebBridgeRequestOptions ): Promise { - return this.host(schema, payload, method, params, options).then(() => undefined) + return this.hostWrite(schema, payload, method, params, options).then(() => undefined) } } diff --git a/src/shared/mobile-web/source-control-history-item-presentation.ts b/src/shared/mobile-web/source-control-history-item-presentation.ts deleted file mode 100644 index a27f7b86aca..00000000000 --- a/src/shared/mobile-web/source-control-history-item-presentation.ts +++ /dev/null @@ -1,106 +0,0 @@ -import { - MOBILE_WEB_SOURCE_CONTROL_HISTORY_PARENT_LIMIT, - MOBILE_WEB_SOURCE_CONTROL_HISTORY_REFERENCE_LIMIT, - MobileWebGitObjectIdSchema, - MobileWebSourceControlHistoryItemSchema, - MobileWebSourceControlHistoryRefSchema, - type MobileWebSourceControlHistoryItem, - type MobileWebSourceControlHistoryRef -} from './source-control-history-contract' - -export function sanitizeMobileWebHistoryItem( - candidate: unknown -): MobileWebSourceControlHistoryItem | null { - if (!isRecord(candidate)) { - return null - } - const id = sanitizeObjectId(candidate.id) - if (!id) { - return null - } - const parentIds = Array.isArray(candidate.parentIds) - ? candidate.parentIds - .slice(0, MOBILE_WEB_SOURCE_CONTROL_HISTORY_PARENT_LIMIT) - .flatMap((value) => { - const parent = sanitizeObjectId(value) - return parent ? [parent] : [] - }) - : [] - const references = Array.isArray(candidate.references) - ? candidate.references - .slice(0, MOBILE_WEB_SOURCE_CONTROL_HISTORY_REFERENCE_LIMIT) - .flatMap((value) => { - const reference = sanitizeMobileWebHistoryRef(value) - return reference ? [reference] : [] - }) - : [] - const message = boundedString(candidate.message, 8 * 1024) ?? '' - const subject = - boundedString(candidate.subject, 512) ?? - boundedString(message.split(/\r?\n/, 1)[0], 512) ?? - '(no commit message)' - const author = boundedString(candidate.author, 256) - const timestamp = boundedTimestamp(candidate.timestamp) - return MobileWebSourceControlHistoryItemSchema.parse({ - id, - parentIds, - displayId: id.slice(0, 12), - subject, - message, - ...(author === undefined ? {} : { author }), - ...(timestamp === undefined ? {} : { timestamp }), - references - }) -} - -export function sanitizeMobileWebHistoryRef( - candidate: unknown -): MobileWebSourceControlHistoryRef | null { - if (!isRecord(candidate)) { - return null - } - const id = boundedString(candidate.id, 320) - const name = boundedString(candidate.name, 240) - if (!id || !name) { - return null - } - const revision = sanitizeObjectId(candidate.revision) - const category = - candidate.category === 'branches' || - candidate.category === 'remote branches' || - candidate.category === 'tags' || - candidate.category === 'commits' - ? candidate.category - : undefined - const description = boundedString(candidate.description, 512) - const parsed = MobileWebSourceControlHistoryRefSchema.safeParse({ - id, - name, - ...(revision ? { revision } : {}), - ...(category ? { category } : {}), - ...(description === undefined ? {} : { description }) - }) - return parsed.success ? parsed.data : null -} - -function sanitizeObjectId(value: unknown): string | null { - const parsed = MobileWebGitObjectIdSchema.safeParse(value) - return parsed.success ? parsed.data : null -} - -function boundedTimestamp(value: unknown): number | undefined { - return typeof value === 'number' && - Number.isSafeInteger(value) && - value >= -8_640_000_000_000_000 && - value <= 8_640_000_000_000_000 - ? value - : undefined -} - -function boundedString(value: unknown, limit: number): string | undefined { - return typeof value === 'string' && value.length > 0 ? value.slice(0, limit) : undefined -} - -function isRecord(value: unknown): value is Record { - return typeof value === 'object' && value !== null && !Array.isArray(value) -} diff --git a/src/shared/mobile-web/source-control-history-presentation.ts b/src/shared/mobile-web/source-control-history-presentation.ts deleted file mode 100644 index 7bba40d866d..00000000000 --- a/src/shared/mobile-web/source-control-history-presentation.ts +++ /dev/null @@ -1,249 +0,0 @@ -import { - MOBILE_WEB_SOURCE_CONTROL_BRANCH_LIMIT, - MOBILE_WEB_SOURCE_CONTROL_COMPARE_MAX_ENTRIES, - MOBILE_WEB_SOURCE_CONTROL_COMPARE_RESPONSE_MAX_BYTES, - MOBILE_WEB_SOURCE_CONTROL_HISTORY_RESPONSE_MAX_BYTES, - MobileWebGitObjectIdSchema, - MobileWebGitRefNameSchema, - MobileWebSourceControlBranchCompareResultSchema, - MobileWebSourceControlBranchesResultSchema, - MobileWebSourceControlCommitCompareResultSchema, - MobileWebSourceControlCompareEntrySchema, - MobileWebSourceControlHistoryResultSchema, - type MobileWebSourceControlBranchCompareResult, - type MobileWebSourceControlBranchesResult, - type MobileWebSourceControlCommitCompareResult, - type MobileWebSourceControlCompareEntry, - type MobileWebSourceControlHistoryItem, - type MobileWebSourceControlHistoryResult -} from './source-control-history-contract' -import { MobileWebBrokerError } from './bridge-operation-error' -import { - sanitizeMobileWebHistoryItem, - sanitizeMobileWebHistoryRef -} from './source-control-history-item-presentation' - -const RESPONSE_BUDGET_RESERVE_BYTES = 8 * 1024 - -export function projectMobileWebBranches( - result: unknown, - workspaceId: string -): MobileWebSourceControlBranchesResult { - if (!isRecord(result) || !Array.isArray(result.branches)) { - throw new MobileWebBrokerError('host_error') - } - const branches = result.branches - .slice(0, MOBILE_WEB_SOURCE_CONTROL_BRANCH_LIMIT) - .flatMap((candidate) => { - const branch = safeGitRef(candidate) - return branch ? [branch] : [] - }) - return MobileWebSourceControlBranchesResultSchema.parse({ - workspaceId, - current: safeGitRef(result.current), - branches, - totalCount: result.branches.length, - truncated: branches.length < result.branches.length - }) -} - -export function projectMobileWebHistory( - result: unknown, - workspaceId: string, - limit: number -): MobileWebSourceControlHistoryResult { - if (!isRecord(result) || !Array.isArray(result.items)) { - throw new MobileWebBrokerError('host_error') - } - const items: MobileWebSourceControlHistoryItem[] = [] - let retainedBytes = 0 - let droppedByBudget = false - for (const candidate of result.items.slice(0, limit)) { - const item = sanitizeMobileWebHistoryItem(candidate) - if (!item) { - continue - } - const nextBytes = encodedByteLength(item) + 1 - if ( - retainedBytes + nextBytes > - MOBILE_WEB_SOURCE_CONTROL_HISTORY_RESPONSE_MAX_BYTES - RESPONSE_BUDGET_RESERVE_BYTES - ) { - droppedByBudget = true - break - } - retainedBytes += nextBytes - items.push(item) - } - const currentRef = sanitizeMobileWebHistoryRef(result.currentRef) - const remoteRef = sanitizeMobileWebHistoryRef(result.remoteRef) - const baseRef = sanitizeMobileWebHistoryRef(result.baseRef) - const mergeBase = safeObjectId(result.mergeBase) - return MobileWebSourceControlHistoryResultSchema.parse({ - workspaceId, - items, - ...(currentRef ? { currentRef } : {}), - ...(remoteRef ? { remoteRef } : {}), - ...(baseRef ? { baseRef } : {}), - ...(mergeBase ? { mergeBase } : {}), - hasIncomingChanges: result.hasIncomingChanges === true, - hasOutgoingChanges: result.hasOutgoingChanges === true, - hasMore: - result.hasMore === true || - droppedByBudget || - result.items.length > limit || - items.length < Math.min(result.items.length, limit), - limit - }) -} - -export function projectMobileWebBranchCompare( - result: unknown, - workspaceId: string, - baseRef: string -): MobileWebSourceControlBranchCompareResult { - const summary = compareSummary(result) - const page = compareEntryPage(result) - const changedFiles = Math.max(page.reportedCount, safeNonnegativeInteger(summary.changedFiles)) - const commitsAhead = optionalNonnegativeInteger(summary.commitsAhead) - return MobileWebSourceControlBranchCompareResultSchema.parse({ - workspaceId, - baseRef, - compareRef: boundedString(summary.compareRef, 240) ?? 'HEAD', - baseOid: safeObjectId(summary.baseOid), - headOid: safeObjectId(summary.headOid), - mergeBase: safeObjectId(summary.mergeBase), - changedFiles, - ...(commitsAhead === undefined ? {} : { commitsAhead }), - status: branchCompareStatus(summary.status), - entries: page.entries, - truncated: page.truncated || changedFiles > page.entries.length - }) -} - -export function projectMobileWebCommitCompare( - result: unknown, - workspaceId: string, - commitId: string -): MobileWebSourceControlCommitCompareResult { - const summary = compareSummary(result) - const page = compareEntryPage(result) - const changedFiles = Math.max(page.reportedCount, safeNonnegativeInteger(summary.changedFiles)) - return MobileWebSourceControlCommitCompareResultSchema.parse({ - workspaceId, - commitId, - commitOid: safeObjectId(summary.commitOid), - parentOid: safeObjectId(summary.parentOid), - compareRef: boundedString(summary.compareRef, 240) ?? commitId.slice(0, 12), - baseRef: boundedString(summary.baseRef, 240) ?? 'parent', - changedFiles, - status: commitCompareStatus(summary.status), - entries: page.entries, - truncated: page.truncated || changedFiles > page.entries.length - }) -} - -/** One response carries the whole compare, so the entry list is bounded by both the entry cap and - * the bridge response budget rather than by a resumable offset the page would have to drive. */ -function compareEntryPage(result: unknown): { - entries: MobileWebSourceControlCompareEntry[] - reportedCount: number - truncated: boolean -} { - if (!isRecord(result) || !Array.isArray(result.entries)) { - throw new MobileWebBrokerError('host_error') - } - const entries: MobileWebSourceControlCompareEntry[] = [] - let retainedBytes = 0 - let droppedByBudget = false - for (const candidate of result.entries.slice(0, MOBILE_WEB_SOURCE_CONTROL_COMPARE_MAX_ENTRIES)) { - const entry = compareEntry(candidate) - if (!entry) { - continue - } - const nextBytes = encodedByteLength(entry) + 1 - if ( - retainedBytes + nextBytes > - MOBILE_WEB_SOURCE_CONTROL_COMPARE_RESPONSE_MAX_BYTES - RESPONSE_BUDGET_RESERVE_BYTES - ) { - droppedByBudget = true - break - } - retainedBytes += nextBytes - entries.push(entry) - } - return { - entries, - reportedCount: result.entries.length, - truncated: droppedByBudget || entries.length < result.entries.length - } -} - -function compareEntry(candidate: unknown): MobileWebSourceControlCompareEntry | null { - if (!isRecord(candidate)) { - return null - } - const parsed = MobileWebSourceControlCompareEntrySchema.safeParse({ - relativePath: candidate.path, - ...(candidate.oldPath === undefined ? {} : { oldRelativePath: candidate.oldPath }), - status: candidate.status, - ...(optionalNonnegativeInteger(candidate.added) === undefined - ? {} - : { added: candidate.added }), - ...(optionalNonnegativeInteger(candidate.removed) === undefined - ? {} - : { removed: candidate.removed }) - }) - return parsed.success ? parsed.data : null -} - -function compareSummary(result: unknown): Record { - if (!isRecord(result) || !isRecord(result.summary)) { - throw new MobileWebBrokerError('host_error') - } - return result.summary -} - -function branchCompareStatus( - value: unknown -): 'ready' | 'invalid-base' | 'unborn-head' | 'no-merge-base' | 'error' { - return value === 'ready' || - value === 'invalid-base' || - value === 'unborn-head' || - value === 'no-merge-base' - ? value - : 'error' -} - -function commitCompareStatus(value: unknown): 'ready' | 'invalid-commit' | 'error' { - return value === 'ready' || value === 'invalid-commit' ? value : 'error' -} - -function safeGitRef(value: unknown): string | null { - const parsed = MobileWebGitRefNameSchema.safeParse(value) - return parsed.success ? parsed.data : null -} - -function safeObjectId(value: unknown): string | null { - const parsed = MobileWebGitObjectIdSchema.safeParse(value) - return parsed.success ? parsed.data : null -} - -function safeNonnegativeInteger(value: unknown): number { - return optionalNonnegativeInteger(value) ?? 0 -} - -function optionalNonnegativeInteger(value: unknown): number | undefined { - return typeof value === 'number' && Number.isSafeInteger(value) && value >= 0 ? value : undefined -} - -function boundedString(value: unknown, limit: number): string | undefined { - return typeof value === 'string' && value.length > 0 ? value.slice(0, limit) : undefined -} - -function encodedByteLength(value: unknown): number { - return new TextEncoder().encode(JSON.stringify(value)).byteLength -} - -function isRecord(value: unknown): value is Record { - return typeof value === 'object' && value !== null && !Array.isArray(value) -} diff --git a/src/shared/mobile-web/source-control-repository-presentation.ts b/src/shared/mobile-web/source-control-repository-presentation.ts deleted file mode 100644 index e099dc43d5c..00000000000 --- a/src/shared/mobile-web/source-control-repository-presentation.ts +++ /dev/null @@ -1,71 +0,0 @@ -import { MobileWebGitRefNameSchema } from './source-control-history-contract' -import { - MobileWebSourceControlRepositoryStateSchema, - MobileWebSourceControlUpstreamSnapshotSchema, - type MobileWebSourceControlRepositoryState, - type MobileWebSourceControlUpstreamSnapshot -} from './source-control-sync-contract' -import { MobileWebBrokerError } from './bridge-operation-error' - -export function projectMobileWebRepositoryState(args: { - status: unknown - upstream: unknown - baseRef: unknown - workspaceId: string -}): MobileWebSourceControlRepositoryState { - if (!isRecord(args.status)) { - throw new MobileWebBrokerError('host_error') - } - return MobileWebSourceControlRepositoryStateSchema.parse({ - workspaceId: args.workspaceId, - head: safeHead(args.status.head), - branch: safeBranch(args.status.branch), - conflictOperation: safeConflictOperation(args.status.conflictOperation), - baseRef: safeBranch(args.baseRef), - upstream: projectMobileWebUpstreamSnapshot(args.upstream) - }) -} - -export function projectMobileWebUpstreamSnapshot( - value: unknown -): MobileWebSourceControlUpstreamSnapshot { - if (!isRecord(value)) { - throw new MobileWebBrokerError('host_error') - } - const upstreamName = boundedString(value.upstreamName, 240) - return MobileWebSourceControlUpstreamSnapshotSchema.parse({ - hasUpstream: value.hasUpstream === true, - ...(upstreamName ? { upstreamName } : {}), - ahead: safeNonnegativeInteger(value.ahead), - behind: safeNonnegativeInteger(value.behind), - hasConfiguredPushTarget: value.hasConfiguredPushTarget === true, - behindCommitsArePatchEquivalent: value.behindCommitsArePatchEquivalent === true - }) -} - -function safeHead(value: unknown): string | null { - return typeof value === 'string' && /^(?:[0-9a-f]{40}|[0-9a-f]{64})$/i.test(value) ? value : null -} - -function safeBranch(value: unknown): string | null { - const parsed = MobileWebGitRefNameSchema.safeParse(value) - return parsed.success ? parsed.data : null -} - -function safeConflictOperation(value: unknown): 'merge' | 'rebase' | 'cherry-pick' | 'unknown' { - return value === 'merge' || value === 'rebase' || value === 'cherry-pick' ? value : 'unknown' -} - -function safeNonnegativeInteger(value: unknown): number { - return typeof value === 'number' && Number.isSafeInteger(value) && value >= 0 ? value : 0 -} - -function boundedString(value: unknown, limit: number): string | undefined { - return typeof value === 'string' && value.trim().length > 0 - ? value.trim().slice(0, limit) - : undefined -} - -function isRecord(value: unknown): value is Record { - return typeof value === 'object' && value !== null && !Array.isArray(value) -} diff --git a/src/shared/mobile-web/source-control-review-presentation.ts b/src/shared/mobile-web/source-control-review-presentation.ts deleted file mode 100644 index 71c4fd89f06..00000000000 --- a/src/shared/mobile-web/source-control-review-presentation.ts +++ /dev/null @@ -1,212 +0,0 @@ -import { sha256 } from '../sha256' -import { - MOBILE_WEB_REVIEW_COMMENT_LIMIT, - MOBILE_WEB_REVIEW_FILE_STATE_LIMIT, - MobileWebSourceControlReviewCommentSchema, - MobileWebSourceControlReviewFileStateSchema, - MobileWebSourceControlReviewLinkResultSchema, - MobileWebSourceControlReviewMetadataResultSchema, - type MobileWebSourceControlReviewComment, - type MobileWebSourceControlReviewLinkResult, - type MobileWebSourceControlReviewMetadataResult, - type MobileWebSourceControlReviewState -} from './source-control-review-contract' -import { MobileWebBrokerError } from './bridge-operation-error' - -export function projectMobileWebReviewMetadata( - worktree: unknown, - workspaceId: string -): MobileWebSourceControlReviewMetadataResult { - if (!isRecord(worktree)) { - throw new MobileWebBrokerError('host_error') - } - const rawComments = Array.isArray(worktree.diffComments) ? worktree.diffComments : [] - const rawReview = isRecord(worktree.mobileDiffReview) ? worktree.mobileDiffReview : {} - const rawFiles = isRecord(rawReview.files) ? Object.values(rawReview.files) : [] - if ( - rawComments.length > MOBILE_WEB_REVIEW_COMMENT_LIMIT || - rawFiles.length > MOBILE_WEB_REVIEW_FILE_STATE_LIMIT - ) { - throw new MobileWebBrokerError('too_large') - } - const comments = rawComments.map(projectComment) - const reviewState: MobileWebSourceControlReviewState = { - version: 1, - ...(safeTimestamp(rawReview.updatedAt) === undefined - ? {} - : { updatedAt: safeTimestamp(rawReview.updatedAt) }), - ...(safeTimestamp(rawReview.completedAt) === undefined - ? {} - : { completedAt: safeTimestamp(rawReview.completedAt) }), - files: rawFiles.map(projectFileState) - } - return MobileWebSourceControlReviewMetadataResultSchema.parse({ - workspaceId, - revision: mobileWebReviewMetadataRevision({ comments, reviewState }), - comments, - reviewState - }) -} - -export function mobileWebReviewMetadataRevision(value: unknown): string { - return Array.from(sha256(new TextEncoder().encode(JSON.stringify(value))), (byte) => - byte.toString(16).padStart(2, '0') - ).join('') -} - -/** The only worktree fields a review write may touch. Everything else on the record stays out of - * reach of the page. */ -export function mobileWebReviewMetadataWorktreeFields(args: { - worktreeId: string - comments: readonly MobileWebSourceControlReviewComment[] - reviewState: MobileWebSourceControlReviewState -}) { - return { - diffComments: args.comments.map((comment) => ({ - id: comment.id, - worktreeId: args.worktreeId, - filePath: comment.relativePath, - ...(comment.oldRelativePath ? { oldPath: comment.oldRelativePath } : {}), - ...(comment.source ? { source: comment.source } : {}), - ...(comment.selectedText === undefined ? {} : { selectedText: comment.selectedText }), - ...(comment.startLine === undefined ? {} : { startLine: comment.startLine }), - lineNumber: comment.lineNumber, - body: comment.body, - createdAt: comment.createdAt, - ...(comment.updatedAt === undefined ? {} : { updatedAt: comment.updatedAt }), - ...(comment.sentAt === undefined ? {} : { sentAt: comment.sentAt }), - ...(comment.scope ? { scope: comment.scope } : {}), - ...(comment.diffIdentity ? { diffIdentity: comment.diffIdentity } : {}), - side: 'modified' - })), - mobileDiffReview: { - version: 1, - ...(args.reviewState.updatedAt === undefined - ? {} - : { updatedAt: args.reviewState.updatedAt }), - ...(args.reviewState.completedAt === undefined - ? {} - : { completedAt: args.reviewState.completedAt }), - files: Object.fromEntries( - args.reviewState.files.map((file) => [ - file.key, - { - key: file.key, - filePath: file.relativePath, - ...(file.oldRelativePath ? { oldPath: file.oldRelativePath } : {}), - scope: file.scope, - ...(file.lastOpenedAt === undefined ? {} : { lastOpenedAt: file.lastOpenedAt }), - ...(file.lastSeenDiffIdentity - ? { lastSeenDiffIdentity: file.lastSeenDiffIdentity } - : {}), - ...(file.reviewedAt === undefined ? {} : { reviewedAt: file.reviewedAt }), - ...(file.reviewDiffIdentity ? { reviewDiffIdentity: file.reviewDiffIdentity } : {}) - } - ]) - ) - } - } -} - -export function projectMobileWebReviewLink( - worktree: unknown, - workspaceId: string -): MobileWebSourceControlReviewLinkResult { - if (!isRecord(worktree)) { - throw new MobileWebBrokerError('host_error') - } - return MobileWebSourceControlReviewLinkResultSchema.parse({ - workspaceId, - baseRef: boundedText(worktree.baseRef, 512), - linkedGitHubPR: positiveInteger(worktree.linkedPR), - linkedGitLabMR: positiveInteger(worktree.linkedGitLabMR), - linkedBitbucketPR: positiveInteger(worktree.linkedBitbucketPR), - linkedAzureDevOpsPR: positiveInteger(worktree.linkedAzureDevOpsPR), - linkedGiteaPR: positiveInteger(worktree.linkedGiteaPR) - }) -} - -export function mobileWebReviewLinkWorktreeField( - provider: string, - number: number | null -): Record { - if (provider === 'github') { - return { linkedPR: number } - } - if (provider === 'gitlab') { - return { linkedGitLabMR: number } - } - if (provider === 'bitbucket') { - return { linkedBitbucketPR: number } - } - if (provider === 'azure-devops') { - return { linkedAzureDevOpsPR: number } - } - return { linkedGiteaPR: number } -} - -function projectComment(value: unknown): MobileWebSourceControlReviewComment { - if (!isRecord(value)) { - throw new MobileWebBrokerError('host_error') - } - const parsed = MobileWebSourceControlReviewCommentSchema.safeParse({ - id: value.id, - relativePath: value.filePath, - ...(value.oldPath === undefined ? {} : { oldRelativePath: value.oldPath }), - ...(value.source === undefined ? {} : { source: value.source }), - ...(value.selectedText === undefined ? {} : { selectedText: value.selectedText }), - ...(value.startLine === undefined ? {} : { startLine: value.startLine }), - lineNumber: value.lineNumber, - body: value.body, - createdAt: value.createdAt, - ...(value.updatedAt === undefined ? {} : { updatedAt: value.updatedAt }), - ...(value.sentAt === undefined ? {} : { sentAt: value.sentAt }), - ...(value.scope === undefined ? {} : { scope: value.scope }), - ...(value.diffIdentity === undefined ? {} : { diffIdentity: value.diffIdentity }), - side: 'modified' - }) - if (!parsed.success) { - throw new MobileWebBrokerError('host_error') - } - return parsed.data -} - -function projectFileState(value: unknown) { - if (!isRecord(value)) { - throw new MobileWebBrokerError('host_error') - } - const parsed = MobileWebSourceControlReviewFileStateSchema.safeParse({ - key: value.key, - relativePath: value.filePath, - ...(value.oldPath === undefined ? {} : { oldRelativePath: value.oldPath }), - scope: value.scope, - ...(value.lastOpenedAt === undefined ? {} : { lastOpenedAt: value.lastOpenedAt }), - ...(value.lastSeenDiffIdentity === undefined - ? {} - : { lastSeenDiffIdentity: value.lastSeenDiffIdentity }), - ...(value.reviewedAt === undefined ? {} : { reviewedAt: value.reviewedAt }), - ...(value.reviewDiffIdentity === undefined - ? {} - : { reviewDiffIdentity: value.reviewDiffIdentity }) - }) - if (!parsed.success) { - throw new MobileWebBrokerError('host_error') - } - return parsed.data -} - -function safeTimestamp(value: unknown): number | undefined { - return typeof value === 'number' && Number.isSafeInteger(value) && value >= 0 ? value : undefined -} - -function positiveInteger(value: unknown): number | null { - return typeof value === 'number' && Number.isSafeInteger(value) && value > 0 ? value : null -} - -function boundedText(value: unknown, limit: number): string | null { - return typeof value === 'string' && value.length > 0 ? value.slice(0, limit) : null -} - -function isRecord(value: unknown): value is Record { - return typeof value === 'object' && value !== null && !Array.isArray(value) -}