From 339a0bd9da6502d3368657e34db802e0340524ab Mon Sep 17 00:00:00 2001 From: DeliciousBuding Date: Fri, 21 Aug 2026 04:22:33 +0800 Subject: [PATCH 1/4] =?UTF-8?q?fix(shared):=20=E6=89=93=E9=80=9A=20diff=20?= =?UTF-8?q?apply=20=E7=9A=84=20PreviewPort=20=E5=B9=B6=E4=BF=AE=E5=A4=8D?= =?UTF-8?q?=E9=A2=84=E8=A7=88=E5=8D=A0=E4=BD=8D=20(#1817)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Cursor --- app/desktop/src/__tests__/edgeClient.test.ts | 90 +++++++ app/desktop/src/api/edgeClient.ts | 61 +++++ app/desktop/src/api/schemas.ts | 15 ++ app/desktop/src/platform/desktopPlatform.ts | 28 +- .../src/platform/desktopPreview.test.ts | 29 +++ app/desktop/src/platform/desktopPreview.ts | 20 ++ app/shared/src/platform/index.ts | 3 + app/shared/src/platform/types.ts | 47 ++++ app/shared/src/workbench/RightInspector.tsx | 2 + .../src/workbench/RightInspectorModePanel.tsx | 5 + .../workbench/__tests__/inspector.test.tsx | 4 +- .../inspector/FilePreviewHelpers.test.ts | 40 ++- .../workbench/inspector/FilePreviewHelpers.ts | 26 ++ .../FilePreviewRouter.apply.test.tsx | 238 +++++++++++++++++ .../workbench/inspector/FilePreviewRouter.tsx | 246 +++++++++++++----- .../inspector/InspectorModeBodies.tsx | 5 + .../inspector/RuntimeEvidenceParts.tsx | 4 +- .../src/workbench/rightInspectorTypes.ts | 7 + .../workbench/workbenchFramePartsHelpers.ts | 1 + app/web/src/platform/webPlatform.ts | 7 +- app/web/src/platform/webPreview.ts | 16 ++ 21 files changed, 818 insertions(+), 76 deletions(-) create mode 100644 app/desktop/src/platform/desktopPreview.test.ts create mode 100644 app/shared/src/workbench/inspector/FilePreviewRouter.apply.test.tsx diff --git a/app/desktop/src/__tests__/edgeClient.test.ts b/app/desktop/src/__tests__/edgeClient.test.ts index b0dbe5964..052358fd9 100644 --- a/app/desktop/src/__tests__/edgeClient.test.ts +++ b/app/desktop/src/__tests__/edgeClient.test.ts @@ -15,6 +15,8 @@ import { fetchRunDiff, fetchArtifacts, fetchPreviews, + applyRunDiff, + applyAllRunDiffs, } from '../api/edgeClient'; import { createDesktopPlatform } from '../platform/desktopPlatform'; import { mapEdgeAgentsToWorkbenchAgents } from '../platform/edgeCapabilityMapper'; @@ -709,4 +711,92 @@ describe('edgeClient', () => { ); }); }); + + describe('diff apply write-back (#1817)', () => { + it('posts a single hunk decision to Edge /v1/runs/{runId}/apply with snake_case fields', async () => { + const fetchSpy = vi.spyOn(globalThis, 'fetch').mockResolvedValueOnce({ + ok: true, + json: () => Promise.resolve({ + code: 'OK', + data: { + runId: 'run_abc123', + filePath: 'src/app.ts', + hunkIndex: 2, + accepted: true, + applied: true, + }, + }), + } as Response); + + const result = await applyRunDiff('run_abc123', { + filePath: 'src/app.ts', + hunkIndex: 2, + accepted: true, + workDir: '/work/project', + }); + + expect(result.applied).toBe(true); + expect(fetchSpy).toHaveBeenCalledWith( + expect.stringMatching(/\/v1\/runs\/run_abc123\/apply$/), + expect.objectContaining({ + method: 'POST', + body: JSON.stringify({ + file_path: 'src/app.ts', + hunk_index: 2, + accepted: true, + workDir: '/work/project', + }), + }), + ); + }); + + it('posts batch decisions to Edge /v1/runs/{runId}/apply-all', async () => { + const fetchSpy = vi.spyOn(globalThis, 'fetch').mockResolvedValueOnce({ + ok: true, + json: () => Promise.resolve({ + code: 'OK', + data: { runId: 'run_abc123', applied: 2 }, + }), + } as Response); + + const result = await applyAllRunDiffs('run_abc123', { + decisions: [ + { filePath: 'src/app.ts', hunkIndex: 0, accepted: true }, + { filePath: 'src/app.ts', hunkIndex: 1, accepted: false }, + ], + workDir: '/work/project', + }); + + expect(result.applied).toBe(2); + expect(fetchSpy).toHaveBeenCalledWith( + expect.stringMatching(/\/v1\/runs\/run_abc123\/apply-all$/), + expect.objectContaining({ + method: 'POST', + body: JSON.stringify({ + decisions: [ + { file_path: 'src/app.ts', hunk_index: 0, accepted: true }, + { file_path: 'src/app.ts', hunk_index: 1, accepted: false }, + ], + workDir: '/work/project', + }), + }), + ); + }); + + it('surfaces Edge apply failures as errors for the UI toast path', async () => { + vi.spyOn(globalThis, 'fetch').mockResolvedValueOnce({ + ok: false, + status: 403, + statusText: 'Forbidden', + json: () => Promise.resolve({ error: { code: 'workspace_not_allowed', message: 'workdir not allowed' } }), + } as Response); + + await expect(applyRunDiff('run_abc123', { + filePath: 'src/app.ts', + hunkIndex: 0, + accepted: true, + workDir: '/forbidden', + })).rejects.toThrow('workdir not allowed'); + }); + }); }); diff --git a/app/desktop/src/api/edgeClient.ts b/app/desktop/src/api/edgeClient.ts index ec1b31cde..99ae0e9f6 100644 --- a/app/desktop/src/api/edgeClient.ts +++ b/app/desktop/src/api/edgeClient.ts @@ -27,6 +27,8 @@ import { AgentInfoSchema, RunInfoSchema, RunDiffSchema, + ApplyRunDiffResponseSchema, + ApplyAllRunDiffsResponseSchema, ArtifactSchema, PreviewSchema, ThreadInfoSchema, @@ -307,6 +309,65 @@ export async function fetchRunDiff(runId: string): Promise { return safeParse(RunDiffSchema, unwrapEdgeResponse(await res.json()), 'runDiff'); } +export interface ApplyRunDiffRequest { + filePath: string; + hunkIndex: number; + accepted: boolean; + workDir: string; +} + +export interface ApplyRunDiffResponse { + runId: string; + filePath: string; + hunkIndex: number; + accepted: boolean; + applied: boolean; +} + +export interface ApplyAllRunDiffsRequest { + decisions: Array>; + workDir: string; +} + +export interface ApplyAllRunDiffsResponse { + runId: string; + applied: number; +} + +/** Write one hunk accept/reject decision back into the run workdir. */ +export async function applyRunDiff(runId: string, request: ApplyRunDiffRequest): Promise { + const res = await edgeFetch(`${BASE}/v1/runs/${encodeURIComponent(runId)}/apply`, { + method: 'POST', + ...edgeDevRequestInit({}, { 'Content-Type': 'application/json' }), + body: JSON.stringify({ + file_path: request.filePath, + hunk_index: request.hunkIndex, + accepted: request.accepted, + workDir: request.workDir, + }), + }); + if (!res.ok) throw await parseError(res); + return safeParse(ApplyRunDiffResponseSchema, unwrapEdgeResponse(await res.json()), 'applyRunDiff'); +} + +/** Batch variant of applyRunDiff for accept-all / reject-all decisions. */ +export async function applyAllRunDiffs(runId: string, request: ApplyAllRunDiffsRequest): Promise { + const res = await edgeFetch(`${BASE}/v1/runs/${encodeURIComponent(runId)}/apply-all`, { + method: 'POST', + ...edgeDevRequestInit({}, { 'Content-Type': 'application/json' }), + body: JSON.stringify({ + decisions: request.decisions.map((decision) => ({ + file_path: decision.filePath, + hunk_index: decision.hunkIndex, + accepted: decision.accepted, + })), + workDir: request.workDir, + }), + }); + if (!res.ok) throw await parseError(res); + return safeParse(ApplyAllRunDiffsResponseSchema, unwrapEdgeResponse(await res.json()), 'applyAllRunDiffs'); +} + export async function fetchArtifacts(): Promise> { const res = await edgeFetch(`${BASE}/v1/artifacts`, edgeDevRequestInit()); if (!res.ok) throw await parseError(res); diff --git a/app/desktop/src/api/schemas.ts b/app/desktop/src/api/schemas.ts index c476aefdc..fa479d6eb 100644 --- a/app/desktop/src/api/schemas.ts +++ b/app/desktop/src/api/schemas.ts @@ -173,6 +173,21 @@ export const RunDiffSchema = z.object({ files: z.array(RunDiffFileSchema), }); +// POST /v1/runs/{runId}/apply — single hunk decision write-back. +export const ApplyRunDiffResponseSchema = z.object({ + runId: z.string(), + filePath: z.string(), + hunkIndex: z.number(), + accepted: z.boolean(), + applied: z.boolean(), +}); + +// POST /v1/runs/{runId}/apply-all — batch hunk decision write-back. +export const ApplyAllRunDiffsResponseSchema = z.object({ + runId: z.string(), + applied: z.number(), +}); + export const ArtifactSchema = z.object({ id: z.string(), runId: z.string(), diff --git a/app/desktop/src/platform/desktopPlatform.ts b/app/desktop/src/platform/desktopPlatform.ts index 4919a59f4..d629102e7 100644 --- a/app/desktop/src/platform/desktopPlatform.ts +++ b/app/desktop/src/platform/desktopPlatform.ts @@ -20,12 +20,17 @@ import type { EvidenceRef } from '@shared/transcript'; import type { TranscriptBlock } from '@shared/transcript'; import type { RunInfo, StartRunRequest } from '@shared/types'; import { createHubClient } from '@/api/hubClient'; +import { applyAllRunDiffs, applyRunDiff } from '@/api/edgeClient'; import { edgeAuthHeaders } from '@/api/edgeAuth'; import { getAccessToken } from '@/hooks/useAuth'; import { getEdgeBaseUrl } from '@/config'; import { fetchRuntimeSessions } from '@shared/workbench'; import { pickDesktopComposerAttachments } from './desktopAttachments'; -import { canOpenDesktopEvidencePreview, openDesktopEvidencePreview } from './desktopPreview'; +import { + canOpenDesktopEvidencePreview, + openDesktopEvidencePreview, + resolveDesktopEvidenceContentUrl, +} from './desktopPreview'; import { resolveDesktopTargetPreference, type DesktopTargetPreference } from './targetPreference'; import { createDesktopSettingsAdapter } from './desktopSettingsAdapter'; @@ -176,6 +181,27 @@ export function createDesktopPlatform(options: DesktopPlatformOptions = {}): Des preview: { canOpenEvidence: canOpenDesktopEvidencePreview, openEvidence: options.openPreview ?? openDesktopEvidencePreview, + // Interactive diff write-back goes through the Local Edge apply endpoints + // (#1817). Desktop owns the Edge connection, so it owns this port leg. + async applyRunDiff(input) { + await applyRunDiff(input.runId, { + filePath: input.decision.filePath, + hunkIndex: input.decision.hunkIndex, + accepted: input.decision.accepted, + workDir: input.workDir, + }); + }, + async applyAllRunDiffs(input) { + await applyAllRunDiffs(input.runId, { + decisions: input.decisions.map((decision) => ({ + filePath: decision.filePath, + hunkIndex: decision.hunkIndex, + accepted: decision.accepted, + })), + workDir: input.workDir, + }); + }, + resolveContentUrl: resolveDesktopEvidenceContentUrl, }, settings: createDesktopSettingsAdapter(), // exactOptionalPropertyTypes: omit key when undefined rather than assign undefined. diff --git a/app/desktop/src/platform/desktopPreview.test.ts b/app/desktop/src/platform/desktopPreview.test.ts new file mode 100644 index 000000000..77a597d54 --- /dev/null +++ b/app/desktop/src/platform/desktopPreview.test.ts @@ -0,0 +1,29 @@ +// Desktop PreviewPort content-URL resolution (#1817): absolute evidence URLs +// pass through; host-relative API paths resolve against the Local Edge base +// URL because Desktop owns the Edge connection. +import { describe, expect, it } from 'vitest'; +import { resolveDesktopEvidenceContentUrl } from './desktopPreview'; + +describe('resolveDesktopEvidenceContentUrl', () => { + it('returns absolute evidence URLs unchanged', () => { + expect(resolveDesktopEvidenceContentUrl('http://127.0.0.1:4173/preview')) + .toBe('http://127.0.0.1:4173/preview'); + expect(resolveDesktopEvidenceContentUrl('https://preview.example.com/app')) + .toBe('https://preview.example.com/app'); + }); + + it('resolves Edge-relative content paths against the Local Edge base URL', () => { + // Default test-env Edge base URL (no override configured). + expect(resolveDesktopEvidenceContentUrl('/v1/runs/run-1/artifacts/artifact-1/content')) + .toBe('http://127.0.0.1:3210/v1/runs/run-1/artifacts/artifact-1/content'); + expect(resolveDesktopEvidenceContentUrl('/v1/runs/run-1/previews/preview-1/content')) + .toBe('http://127.0.0.1:3210/v1/runs/run-1/previews/preview-1/content'); + }); + + it('yields undefined for empty or non-URL references', () => { + expect(resolveDesktopEvidenceContentUrl('')).toBeUndefined(); + expect(resolveDesktopEvidenceContentUrl(' ')).toBeUndefined(); + expect(resolveDesktopEvidenceContentUrl('# reports/runtime.patch')).toBeUndefined(); + expect(resolveDesktopEvidenceContentUrl('data:text/plain;base64,abc')).toBeUndefined(); + }); +}); diff --git a/app/desktop/src/platform/desktopPreview.ts b/app/desktop/src/platform/desktopPreview.ts index 3de080fac..5be1234bb 100644 --- a/app/desktop/src/platform/desktopPreview.ts +++ b/app/desktop/src/platform/desktopPreview.ts @@ -1,6 +1,7 @@ import { open } from '@tauri-apps/plugin-shell'; import { resolveEvidencePreviewTarget } from '@shared/platform'; import type { EvidenceRef } from '@shared/transcript'; +import { getEdgeBaseUrl } from '@/config'; export function canOpenDesktopEvidencePreview(evidence: EvidenceRef): boolean { return Boolean(resolveEvidencePreviewTarget(evidence)); @@ -14,3 +15,22 @@ export async function openDesktopEvidencePreview(evidence: EvidenceRef): Promise await open(target); } + +/** + * PreviewPort.resolveContentUrl for Desktop (#1817). + * Absolute http(s) URLs (e.g. a runtime preview server) are displayed as-is; + * host-relative API paths (`/v1/runs/…/content`) are owned by the Local Edge, + * so they are resolved against the Edge base URL. Anything else has no + * displayable source and yields undefined (UI shows an honest notice). + */ +export function resolveDesktopEvidenceContentUrl(contentRef: string): string | undefined { + const trimmed = contentRef.trim(); + if (!trimmed) return undefined; + if (/^https?:\/\//i.test(trimmed)) return trimmed; + if (trimmed.startsWith('/')) { + const edgeBase = getEdgeBaseUrl().replace(/\/+$/, ''); + if (!edgeBase) return undefined; + return `${edgeBase}${trimmed}`; + } + return undefined; +} diff --git a/app/shared/src/platform/index.ts b/app/shared/src/platform/index.ts index 60b151612..c98f8917f 100644 --- a/app/shared/src/platform/index.ts +++ b/app/shared/src/platform/index.ts @@ -5,6 +5,8 @@ export { resolveEvidencePreviewTarget } from './previewTargets'; export type { AgentHubPlatform, AgentHubSurface, + ApplyAllRunDiffsInput, + ApplyRunDiffInput, AttachmentPort, ConversationKind, ConversationPort, @@ -14,6 +16,7 @@ export type { LocalCliRuntimeId, PreviewPort, RedispatchTaskResult, + RunDiffHunkDecision, RunPort, RuntimeSessionSummary, SurfaceCapabilities, diff --git a/app/shared/src/platform/types.ts b/app/shared/src/platform/types.ts index e170d1df5..4533065fb 100644 --- a/app/shared/src/platform/types.ts +++ b/app/shared/src/platform/types.ts @@ -107,9 +107,56 @@ export interface AttachmentPort { uploadAttachment(file: File): Promise; } +/** + * A single hunk accept/reject decision from the interactive diff reviewer. + * Mirrors the Edge apply request body field-by-field (file_path / hunk_index / + * accepted) so host adapters can forward it without re-mapping. + */ +export interface RunDiffHunkDecision { + filePath: string; + hunkIndex: number; + accepted: boolean; +} + +/** Input for `PreviewPort.applyRunDiff` — one hunk decision plus run/workdir context. */ +export interface ApplyRunDiffInput { + runId: string; + /** Run working directory the hunk is written back into (Edge validates it). */ + workDir: string; + decision: RunDiffHunkDecision; +} + +/** Input for `PreviewPort.applyAllRunDiffs` — batch decisions plus run/workdir context. */ +export interface ApplyAllRunDiffsInput { + runId: string; + workDir: string; + decisions: RunDiffHunkDecision[]; +} + export interface PreviewPort { canOpenEvidence?(evidence: EvidenceRef): boolean; openEvidence(evidence: EvidenceRef): Promise; + /** + * Write one interactive-diff hunk decision back into the run workdir + * (Edge POST /v1/runs/{runId}/apply). Only surfaces with a Local Edge + * implement this (Desktop); Web omits it and the inspector degrades to an + * explicit read-only review notice instead of silently dropping the click. + */ + applyRunDiff?(input: ApplyRunDiffInput): Promise; + /** + * Batch variant of `applyRunDiff` for accept-all / reject-all + * (Edge POST /v1/runs/{runId}/apply-all). Same surface contract. + */ + applyAllRunDiffs?(input: ApplyAllRunDiffsInput): Promise; + /** + * Resolve an evidence content reference into a displayable URL. + * Absolute http(s) URLs are typically returned unchanged; host-relative + * API paths (e.g. `/v1/runs/…/content`) become absolute host URLs on + * surfaces that own the host (Desktop → Local Edge). Return `undefined` + * when the surface cannot serve the content so the UI can render an + * honest capability notice instead of a broken frame. + */ + resolveContentUrl?(contentRef: string): string | undefined; } export type LocalCliRuntimeId = 'codex' | 'claude-code' | 'opencode'; diff --git a/app/shared/src/workbench/RightInspector.tsx b/app/shared/src/workbench/RightInspector.tsx index 5ae572cad..7bb0eb631 100644 --- a/app/shared/src/workbench/RightInspector.tsx +++ b/app/shared/src/workbench/RightInspector.tsx @@ -45,6 +45,7 @@ export function RightInspector({ maxWidth, minWidth, onOpenPreview, + previewPort, reviewFileRequest, runtimeEvidence, workDir, @@ -196,6 +197,7 @@ export function RightInspector({ overviewFiles={overviewFiles} overviewTasks={overviewTasks} previewFile={previewFile} + previewPort={previewPort} runResult={runResult} runtimeEvidence={runtimeEvidence} visibleTabs={visibleTabs} diff --git a/app/shared/src/workbench/RightInspectorModePanel.tsx b/app/shared/src/workbench/RightInspectorModePanel.tsx index 4c5aab089..500c7edb0 100644 --- a/app/shared/src/workbench/RightInspectorModePanel.tsx +++ b/app/shared/src/workbench/RightInspectorModePanel.tsx @@ -1,5 +1,6 @@ import React from 'react'; import type { RuntimeEvidenceSnapshot } from '../inspector'; +import type { PreviewPort } from '../platform'; import type { EvidenceRef, ContextUsageTranscriptBlock } from '../transcript'; import { BrowserModeBody, @@ -39,6 +40,8 @@ export interface RightInspectorModePanelProps { deployStatus: 'pending' | 'building' | 'deploying' | 'deployed' | 'failed' | undefined; files: EvidenceRef[]; previewFile: PreviewFile | null; + /** Platform preview port for the file preview router (#1817). */ + previewPort?: PreviewPort | undefined; onFileClick: (file: FileItem) => void; onClosePreview: () => void; onOpenPreview: ((evidence: EvidenceRef) => Promise) | undefined; @@ -64,6 +67,7 @@ export function RightInspectorModePanel({ deployStatus, files, previewFile, + previewPort, onFileClick, onClosePreview, onOpenPreview, @@ -109,6 +113,7 @@ export function RightInspectorModePanel({ files={files} overviewFiles={overviewFiles} previewFile={previewFile} + previewPort={previewPort} runtimeEvidence={runtimeEvidence} onClose={onClosePreview} onFallbackFileClick={onFileClick} diff --git a/app/shared/src/workbench/__tests__/inspector.test.tsx b/app/shared/src/workbench/__tests__/inspector.test.tsx index 76edc28e1..aee911019 100644 --- a/app/shared/src/workbench/__tests__/inspector.test.tsx +++ b/app/shared/src/workbench/__tests__/inspector.test.tsx @@ -107,8 +107,8 @@ describe('AgentHubWorkbench', () => { expect(screen.getByText('Topic: thread-1')).toBeInTheDocument(); expect(screen.getByText('Version: run-edge-1')).toBeInTheDocument(); expect(screen.getByText('Preview: ready')).toBeInTheDocument(); - expect(screen.getByText('Download: metadata only')).toBeInTheDocument(); - expect(screen.getByText('Export: evidence bundle ready')).toBeInTheDocument(); + expect(screen.getByText('Download: unavailable — Hub/Edge expose no artifact content endpoint (metadata only)')).toBeInTheDocument(); + expect(screen.getByText('Export: unavailable — this panel has no export action (review-only evidence)')).toBeInTheDocument(); expect(screen.getByText('Evidence: Edge')).toBeInTheDocument(); expect(screen.getByText('Diff projection: 1 file')).toBeInTheDocument(); expect(screen.queryByRole('button', { name: '查看产物 reports/runtime.patch' })).not.toBeInTheDocument(); diff --git a/app/shared/src/workbench/inspector/FilePreviewHelpers.test.ts b/app/shared/src/workbench/inspector/FilePreviewHelpers.test.ts index 3da10e62a..7c1bcc799 100644 --- a/app/shared/src/workbench/inspector/FilePreviewHelpers.test.ts +++ b/app/shared/src/workbench/inspector/FilePreviewHelpers.test.ts @@ -1,4 +1,5 @@ -import { describe, expect, it } from 'vitest'; +import { describe, expect, it, vi } from 'vitest'; +import type { PreviewPort } from '../../platform'; import { defaultPreviewMode, diffLineClass, @@ -13,6 +14,7 @@ import { openWithIconClass, openWithItems, resolveNativeMode, + resolvePreviewContentUrl, syntheticDiff, } from './FilePreviewHelpers'; import styles from './FilePreview.module.css'; @@ -80,3 +82,39 @@ describe('FilePreviewHelpers', () => { expect(openWithIconClass('vscode')).toBeTruthy(); }); }); + +describe('resolvePreviewContentUrl (#1817)', () => { + it('returns undefined for empty or prose fallback content', () => { + expect(resolvePreviewContentUrl(undefined, undefined)).toBeUndefined(); + expect(resolvePreviewContentUrl('', undefined)).toBeUndefined(); + expect(resolvePreviewContentUrl('# reports/runtime.patch', undefined)).toBeUndefined(); + expect(resolvePreviewContentUrl('Read-only runtime diff evidence.', undefined)).toBeUndefined(); + }); + + it('passes absolute http(s) URLs through without a port', () => { + expect(resolvePreviewContentUrl('http://127.0.0.1:4173/preview', undefined)) + .toBe('http://127.0.0.1:4173/preview'); + expect(resolvePreviewContentUrl('https://preview.example.com/app', undefined)) + .toBe('https://preview.example.com/app'); + }); + + it('delegates host-relative API paths to the port resolver', () => { + const resolveContentUrl = vi.fn((ref: string) => `http://127.0.0.1:3210${ref}`); + const port: PreviewPort = { + openEvidence: vi.fn(), + resolveContentUrl, + }; + expect(resolvePreviewContentUrl('/v1/runs/run-1/artifacts/artifact-1/content', port)) + .toBe('http://127.0.0.1:3210/v1/runs/run-1/artifacts/artifact-1/content'); + expect(resolveContentUrl).toHaveBeenCalledWith('/v1/runs/run-1/artifacts/artifact-1/content'); + }); + + it('yields undefined for host-relative paths when the port cannot resolve them (web boundary)', () => { + const port: PreviewPort = { + openEvidence: vi.fn(), + resolveContentUrl: () => undefined, + }; + expect(resolvePreviewContentUrl('/v1/runs/run-1/previews/preview-1/content', port)).toBeUndefined(); + expect(resolvePreviewContentUrl('/v1/runs/run-1/artifacts/artifact-1/content', undefined)).toBeUndefined(); + }); +}); diff --git a/app/shared/src/workbench/inspector/FilePreviewHelpers.ts b/app/shared/src/workbench/inspector/FilePreviewHelpers.ts index 03e77eb9d..5a2b4554f 100644 --- a/app/shared/src/workbench/inspector/FilePreviewHelpers.ts +++ b/app/shared/src/workbench/inspector/FilePreviewHelpers.ts @@ -1,3 +1,4 @@ +import type { PreviewPort } from '../../platform'; import { highlightLine } from '../../ui/syntaxHighlight'; import type { DesignOpenWithIconName } from '../designIcons'; import styles from './FilePreview.module.css'; @@ -122,3 +123,28 @@ export function openWithIconClass(name: DesignOpenWithIconName): string { return styles.brandIconSvg ?? ''; } } + +/** + * Resolve an evidence content reference (from `PreviewFile.content`) into a + * displayable URL for native previews (PDF iframe / image). + * + * - Empty/non-URL references (fallback prose such as `# path` metadata) yield + * `undefined` so the renderer shows an honest capability notice instead of + * an empty frame. + * - Absolute http(s) URLs are used unchanged on every surface. + * - Host-relative API paths (`/v1/runs/…/content`) require a `PreviewPort` + * that owns the host: Desktop resolves them against the Local Edge base + * URL, Web returns `undefined` (no Local Edge access, Hub-only boundary). + */ +export function resolvePreviewContentUrl( + contentRef: string | undefined, + previewPort: PreviewPort | undefined, +): string | undefined { + const trimmed = contentRef?.trim() ?? ''; + if (!trimmed) return undefined; + if (/^https?:\/\//i.test(trimmed)) return trimmed; + if (trimmed.startsWith('/')) { + return previewPort?.resolveContentUrl?.(trimmed); + } + return undefined; +} diff --git a/app/shared/src/workbench/inspector/FilePreviewRouter.apply.test.tsx b/app/shared/src/workbench/inspector/FilePreviewRouter.apply.test.tsx new file mode 100644 index 000000000..badd5e735 --- /dev/null +++ b/app/shared/src/workbench/inspector/FilePreviewRouter.apply.test.tsx @@ -0,0 +1,238 @@ +// Interactive diff apply dispatch through the platform PreviewPort (#1817). +// +// The shared package owns no Local Edge, so hunk write-back must be routed +// through the port: desktop-shaped ports (apply methods present) receive the +// decisions, web-shaped ports (apply methods absent — Hub-only boundary) +// degrade to explicit read-only feedback instead of silent console errors. +import { fireEvent, render, screen, waitFor } from '@testing-library/react'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import type { PreviewPort } from '../../platform'; +import { useToastStore } from '../../ui/toast/toastStore'; +import { FilePreviewRouter, type PreviewFile } from './FilePreviewRouter'; + +function interactiveDiffFile(): PreviewFile { + return { + name: 'src/app.ts', + type: 'diff', + interactiveDiff: { + runId: 'run-1', + workDir: '/work/project', + fileDiff: { + filePath: 'src/app.ts', + status: 'modified', + additions: 1, + deletions: 1, + hunks: [{ + header: '@@ -1 +1 @@', + lines: [ + { type: 'deleted', content: 'const legacy = true;' }, + { type: 'added', content: 'const modern = true;' }, + ], + }], + }, + }, + }; +} + +/** Desktop-shaped port: apply methods present (Local Edge write-back). */ +function desktopShapedPort(overrides: { + applyRunDiff?: PreviewPort['applyRunDiff']; + applyAllRunDiffs?: PreviewPort['applyAllRunDiffs']; +} = {}): PreviewPort { + return { + openEvidence: vi.fn().mockResolvedValue(undefined), + applyRunDiff: overrides.applyRunDiff ?? vi.fn().mockResolvedValue(undefined), + applyAllRunDiffs: overrides.applyAllRunDiffs ?? vi.fn().mockResolvedValue(undefined), + }; +} + +/** Web-shaped port: Hub-only surface, no apply methods, content resolution only. */ +function webShapedPort(): PreviewPort { + return { + openEvidence: vi.fn().mockResolvedValue(undefined), + resolveContentUrl: (ref: string) => (/^https?:\/\//i.test(ref) ? ref : undefined), + }; +} + +function currentToasts() { + return useToastStore.getState().toasts; +} + +describe('FilePreviewRouter interactive diff apply dispatch (#1817)', () => { + beforeEach(() => { + useToastStore.setState({ toasts: [] }); + }); + + afterEach(() => { + vi.restoreAllMocks(); + useToastStore.setState({ toasts: [] }); + }); + + it('routes a single hunk accept through PreviewPort.applyRunDiff and toasts success', async () => { + const port = desktopShapedPort(); + render( + , + ); + + // Supported surface: no read-only capability notice. + expect(screen.queryByRole('note')).toBeNull(); + + fireEvent.click(screen.getAllByRole('button', { name: 'Accept line' })[0]!); + + await waitFor(() => { + expect(port.applyRunDiff).toHaveBeenCalledTimes(1); + }); + expect(port.applyRunDiff).toHaveBeenCalledWith({ + runId: 'run-1', + workDir: '/work/project', + decision: { filePath: 'src/app.ts', hunkIndex: 0, accepted: true }, + }); + + await waitFor(() => { + expect(currentToasts().some((toast) => toast.type === 'success')).toBe(true); + }); + }); + + it('routes a single hunk reject through PreviewPort.applyRunDiff with accepted=false', async () => { + const port = desktopShapedPort(); + render( + , + ); + + fireEvent.click(screen.getAllByRole('button', { name: 'Reject line' })[0]!); + + await waitFor(() => { + expect(port.applyRunDiff).toHaveBeenCalledWith({ + runId: 'run-1', + workDir: '/work/project', + decision: { filePath: 'src/app.ts', hunkIndex: 0, accepted: false }, + }); + }); + }); + + it('surfaces port failures as an error toast instead of failing silently', async () => { + const consoleError = vi.spyOn(console, 'error').mockImplementation(() => {}); + const port = desktopShapedPort({ + applyRunDiff: vi.fn().mockRejectedValue(new Error('workdir not allowed')), + }); + render( + , + ); + + fireEvent.click(screen.getAllByRole('button', { name: 'Accept line' })[0]!); + + await waitFor(() => { + expect(currentToasts().some((toast) => toast.type === 'error')).toBe(true); + }); + // Silent console fallback is the defect this lane removes. + expect(consoleError).not.toHaveBeenCalled(); + }); + + it('routes accept-all through PreviewPort.applyAllRunDiffs with every hunk decision', async () => { + const port = desktopShapedPort(); + render( + , + ); + + fireEvent.click(screen.getByRole('button', { name: 'Accept All' })); + + await waitFor(() => { + expect(port.applyAllRunDiffs).toHaveBeenCalledTimes(1); + }); + expect(port.applyAllRunDiffs).toHaveBeenCalledWith({ + runId: 'run-1', + workDir: '/work/project', + decisions: [{ filePath: 'src/app.ts', hunkIndex: 0, accepted: true }], + }); + + await waitFor(() => { + expect(currentToasts().some((toast) => toast.type === 'success')).toBe(true); + }); + }); + + it('routes reject-all through PreviewPort.applyAllRunDiffs with accepted=false decisions', async () => { + const port = desktopShapedPort(); + render( + , + ); + + fireEvent.click(screen.getByRole('button', { name: 'Reject All' })); + + await waitFor(() => { + expect(port.applyAllRunDiffs).toHaveBeenCalledWith({ + runId: 'run-1', + workDir: '/work/project', + decisions: [{ filePath: 'src/app.ts', hunkIndex: 0, accepted: false }], + }); + }); + }); + + it('web-shaped port renders the read-only notice and warns on apply attempts', async () => { + const port = webShapedPort(); + render( + , + ); + + const notice = screen.getByRole('note'); + expect(notice).toBeInTheDocument(); + + fireEvent.click(screen.getAllByRole('button', { name: 'Accept line' })[0]!); + + await waitFor(() => { + expect(currentToasts().some((toast) => toast.type === 'warning')).toBe(true); + }); + // Unsupported surface never attempts a write-back. + expect(port.applyRunDiff).toBeUndefined(); + expect(port.applyAllRunDiffs).toBeUndefined(); + }); + + it('accept-all on an unsupported surface warns instead of dispatching', async () => { + render( + , + ); + + fireEvent.click(screen.getByRole('button', { name: 'Accept All' })); + + await waitFor(() => { + expect(currentToasts().some((toast) => toast.type === 'warning')).toBe(true); + }); + }); + + it('missing port entirely still degrades to the explicit read-only notice', () => { + render( + , + ); + expect(screen.getByRole('note')).toBeInTheDocument(); + }); +}); diff --git a/app/shared/src/workbench/inspector/FilePreviewRouter.tsx b/app/shared/src/workbench/inspector/FilePreviewRouter.tsx index 02b5a6823..4091f0c10 100644 --- a/app/shared/src/workbench/inspector/FilePreviewRouter.tsx +++ b/app/shared/src/workbench/inspector/FilePreviewRouter.tsx @@ -1,14 +1,16 @@ -import React, { useCallback, useMemo } from 'react'; -import { parseError } from '../../errors'; +import React, { useCallback, useMemo, useState } from 'react'; +import type { PreviewPort } from '../../platform'; import type { FileDiff } from '../../types/chat'; import { DiffReviewPanel, type DiffHunkDecision, type DiffReviewFile } from '../../ui/DiffReviewPanel'; import { DocxPreview } from '../../ui/DocxPreview'; import { SlideshowPreview } from '../../ui/SlideshowPreview'; import { TablePreview } from '../../ui/TablePreview'; import { PREVIEW_SANDBOX_SRCDOC } from '../../ui/previewSandbox'; +import { useToastStore } from '../../ui/toast/toastStore'; import { DesignFileIcon } from '../designIcons'; import styles from '../AgentHubWorkbench.module.css'; import { FilePreview } from './FilePreview'; +import { resolvePreviewContentUrl } from './FilePreviewHelpers'; import type { FileItem } from './OverviewPanel'; /* ═══════════════════════════════════════════════════════════════════════ @@ -16,16 +18,26 @@ import type { FileItem } from './OverviewPanel'; code viewer based on the filename extension. Routing table: - interactiveDiff -> InteractiveDiffPreview (accept/reject write-back) + interactiveDiff -> InteractiveDiffPreview (accept/reject write-back + via PreviewPort; read-only notice on surfaces + without a Local Edge) .pptx -> SlideshowPreview .ppt -> SlideshowPreview (legacy kind) .xlsx / .xls / .csv -> TablePreview .docx -> DocxPreview - .pdf -> browser-native PDF iframe + .pdf -> browser-native PDF iframe (needs a resolvable + content URL; honest notice otherwise) .html / .htm -> sandboxed HTML iframe (srcDoc) - .png/.jpg/... -> image placeholder (URL-loaded later) + .png/.jpg/... -> image via evidence content URL (honest notice + when no resolvable URL exists) .txt / .log -> plain
      everything else       -> FilePreview (code / diff / markdown)
+
+   Interactive diff apply and content-URL resolution go through the platform
+   `PreviewPort` (#1817): the shared package owns no Local Edge, so the
+   renderer never hardcodes an Edge base URL. Desktop implements the port
+   against Edge REST; Web omits apply (Hub-only boundary) and the router
+   degrades to explicit read-only feedback instead of silent console errors.
    ═══════════════════════════════════════════════════════════════════════ */
 
 export type PreviewFile = FileItem & {
@@ -43,6 +55,13 @@ export type PreviewFile = FileItem & {
 export interface FilePreviewRouterProps {
   file: PreviewFile;
   onClose: () => void;
+  /**
+   * Platform preview port for capabilities the shared package cannot own:
+   * diff hunk write-back (Edge apply) and evidence content-URL resolution.
+   * Optional so fixture/demo shells keep rendering; absent capabilities
+   * degrade to explicit user-facing notices.
+   */
+  previewPort?: PreviewPort | undefined;
 }
 
 type FilePreviewKind =
@@ -85,51 +104,26 @@ function extractFileUrl(content: string | undefined): string {
   return '';
 }
 
-/** Replaces the now-deleted shared Edge REST client apply fns (RFC A-V3 §4.1 —
- *  zero external consumers, stored in shared/src as dead surface).  edgeBaseUrl is unconfigured here
- *  because the shared package has no Local Edge; Desktop drives the Edge
- *  connection through its own wrappers.  InteractiveDiffPreview was already
- *  a known defect per verify-shared-boundary.py (audit-A P → PreviewPort). */
-const edgeBaseUrl = '';
-
-async function postJson(path: string, body: unknown): Promise {
-  if (!edgeBaseUrl) {
-    throw new Error('Edge base URL not configured — route through PreviewPort instead');
-  }
-  const res = await fetch(`${edgeBaseUrl}${path}`, {
-    method: 'POST',
-    headers: { 'Content-Type': 'application/json' },
-    body: JSON.stringify(body),
-  });
-  if (!res.ok) {
-    throw await parseError(res);
-  }
-  return res.json() as Promise;
+function describeError(err: unknown): string {
+  if (err instanceof Error) return err.message;
+  return String(err);
 }
 
-async function applyRunDiff(
-  runId: string,
-  body: { file_path: string; hunk_index: number; accepted: boolean; workDir: string },
-): Promise<{ code: string; data: unknown }> {
-  return postJson(`/v1/runs/${encodeURIComponent(runId)}/apply`, body);
-}
+const APPLY_UNSUPPORTED_NOTE = '当前端不支持将 diff 写回工作区(仅桌面本地 Edge 支持),当前为只读评审。';
 
-async function applyAllRunDiffs(
-  runId: string,
-  body: { decisions: Array<{ file_path: string; hunk_index: number; accepted: boolean }>; workDir: string },
-): Promise<{ code: string; data: unknown }> {
-  return postJson(`/v1/runs/${encodeURIComponent(runId)}/apply-all`, body);
-}
-
-/** Interactive diff preview with hunk accept/reject that writes back to the workdir via Edge API. */
+/** Interactive diff preview with hunk accept/reject that writes back to the workdir via the platform PreviewPort. */
 function InteractiveDiffPreview({
   file,
   onClose,
+  previewPort,
 }: {
   file: PreviewFile;
   onClose: () => void;
+  previewPort?: PreviewPort | undefined;
 }): React.ReactElement {
   const interactiveDiff = file.interactiveDiff;
+  const showToast = useToastStore((state) => state.showToast);
+  const applySupported = Boolean(previewPort?.applyRunDiff && previewPort?.applyAllRunDiffs);
 
   // Hooks run unconditionally so the hook order is stable if a file toggles
   // between interactive and non-interactive diff states across renders.
@@ -148,37 +142,60 @@ function InteractiveDiffPreview({
   const handleApplyHunk = useCallback(
     async (decision: DiffHunkDecision) => {
       if (!interactiveDiff) return;
+      if (!previewPort?.applyRunDiff) {
+        showToast('warning', APPLY_UNSUPPORTED_NOTE);
+        return;
+      }
       try {
-        await applyRunDiff(interactiveDiff.runId, {
-          file_path: decision.filePath,
-          hunk_index: decision.hunkIndex,
-          accepted: decision.accepted,
+        await previewPort.applyRunDiff({
+          runId: interactiveDiff.runId,
           workDir: interactiveDiff.workDir,
+          decision: {
+            filePath: decision.filePath,
+            hunkIndex: decision.hunkIndex,
+            accepted: decision.accepted,
+          },
         });
+        showToast(
+          'success',
+          decision.accepted
+            ? `已应用 hunk:${decision.filePath} #${decision.hunkIndex + 1}`
+            : `已拒绝 hunk:${decision.filePath} #${decision.hunkIndex + 1}`,
+        );
       } catch (err) {
-        console.error('RightInspector: applyRunDiff failed for hunk:', decision.filePath, decision.hunkIndex, err);
+        showToast('error', `Diff 应用失败:${describeError(err)}`);
       }
     },
-    [interactiveDiff],
+    [interactiveDiff, previewPort, showToast],
   );
 
   const handleApplyAllHunks = useCallback(
     async (decisions: DiffHunkDecision[]) => {
       if (!interactiveDiff) return;
+      if (!previewPort?.applyAllRunDiffs) {
+        showToast('warning', APPLY_UNSUPPORTED_NOTE);
+        return;
+      }
       try {
-        await applyAllRunDiffs(interactiveDiff.runId, {
-          decisions: decisions.map((d) => ({
-            file_path: d.filePath,
-            hunk_index: d.hunkIndex,
-            accepted: d.accepted,
-          })),
+        await previewPort.applyAllRunDiffs({
+          runId: interactiveDiff.runId,
           workDir: interactiveDiff.workDir,
+          decisions: decisions.map((item) => ({
+            filePath: item.filePath,
+            hunkIndex: item.hunkIndex,
+            accepted: item.accepted,
+          })),
         });
+        const acceptedCount = decisions.filter((item) => item.accepted).length;
+        showToast(
+          'success',
+          `已批量处理 ${decisions.length} 个 hunk(应用 ${acceptedCount},拒绝 ${decisions.length - acceptedCount})`,
+        );
       } catch (err) {
-        console.error('RightInspector: applyAllRunDiffs failed:', decisions.length, 'hunks,', err);
+        showToast('error', `Diff 批量应用失败:${describeError(err)}`);
       }
     },
-    [interactiveDiff],
+    [interactiveDiff, previewPort, showToast],
   );
 
   if (!interactiveDiff) return (<>);
@@ -192,6 +209,20 @@ function InteractiveDiffPreview({
         
         {fileDiff.filePath}
       
+      {!applySupported && (
+        
+ {APPLY_UNSUPPORTED_NOTE} +
+ )} ); } @@ -219,6 +252,7 @@ export function FilePreviewRouter({ const kind = detectFilePreviewKind(file.name); const content = file.content ?? `${file.name}\n\n暂无文件内容。`; const fileUrl = extractFileUrl(file.content); + const contentUrl = resolvePreviewContentUrl(file.content, previewPort); switch (kind) { case 'pptx': @@ -252,13 +286,13 @@ export function FilePreviewRouter({ ); case 'pdf': - return ; + return ; case 'html': return ; case 'image': - return ; + return ; case 'text': return ; @@ -279,11 +313,63 @@ export function FilePreviewRouter({ /* ═══ Native File Previews (zero extra libraries) ═══ */ -function NativePdfPreview({ filename }: { filename: string }): React.ReactElement { +function NativePreviewFallback({ + detail, + filename, + title, +}: { + detail: string; + filename: string; + title: string; +}): React.ReactElement { + return ( +
+
+ + {title} + {detail} +
+
+ ); +} + +function NativePdfPreview({ + contentUrl, + filename, +}: { + contentUrl?: string | undefined; + filename: string; +}): React.ReactElement { + if (!contentUrl) { + return ( + + ); + } return (