refactor(web): open produced files through the Host, not over HTTP
Scope decision: previews for a browser that is not on the Host machine are not supported. With that settled, host.openPath answers the supported case completely — a file:// document in a real browser has full page capabilities and no reach into /api — and the HTTP serving this branch had built answered only the unsupported one. Removed: the /f route and its listener, the workspace-file URL shape, ApiProxy.workspaceRootOf, ConnectionHandle.fileUrl, and the port published into the index page. Kept, and finished: - the produced-files row a turn ends with, derived from mutation locations; - the path link now reads as a link at rest, not only on hover — the reported "I can't open what it made" was this, sitting on a working capability; - the Host opener prefers the default BROWSER for .html/.htm/.xhtml/.svg, so a developer who binds .html to an editor still gets a rendered page (macOS via the LaunchServices https handler, Linux via $BROWSER, every failure falling back to the default application). The retired designs and their measurements stay in the Agent Note, including why same-origin serving was unsafe and why the sandbox that fixed it broke the pages invisibly.
This commit is contained in:
ZiyaZhang
@@ -62,11 +62,7 @@ function stubAgent(session: Session): Agent {
|
||||
async function harness(
|
||||
workspaceRoot = realpathSync(mkdtempSync(join(tmpdir(), 'dsh-apiproxy-workspace-'))),
|
||||
picker: DirectoryPickerCapability = { kind: 'native', pick: async () => null },
|
||||
extras: {
|
||||
openPath?: (path: string, signal: AbortSignal) => Promise<void>
|
||||
/** Store contents behind the gateway, or 'absent' for a composition with no persistence at all. */
|
||||
persisted?: { id: SessionId; cwd?: string }[] | 'absent'
|
||||
} = {},
|
||||
extras: { openPath?: (path: string, signal: AbortSignal) => Promise<void> } = {},
|
||||
) {
|
||||
const ctx = new Context()
|
||||
await ctx.plugin(SessionStore)
|
||||
@@ -77,10 +73,7 @@ async function harness(
|
||||
const storageDomain = new DomainFacility(ctx, { backend: 'memory', routes: {} })
|
||||
ctx.storage.mount('domain', storageDomain)
|
||||
ctx.provide('storageDomain', storageDomain)
|
||||
if (extras.persisted !== 'absent') {
|
||||
const persisted = extras.persisted ?? []
|
||||
ctx.provide('sessionPersistence', { list: () => Promise.resolve(persisted) } as never)
|
||||
}
|
||||
ctx.provide('sessionPersistence', { list: () => Promise.resolve([]) } as never)
|
||||
await ctx.plugin(WorkspaceRegistry)
|
||||
|
||||
const factory: AgentFactory = {
|
||||
@@ -251,27 +244,6 @@ describe('host.openPath', () => {
|
||||
})
|
||||
})
|
||||
|
||||
describe('workspaceRootOf', () => {
|
||||
it('answers from the live agent, then the store, and names nothing for an unknown session', async () => {
|
||||
const { api, workspaceRoot } = await harness(undefined, undefined, {
|
||||
persisted: [{ id: 's-cold' as SessionId, cwd: '/w/cold' }],
|
||||
})
|
||||
const created = await api.sessions.create(request({ cwd: workspaceRoot }))
|
||||
const sessionId = (created.result as { ok: true; value: { sessionId: SessionId } }).value.sessionId
|
||||
// Live: the agent's own header, no store read involved.
|
||||
await expect(api.workspaceRootOf(sessionId)).resolves.toBe(workspaceRoot)
|
||||
// Not live: the store answers, and the lookup never resumes an agent —
|
||||
// this harness's factory throws on resume, so a resuming lookup would fail.
|
||||
await expect(api.workspaceRootOf('s-cold' as SessionId)).resolves.toBe('/w/cold')
|
||||
await expect(api.workspaceRootOf('s-absent' as SessionId)).resolves.toBeUndefined()
|
||||
})
|
||||
|
||||
it('names nothing at all when the host keeps no session store', async () => {
|
||||
const { api } = await harness(undefined, undefined, { persisted: 'absent' })
|
||||
await expect(api.workspaceRootOf('s-any' as SessionId)).resolves.toBeUndefined()
|
||||
})
|
||||
})
|
||||
|
||||
describe('workspace.create', () => {
|
||||
it('serializes concurrent names and rejects the duplicate', async () => {
|
||||
const { api, workspaceRoot } = await harness()
|
||||
|
||||
@@ -108,8 +108,6 @@ function scriptedApi(overrides: {
|
||||
},
|
||||
events: { mux: () => empty<MuxFrame>(), host: () => empty<HostFrame>(), ...overrides.events },
|
||||
respond: overrides.respond ?? (() => Promise.resolve({ accepted: false as const, reason: 'not-pending' as const })),
|
||||
// No wire face, so the handler map never reaches it.
|
||||
workspaceRootOf: () => Promise.resolve(undefined),
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -233,8 +233,6 @@ function fakeApi(overrides: Partial<{ muxFrames: MuxFrame[]; hostFrames: HostFra
|
||||
async respond(message: ClientResponse): Promise<RpcReceipt> {
|
||||
return message.rpcId === 'known' ? { accepted: true } : { accepted: false, reason: 'not-pending' }
|
||||
},
|
||||
// No wire face, so the carrier never reaches it.
|
||||
workspaceRootOf: () => Promise.resolve(undefined),
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -1,74 +0,0 @@
|
||||
/** The /f URL shape: one encoding decision, asserted from both ends. */
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import {
|
||||
FILES_PATH, parseWorkspaceFilePath, workspaceFileSegments, workspaceFileUrl,
|
||||
} from '../src/api/files.ts'
|
||||
|
||||
describe('workspaceFileSegments', () => {
|
||||
it('keeps a relative path as its own segments', () => {
|
||||
expect(workspaceFileSegments('/w', 'out/index.html')).toEqual(['out', 'index.html'])
|
||||
expect(workspaceFileSegments(undefined, 'index.html')).toEqual(['index.html'])
|
||||
expect(workspaceFileSegments('/w', './a/./b.txt')).toEqual(['a', 'b.txt'])
|
||||
})
|
||||
|
||||
it('strips the cwd prefix from an absolute path inside the workspace', () => {
|
||||
expect(workspaceFileSegments('/w', '/w/a/b.html')).toEqual(['a', 'b.html'])
|
||||
// A trailing separator on the cwd must not shift the split.
|
||||
expect(workspaceFileSegments('/w/', '/w/a.html')).toEqual(['a.html'])
|
||||
})
|
||||
|
||||
it('reads Windows paths on either separator', () => {
|
||||
expect(workspaceFileSegments('C:\\w', 'C:\\w\\a\\b.html')).toEqual(['a', 'b.html'])
|
||||
expect(workspaceFileSegments('C:/w', 'C:\\w\\a.html')).toEqual(['a.html'])
|
||||
})
|
||||
|
||||
it('refuses everything the route would not serve', () => {
|
||||
// Absolute, but not under this workspace.
|
||||
expect(workspaceFileSegments('/w', '/etc/hosts')).toBeUndefined()
|
||||
// A sibling directory sharing the cwd's name prefix is not inside it.
|
||||
expect(workspaceFileSegments('/w', '/workspace-other/a')).toBeUndefined()
|
||||
// Absolute with no cwd to anchor against.
|
||||
expect(workspaceFileSegments(undefined, '/w/a.html')).toBeUndefined()
|
||||
expect(workspaceFileSegments('', '/w/a.html')).toBeUndefined()
|
||||
// Traversal, in either spelling.
|
||||
expect(workspaceFileSegments('/w', '../secret')).toBeUndefined()
|
||||
expect(workspaceFileSegments('/w', 'a/../../secret')).toBeUndefined()
|
||||
// The workspace directory itself is not a file.
|
||||
expect(workspaceFileSegments('/w', '/w')).toBeUndefined()
|
||||
expect(workspaceFileSegments('/w', '.')).toBeUndefined()
|
||||
})
|
||||
})
|
||||
|
||||
describe('workspaceFileUrl', () => {
|
||||
it('percent-encodes each segment but keeps the separators structural', () => {
|
||||
expect(workspaceFileUrl('s-1', ['out', 'a b.html'])).toBe(`${FILES_PATH}/s-1/out/a%20b.html`)
|
||||
expect(workspaceFileUrl('s/1', ['a#b.html'])).toBe(`${FILES_PATH}/s%2F1/a%23b.html`)
|
||||
})
|
||||
})
|
||||
|
||||
describe('parseWorkspaceFilePath', () => {
|
||||
it('round-trips what the browser half builds', () => {
|
||||
const url = workspaceFileUrl('s-1', ['out', 'a b.html'])
|
||||
expect(parseWorkspaceFilePath(url)).toEqual({ sessionId: 's-1', segments: ['out', 'a b.html'] })
|
||||
})
|
||||
|
||||
it('refuses malformed, prefix-foreign, and traversal pathnames', () => {
|
||||
expect(parseWorkspaceFilePath('/api/session.list')).toBeUndefined()
|
||||
expect(parseWorkspaceFilePath(FILES_PATH)).toBeUndefined()
|
||||
// Session named but no file below it.
|
||||
expect(parseWorkspaceFilePath(`${FILES_PATH}/s-1`)).toBeUndefined()
|
||||
expect(parseWorkspaceFilePath(`${FILES_PATH}//a.html`)).toBeUndefined()
|
||||
// Traversal is refused at parse time, before any filesystem call.
|
||||
expect(parseWorkspaceFilePath(`${FILES_PATH}/s-1/../etc/hosts`)).toBeUndefined()
|
||||
expect(parseWorkspaceFilePath(`${FILES_PATH}/s-1/a/./b`)).toBeUndefined()
|
||||
expect(parseWorkspaceFilePath(`${FILES_PATH}/s-1/a//b`)).toBeUndefined()
|
||||
// A separator smuggled through percent-encoding stays one segment's problem.
|
||||
expect(parseWorkspaceFilePath(`${FILES_PATH}/s-1/a%2F..%2Fb`)).toBeUndefined()
|
||||
expect(parseWorkspaceFilePath(`${FILES_PATH}/s-1/a%5Cb`)).toBeUndefined()
|
||||
expect(parseWorkspaceFilePath(`${FILES_PATH}/s-1/a%00b`)).toBeUndefined()
|
||||
// Malformed percent-escapes are uninterpretable, not a miss to resolve.
|
||||
expect(parseWorkspaceFilePath(`${FILES_PATH}/s-1/a%zz`)).toBeUndefined()
|
||||
expect(parseWorkspaceFilePath(`${FILES_PATH}/%zz/a.html`)).toBeUndefined()
|
||||
expect(parseWorkspaceFilePath(`${FILES_PATH}//`)).toBeUndefined()
|
||||
})
|
||||
})
|
||||
@@ -80,3 +80,96 @@ describe('native path opener', () => {
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
describe('browser-renderable documents', () => {
|
||||
const LS_PLIST = `{
|
||||
LSHandlers = (
|
||||
{
|
||||
LSHandlerPreferredVersions = {
|
||||
LSHandlerRoleAll = "-";
|
||||
};
|
||||
LSHandlerRoleAll = "com.google.chrome";
|
||||
LSHandlerURLScheme = https;
|
||||
}
|
||||
);
|
||||
}`
|
||||
|
||||
it('opens a page with the default browser rather than the .html handler on darwin', async () => {
|
||||
const calls: { command: string; args: readonly string[] }[] = []
|
||||
const run = async (command: string, args: readonly string[]) => {
|
||||
calls.push({ command, args })
|
||||
return { stdout: command === 'defaults' ? LS_PLIST : '', stderr: '' }
|
||||
}
|
||||
await openNativePath('/w/page.html', new AbortController().signal, { platform: 'darwin', run })
|
||||
// A developer who bound .html to an editor still gets a rendered page.
|
||||
expect(calls.map(c => [c.command, ...c.args])).toEqual([
|
||||
['defaults', 'read', 'com.apple.LaunchServices/com.apple.launchservices.secure'],
|
||||
['open', '-b', 'com.google.chrome', '/w/page.html'],
|
||||
])
|
||||
})
|
||||
|
||||
it('leaves every other document to the default application', async () => {
|
||||
const calls: string[][] = []
|
||||
const run = async (command: string, args: readonly string[]) => {
|
||||
calls.push([command, ...args])
|
||||
return { stdout: '', stderr: '' }
|
||||
}
|
||||
await openNativePath('/w/report.md', new AbortController().signal, { platform: 'darwin', run })
|
||||
// No LaunchServices read at all: markdown is not a browser document.
|
||||
expect(calls).toEqual([['open', '/w/report.md']])
|
||||
})
|
||||
|
||||
it('falls back to the default application when no browser can be named', async () => {
|
||||
// LaunchServices has no https record (a fresh account), so the system's
|
||||
// own content-type choice is the best answer available.
|
||||
const calls: string[][] = []
|
||||
const run = async (command: string, args: readonly string[]) => {
|
||||
calls.push([command, ...args])
|
||||
if (command === 'defaults') throw new Error('domain not found')
|
||||
return { stdout: '', stderr: '' }
|
||||
}
|
||||
await openNativePath('/w/page.html', new AbortController().signal, { platform: 'darwin', run })
|
||||
expect(calls).toEqual([
|
||||
['defaults', 'read', 'com.apple.LaunchServices/com.apple.launchservices.secure'],
|
||||
['open', '/w/page.html'],
|
||||
])
|
||||
|
||||
// A record without an https handler is the same answer.
|
||||
const bare: string[][] = []
|
||||
await openNativePath('/w/page.html', new AbortController().signal, {
|
||||
platform: 'darwin',
|
||||
run: async (command, args) => {
|
||||
bare.push([command, ...args])
|
||||
return { stdout: '{ LSHandlers = ( ); }', stderr: '' }
|
||||
},
|
||||
})
|
||||
expect(bare[1]).toEqual(['open', '/w/page.html'])
|
||||
})
|
||||
|
||||
it('honors $BROWSER on linux and leaves windows to its association', async () => {
|
||||
const linux: string[][] = []
|
||||
await openNativePath('/w/page.html', new AbortController().signal, {
|
||||
platform: 'linux',
|
||||
env: { BROWSER: 'firefox' },
|
||||
run: async (command, args) => { linux.push([command, ...args]); return { stdout: '', stderr: '' } },
|
||||
})
|
||||
expect(linux).toEqual([['firefox', '/w/page.html']])
|
||||
|
||||
// Unset $BROWSER: xdg-open's association is the fallback.
|
||||
const bare: string[][] = []
|
||||
await openNativePath('/w/page.html', new AbortController().signal, {
|
||||
platform: 'linux',
|
||||
env: {},
|
||||
run: async (command, args) => { bare.push([command, ...args]); return { stdout: '', stderr: '' } },
|
||||
})
|
||||
expect(bare).toEqual([['xdg-open', '/w/page.html']])
|
||||
|
||||
// Windows names no browser without the UserChoice registry.
|
||||
const win: string[][] = []
|
||||
await openNativePath('C:\\w\\page.html', new AbortController().signal, {
|
||||
platform: 'win32',
|
||||
run: async (command, args) => { win.push([command, ...args]); return { stdout: '', stderr: '' } },
|
||||
})
|
||||
expect(win[0]?.[0]).toBe('powershell.exe')
|
||||
})
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user