From dc2c347d250c85117afe6c1c865aa3c48f6ac5be Mon Sep 17 00:00:00 2001 From: ying-hua <60057611+ying-hua@users.noreply.github.com> Date: Sun, 13 Sep 2026 17:15:19 +0800 Subject: [PATCH 1/3] feat(harness): add local preview capability preflight MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Showing generated HTML locally depends on four independent things, and each fails differently: the client may have no GUI surface, the embedded browser refuses file:// by policy, the process that writes the file may not share a filesystem with the surface that renders it, and the conversation's own view may not be on screen. Attempting the preview and reading the first error conflates all four. A refused loopback connection is the worst of them — "nothing listening yet", "still starting", and "separate namespace" are indistinguishable at the socket, yet the refusal reads like proof of the last one. preview_preflight answers each capability with verified, unsupported, or unknown, and names the observation behind it. verified needs a positive observation; unsupported is reserved for settled boundaries statable without probing; everything else is unknown and explicitly disclaims a sandbox or isolation cause. A connection outcome can therefore only ever produce verified or unknown, never unsupported. The honesty runs both ways: an endpoint that answers proves only that something listens for this process, not that the embedded browser shares that loopback namespace, and a staging root that round-trips proves only that the main process reaches it. `surface` reports whether this client can display a page at all and stays separate from `endpoint`, which is not_checked when no origin was given; `ready` needs both, and is never inferred from an absent error. The loopback probe settles on every terminal path and owns an absolute deadline. A server answering 101 Switching Protocols makes Node route the exchange to `upgrade` and close the request without emitting `response` or `error`, taking the socket-bound timeout with it; the probe previously outlived both its stated bound and its caller's cancellation. Published as its own desktop_preview offer rather than in the Browser offer: every Browser tool is admitted against an Origin resolved from the view's current URL, so a capability check published there would need a page already loaded and would fail with the very "requires an HTTP origin" error it exists to explain. Generated-by: Claude Opus 5 via Claude Code Co-Authored-By: Claude Opus 5 (1M context) --- .../main/__tests__/preview-preflight.test.ts | 542 ++++++++++++++++++ .../runtime-host-artifacts-ipc-main.test.ts | 62 +- .../src/main/preview-preflight-probes.ts | 176 ++++++ apps/desktop/src/main/preview-preflight.ts | 490 ++++++++++++++++ .../main/runtime-host-artifacts-ipc-main.ts | 17 +- apps/desktop/src/main/runtime-host-boot.ts | 33 +- 6 files changed, 1316 insertions(+), 4 deletions(-) create mode 100644 apps/desktop/src/main/__tests__/preview-preflight.test.ts create mode 100644 apps/desktop/src/main/preview-preflight-probes.ts create mode 100644 apps/desktop/src/main/preview-preflight.ts diff --git a/apps/desktop/src/main/__tests__/preview-preflight.test.ts b/apps/desktop/src/main/__tests__/preview-preflight.test.ts new file mode 100644 index 00000000000..d2478c28a06 --- /dev/null +++ b/apps/desktop/src/main/__tests__/preview-preflight.test.ts @@ -0,0 +1,542 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +/** + * Local preview preflight: the verified / unsupported / unknown classification + * for every capability, the rule that a connection outcome may never become + * `unsupported`, and the probes driven against a real directory and a real + * loopback socket. + */ + +import { strict as assert } from 'node:assert'; +import { writeFile, mkdtemp, rm } from 'node:fs/promises'; +import { createServer, type Server } from 'node:http'; +import type { Socket } from 'node:net'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { after, describe, it } from 'node:test'; +import type { MakaTool, MakaToolContext } from '@maka/runtime/tool-runtime'; +import { + assertLoopbackOrigin, + buildPreviewPreflightTools, + classifyBrowserReachability, + classifyFilesystemVisibility, + classifyGuiSurface, + classifyLoopbackEndpoint, + classifyUrlSchemes, + probeNavigableSchemes, + type PreviewCapability, + type PreviewCapabilityId, + type PreviewPreflightAuthority, + type PreviewPreflightResult, +} from '../preview-preflight.js'; +import { + createPreviewPreflightAuthority, + LOOPBACK_PROBE_TIMEOUT_MS, +} from '../preview-preflight-probes.js'; + +function ctx(signal?: AbortSignal): MakaToolContext { + return { + sessionId: 's1', + turnId: 't1', + cwd: '/tmp', + toolCallId: 'c1', + abortSignal: signal ?? new AbortController().signal, + emitOutput: () => {}, + }; +} + +/** Everything observable and healthy; each test spoils only what it is about. */ +function healthyAuthority( + overrides: Partial = {}, +): PreviewPreflightAuthority { + return { + guiSurfaceAvailable: () => true, + browserDrivable: () => true, + probeStagingRoot: () => ({ kind: 'round_tripped', root: '/tmp/maka-runtime-host-artifacts' }), + probeLoopback: () => ({ kind: 'answered', status: 200 }), + ...overrides, + }; +} + +function preflightTool( + authority: PreviewPreflightAuthority, +): MakaTool<{ origin?: string }, PreviewPreflightResult> { + const [tool] = buildPreviewPreflightTools(authority); + assert.ok(tool, 'the preflight offer publishes a tool'); + return tool as MakaTool<{ origin?: string }, PreviewPreflightResult>; +} + +function run( + authority: PreviewPreflightAuthority, + args: { origin?: string } = {}, + signal?: AbortSignal, +): Promise { + return Promise.resolve(preflightTool(authority).impl(args, ctx(signal))); +} + +function capability( + result: PreviewPreflightResult, + id: PreviewCapabilityId, +): PreviewCapability { + const found = result.capabilities.find((entry) => entry.id === id); + assert.ok(found, `the report covers ${id}`); + return found; +} + +/** No status may read as a cause the probe did not establish. */ +function assertClaimsNoCause(capability: PreviewCapability): void { + const text = `${capability.evidence} ${capability.boundary ?? ''}`; + assert.doesNotMatch( + text, + /(?:is|are|was|were) (?:sandbox|isolat)/iu, + `${capability.id} must not assert an isolation cause: ${text}`, + ); +} + +describe('preview preflight capability classification', () => { + it('reports the real address policy rather than a restatement of it', () => { + const probes = probeNavigableSchemes(); + const navigable = new Map(probes.map((probe) => [probe.scheme, probe.navigable])); + assert.equal(navigable.get('https:'), true); + assert.equal(navigable.get('http:'), true); + assert.equal(navigable.get('file:'), false); + assert.equal(navigable.get('data:'), false); + }); + + it('calls a rejected file:// scheme an unsupported boundary and explains it', () => { + const capability = classifyUrlSchemes(probeNavigableSchemes()); + assert.equal(capability.status, 'unsupported'); + assert.match(capability.evidence, /accepted https:, http:/u); + assert.match(capability.evidence, /rejected file:, data:/u); + assert.match(capability.boundary ?? '', /requires an HTTP origin/u); + assert.match(capability.boundary ?? '', /deliberate boundary, not a missing feature/u); + // A boundary the caller cannot move must name the way around it. + assert.match(capability.boundary ?? '', /use ArtifactPreview to serve the same content over http/u); + }); + + it('follows the policy if file:// ever becomes navigable', () => { + const capability = classifyUrlSchemes([ + { scheme: 'https:', navigable: true }, + { scheme: 'file:', navigable: true }, + ]); + assert.equal(capability.status, 'verified'); + }); + + it('separates a missing view host from a check that did not complete', () => { + assert.equal(classifyGuiSurface({ ok: true, value: true }).status, 'verified'); + + const absent = classifyGuiSurface({ ok: true, value: false }); + assert.equal(absent.status, 'unsupported'); + assert.match(absent.boundary ?? '', /only inside the desktop app/u); + + const unreadable = classifyGuiSurface({ ok: false, cause: 'provider exploded' }); + assert.equal(unreadable.status, 'unknown'); + assert.match(unreadable.evidence, /provider exploded/u); + assert.match(unreadable.boundary ?? '', /does not mean the client is headless/u); + }); + + it('keeps a verified staging root from implying the generating process shares it', () => { + const capability = classifyFilesystemVisibility({ + ok: true, + value: { kind: 'round_tripped', root: '/tmp/staging' }, + }); + assert.equal(capability.status, 'verified'); + assert.match(capability.evidence, /Created, read back, and removed a probe file under \/tmp\/staging/u); + assert.match(capability.boundary ?? '', /does not prove that the shell or sandboxed runtime/u); + }); + + it('reports an unreachable staging root as unknown with its cause', () => { + const capability = classifyFilesystemVisibility({ + ok: true, + value: { kind: 'unavailable', root: '/tmp/staging', cause: 'EACCES' }, + }); + assert.equal(capability.status, 'unknown'); + assert.match(capability.evidence, /EACCES/u); + assert.match(capability.boundary ?? '', /does not distinguish a missing directory/u); + assertClaimsNoCause(capability); + }); + + it('reports a client with no staging root as unsupported', () => { + const capability = classifyFilesystemVisibility({ ok: true, value: { kind: 'not_configured' } }); + assert.equal(capability.status, 'unsupported'); + }); + + it('treats a refused view as unknown and visibility-scoped, never as isolation', () => { + const capability = classifyBrowserReachability({ + guiSurface: classifyGuiSurface({ ok: true, value: true }), + drivable: { ok: true, value: false }, + }); + assert.equal(capability.status, 'unknown'); + assert.match(capability.boundary ?? '', /conversation the user is looking at/u); + assert.match(capability.boundary ?? '', /does not prove the browser is unreachable/u); + assertClaimsNoCause(capability); + }); + + it('will not call a view reachable when it was never asked', () => { + const capability = classifyBrowserReachability({ + guiSurface: classifyGuiSurface({ ok: true, value: true }), + }); + assert.equal(capability.status, 'unknown'); + assert.match(capability.evidence, /was not asked/u); + }); + + it('reports reachability as unsupported only when there is no view host at all', () => { + const capability = classifyBrowserReachability({ + guiSurface: classifyGuiSurface({ ok: true, value: false }), + drivable: { ok: true, value: false }, + }); + assert.equal(capability.status, 'unsupported'); + }); + + it('accepts any HTTP answer as evidence that something listened', () => { + const capability = classifyLoopbackEndpoint({ + origin: 'http://127.0.0.1:8765/', + observed: { ok: true, value: { kind: 'answered', status: 404 } }, + }); + assert.equal(capability.status, 'verified'); + assert.match(capability.evidence, /answered with HTTP 404/u); + // Symmetry: an answer to this process is not an answer for the browser. + assert.match(capability.boundary ?? '', /does not prove the embedded browser shares/u); + }); + + it('never turns a failed connection into an unsupported capability', () => { + for (const cause of ['ECONNREFUSED', 'ETIMEDOUT', 'EHOSTUNREACH']) { + const capability = classifyLoopbackEndpoint({ + origin: 'http://127.0.0.1:8765/', + observed: { ok: true, value: { kind: 'no_answer', cause } }, + }); + assert.equal(capability.status, 'unknown', `${cause} is not a settled boundary`); + assert.match(capability.evidence, new RegExp(cause, 'u')); + assert.match(capability.boundary ?? '', /does not prove sandbox isolation/u); + assert.match(capability.boundary ?? '', /nothing answered this process at that address/u); + assertClaimsNoCause(capability); + } + }); +}); + +describe('preview preflight loopback argument', () => { + it('accepts a bounded loopback URL', () => { + assert.equal( + assertLoopbackOrigin('http://127.0.0.1:8765/preview.html'), + 'http://127.0.0.1:8765/preview.html', + ); + assert.equal(assertLoopbackOrigin('http://[::1]:8765/'), 'http://[::1]:8765/'); + }); + + it('rejects anything that would make the check an arbitrary outbound request', () => { + assert.throws(() => assertLoopbackOrigin('example.com'), /Not a URL/u); + assert.throws(() => assertLoopbackOrigin('file:///tmp/preview.html'), /Only http:\/\/ and https:\/\//u); + assert.throws(() => assertLoopbackOrigin('http://192.168.1.4:8765/'), /Only the loopback interface/u); + assert.throws(() => assertLoopbackOrigin('http://example.com/'), /Only the loopback interface/u); + // Resolves through the host, so it can point somewhere other than loopback. + assert.throws(() => assertLoopbackOrigin('http://localhost:8765/'), /"localhost" is excluded/u); + assert.throws(() => assertLoopbackOrigin('http://user:pw@127.0.0.1:8765/'), /Credentials are not accepted/u); + assert.throws(() => assertLoopbackOrigin('http://127.0.0.1:8765/?token=x'), /no query or fragment/u); + assert.throws(() => assertLoopbackOrigin('http://127.0.0.1:8765/#frag'), /no query or fragment/u); + }); +}); + +describe('preview preflight tool', () => { + it('claims ready only on a verified surface and an endpoint that answered', async () => { + const result = await run(healthyAuthority(), { origin: 'http://127.0.0.1:8765/preview.html' }); + assert.equal(result.ready, true); + assert.equal(result.surface, 'verified'); + assert.equal(result.endpoint, 'verified'); + assert.equal(result.alternative, undefined); + assert.equal(capability(result, 'loopback_endpoint').status, 'verified'); + }); + + it('never claims ready from the absence of an error', async () => { + // Everything else is verified; only the endpoint is unproven. + const result = await run( + healthyAuthority({ probeLoopback: () => ({ kind: 'no_answer', cause: 'ECONNREFUSED' }) }), + { origin: 'http://127.0.0.1:8765/preview.html' }, + ); + assert.equal(result.ready, false); + assert.equal(result.surface, 'verified'); + assert.equal(result.endpoint, 'unknown'); + assert.equal(capability(result, 'loopback_endpoint').status, 'unknown'); + }); + + it('keeps an unnamed endpoint from reading as a missing capability', async () => { + const result = await run(healthyAuthority()); + // The surface stands on its own evidence; only the endpoint is unanswered. + assert.equal(result.surface, 'verified'); + assert.equal(result.endpoint, 'not_checked'); + assert.equal(result.ready, false); + assert.equal( + result.capabilities.some((entry) => entry.id === 'loopback_endpoint'), + false, + 'an endpoint that was never named produces no capability entry', + ); + }); + + it('separates a settled surface boundary from an unproven one', async () => { + const headless = await run(healthyAuthority({ guiSurfaceAvailable: () => false })); + assert.equal(headless.surface, 'unsupported'); + + const offScreen = await run(healthyAuthority({ browserDrivable: () => false })); + assert.equal(offScreen.surface, 'unknown'); + assert.doesNotMatch(offScreen.summary, /cannot display/u); + }); + + it('points an unready report at the managed preview endpoint', async () => { + const result = await run(healthyAuthority()); + assert.match(result.alternative ?? '', /ArtifactPreview/u); + assert.match(result.alternative ?? '', /no shell server and no file:\/\/ navigation/u); + // The handoff must carry its own caveat, not just its happy path. + assert.match(result.alternative ?? '', /not a browser load/u); + }); + + it('covers every capability with an observation behind its status', async () => { + const result = await run(healthyAuthority(), { origin: 'http://127.0.0.1:8765/' }); + assert.deepEqual( + result.capabilities.map((entry) => entry.id), + ['gui_surface', 'url_schemes', 'filesystem_visibility', 'browser_reachability', 'loopback_endpoint'], + ); + for (const entry of result.capabilities) { + assert.notEqual(entry.evidence.trim(), '', `${entry.id} names its observation`); + } + }); + + it('reports a headless client without asking a view host that cannot answer', async () => { + let asked = 0; + const result = await run( + healthyAuthority({ + guiSurfaceAvailable: () => false, + browserDrivable: () => { + asked += 1; + throw new Error('Browser automation is only available inside the desktop app.'); + }, + }), + ); + assert.equal(asked, 0, 'a settled unsupported answer is not downgraded to unknown by a throw'); + assert.equal(capability(result, 'gui_surface').status, 'unsupported'); + assert.equal(capability(result, 'browser_reachability').status, 'unsupported'); + assert.ok(result.alternative, 'a client that cannot preview is still told what to do instead'); + }); + + it('turns a probe that fails into an unknown carrying its cause', async () => { + const result = await run( + healthyAuthority({ + probeStagingRoot: () => { + throw new Error('staging probe exploded'); + }, + }), + ); + const filesystem = capability(result, 'filesystem_visibility'); + assert.equal(filesystem.status, 'unknown'); + assert.match(filesystem.evidence, /staging probe exploded/u); + // Materializing a file is a separate question from displaying a page, so a + // failed staging probe must not drag the surface down with it. + assert.equal(result.surface, 'verified'); + }); + + it('rejects a bad origin as an argument error instead of an unknown capability', async () => { + await assert.rejects( + () => run(healthyAuthority(), { origin: 'http://192.168.1.4:8765/' }), + /Only the loopback interface/u, + ); + }); + + it('propagates cancellation instead of reporting it as a finding', async () => { + const controller = new AbortController(); + await assert.rejects( + () => + run( + healthyAuthority({ + guiSurfaceAvailable: () => { + controller.abort(new Error('turn ended')); + throw new Error('turn ended'); + }, + }), + {}, + controller.signal, + ), + /turn ended/u, + ); + + const already = AbortSignal.abort(new Error('already gone')); + await assert.rejects(() => run(healthyAuthority(), {}, already), /already gone/u); + }); +}); + +describe('preview preflight probes', () => { + const servers: Server[] = []; + const sockets: Socket[] = []; + const roots: string[] = []; + + after(async () => { + for (const socket of sockets) socket.destroy(); + for (const server of servers) await new Promise((resolve) => server.close(resolve)); + for (const root of roots) await rm(root, { recursive: true, force: true }).catch(() => undefined); + }); + + async function tempRoot(): Promise { + const root = await mkdtemp(join(tmpdir(), 'maka-preflight-')); + roots.push(root); + return root; + } + + function listen(): Promise { + const server = createServer((_request, response) => { + response.writeHead(404).end('nope'); + }); + servers.push(server); + return new Promise((resolve) => server.listen(0, '127.0.0.1', () => resolve(server))); + } + + function originOf(server: Server): string { + const address = server.address(); + assert.ok(address && typeof address === 'object', 'the probe server is bound'); + return `http://127.0.0.1:${address.port}/`; + } + + const signal = new AbortController().signal; + + it('round-trips a probe file under a real staging root and leaves nothing behind', async () => { + const root = await tempRoot(); + const authority = createPreviewPreflightAuthority({ stagingRoot: () => root }); + assert.deepEqual(await authority.probeStagingRoot({ signal }), { kind: 'round_tripped', root }); + const { readdir } = await import('node:fs/promises'); + assert.deepEqual(await readdir(root), []); + }); + + it('reports an unusable staging root as unavailable with its errno', async () => { + const root = await tempRoot(); + const blocked = join(root, 'not-a-directory'); + await writeFile(blocked, 'x', 'utf8'); + const authority = createPreviewPreflightAuthority({ stagingRoot: () => blocked }); + const probe = await authority.probeStagingRoot({ signal }); + assert.equal(probe.kind, 'unavailable'); + assert.match(probe.kind === 'unavailable' ? probe.cause : '', /^E[A-Z]+$/u); + }); + + it('reports a client with no staging root instead of inventing a path', async () => { + const authority = createPreviewPreflightAuthority({ stagingRoot: () => undefined }); + assert.deepEqual(await authority.probeStagingRoot({ signal }), { kind: 'not_configured' }); + }); + + it('records the status a live loopback endpoint answers with', async () => { + const server = await listen(); + const authority = createPreviewPreflightAuthority({ stagingRoot: () => undefined }); + assert.deepEqual(await authority.probeLoopback({ origin: originOf(server), signal }), { + kind: 'answered', + status: 404, + }); + }); + + it('answers from the status line without waiting for or reading a large body', async () => { + const server = createServer((_request, response) => { + response.writeHead(200, { 'content-type': 'text/html' }); + // Larger than the probe will ever drain, and deliberately still open when + // the probe answers: a preview page's bytes must not gate the report. + response.write('x'.repeat(4 * 1024 * 1024)); + setTimeout(() => response.end(), 50); + }); + servers.push(server); + await new Promise((resolve) => server.listen(0, '127.0.0.1', () => resolve(undefined))); + const authority = createPreviewPreflightAuthority({ stagingRoot: () => undefined }); + assert.deepEqual(await authority.probeLoopback({ origin: originOf(server), signal }), { + kind: 'answered', + status: 200, + }); + // Give the torn-down body a turn of the loop to surface anything it emits. + await new Promise((resolve) => setTimeout(resolve, 100)); + }); + + /** A server that answers 101, which Node routes to `upgrade`, not `response`. */ + async function listenUpgrading(): Promise { + const server = createServer(); + server.on('connection', (socket) => { + socket.once('data', () => { + socket.write( + 'HTTP/1.1 101 Switching Protocols\r\nConnection: Upgrade\r\nUpgrade: websocket\r\n\r\n', + ); + }); + }); + servers.push(server); + await new Promise((resolve) => server.listen(0, '127.0.0.1', () => resolve(undefined))); + return server; + } + + /** A server that accepts the connection and then says nothing at all. */ + async function listenSilent(): Promise { + const server = createServer(); + server.on('connection', (socket) => sockets.push(socket)); + servers.push(server); + await new Promise((resolve) => server.listen(0, '127.0.0.1', () => resolve(undefined))); + return server; + } + + it('settles on a 101 that never becomes a response', async () => { + const server = await listenUpgrading(); + const authority = createPreviewPreflightAuthority({ stagingRoot: () => undefined }); + const started = Date.now(); + // A 101 still proves something listened, which is all this capability claims. + assert.deepEqual(await authority.probeLoopback({ origin: originOf(server), signal }), { + kind: 'answered', + status: 101, + }); + assert.ok( + Date.now() - started < LOOPBACK_PROBE_TIMEOUT_MS, + 'the answer arrives without waiting out the deadline', + ); + }); + + it('propagates cancellation from a server that is saying nothing', async () => { + const silent = await listenSilent(); + const controller = new AbortController(); + const authority = createPreviewPreflightAuthority({ stagingRoot: () => undefined }); + // Nothing will ever arrive on this socket, so the probe's own abort + // listener is the only thing that can settle it before the deadline. + const cancelled = Promise.resolve( + authority.probeLoopback({ origin: originOf(silent), signal: controller.signal }), + ); + controller.abort(new Error('turn ended')); + await assert.rejects(() => cancelled, /turn ended/u); + }); + + it('enforces its deadline against a server that never answers', async () => { + const server = await listenSilent(); + const authority = createPreviewPreflightAuthority({ + stagingRoot: () => undefined, + loopbackTimeoutMs: 120, + }); + const probe = await authority.probeLoopback({ origin: originOf(server), signal }); + assert.equal(probe.kind, 'no_answer'); + assert.match(probe.kind === 'no_answer' ? probe.cause : '', /no response within 120ms/u); + }); + + it('returns a refused connection as a value, not a throw', async () => { + const server = await listen(); + const origin = originOf(server); + await new Promise((resolve) => server.close(resolve)); + const authority = createPreviewPreflightAuthority({ stagingRoot: () => undefined }); + const probe = await authority.probeLoopback({ origin, signal }); + assert.equal(probe.kind, 'no_answer'); + assert.match(probe.kind === 'no_answer' ? probe.cause : '', /^E[A-Z]+$/u); + }); + + it('reports no GUI surface when no Desktop view host is registered', () => { + const authority = createPreviewPreflightAuthority({ stagingRoot: () => undefined }); + assert.equal(authority.guiSurfaceAvailable(), false); + }); +}); diff --git a/apps/desktop/src/main/__tests__/runtime-host-artifacts-ipc-main.test.ts b/apps/desktop/src/main/__tests__/runtime-host-artifacts-ipc-main.test.ts index 420765c7e0d..83aa444a7f1 100644 --- a/apps/desktop/src/main/__tests__/runtime-host-artifacts-ipc-main.test.ts +++ b/apps/desktop/src/main/__tests__/runtime-host-artifacts-ipc-main.test.ts @@ -19,11 +19,15 @@ import assert from "node:assert/strict"; import fs, { mkdir, mkdtemp, readFile, readdir, rm, writeFile } from "node:fs/promises"; +import { randomUUID } from "node:crypto"; import { syncBuiltinESMExports } from "node:module"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { test } from "node:test"; -import { registerRuntimeHostArtifactsIpc } from "../runtime-host-artifacts-ipc-main.js"; +import { + artifactPresentationRoot, + registerRuntimeHostArtifactsIpc, +} from "../runtime-host-artifacts-ipc-main.js"; import { ManagedArtifactPreview } from '../managed-artifact-preview.js'; for (const launchFails of [false, true]) { @@ -498,3 +502,59 @@ test("Attachment byte IPC stops a stream that exceeds its preview admission", as { ok: false, reason: "too_large" }, ); }); + +test("Open materializes under the generation root the preview preflight probes", async () => { + // The preflight reports on artifactPresentationRoot(hostEpoch). If the Open + // action ever stopped using that same value, the preflight would describe a + // directory nothing writes to, and its verdict would be about the wrong path. + const hostEpoch = `preflight-wiring-${randomUUID()}`; + const expectedRoot = artifactPresentationRoot(hostEpoch); + const content = Buffer.from("materialized"); + const handlers = new Map(); + const opened: string[] = []; + const artifact = { + id: "artifact-1", + sessionId: "session-1", + turnId: "turn-1", + createdAt: 1, + name: "result.bin", + kind: "image", + sizeBytes: content.byteLength, + mimeType: "image/png", + status: "live", + } as const; + + try { + registerRuntimeHostArtifactsIpc({ + uiLocale: () => "en" as const, + ipcMain: { handle: (channel, handler) => handlers.set(channel, handler as Handler) }, + // No presentationRoot override: this is the production default path. + client: { + hostEpoch, + async getArtifact() { + return artifact; + }, + async streamArtifact( + _sessionId: string, + _artifactId: string, + writeChunk: (chunk: Uint8Array) => Promise, + ) { + await writeChunk(content); + return content.byteLength; + }, + } as never, + mainWindowController: {} as never, + showItemInFolder: (path) => opened.push(path), + }); + + assert.deepEqual(await handlers.get("app:openArtifactPath")?.({}, "session-1", "artifact-1"), { + ok: true, + opened: "result.bin", + }); + assert.equal(opened.length, 1); + assert.equal(opened[0], join(expectedRoot, "session-1", "artifact-1-result.bin")); + assert.deepEqual(await readFile(opened[0] as string), content); + } finally { + await rm(expectedRoot, { recursive: true, force: true }); + } +}); diff --git a/apps/desktop/src/main/preview-preflight-probes.ts b/apps/desktop/src/main/preview-preflight-probes.ts new file mode 100644 index 00000000000..09de1ced522 --- /dev/null +++ b/apps/desktop/src/main/preview-preflight-probes.ts @@ -0,0 +1,176 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +/** + * The OS-touching half of the local preview preflight: the probes that decide + * a capability's status by actually trying something. Classification lives in + * preview-preflight.ts and never imports this file, so the statuses stay + * testable without a filesystem or a socket. + * + * Two rules hold for everything here. A probe reports a connection or access + * outcome as a value, never as a throw, so the classifier — not an exception + * handler — decides what the outcome proves. And a probe never reads a + * response body or file content back into the result: the evidence a caller + * sees is a status code, an errno, and a path. + */ + +import { randomUUID } from 'node:crypto'; +import { mkdir, readFile, rm, writeFile } from 'node:fs/promises'; +import { request as httpRequest } from 'node:http'; +import { request as httpsRequest } from 'node:https'; +import { join } from 'node:path'; +import { browserAutomationAvailable, browserViewHost } from './browser/browser-host.js'; +import type { + LoopbackProbe, + PreviewPreflightAuthority, + StagingRootProbe, +} from './preview-preflight.js'; + +/** + * A preview endpoint on loopback either answers immediately or is not up yet; + * a long wait here would only turn a fast, honest `unknown` into a slow one. + */ +export const LOOPBACK_PROBE_TIMEOUT_MS = 2_000; + +const PROBE_CONTENT = 'maka-preview-preflight'; + +export interface PreviewPreflightProbeDeps { + /** The Artifact staging root, or undefined when this client has none. */ + readonly stagingRoot: () => string | undefined; + /** Overrides the loopback deadline so a test can reach it without a real wait. */ + readonly loopbackTimeoutMs?: number; +} + +export function createPreviewPreflightAuthority( + deps: PreviewPreflightProbeDeps, +): PreviewPreflightAuthority { + const loopbackTimeoutMs = deps.loopbackTimeoutMs ?? LOOPBACK_PROBE_TIMEOUT_MS; + return { + guiSurfaceAvailable: () => browserAutomationAvailable(), + browserDrivable: ({ sessionId, signal }) => { + signal.throwIfAborted(); + return browserViewHost().canDrive(sessionId, 'observe', { signal }); + }, + probeStagingRoot: ({ signal }) => probeStagingRoot(deps.stagingRoot(), signal), + probeLoopback: ({ origin, signal }) => probeLoopback(origin, signal, loopbackTimeoutMs), + }; +} + +async function probeStagingRoot( + root: string | undefined, + signal: AbortSignal, +): Promise { + signal.throwIfAborted(); + if (root === undefined) return { kind: 'not_configured' }; + const path = join(root, `.preview-preflight-${randomUUID()}.probe`); + try { + // Creating the root is what the Open action itself does before it + // materializes an artifact, so probing that way answers the question the + // caller actually has rather than reporting a directory nobody made yet. + await mkdir(root, { recursive: true }); + await writeFile(path, PROBE_CONTENT, { encoding: 'utf8', mode: 0o600 }); + // Write-then-read, because a directory that accepts a write but hands back + // something else is exactly the "not the filesystem you think" case this + // capability is meant to catch. + const read = await readFile(path, { encoding: 'utf8' }); + if (read !== PROBE_CONTENT) { + return { kind: 'unavailable', root, cause: 'a probe file read back different content' }; + } + return { kind: 'round_tripped', root }; + } catch (error) { + return { kind: 'unavailable', root, cause: describeCause(error) }; + } finally { + await rm(path, { force: true }).catch(() => undefined); + } +} + +/** + * Every terminal path settles, and the deadline lives here rather than on the + * socket. Both matter for the same reason: a server answering `101 Switching + * Protocols` makes Node route the exchange to `upgrade` and close the request + * without ever emitting `response` or `error`, taking the socket — and any + * socket-bound timeout — with it. A probe that only listened for those two + * events outlived both its stated bound and its caller's cancellation. + */ +function probeLoopback( + origin: string, + signal: AbortSignal, + timeoutMs: number, +): Promise { + signal.throwIfAborted(); + const url = new URL(origin); + const send = url.protocol === 'https:' ? httpsRequest : httpRequest; + return new Promise((resolve, reject) => { + let settled = false; + let deadline: ReturnType | undefined; + + function finish(settle: () => void): void { + if (settled) return; + settled = true; + if (deadline !== undefined) clearTimeout(deadline); + signal.removeEventListener('abort', abandon); + // Marked settled first, so the teardown's own close/error cannot re-enter. + probe.destroy(); + settle(); + } + function answer(result: LoopbackProbe): void { + finish(() => resolve(result)); + } + // An aborted turn is a cancellation, not a statement about the endpoint. + function abandon(): void { + finish(() => reject(signal.reason)); + } + + const probe = send(url, { method: 'GET' }, (response) => { + const status = response.statusCode ?? 0; + // Drain without reading: the status is the whole observation, and a + // preview page's content has no business in a capability report. + response.resume(); + answer({ kind: 'answered', status }); + }); + probe.on('upgrade', (response, socket) => { + // The upgraded socket is detached from the request, so destroying the + // request would leave it open. A 101 is still an answer: something + // listened at that address, which is all this capability claims. + socket.destroy(); + answer({ kind: 'answered', status: response.statusCode ?? 0 }); + }); + probe.on('error', (error) => { + if (signal.aborted) abandon(); + else answer({ kind: 'no_answer', cause: describeCause(error) }); + }); + // The catch-all: a socket that closes without telling us anything else. + probe.on('close', () => answer({ kind: 'no_answer', cause: 'closed without a response' })); + deadline = setTimeout( + () => answer({ kind: 'no_answer', cause: `no response within ${timeoutMs}ms` }), + timeoutMs, + ); + signal.addEventListener('abort', abandon, { once: true }); + probe.end(); + }); +} + +/** Prefer the errno: ECONNREFUSED says more to a caller than its sentence does. */ +function describeCause(error: unknown): string { + if (typeof error === 'object' && error !== null && 'code' in error) { + const code = (error as { readonly code?: unknown }).code; + if (typeof code === 'string' && code !== '') return code; + } + return error instanceof Error ? error.message : String(error); +} diff --git a/apps/desktop/src/main/preview-preflight.ts b/apps/desktop/src/main/preview-preflight.ts new file mode 100644 index 00000000000..505ff549295 --- /dev/null +++ b/apps/desktop/src/main/preview-preflight.ts @@ -0,0 +1,490 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +/** + * Local preview capability preflight — what this client can actually show, + * asked before anything is attempted. + * + * Showing generated HTML locally depends on four independent things, and each + * one fails differently: the client may have no GUI surface at all, the + * embedded browser refuses `file://` by policy, the process that writes the + * file may not share a filesystem with the surface that would render it, and + * the conversation's own view may not be the one on screen. Attempting the + * preview and reading the first error conflates all four. A refused loopback + * connection is the worst of them: "nothing listening yet", "server still + * starting", and "separate network namespace" are indistinguishable at the + * socket, yet the refusal reads like proof of the last one. + * + * So every capability below answers with one of three statuses and names the + * observation behind it. `verified` requires a positive observation. + * `unsupported` is a settled product boundary this code can state without + * probing. Everything else is `unknown` — explicitly NOT a claim that a + * sandbox, an isolated localhost, or a missing capability caused it. A caller + * may describe a preview as ready only on `verified`, which is why `ready` + * below is derived from positive evidence alone and never from the absence of + * an error. + * + * Pure by construction: every observation arrives through + * PreviewPreflightAuthority, so each status — including the ones that need a + * refused socket or an unreadable directory — is reachable from a plain unit + * test. The probes that touch the OS live in preview-preflight-probes.ts. + */ + +import type { MakaTool } from '@maka/runtime/tool-runtime'; +import { z } from 'zod'; +import { parseNavigable } from './browser/logic.js'; + +export type PreviewCapabilityStatus = 'verified' | 'unsupported' | 'unknown'; + +export type PreviewCapabilityId = + | 'gui_surface' + | 'url_schemes' + | 'filesystem_visibility' + | 'browser_reachability' + | 'loopback_endpoint'; + +export interface PreviewCapability { + readonly id: PreviewCapabilityId; + readonly status: PreviewCapabilityStatus; + /** The observation this status rests on. Every status names one. */ + readonly evidence: string; + /** What the status does not prove, or why the boundary exists. */ + readonly boundary?: string; +} + +/** An endpoint nobody named was not found wanting; it was never asked about. */ +export type PreviewEndpointStatus = PreviewCapabilityStatus | 'not_checked'; + +export interface PreviewPreflightResult { + readonly kind: 'preview_preflight'; + /** + * Whether this client can display a page at all, independent of any + * particular endpoint: a GUI surface plus a view that accepts actions. + */ + readonly surface: PreviewCapabilityStatus; + /** The health of the named endpoint, kept separate from the surface above. */ + readonly endpoint: PreviewEndpointStatus; + /** + * True only when a page can be shown on positive evidence right now: a + * verified surface AND an endpoint that answered. An absent error is never + * enough. Read `surface` and `endpoint` to see which half is missing — a + * false `ready` with no endpoint named is not a capability finding. + */ + readonly ready: boolean; + readonly capabilities: readonly PreviewCapability[]; + readonly summary: string; + /** The supported handoff to use instead. Always present when `ready` is false. */ + readonly alternative?: string; +} + +/** An observation that may not have been obtainable, kept distinct from a negative one. */ +export type Observed = + | { readonly ok: true; readonly value: T } + | { readonly ok: false; readonly cause: string }; + +export type StagingRootProbe = + | { readonly kind: 'round_tripped'; readonly root: string } + | { readonly kind: 'unavailable'; readonly root: string; readonly cause: string } + | { readonly kind: 'not_configured' }; + +export type LoopbackProbe = + // Any HTTP status answers the question: a 404 still proves something listened. + | { readonly kind: 'answered'; readonly status: number } + | { readonly kind: 'no_answer'; readonly cause: string }; + +export interface PreviewPreflightAuthority { + /** Whether a Desktop browser view host is registered for this client. */ + guiSurfaceAvailable(): boolean | Promise; + /** Whether this conversation's embedded view accepts an observe action right now. */ + browserDrivable(input: { + readonly sessionId: string; + readonly signal: AbortSignal; + }): boolean | Promise; + /** Round-trip a probe file under the Artifact staging root. */ + probeStagingRoot(input: { readonly signal: AbortSignal }): StagingRootProbe | Promise; + /** Bounded loopback request. A connection outcome is a result, never a throw. */ + probeLoopback(input: { + readonly origin: string; + readonly signal: AbortSignal; + }): LoopbackProbe | Promise; +} + +/** + * The supported handoff: it needs neither `file://` navigation in the embedded + * browser nor an ad-hoc localhost server, so it stays available even when every + * capability below is unknown. Its own honesty caveat is repeated here, because + * a caller reading this line is exactly the one about to overclaim. + */ +export const ARTIFACT_PREVIEW_HANDOFF = + 'Use the ArtifactPreview tool: it turns an HTML Artifact in this session into a Desktop-managed, temporary HTTP URL the browser can open, with no shell server and no file:// navigation. Its `reachable` flag reports a Desktop-side HTTP check and not a browser load, so confirm rendering by navigating and observing the page. If it fails, fall back to Generated Files → Save As or Show in Folder rather than reporting that the preview opened.'; + +/** Addresses the embedded browser is asked about, one per scheme worth reporting. */ +const SCHEME_PROBES = [ + 'https://example.invalid/preview.html', + 'http://127.0.0.1:8765/preview.html', + 'file:///tmp/preview.html', + 'data:text/html,

preview

', +] as const; + +/** + * `localhost` is deliberately not accepted: it resolves through the host's + * name resolution and can point somewhere other than the loopback interface, + * which would turn a capability check into an arbitrary outbound request. + */ +const LOOPBACK_HOSTS = new Set(['127.0.0.1', '[::1]']); + +export interface SchemeProbeResult { + readonly scheme: string; + readonly navigable: boolean; +} + +/** + * Ask the real address policy rather than restating it. A hardcoded list would + * keep reporting `file:` as rejected long after someone changed the rule. + */ +export function probeNavigableSchemes(): readonly SchemeProbeResult[] { + return SCHEME_PROBES.map((sample) => ({ + scheme: sample.slice(0, sample.indexOf(':') + 1), + navigable: parseNavigable(sample) !== null, + })); +} + +export function classifyGuiSurface(observed: Observed): PreviewCapability { + if (!observed.ok) { + return { + id: 'gui_surface', + status: 'unknown', + evidence: `Could not determine whether a browser view host is registered: ${observed.cause}.`, + boundary: 'This does not mean the client is headless; the check itself did not complete.', + }; + } + if (observed.value) { + return { + id: 'gui_surface', + status: 'verified', + evidence: 'A Desktop browser view host is registered for this client.', + }; + } + return { + id: 'gui_surface', + status: 'unsupported', + evidence: 'No Desktop browser view host is registered.', + boundary: + 'Browser automation exists only inside the desktop app, so this runtime has no embedded view to preview into. Nothing this runtime does will make one appear.', + }; +} + +export function classifyUrlSchemes(probes: readonly SchemeProbeResult[]): PreviewCapability { + const navigable = probes.filter((probe) => probe.navigable).map((probe) => probe.scheme); + const rejected = probes.filter((probe) => !probe.navigable).map((probe) => probe.scheme); + const evidence = `The embedded browser's address policy accepted ${describeSchemes(navigable)} and rejected ${describeSchemes(rejected)}.`; + if (probes.some((probe) => probe.scheme === 'file:' && probe.navigable)) { + return { id: 'url_schemes', status: 'verified', evidence }; + } + return { + id: 'url_schemes', + status: 'unsupported', + evidence, + boundary: + 'Admission requires an HTTP origin. file:// is rejected so that a typed address or an in-page link can never reach the local filesystem, so a generated file cannot be previewed by pointing the embedded browser at its path. This is a deliberate boundary, not a missing feature: use ArtifactPreview to serve the same content over http:// instead of working around it.', + }; +} + +export function classifyFilesystemVisibility(observed: Observed): PreviewCapability { + if (!observed.ok) { + return { + id: 'filesystem_visibility', + status: 'unknown', + evidence: `Could not probe the Artifact staging root: ${observed.cause}.`, + boundary: 'The probe did not complete, so nothing was learned about the path.', + }; + } + const probe = observed.value; + if (probe.kind === 'not_configured') { + return { + id: 'filesystem_visibility', + status: 'unsupported', + evidence: 'This client has no Artifact staging root configured.', + boundary: 'Without a staging root there is no local path for a preview to be materialized into.', + }; + } + if (probe.kind === 'round_tripped') { + return { + id: 'filesystem_visibility', + status: 'verified', + evidence: `Created, read back, and removed a probe file under ${probe.root}.`, + boundary: + 'This proves only that the Desktop main process reaches that directory. It does not prove that the shell or sandboxed runtime which generates a file writes into the same filesystem.', + }; + } + return { + id: 'filesystem_visibility', + status: 'unknown', + evidence: `A probe file under ${probe.root} could not be round-tripped: ${probe.cause}.`, + boundary: + 'A failed access does not distinguish a missing directory, a permission denial, and a sandbox that hides the path. Read the reported cause rather than assuming isolation.', + }; +} + +export function classifyBrowserReachability(input: { + readonly guiSurface: PreviewCapability; + /** Omitted when the view was never asked, which only a settled `unsupported` justifies. */ + readonly drivable?: Observed; +}): PreviewCapability { + if (input.guiSurface.status === 'unsupported') { + return { + id: 'browser_reachability', + status: 'unsupported', + evidence: 'There is no registered browser view host, so this client has no embedded view to reach.', + }; + } + if (!input.drivable) { + return { + id: 'browser_reachability', + status: 'unknown', + evidence: "This conversation's view was not asked whether it accepts actions.", + }; + } + if (!input.drivable.ok) { + return { + id: 'browser_reachability', + status: 'unknown', + evidence: `Could not determine whether this conversation's view accepts actions: ${input.drivable.cause}.`, + }; + } + if (input.drivable.value) { + return { + id: 'browser_reachability', + status: 'verified', + evidence: "This conversation's embedded view accepts an observe action, so a page can be driven into it now.", + }; + } + return { + id: 'browser_reachability', + status: 'unknown', + evidence: "This conversation's embedded view refused an observe action right now.", + boundary: + 'Every browser action must run in the conversation the user is looking at. A refusal usually means this conversation is not the one on screen, which is transient and visibility-scoped — it does not prove the browser is unreachable or that a sandbox separates it.', + }; +} + +export function classifyLoopbackEndpoint(input: { + readonly origin: string; + readonly observed: Observed; +}): PreviewCapability { + if (!input.observed.ok) { + return { + id: 'loopback_endpoint', + status: 'unknown', + evidence: `Could not probe ${input.origin}: ${input.observed.cause}.`, + }; + } + const probe = input.observed.value; + if (probe.kind === 'answered') { + return { + id: 'loopback_endpoint', + status: 'verified', + evidence: `${input.origin} answered with HTTP ${probe.status} to the Desktop main process.`, + boundary: + 'The answer proves something is listening for this process. It does not prove the embedded browser shares that loopback namespace.', + }; + } + // The rule this whole tool exists for: a connection outcome can produce + // `verified` or `unknown`, never `unsupported`. Nothing observable at a + // refused socket distinguishes a slow start from an isolated namespace. + return { + id: 'loopback_endpoint', + status: 'unknown', + evidence: `Nothing answered at ${input.origin}: ${probe.cause}.`, + boundary: + 'A refused or timed-out connection does not prove sandbox isolation, a separate localhost namespace, or that the endpoint is unusable. It shows only that nothing answered this process at that address at this moment. Retry once the server reports that it is listening, or read the server\'s own startup output.', + }; +} + +/** + * Reject anything that would turn a capability check into an arbitrary + * outbound request, and say what is accepted instead. Mirrors the bounded + * loopback contract the background-task health check uses (#5237). + */ +export function assertLoopbackOrigin(raw: string): string { + const guidance = 'Pass a loopback URL such as http://127.0.0.1:8765/preview.html.'; + let url: URL; + try { + url = new URL(raw); + } catch { + throw new Error(`Not a URL: ${JSON.stringify(raw)}. ${guidance}`); + } + if (url.protocol !== 'http:' && url.protocol !== 'https:') { + throw new Error(`Only http:// and https:// origins can be probed, not ${url.protocol}. ${guidance}`); + } + if (url.username !== '' || url.password !== '') { + throw new Error(`Credentials are not accepted in a probe URL. ${guidance}`); + } + if (url.search !== '' || url.hash !== '') { + throw new Error(`A probe URL carries no query or fragment. ${guidance}`); + } + if (!LOOPBACK_HOSTS.has(url.hostname)) { + throw new Error( + `Only the loopback interface can be probed, not ${JSON.stringify(url.hostname)}. "localhost" is excluded because it resolves through the host and can point elsewhere. ${guidance}`, + ); + } + return url.toString(); +} + +/** + * Whether this client can display a page at all. Deliberately independent of + * any endpoint, so "nothing was named to check" cannot read as "this client + * cannot preview". A settled boundary on either half settles the whole. + */ +export function derivePreviewSurface(input: { + readonly guiSurface: PreviewCapability; + readonly browserReachability: PreviewCapability; +}): PreviewCapabilityStatus { + const statuses = [input.guiSurface.status, input.browserReachability.status]; + if (statuses.includes('unsupported')) return 'unsupported'; + return statuses.every((status) => status === 'verified') ? 'verified' : 'unknown'; +} + +export function summarizePreviewPreflight(input: { + readonly capabilities: readonly PreviewCapability[]; + readonly surface: PreviewCapabilityStatus; + readonly endpoint: PreviewEndpointStatus; + readonly ready: boolean; +}): string { + if (input.ready) { + return 'A page can be shown now: this client has a usable preview surface and the endpoint answered.'; + } + if (input.surface === 'unsupported') { + return 'This client cannot display a page locally. The unsupported entries below say why, and that will not change at runtime — take the alternative rather than retrying.'; + } + if (input.endpoint === 'not_checked') { + return input.surface === 'verified' + ? 'This client has a usable preview surface. No endpoint was named, so none was checked — an absent question, not a negative result.' + : 'No endpoint was named, so none was checked, and the surface itself is unproven. The entries below report what was observed, not a cause.'; + } + const counts = { verified: 0, unsupported: 0, unknown: 0 }; + for (const capability of input.capabilities) counts[capability.status] += 1; + return ( + `No verified way to show a page right now — ${counts.verified} verified, ${counts.unsupported} unsupported, ${counts.unknown} unknown. ` + + 'Only the unsupported entries are settled boundaries; an unknown entry reports what was observed and not a cause.' + ); +} + +function describeSchemes(schemes: readonly string[]): string { + return schemes.length === 0 ? 'nothing' : schemes.join(', '); +} + +/** + * Cancellation is not an observation. Without this the abort that ends a turn + * would be reported as an `unknown` capability, which reads like a finding. + */ +async function observe(signal: AbortSignal, run: () => T | Promise): Promise> { + try { + return { ok: true, value: await run() }; + } catch (error) { + if (signal.aborted) throw error; + return { ok: false, cause: error instanceof Error ? error.message : String(error) }; + } +} + +/** The local-preview capability preflight, published as its own capability offer. */ +export function buildPreviewPreflightTools( + authority: PreviewPreflightAuthority, +): readonly MakaTool[] { + const preflight: MakaTool<{ origin?: string }, PreviewPreflightResult> = { + name: 'preview_preflight', + displayName: 'Check local preview capabilities', + description: + 'Report what this Maka client can actually preview locally BEFORE attempting it: GUI availability, which URL schemes the embedded browser accepts, whether the Artifact staging path round-trips, and whether this conversation\'s view accepts actions. ' + + 'Pass `origin` to also check a loopback endpoint you started. Each capability answers verified, unsupported, or unknown, with the observation behind it — an unknown reports what was seen and never asserts a sandbox or isolation cause. ' + + '`surface` says whether this client can display a page at all and is reported separately from `endpoint`, which is `not_checked` when no origin was passed; `ready` needs both. ' + + 'Call this instead of inferring a boundary from a failed navigation or a refused connection.', + parameters: z + .object({ + origin: z + .string() + .min(1) + .max(2000) + .optional() + .describe( + 'Optional loopback URL to probe, such as http://127.0.0.1:8765/preview.html. Only 127.0.0.1 and [::1] are accepted; no credentials, query, or fragment.', + ), + }) + .strict(), + categoryHint: 'read', + recoveryMode: 'replay_safe', + impl: async ({ origin: rawOrigin }, { sessionId, abortSignal }) => { + abortSignal.throwIfAborted(); + // Validate before probing so a malformed origin is an argument error the + // caller can fix, not an `unknown` capability that reads like a finding. + const origin = rawOrigin === undefined ? undefined : assertLoopbackOrigin(rawOrigin); + + const guiSurface = classifyGuiSurface( + await observe(abortSignal, () => authority.guiSurfaceAvailable()), + ); + const urlSchemes = classifyUrlSchemes(probeNavigableSchemes()); + const filesystemVisibility = classifyFilesystemVisibility( + await observe(abortSignal, () => authority.probeStagingRoot({ signal: abortSignal })), + ); + // Skip the drive check when there is demonstrably no view host: asking + // would throw, and an exception dressed as `unknown` would hide the + // settled `unsupported` answer the caller needs. + const browserReachability = classifyBrowserReachability({ + guiSurface, + ...(guiSurface.status === 'unsupported' + ? {} + : { + drivable: await observe(abortSignal, () => + authority.browserDrivable({ sessionId, signal: abortSignal }), + ), + }), + }); + const loopbackEndpoint = + origin === undefined + ? undefined + : classifyLoopbackEndpoint({ + origin, + observed: await observe(abortSignal, () => + authority.probeLoopback({ origin, signal: abortSignal }), + ), + }); + + const capabilities = [ + guiSurface, + urlSchemes, + filesystemVisibility, + browserReachability, + ...(loopbackEndpoint ? [loopbackEndpoint] : []), + ]; + const surface = derivePreviewSurface({ guiSurface, browserReachability }); + const endpoint: PreviewEndpointStatus = loopbackEndpoint?.status ?? 'not_checked'; + const ready = surface === 'verified' && endpoint === 'verified'; + return { + kind: 'preview_preflight', + surface, + endpoint, + ready, + capabilities, + summary: summarizePreviewPreflight({ capabilities, surface, endpoint, ready }), + ...(ready ? {} : { alternative: ARTIFACT_PREVIEW_HANDOFF }), + }; + }, + }; + return [preflight as MakaTool]; +} diff --git a/apps/desktop/src/main/runtime-host-artifacts-ipc-main.ts b/apps/desktop/src/main/runtime-host-artifacts-ipc-main.ts index 2d7b3b0911f..d7ef8334949 100644 --- a/apps/desktop/src/main/runtime-host-artifacts-ipc-main.ts +++ b/apps/desktop/src/main/runtime-host-artifacts-ipc-main.ts @@ -55,12 +55,25 @@ type RuntimeHostAttachmentPreviewIpcDeps = Pick< const ATTACHMENT_PREVIEW_LIMIT_EXCEEDED = Symbol("attachment-preview-limit-exceeded"); +const ARTIFACT_PRESENTATION_DIR = "maka-runtime-host-artifacts"; + +/** + * Where the Open action materializes an artifact before handing it to the + * operating system. Exported so the local preview preflight probes the very + * directory this path uses, instead of a plausible-looking stand-in; omit the + * epoch to name the root shared by every Host generation. + */ +export function artifactPresentationRoot(hostEpoch?: string): string { + return hostEpoch === undefined + ? join(tmpdir(), ARTIFACT_PRESENTATION_DIR) + : join(tmpdir(), ARTIFACT_PRESENTATION_DIR, hostEpoch); +} + export function registerRuntimeHostArtifactsIpc( deps: RuntimeHostArtifactsIpcDeps, ): void { const presentationRoot = - deps.presentationRoot ?? - join(tmpdir(), "maka-runtime-host-artifacts", deps.client.hostEpoch); + deps.presentationRoot ?? artifactPresentationRoot(deps.client.hostEpoch); handleReconnectableRead( deps.ipcMain, diff --git a/apps/desktop/src/main/runtime-host-boot.ts b/apps/desktop/src/main/runtime-host-boot.ts index 8ecd08ec8fc..21337eb79d1 100644 --- a/apps/desktop/src/main/runtime-host-boot.ts +++ b/apps/desktop/src/main/runtime-host-boot.ts @@ -186,9 +186,14 @@ import { createClientSettingsEffects } from "./client-settings-effects.js"; import { registerClientSettingsIpc } from "./client-settings-ipc-main.js"; import { startClientSettingsWatcher } from "./client-settings-watcher.js"; import { registerRuntimeHostGitHubCopilotIpc } from "./runtime-host-github-copilot-ipc-main.js"; -import { registerRuntimeHostArtifactsIpc } from "./runtime-host-artifacts-ipc-main.js"; +import { + artifactPresentationRoot, + registerRuntimeHostArtifactsIpc, +} from "./runtime-host-artifacts-ipc-main.js"; import { ManagedArtifactPreview } from './managed-artifact-preview.js'; import { buildManagedArtifactPreviewTools } from './managed-artifact-preview-tools.js'; +import { buildPreviewPreflightTools } from "./preview-preflight.js"; +import { createPreviewPreflightAuthority } from "./preview-preflight-probes.js"; import type { DesktopRuntimeHostClient } from "./runtime-host-client.js"; import type { DesktopRuntimeHostCandidateControls, @@ -1082,6 +1087,32 @@ const createLocalRuntimeHostManager = () => createRuntimeHostDesktopManager( "Use durable Rive workflows through this Desktop client.", tools: [riveWorkflowTool], }, + // Deliberately NOT part of the Browser offer. Every tool published + // there is admitted against an Origin resolved from the view's + // current URL, so a capability check would need a page already + // loaded — and would fail with the very "requires an HTTP origin" + // error it exists to explain. + { + offerId: "desktop_preview", + label: "Local preview", + description: + "Report what this Desktop client can actually preview locally before a preview is attempted.", + tools: buildPreviewPreflightTools( + createPreviewPreflightAuthority({ + // The generation-specific directory, resolved per call against + // the live target: reporting on any other path would describe a + // directory the Open action never touches. No live target means + // no staging root to report, which the probe states as such + // rather than inventing one. + stagingRoot: () => { + if (!scope || !runtimeHostManager?.ownsScope(scope)) return undefined; + const target = runtimePolicyTargetsByEpoch.get(scope.targetEpoch); + if (!target?.isActive()) return undefined; + return artifactPresentationRoot(target.client.hostEpoch); + }, + }), + ), + }, // One offer per MCP server keeps grant contracts server-scoped: a // server change re-prompts only that server's tools. ...[...mcpServers.keys()].sort().map((serverId) => ({ From 1d2571f2d56121007a39f4ca81cff23c35338fdb Mon Sep 17 00:00:00 2001 From: ying-hua <60057611+ying-hua@users.noreply.github.com> Date: Fri, 25 Sep 2026 11:55:20 -0400 Subject: [PATCH 2/3] fix(harness): narrow preview preflight to what it can prove MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Drop filesystem_visibility. HTML Open always goes through ManagedArtifactPreview, which never touches the staging root, so the capability measured the non-HTML Show in Folder path rather than a preview question. It also let a transient target loss during a reconnect surface as `unsupported`, the status reserved for settled boundaries. The artifactPresentationRoot export, its boot closure, the wiring test and the mkdir side effect inside a `read` tool go with it. Drop `ready`. A listener's answer — a 404 included — proves a listener, not that the page exists or loads, and a readiness flag built from it claimed more than the evidence held. `surface` and `endpoint` stand on their own; page load remains #5235's question. Probe with HEAD / against the origin instead of GET on the caller's path, so a capability check cannot trip a dev-server route with side effects. The absolute deadline is now the single catch-all, replacing a `close` arm no real server reaches. Publish the preflight inside desktop_artifact_preview, so the ArtifactPreview route it hands off to is always granted with it, and defer to that tool's description instead of paraphrasing a copy that had already drifted. Report a failed probe as "no usable HTTP response" rather than "nothing answered", since a TLS failure or a reset after accept means something did listen. Rename browser_reachability to browser_view and split the host-less case out as a constant, removing a branch the tool could never reach. Generated-by: Claude Opus 5.5 via Claude Code Co-Authored-By: Claude Opus 5.5 (1M context) --- .../main/__tests__/preview-preflight.test.ts | 424 ++++++------------ .../runtime-host-artifacts-ipc-main.test.ts | 62 +-- .../src/main/preview-preflight-probes.ts | 83 +--- apps/desktop/src/main/preview-preflight.ts | 235 ++++------ .../main/runtime-host-artifacts-ipc-main.ts | 17 +- apps/desktop/src/main/runtime-host-boot.ts | 59 +-- 6 files changed, 279 insertions(+), 601 deletions(-) diff --git a/apps/desktop/src/main/__tests__/preview-preflight.test.ts b/apps/desktop/src/main/__tests__/preview-preflight.test.ts index d2478c28a06..6130e080e64 100644 --- a/apps/desktop/src/main/__tests__/preview-preflight.test.ts +++ b/apps/desktop/src/main/__tests__/preview-preflight.test.ts @@ -20,23 +20,20 @@ /** * Local preview preflight: the verified / unsupported / unknown classification * for every capability, the rule that a connection outcome may never become - * `unsupported`, and the probes driven against a real directory and a real - * loopback socket. + * `unsupported`, the rule that no result claims a page is ready, and the + * loopback probe driven against real sockets. */ import { strict as assert } from 'node:assert'; -import { writeFile, mkdtemp, rm } from 'node:fs/promises'; import { createServer, type Server } from 'node:http'; import type { Socket } from 'node:net'; -import { tmpdir } from 'node:os'; -import { join } from 'node:path'; import { after, describe, it } from 'node:test'; import type { MakaTool, MakaToolContext } from '@maka/runtime/tool-runtime'; +import { parseNavigable } from '../browser/logic.js'; import { assertLoopbackOrigin, buildPreviewPreflightTools, - classifyBrowserReachability, - classifyFilesystemVisibility, + classifyBrowserView, classifyGuiSurface, classifyLoopbackEndpoint, classifyUrlSchemes, @@ -69,32 +66,23 @@ function healthyAuthority( return { guiSurfaceAvailable: () => true, browserDrivable: () => true, - probeStagingRoot: () => ({ kind: 'round_tripped', root: '/tmp/maka-runtime-host-artifacts' }), probeLoopback: () => ({ kind: 'answered', status: 200 }), ...overrides, }; } -function preflightTool( - authority: PreviewPreflightAuthority, -): MakaTool<{ origin?: string }, PreviewPreflightResult> { - const [tool] = buildPreviewPreflightTools(authority); - assert.ok(tool, 'the preflight offer publishes a tool'); - return tool as MakaTool<{ origin?: string }, PreviewPreflightResult>; -} - function run( authority: PreviewPreflightAuthority, args: { origin?: string } = {}, signal?: AbortSignal, ): Promise { - return Promise.resolve(preflightTool(authority).impl(args, ctx(signal))); + const [tool] = buildPreviewPreflightTools(authority); + assert.ok(tool, 'the preflight publishes a tool'); + const preflight = tool as MakaTool<{ origin?: string }, PreviewPreflightResult>; + return Promise.resolve(preflight.impl(args, ctx(signal))); } -function capability( - result: PreviewPreflightResult, - id: PreviewCapabilityId, -): PreviewCapability { +function capability(result: PreviewPreflightResult, id: PreviewCapabilityId): PreviewCapability { const found = result.capabilities.find((entry) => entry.id === id); assert.ok(found, `the report covers ${id}`); return found; @@ -111,141 +99,94 @@ function assertClaimsNoCause(capability: PreviewCapability): void { } describe('preview preflight capability classification', () => { - it('reports the real address policy rather than a restatement of it', () => { + it('reports exactly what the address policy answers', () => { const probes = probeNavigableSchemes(); - const navigable = new Map(probes.map((probe) => [probe.scheme, probe.navigable])); - assert.equal(navigable.get('https:'), true); - assert.equal(navigable.get('http:'), true); - assert.equal(navigable.get('file:'), false); - assert.equal(navigable.get('data:'), false); + assert.deepEqual( + probes.map((probe) => probe.scheme), + ['https:', 'http:', 'file:', 'data:'], + ); + for (const probe of probes) { + assert.equal( + probe.navigable, + parseNavigable(probe.sample) !== null, + `${probe.scheme} agrees with parseNavigable`, + ); + } }); - it('calls a rejected file:// scheme an unsupported boundary and explains it', () => { + it('calls a rejected file:// an unsupported boundary and names the way around it', () => { const capability = classifyUrlSchemes(probeNavigableSchemes()); assert.equal(capability.status, 'unsupported'); - assert.match(capability.evidence, /accepted https:, http:/u); - assert.match(capability.evidence, /rejected file:, data:/u); + assert.match(capability.evidence, /rejected file:/u); assert.match(capability.boundary ?? '', /requires an HTTP origin/u); - assert.match(capability.boundary ?? '', /deliberate boundary, not a missing feature/u); - // A boundary the caller cannot move must name the way around it. - assert.match(capability.boundary ?? '', /use ArtifactPreview to serve the same content over http/u); + assert.match(capability.boundary ?? '', /use ArtifactPreview/u); }); it('follows the policy if file:// ever becomes navigable', () => { const capability = classifyUrlSchemes([ - { scheme: 'https:', navigable: true }, - { scheme: 'file:', navigable: true }, + { sample: 'https://x.invalid/', scheme: 'https:', navigable: true }, + { sample: 'file:///x', scheme: 'file:', navigable: true }, ]); assert.equal(capability.status, 'verified'); }); it('separates a missing view host from a check that did not complete', () => { assert.equal(classifyGuiSurface({ ok: true, value: true }).status, 'verified'); - - const absent = classifyGuiSurface({ ok: true, value: false }); - assert.equal(absent.status, 'unsupported'); - assert.match(absent.boundary ?? '', /only inside the desktop app/u); + assert.equal(classifyGuiSurface({ ok: true, value: false }).status, 'unsupported'); const unreadable = classifyGuiSurface({ ok: false, cause: 'provider exploded' }); assert.equal(unreadable.status, 'unknown'); assert.match(unreadable.evidence, /provider exploded/u); - assert.match(unreadable.boundary ?? '', /does not mean the client is headless/u); }); - it('keeps a verified staging root from implying the generating process shares it', () => { - const capability = classifyFilesystemVisibility({ - ok: true, - value: { kind: 'round_tripped', root: '/tmp/staging' }, - }); - assert.equal(capability.status, 'verified'); - assert.match(capability.evidence, /Created, read back, and removed a probe file under \/tmp\/staging/u); - assert.match(capability.boundary ?? '', /does not prove that the shell or sandboxed runtime/u); - }); + it('never calls a refused view unsupported', () => { + assert.equal(classifyBrowserView({ ok: true, value: true }).status, 'verified'); - it('reports an unreachable staging root as unknown with its cause', () => { - const capability = classifyFilesystemVisibility({ - ok: true, - value: { kind: 'unavailable', root: '/tmp/staging', cause: 'EACCES' }, - }); - assert.equal(capability.status, 'unknown'); - assert.match(capability.evidence, /EACCES/u); - assert.match(capability.boundary ?? '', /does not distinguish a missing directory/u); - assertClaimsNoCause(capability); - }); + const refused = classifyBrowserView({ ok: true, value: false }); + assert.equal(refused.status, 'unknown'); + assert.match(refused.boundary ?? '', /does not prove the browser is unreachable/u); + assertClaimsNoCause(refused); - it('reports a client with no staging root as unsupported', () => { - const capability = classifyFilesystemVisibility({ ok: true, value: { kind: 'not_configured' } }); - assert.equal(capability.status, 'unsupported'); + assert.equal(classifyBrowserView({ ok: false, cause: 'host gone' }).status, 'unknown'); }); - it('treats a refused view as unknown and visibility-scoped, never as isolation', () => { - const capability = classifyBrowserReachability({ - guiSurface: classifyGuiSurface({ ok: true, value: true }), - drivable: { ok: true, value: false }, - }); - assert.equal(capability.status, 'unknown'); - assert.match(capability.boundary ?? '', /conversation the user is looking at/u); - assert.match(capability.boundary ?? '', /does not prove the browser is unreachable/u); - assertClaimsNoCause(capability); - }); - - it('will not call a view reachable when it was never asked', () => { - const capability = classifyBrowserReachability({ - guiSurface: classifyGuiSurface({ ok: true, value: true }), - }); - assert.equal(capability.status, 'unknown'); - assert.match(capability.evidence, /was not asked/u); - }); - - it('reports reachability as unsupported only when there is no view host at all', () => { - const capability = classifyBrowserReachability({ - guiSurface: classifyGuiSurface({ ok: true, value: false }), - drivable: { ok: true, value: false }, - }); - assert.equal(capability.status, 'unsupported'); - }); - - it('accepts any HTTP answer as evidence that something listened', () => { + it('treats any HTTP status as proof of a listener, and of nothing more', () => { const capability = classifyLoopbackEndpoint({ - origin: 'http://127.0.0.1:8765/', + origin: 'http://127.0.0.1:8765', observed: { ok: true, value: { kind: 'answered', status: 404 } }, }); assert.equal(capability.status, 'verified'); - assert.match(capability.evidence, /answered with HTTP 404/u); - // Symmetry: an answer to this process is not an answer for the browser. - assert.match(capability.boundary ?? '', /does not prove the embedded browser shares/u); + assert.match(capability.evidence, /HTTP 404/u); + assert.match(capability.boundary ?? '', /not that any particular page exists there/u); }); it('never turns a failed connection into an unsupported capability', () => { - for (const cause of ['ECONNREFUSED', 'ETIMEDOUT', 'EHOSTUNREACH']) { + for (const cause of ['ECONNREFUSED', 'ETIMEDOUT', 'EHOSTUNREACH', 'DEPTH_ZERO_SELF_SIGNED_CERT']) { const capability = classifyLoopbackEndpoint({ - origin: 'http://127.0.0.1:8765/', + origin: 'http://127.0.0.1:8765', observed: { ok: true, value: { kind: 'no_answer', cause } }, }); assert.equal(capability.status, 'unknown', `${cause} is not a settled boundary`); assert.match(capability.evidence, new RegExp(cause, 'u')); + // Something may well have listened: a TLS failure means it did. + assert.doesNotMatch(capability.evidence, /nothing answered/iu); assert.match(capability.boundary ?? '', /does not prove sandbox isolation/u); - assert.match(capability.boundary ?? '', /nothing answered this process at that address/u); assertClaimsNoCause(capability); } }); }); describe('preview preflight loopback argument', () => { - it('accepts a bounded loopback URL', () => { - assert.equal( - assertLoopbackOrigin('http://127.0.0.1:8765/preview.html'), - 'http://127.0.0.1:8765/preview.html', - ); - assert.equal(assertLoopbackOrigin('http://[::1]:8765/'), 'http://[::1]:8765/'); + it('reduces an accepted URL to its origin', () => { + assert.equal(assertLoopbackOrigin('http://127.0.0.1:8765/preview.html'), 'http://127.0.0.1:8765'); + assert.equal(assertLoopbackOrigin('http://[::1]:8765/'), 'http://[::1]:8765'); }); it('rejects anything that would make the check an arbitrary outbound request', () => { assert.throws(() => assertLoopbackOrigin('example.com'), /Not a URL/u); assert.throws(() => assertLoopbackOrigin('file:///tmp/preview.html'), /Only http:\/\/ and https:\/\//u); - assert.throws(() => assertLoopbackOrigin('http://192.168.1.4:8765/'), /Only the loopback interface/u); - assert.throws(() => assertLoopbackOrigin('http://example.com/'), /Only the loopback interface/u); - // Resolves through the host, so it can point somewhere other than loopback. + assert.throws(() => assertLoopbackOrigin('http://192.168.1.4:8765/'), /Only 127\.0\.0\.1 and \[::1\]/u); + assert.throws(() => assertLoopbackOrigin('http://example.com/'), /Only 127\.0\.0\.1 and \[::1\]/u); assert.throws(() => assertLoopbackOrigin('http://localhost:8765/'), /"localhost" is excluded/u); assert.throws(() => assertLoopbackOrigin('http://user:pw@127.0.0.1:8765/'), /Credentials are not accepted/u); assert.throws(() => assertLoopbackOrigin('http://127.0.0.1:8765/?token=x'), /no query or fragment/u); @@ -254,66 +195,57 @@ describe('preview preflight loopback argument', () => { }); describe('preview preflight tool', () => { - it('claims ready only on a verified surface and an endpoint that answered', async () => { + it('reports surface and endpoint separately and claims no page readiness', async () => { const result = await run(healthyAuthority(), { origin: 'http://127.0.0.1:8765/preview.html' }); - assert.equal(result.ready, true); assert.equal(result.surface, 'verified'); assert.equal(result.endpoint, 'verified'); + assert.equal('ready' in result, false, 'a listener is not a loaded page; #5235 owns that'); assert.equal(result.alternative, undefined); - assert.equal(capability(result, 'loopback_endpoint').status, 'verified'); }); - it('never claims ready from the absence of an error', async () => { - // Everything else is verified; only the endpoint is unproven. + it('does not report a missing page as showable', async () => { + // A 404 proves a listener, not the page. The summary must not claim more. const result = await run( - healthyAuthority({ probeLoopback: () => ({ kind: 'no_answer', cause: 'ECONNREFUSED' }) }), - { origin: 'http://127.0.0.1:8765/preview.html' }, + healthyAuthority({ probeLoopback: () => ({ kind: 'answered', status: 404 }) }), + { origin: 'http://127.0.0.1:8765/missing.html' }, ); - assert.equal(result.ready, false); - assert.equal(result.surface, 'verified'); - assert.equal(result.endpoint, 'unknown'); - assert.equal(capability(result, 'loopback_endpoint').status, 'unknown'); + assert.equal(result.endpoint, 'verified'); + assert.doesNotMatch(result.summary, /can be shown/iu); + assert.match(result.summary, /does not prove the page you want exists there or loads/u); }); it('keeps an unnamed endpoint from reading as a missing capability', async () => { const result = await run(healthyAuthority()); - // The surface stands on its own evidence; only the endpoint is unanswered. assert.equal(result.surface, 'verified'); assert.equal(result.endpoint, 'not_checked'); - assert.equal(result.ready, false); - assert.equal( - result.capabilities.some((entry) => entry.id === 'loopback_endpoint'), - false, - 'an endpoint that was never named produces no capability entry', - ); - }); - - it('separates a settled surface boundary from an unproven one', async () => { - const headless = await run(healthyAuthority({ guiSurfaceAvailable: () => false })); - assert.equal(headless.surface, 'unsupported'); - - const offScreen = await run(healthyAuthority({ browserDrivable: () => false })); - assert.equal(offScreen.surface, 'unknown'); - assert.doesNotMatch(offScreen.summary, /cannot display/u); - }); - - it('points an unready report at the managed preview endpoint', async () => { + assert.equal(result.capabilities.some((entry) => entry.id === 'loopback_endpoint'), false); + }); + + it('gives each summary branch its own answer', async () => { + const summaries = { + unsupported: (await run(healthyAuthority({ guiSurfaceAvailable: () => false }))).summary, + listening: (await run(healthyAuthority(), { origin: 'http://127.0.0.1:8765' })).summary, + unnamed: (await run(healthyAuthority())).summary, + unnamedUnproven: (await run(healthyAuthority({ browserDrivable: () => false }))).summary, + mixed: ( + await run( + healthyAuthority({ probeLoopback: () => ({ kind: 'no_answer', cause: 'ECONNREFUSED' }) }), + { origin: 'http://127.0.0.1:8765' }, + ) + ).summary, + }; + assert.match(summaries.unsupported, /cannot display a page locally/u); + assert.match(summaries.listening, /something is listening/u); + assert.match(summaries.unnamed, /No endpoint was named/u); + assert.match(summaries.unnamedUnproven, /surface itself is unproven/u); + assert.match(summaries.mixed, /2 verified, 1 unsupported, 1 unknown/u); + assert.equal(new Set(Object.values(summaries)).size, 5, 'no two branches share a summary'); + }); + + it('points an uncovered report at ArtifactPreview without restating its guarantees', async () => { const result = await run(healthyAuthority()); assert.match(result.alternative ?? '', /ArtifactPreview/u); - assert.match(result.alternative ?? '', /no shell server and no file:\/\/ navigation/u); - // The handoff must carry its own caveat, not just its happy path. - assert.match(result.alternative ?? '', /not a browser load/u); - }); - - it('covers every capability with an observation behind its status', async () => { - const result = await run(healthyAuthority(), { origin: 'http://127.0.0.1:8765/' }); - assert.deepEqual( - result.capabilities.map((entry) => entry.id), - ['gui_surface', 'url_schemes', 'filesystem_visibility', 'browser_reachability', 'loopback_endpoint'], - ); - for (const entry of result.capabilities) { - assert.notEqual(entry.evidence.trim(), '', `${entry.id} names its observation`); - } + assert.match(result.alternative ?? '', /Its description states what the URL/u); }); it('reports a headless client without asking a view host that cannot answer', async () => { @@ -328,31 +260,21 @@ describe('preview preflight tool', () => { }), ); assert.equal(asked, 0, 'a settled unsupported answer is not downgraded to unknown by a throw'); - assert.equal(capability(result, 'gui_surface').status, 'unsupported'); - assert.equal(capability(result, 'browser_reachability').status, 'unsupported'); - assert.ok(result.alternative, 'a client that cannot preview is still told what to do instead'); + assert.equal(result.surface, 'unsupported'); + assert.equal(capability(result, 'browser_view').status, 'unsupported'); + assert.ok(result.alternative); }); - it('turns a probe that fails into an unknown carrying its cause', async () => { - const result = await run( - healthyAuthority({ - probeStagingRoot: () => { - throw new Error('staging probe exploded'); - }, - }), - ); - const filesystem = capability(result, 'filesystem_visibility'); - assert.equal(filesystem.status, 'unknown'); - assert.match(filesystem.evidence, /staging probe exploded/u); - // Materializing a file is a separate question from displaying a page, so a - // failed staging probe must not drag the surface down with it. - assert.equal(result.surface, 'verified'); + it('keeps an off-screen conversation unproven rather than unsupported', async () => { + const result = await run(healthyAuthority({ browserDrivable: () => false })); + assert.equal(result.surface, 'unknown'); + assert.equal(capability(result, 'browser_view').status, 'unknown'); }); it('rejects a bad origin as an argument error instead of an unknown capability', async () => { await assert.rejects( () => run(healthyAuthority(), { origin: 'http://192.168.1.4:8765/' }), - /Only the loopback interface/u, + /Only 127\.0\.0\.1 and \[::1\]/u, ); }); @@ -372,171 +294,117 @@ describe('preview preflight tool', () => { ), /turn ended/u, ); - - const already = AbortSignal.abort(new Error('already gone')); - await assert.rejects(() => run(healthyAuthority(), {}, already), /already gone/u); + await assert.rejects(() => run(healthyAuthority(), {}, AbortSignal.abort(new Error('gone'))), /gone/u); }); }); -describe('preview preflight probes', () => { +describe('preview preflight loopback probe', () => { const servers: Server[] = []; const sockets: Socket[] = []; - const roots: string[] = []; after(async () => { for (const socket of sockets) socket.destroy(); for (const server of servers) await new Promise((resolve) => server.close(resolve)); - for (const root of roots) await rm(root, { recursive: true, force: true }).catch(() => undefined); }); - async function tempRoot(): Promise { - const root = await mkdtemp(join(tmpdir(), 'maka-preflight-')); - roots.push(root); - return root; - } - - function listen(): Promise { - const server = createServer((_request, response) => { - response.writeHead(404).end('nope'); - }); + async function listen(server: Server): Promise { servers.push(server); - return new Promise((resolve) => server.listen(0, '127.0.0.1', () => resolve(server))); - } - - function originOf(server: Server): string { + await new Promise((resolve) => server.listen(0, '127.0.0.1', () => resolve(undefined))); const address = server.address(); assert.ok(address && typeof address === 'object', 'the probe server is bound'); - return `http://127.0.0.1:${address.port}/`; + return `http://127.0.0.1:${address.port}`; } const signal = new AbortController().signal; - it('round-trips a probe file under a real staging root and leaves nothing behind', async () => { - const root = await tempRoot(); - const authority = createPreviewPreflightAuthority({ stagingRoot: () => root }); - assert.deepEqual(await authority.probeStagingRoot({ signal }), { kind: 'round_tripped', root }); - const { readdir } = await import('node:fs/promises'); - assert.deepEqual(await readdir(root), []); - }); - - it('reports an unusable staging root as unavailable with its errno', async () => { - const root = await tempRoot(); - const blocked = join(root, 'not-a-directory'); - await writeFile(blocked, 'x', 'utf8'); - const authority = createPreviewPreflightAuthority({ stagingRoot: () => blocked }); - const probe = await authority.probeStagingRoot({ signal }); - assert.equal(probe.kind, 'unavailable'); - assert.match(probe.kind === 'unavailable' ? probe.cause : '', /^E[A-Z]+$/u); - }); - - it('reports a client with no staging root instead of inventing a path', async () => { - const authority = createPreviewPreflightAuthority({ stagingRoot: () => undefined }); - assert.deepEqual(await authority.probeStagingRoot({ signal }), { kind: 'not_configured' }); + it('sends HEAD / and never requests the path it was given', async () => { + const seen: string[] = []; + const origin = await listen( + createServer((request, response) => { + seen.push(`${request.method} ${request.url}`); + response.writeHead(404).end(); + }), + ); + const authority = createPreviewPreflightAuthority(); + const [tool] = buildPreviewPreflightTools(authority); + const preflight = tool as MakaTool<{ origin?: string }, PreviewPreflightResult>; + await preflight.impl({ origin: `${origin}/delete-everything` }, ctx()); + assert.deepEqual(seen, ['HEAD /']); }); - it('records the status a live loopback endpoint answers with', async () => { - const server = await listen(); - const authority = createPreviewPreflightAuthority({ stagingRoot: () => undefined }); - assert.deepEqual(await authority.probeLoopback({ origin: originOf(server), signal }), { + it('records the status a live endpoint answers with', async () => { + const origin = await listen(createServer((_request, response) => response.writeHead(404).end())); + assert.deepEqual(await createPreviewPreflightAuthority().probeLoopback({ origin, signal }), { kind: 'answered', status: 404, }); }); - it('answers from the status line without waiting for or reading a large body', async () => { - const server = createServer((_request, response) => { - response.writeHead(200, { 'content-type': 'text/html' }); - // Larger than the probe will ever drain, and deliberately still open when - // the probe answers: a preview page's bytes must not gate the report. - response.write('x'.repeat(4 * 1024 * 1024)); - setTimeout(() => response.end(), 50); - }); - servers.push(server); - await new Promise((resolve) => server.listen(0, '127.0.0.1', () => resolve(undefined))); - const authority = createPreviewPreflightAuthority({ stagingRoot: () => undefined }); - assert.deepEqual(await authority.probeLoopback({ origin: originOf(server), signal }), { - kind: 'answered', - status: 200, - }); - // Give the torn-down body a turn of the loop to surface anything it emits. - await new Promise((resolve) => setTimeout(resolve, 100)); - }); - - /** A server that answers 101, which Node routes to `upgrade`, not `response`. */ - async function listenUpgrading(): Promise { + it('settles on a 101 that never becomes a response', async () => { + // Node routes a 101 to `upgrade` and closes the request without emitting + // `response` or `error`; this is the path that used to hang. const server = createServer(); server.on('connection', (socket) => { socket.once('data', () => { - socket.write( - 'HTTP/1.1 101 Switching Protocols\r\nConnection: Upgrade\r\nUpgrade: websocket\r\n\r\n', - ); + socket.write('HTTP/1.1 101 Switching Protocols\r\nConnection: Upgrade\r\nUpgrade: websocket\r\n\r\n'); }); }); - servers.push(server); - await new Promise((resolve) => server.listen(0, '127.0.0.1', () => resolve(undefined))); - return server; - } - - /** A server that accepts the connection and then says nothing at all. */ - async function listenSilent(): Promise { - const server = createServer(); - server.on('connection', (socket) => sockets.push(socket)); - servers.push(server); - await new Promise((resolve) => server.listen(0, '127.0.0.1', () => resolve(undefined))); - return server; - } - - it('settles on a 101 that never becomes a response', async () => { - const server = await listenUpgrading(); - const authority = createPreviewPreflightAuthority({ stagingRoot: () => undefined }); + const origin = await listen(server); const started = Date.now(); - // A 101 still proves something listened, which is all this capability claims. - assert.deepEqual(await authority.probeLoopback({ origin: originOf(server), signal }), { + assert.deepEqual(await createPreviewPreflightAuthority().probeLoopback({ origin, signal }), { kind: 'answered', status: 101, }); - assert.ok( - Date.now() - started < LOOPBACK_PROBE_TIMEOUT_MS, - 'the answer arrives without waiting out the deadline', - ); + assert.ok(Date.now() - started < LOOPBACK_PROBE_TIMEOUT_MS, 'answered without waiting out the deadline'); }); it('propagates cancellation from a server that is saying nothing', async () => { - const silent = await listenSilent(); + const server = createServer(); + server.on('connection', (socket) => sockets.push(socket)); + const origin = await listen(server); const controller = new AbortController(); - const authority = createPreviewPreflightAuthority({ stagingRoot: () => undefined }); - // Nothing will ever arrive on this socket, so the probe's own abort - // listener is the only thing that can settle it before the deadline. const cancelled = Promise.resolve( - authority.probeLoopback({ origin: originOf(silent), signal: controller.signal }), + createPreviewPreflightAuthority().probeLoopback({ origin, signal: controller.signal }), ); controller.abort(new Error('turn ended')); await assert.rejects(() => cancelled, /turn ended/u); }); it('enforces its deadline against a server that never answers', async () => { - const server = await listenSilent(); - const authority = createPreviewPreflightAuthority({ - stagingRoot: () => undefined, - loopbackTimeoutMs: 120, + const server = createServer(); + server.on('connection', (socket) => sockets.push(socket)); + const origin = await listen(server); + const probe = await createPreviewPreflightAuthority({ loopbackTimeoutMs: 120 }).probeLoopback({ + origin, + signal, }); - const probe = await authority.probeLoopback({ origin: originOf(server), signal }); + assert.deepEqual(probe, { kind: 'no_answer', cause: 'no response within 120ms' }); + }); + + it('reports a TLS handshake against a plain listener as no usable response', async () => { + // Exercises the https branch without a certificate: something listens, + // but nothing usable comes back, which is exactly the case "nothing + // answered" would have misdescribed. + const plain = await listen(createServer((_request, response) => response.writeHead(200).end())); + const origin = plain.replace('http://', 'https://'); + const probe = await createPreviewPreflightAuthority().probeLoopback({ origin, signal }); assert.equal(probe.kind, 'no_answer'); - assert.match(probe.kind === 'no_answer' ? probe.cause : '', /no response within 120ms/u); + const capability = classifyLoopbackEndpoint({ origin, observed: { ok: true, value: probe } }); + assert.equal(capability.status, 'unknown'); + assert.match(capability.evidence, /No usable HTTP response/u); }); it('returns a refused connection as a value, not a throw', async () => { - const server = await listen(); - const origin = originOf(server); + const server = createServer(); + const origin = await listen(server); await new Promise((resolve) => server.close(resolve)); - const authority = createPreviewPreflightAuthority({ stagingRoot: () => undefined }); - const probe = await authority.probeLoopback({ origin, signal }); + servers.splice(servers.indexOf(server), 1); + const probe = await createPreviewPreflightAuthority().probeLoopback({ origin, signal }); assert.equal(probe.kind, 'no_answer'); assert.match(probe.kind === 'no_answer' ? probe.cause : '', /^E[A-Z]+$/u); }); it('reports no GUI surface when no Desktop view host is registered', () => { - const authority = createPreviewPreflightAuthority({ stagingRoot: () => undefined }); - assert.equal(authority.guiSurfaceAvailable(), false); + assert.equal(createPreviewPreflightAuthority().guiSurfaceAvailable(), false); }); }); diff --git a/apps/desktop/src/main/__tests__/runtime-host-artifacts-ipc-main.test.ts b/apps/desktop/src/main/__tests__/runtime-host-artifacts-ipc-main.test.ts index 83aa444a7f1..420765c7e0d 100644 --- a/apps/desktop/src/main/__tests__/runtime-host-artifacts-ipc-main.test.ts +++ b/apps/desktop/src/main/__tests__/runtime-host-artifacts-ipc-main.test.ts @@ -19,15 +19,11 @@ import assert from "node:assert/strict"; import fs, { mkdir, mkdtemp, readFile, readdir, rm, writeFile } from "node:fs/promises"; -import { randomUUID } from "node:crypto"; import { syncBuiltinESMExports } from "node:module"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { test } from "node:test"; -import { - artifactPresentationRoot, - registerRuntimeHostArtifactsIpc, -} from "../runtime-host-artifacts-ipc-main.js"; +import { registerRuntimeHostArtifactsIpc } from "../runtime-host-artifacts-ipc-main.js"; import { ManagedArtifactPreview } from '../managed-artifact-preview.js'; for (const launchFails of [false, true]) { @@ -502,59 +498,3 @@ test("Attachment byte IPC stops a stream that exceeds its preview admission", as { ok: false, reason: "too_large" }, ); }); - -test("Open materializes under the generation root the preview preflight probes", async () => { - // The preflight reports on artifactPresentationRoot(hostEpoch). If the Open - // action ever stopped using that same value, the preflight would describe a - // directory nothing writes to, and its verdict would be about the wrong path. - const hostEpoch = `preflight-wiring-${randomUUID()}`; - const expectedRoot = artifactPresentationRoot(hostEpoch); - const content = Buffer.from("materialized"); - const handlers = new Map(); - const opened: string[] = []; - const artifact = { - id: "artifact-1", - sessionId: "session-1", - turnId: "turn-1", - createdAt: 1, - name: "result.bin", - kind: "image", - sizeBytes: content.byteLength, - mimeType: "image/png", - status: "live", - } as const; - - try { - registerRuntimeHostArtifactsIpc({ - uiLocale: () => "en" as const, - ipcMain: { handle: (channel, handler) => handlers.set(channel, handler as Handler) }, - // No presentationRoot override: this is the production default path. - client: { - hostEpoch, - async getArtifact() { - return artifact; - }, - async streamArtifact( - _sessionId: string, - _artifactId: string, - writeChunk: (chunk: Uint8Array) => Promise, - ) { - await writeChunk(content); - return content.byteLength; - }, - } as never, - mainWindowController: {} as never, - showItemInFolder: (path) => opened.push(path), - }); - - assert.deepEqual(await handlers.get("app:openArtifactPath")?.({}, "session-1", "artifact-1"), { - ok: true, - opened: "result.bin", - }); - assert.equal(opened.length, 1); - assert.equal(opened[0], join(expectedRoot, "session-1", "artifact-1-result.bin")); - assert.deepEqual(await readFile(opened[0] as string), content); - } finally { - await rm(expectedRoot, { recursive: true, force: true }); - } -}); diff --git a/apps/desktop/src/main/preview-preflight-probes.ts b/apps/desktop/src/main/preview-preflight-probes.ts index 09de1ced522..c62c73482bb 100644 --- a/apps/desktop/src/main/preview-preflight-probes.ts +++ b/apps/desktop/src/main/preview-preflight-probes.ts @@ -21,26 +21,16 @@ * The OS-touching half of the local preview preflight: the probes that decide * a capability's status by actually trying something. Classification lives in * preview-preflight.ts and never imports this file, so the statuses stay - * testable without a filesystem or a socket. + * testable without a socket. * - * Two rules hold for everything here. A probe reports a connection or access - * outcome as a value, never as a throw, so the classifier — not an exception - * handler — decides what the outcome proves. And a probe never reads a - * response body or file content back into the result: the evidence a caller - * sees is a status code, an errno, and a path. + * A probe reports a connection outcome as a value, never as a throw, so the + * classifier — not an exception handler — decides what the outcome proves. */ -import { randomUUID } from 'node:crypto'; -import { mkdir, readFile, rm, writeFile } from 'node:fs/promises'; import { request as httpRequest } from 'node:http'; import { request as httpsRequest } from 'node:https'; -import { join } from 'node:path'; import { browserAutomationAvailable, browserViewHost } from './browser/browser-host.js'; -import type { - LoopbackProbe, - PreviewPreflightAuthority, - StagingRootProbe, -} from './preview-preflight.js'; +import type { LoopbackProbe, PreviewPreflightAuthority } from './preview-preflight.js'; /** * A preview endpoint on loopback either answers immediately or is not up yet; @@ -48,17 +38,13 @@ import type { */ export const LOOPBACK_PROBE_TIMEOUT_MS = 2_000; -const PROBE_CONTENT = 'maka-preview-preflight'; - export interface PreviewPreflightProbeDeps { - /** The Artifact staging root, or undefined when this client has none. */ - readonly stagingRoot: () => string | undefined; /** Overrides the loopback deadline so a test can reach it without a real wait. */ readonly loopbackTimeoutMs?: number; } export function createPreviewPreflightAuthority( - deps: PreviewPreflightProbeDeps, + deps: PreviewPreflightProbeDeps = {}, ): PreviewPreflightAuthority { const loopbackTimeoutMs = deps.loopbackTimeoutMs ?? LOOPBACK_PROBE_TIMEOUT_MS; return { @@ -67,46 +53,22 @@ export function createPreviewPreflightAuthority( signal.throwIfAborted(); return browserViewHost().canDrive(sessionId, 'observe', { signal }); }, - probeStagingRoot: ({ signal }) => probeStagingRoot(deps.stagingRoot(), signal), probeLoopback: ({ origin, signal }) => probeLoopback(origin, signal, loopbackTimeoutMs), }; } -async function probeStagingRoot( - root: string | undefined, - signal: AbortSignal, -): Promise { - signal.throwIfAborted(); - if (root === undefined) return { kind: 'not_configured' }; - const path = join(root, `.preview-preflight-${randomUUID()}.probe`); - try { - // Creating the root is what the Open action itself does before it - // materializes an artifact, so probing that way answers the question the - // caller actually has rather than reporting a directory nobody made yet. - await mkdir(root, { recursive: true }); - await writeFile(path, PROBE_CONTENT, { encoding: 'utf8', mode: 0o600 }); - // Write-then-read, because a directory that accepts a write but hands back - // something else is exactly the "not the filesystem you think" case this - // capability is meant to catch. - const read = await readFile(path, { encoding: 'utf8' }); - if (read !== PROBE_CONTENT) { - return { kind: 'unavailable', root, cause: 'a probe file read back different content' }; - } - return { kind: 'round_tripped', root }; - } catch (error) { - return { kind: 'unavailable', root, cause: describeCause(error) }; - } finally { - await rm(path, { force: true }).catch(() => undefined); - } -} - /** - * Every terminal path settles, and the deadline lives here rather than on the - * socket. Both matter for the same reason: a server answering `101 Switching - * Protocols` makes Node route the exchange to `upgrade` and close the request - * without ever emitting `response` or `error`, taking the socket — and any - * socket-bound timeout — with it. A probe that only listened for those two - * events outlived both its stated bound and its caller's cancellation. + * `HEAD /` against the origin, settled by the first of: a response, an + * upgrade, an error, the caller's abort, or the deadline. + * + * HEAD to the root, never the caller's path: a dev server may attach side + * effects to a route, and a capability check must not trip them. + * + * The deadline lives here rather than on the socket. A server answering + * `101 Switching Protocols` makes Node route the exchange to `upgrade` and + * close the request without emitting `response` or `error`, taking any + * socket-bound timeout with it. The deadline is therefore the one catch-all: + * whatever else does or does not fire, the probe settles within it. */ function probeLoopback( origin: string, @@ -114,7 +76,7 @@ function probeLoopback( timeoutMs: number, ): Promise { signal.throwIfAborted(); - const url = new URL(origin); + const url = new URL('/', origin); const send = url.protocol === 'https:' ? httpsRequest : httpRequest; return new Promise((resolve, reject) => { let settled = false; @@ -125,7 +87,7 @@ function probeLoopback( settled = true; if (deadline !== undefined) clearTimeout(deadline); signal.removeEventListener('abort', abandon); - // Marked settled first, so the teardown's own close/error cannot re-enter. + // Marked settled first, so the teardown's own error cannot re-enter. probe.destroy(); settle(); } @@ -137,12 +99,9 @@ function probeLoopback( finish(() => reject(signal.reason)); } - const probe = send(url, { method: 'GET' }, (response) => { - const status = response.statusCode ?? 0; - // Drain without reading: the status is the whole observation, and a - // preview page's content has no business in a capability report. + const probe = send(url, { method: 'HEAD' }, (response) => { response.resume(); - answer({ kind: 'answered', status }); + answer({ kind: 'answered', status: response.statusCode ?? 0 }); }); probe.on('upgrade', (response, socket) => { // The upgraded socket is detached from the request, so destroying the @@ -155,8 +114,6 @@ function probeLoopback( if (signal.aborted) abandon(); else answer({ kind: 'no_answer', cause: describeCause(error) }); }); - // The catch-all: a socket that closes without telling us anything else. - probe.on('close', () => answer({ kind: 'no_answer', cause: 'closed without a response' })); deadline = setTimeout( () => answer({ kind: 'no_answer', cause: `no response within ${timeoutMs}ms` }), timeoutMs, diff --git a/apps/desktop/src/main/preview-preflight.ts b/apps/desktop/src/main/preview-preflight.ts index 505ff549295..3640925c926 100644 --- a/apps/desktop/src/main/preview-preflight.ts +++ b/apps/desktop/src/main/preview-preflight.ts @@ -21,29 +21,30 @@ * Local preview capability preflight — what this client can actually show, * asked before anything is attempted. * - * Showing generated HTML locally depends on four independent things, and each - * one fails differently: the client may have no GUI surface at all, the - * embedded browser refuses `file://` by policy, the process that writes the - * file may not share a filesystem with the surface that would render it, and - * the conversation's own view may not be the one on screen. Attempting the - * preview and reading the first error conflates all four. A refused loopback - * connection is the worst of them: "nothing listening yet", "server still - * starting", and "separate network namespace" are indistinguishable at the - * socket, yet the refusal reads like proof of the last one. + * Showing a page locally depends on independent things that each fail + * differently: the client may have no GUI surface at all, the embedded browser + * refuses `file://` by policy, and the conversation's own view may not be the + * one on screen. Attempting the preview and reading the first error conflates + * them. A refused loopback connection is the worst case: "nothing listening + * yet", "server still starting", and "separate network namespace" are + * indistinguishable at the socket, yet the refusal reads like proof of the + * last one. * * So every capability below answers with one of three statuses and names the * observation behind it. `verified` requires a positive observation. * `unsupported` is a settled product boundary this code can state without * probing. Everything else is `unknown` — explicitly NOT a claim that a - * sandbox, an isolated localhost, or a missing capability caused it. A caller - * may describe a preview as ready only on `verified`, which is why `ready` - * below is derived from positive evidence alone and never from the absence of - * an error. + * sandbox, an isolated localhost, or a missing capability caused it. + * + * No result here claims that a page is ready. The loopback probe observes the + * origin, not the page, and whether a page loads is #5235's question; a + * readiness flag built from a listener's answer would claim more than the + * evidence holds. * * Pure by construction: every observation arrives through * PreviewPreflightAuthority, so each status — including the ones that need a - * refused socket or an unreadable directory — is reachable from a plain unit - * test. The probes that touch the OS live in preview-preflight-probes.ts. + * refused socket — is reachable from a plain unit test. The probes that touch + * the OS live in preview-preflight-probes.ts. */ import type { MakaTool } from '@maka/runtime/tool-runtime'; @@ -52,12 +53,7 @@ import { parseNavigable } from './browser/logic.js'; export type PreviewCapabilityStatus = 'verified' | 'unsupported' | 'unknown'; -export type PreviewCapabilityId = - | 'gui_surface' - | 'url_schemes' - | 'filesystem_visibility' - | 'browser_reachability' - | 'loopback_endpoint'; +export type PreviewCapabilityId = 'gui_surface' | 'url_schemes' | 'browser_view' | 'loopback_endpoint'; export interface PreviewCapability { readonly id: PreviewCapabilityId; @@ -78,18 +74,14 @@ export interface PreviewPreflightResult { * particular endpoint: a GUI surface plus a view that accepts actions. */ readonly surface: PreviewCapabilityStatus; - /** The health of the named endpoint, kept separate from the surface above. */ - readonly endpoint: PreviewEndpointStatus; /** - * True only when a page can be shown on positive evidence right now: a - * verified surface AND an endpoint that answered. An absent error is never - * enough. Read `surface` and `endpoint` to see which half is missing — a - * false `ready` with no endpoint named is not a capability finding. + * Whether something listens at the named origin. `verified` proves a + * listener, not that any page exists there or that it loads. */ - readonly ready: boolean; + readonly endpoint: PreviewEndpointStatus; readonly capabilities: readonly PreviewCapability[]; readonly summary: string; - /** The supported handoff to use instead. Always present when `ready` is false. */ + /** Present unless the caller already has a verified surface and listener. */ readonly alternative?: string; } @@ -98,11 +90,6 @@ export type Observed = | { readonly ok: true; readonly value: T } | { readonly ok: false; readonly cause: string }; -export type StagingRootProbe = - | { readonly kind: 'round_tripped'; readonly root: string } - | { readonly kind: 'unavailable'; readonly root: string; readonly cause: string } - | { readonly kind: 'not_configured' }; - export type LoopbackProbe = // Any HTTP status answers the question: a 404 still proves something listened. | { readonly kind: 'answered'; readonly status: number } @@ -116,9 +103,7 @@ export interface PreviewPreflightAuthority { readonly sessionId: string; readonly signal: AbortSignal; }): boolean | Promise; - /** Round-trip a probe file under the Artifact staging root. */ - probeStagingRoot(input: { readonly signal: AbortSignal }): StagingRootProbe | Promise; - /** Bounded loopback request. A connection outcome is a result, never a throw. */ + /** Bounded `HEAD /` against an origin. A connection outcome is a result, never a throw. */ probeLoopback(input: { readonly origin: string; readonly signal: AbortSignal; @@ -126,13 +111,12 @@ export interface PreviewPreflightAuthority { } /** - * The supported handoff: it needs neither `file://` navigation in the embedded - * browser nor an ad-hoc localhost server, so it stays available even when every - * capability below is unknown. Its own honesty caveat is repeated here, because - * a caller reading this line is exactly the one about to overclaim. + * Names the supported route and defers to it for what that route promises. + * Restating ArtifactPreview's guarantees here would give two answers to the + * same question, and the copy would drift from the tool it describes. */ export const ARTIFACT_PREVIEW_HANDOFF = - 'Use the ArtifactPreview tool: it turns an HTML Artifact in this session into a Desktop-managed, temporary HTTP URL the browser can open, with no shell server and no file:// navigation. Its `reachable` flag reports a Desktop-side HTTP check and not a browser load, so confirm rendering by navigating and observing the page. If it fails, fall back to Generated Files → Save As or Show in Folder rather than reporting that the preview opened.'; + 'To show an HTML Artifact without a shell server or file:// navigation, use the ArtifactPreview tool in this same offer. Its description states what the URL it returns does and does not guarantee; read that before reporting that a preview opened.'; /** Addresses the embedded browser is asked about, one per scheme worth reporting. */ const SCHEME_PROBES = [ @@ -143,13 +127,18 @@ const SCHEME_PROBES = [ ] as const; /** - * `localhost` is deliberately not accepted: it resolves through the host's - * name resolution and can point somewhere other than the loopback interface, - * which would turn a capability check into an arbitrary outbound request. + * Stricter than `isLoopbackHost` in @maka/core/mcp on purpose, so do not widen + * it to match. That predicate decides whether traffic may be trusted as local; + * this one names the exact address a probe dials, because the evidence it + * returns says which address answered or refused. `localhost` can resolve to + * either 127.0.0.1 or [::1] depending on the resolver's order, so a refusal + * there would not say which address was tried — a server bound to one family + * reads as down. The rest of 127/8 is excluded for the same precision. */ const LOOPBACK_HOSTS = new Set(['127.0.0.1', '[::1]']); export interface SchemeProbeResult { + readonly sample: string; readonly scheme: string; readonly navigable: boolean; } @@ -160,6 +149,7 @@ export interface SchemeProbeResult { */ export function probeNavigableSchemes(): readonly SchemeProbeResult[] { return SCHEME_PROBES.map((sample) => ({ + sample, scheme: sample.slice(0, sample.indexOf(':') + 1), navigable: parseNavigable(sample) !== null, })); @@ -206,77 +196,28 @@ export function classifyUrlSchemes(probes: readonly SchemeProbeResult[]): Previe }; } -export function classifyFilesystemVisibility(observed: Observed): PreviewCapability { - if (!observed.ok) { - return { - id: 'filesystem_visibility', - status: 'unknown', - evidence: `Could not probe the Artifact staging root: ${observed.cause}.`, - boundary: 'The probe did not complete, so nothing was learned about the path.', - }; - } - const probe = observed.value; - if (probe.kind === 'not_configured') { - return { - id: 'filesystem_visibility', - status: 'unsupported', - evidence: 'This client has no Artifact staging root configured.', - boundary: 'Without a staging root there is no local path for a preview to be materialized into.', - }; - } - if (probe.kind === 'round_tripped') { - return { - id: 'filesystem_visibility', - status: 'verified', - evidence: `Created, read back, and removed a probe file under ${probe.root}.`, - boundary: - 'This proves only that the Desktop main process reaches that directory. It does not prove that the shell or sandboxed runtime which generates a file writes into the same filesystem.', - }; - } - return { - id: 'filesystem_visibility', - status: 'unknown', - evidence: `A probe file under ${probe.root} could not be round-tripped: ${probe.cause}.`, - boundary: - 'A failed access does not distinguish a missing directory, a permission denial, and a sandbox that hides the path. Read the reported cause rather than assuming isolation.', - }; -} - -export function classifyBrowserReachability(input: { - readonly guiSurface: PreviewCapability; - /** Omitted when the view was never asked, which only a settled `unsupported` justifies. */ - readonly drivable?: Observed; -}): PreviewCapability { - if (input.guiSurface.status === 'unsupported') { - return { - id: 'browser_reachability', - status: 'unsupported', - evidence: 'There is no registered browser view host, so this client has no embedded view to reach.', - }; - } - if (!input.drivable) { - return { - id: 'browser_reachability', - status: 'unknown', - evidence: "This conversation's view was not asked whether it accepts actions.", - }; - } - if (!input.drivable.ok) { +/** + * Reached only when a view host exists, so this answers `verified` or + * `unknown` and never `unsupported`: a refused observe is a visibility + * condition of the moment, not a boundary. + */ +export function classifyBrowserView(drivable: Observed): PreviewCapability { + if (!drivable.ok) { return { - id: 'browser_reachability', + id: 'browser_view', status: 'unknown', - evidence: `Could not determine whether this conversation's view accepts actions: ${input.drivable.cause}.`, + evidence: `Could not determine whether this conversation's view accepts actions: ${drivable.cause}.`, }; } - if (input.drivable.value) { + if (drivable.value) { return { - id: 'browser_reachability', + id: 'browser_view', status: 'verified', evidence: "This conversation's embedded view accepts an observe action, so a page can be driven into it now.", }; } return { - id: 'browser_reachability', + id: 'browser_view', status: 'unknown', evidence: "This conversation's embedded view refused an observe action right now.", boundary: @@ -284,6 +225,13 @@ export function classifyBrowserReachability(input: { }; } +/** The view half when there is demonstrably no view host to ask. */ +export const BROWSER_VIEW_WITHOUT_HOST: PreviewCapability = { + id: 'browser_view', + status: 'unsupported', + evidence: 'There is no registered browser view host, so this client has no embedded view to drive.', +}; + export function classifyLoopbackEndpoint(input: { readonly origin: string; readonly observed: Observed; @@ -300,9 +248,9 @@ export function classifyLoopbackEndpoint(input: { return { id: 'loopback_endpoint', status: 'verified', - evidence: `${input.origin} answered with HTTP ${probe.status} to the Desktop main process.`, + evidence: `HEAD / at ${input.origin} answered with HTTP ${probe.status} to the Desktop main process.`, boundary: - 'The answer proves something is listening for this process. It does not prove the embedded browser shares that loopback namespace.', + 'An answer proves a listener at this origin and nothing more: not that any particular page exists there, not that it loads, and not that the embedded browser shares this loopback namespace.', }; } // The rule this whole tool exists for: a connection outcome can produce @@ -311,19 +259,22 @@ export function classifyLoopbackEndpoint(input: { return { id: 'loopback_endpoint', status: 'unknown', - evidence: `Nothing answered at ${input.origin}: ${probe.cause}.`, + // "No usable response", not "nothing answered": a TLS handshake failure, + // garbage bytes, or a reset after accept all mean something did listen. + evidence: `No usable HTTP response from ${input.origin}: ${probe.cause}.`, boundary: - 'A refused or timed-out connection does not prove sandbox isolation, a separate localhost namespace, or that the endpoint is unusable. It shows only that nothing answered this process at that address at this moment. Retry once the server reports that it is listening, or read the server\'s own startup output.', + 'This does not prove sandbox isolation, a separate localhost namespace, or that the endpoint is unusable. It shows only that no usable HTTP response reached this process from that address at this moment. Retry once the server reports that it is listening, or read the server\'s own startup output.', }; } /** * Reject anything that would turn a capability check into an arbitrary - * outbound request, and say what is accepted instead. Mirrors the bounded - * loopback contract the background-task health check uses (#5237). + * outbound request, and say what is accepted instead. Returns the origin only: + * the probe never requests the caller's path, so a dev-server route with side + * effects on GET cannot be tripped by a capability check. */ export function assertLoopbackOrigin(raw: string): string { - const guidance = 'Pass a loopback URL such as http://127.0.0.1:8765/preview.html.'; + const guidance = 'Pass a loopback origin such as http://127.0.0.1:8765.'; let url: URL; try { url = new URL(raw); @@ -341,10 +292,10 @@ export function assertLoopbackOrigin(raw: string): string { } if (!LOOPBACK_HOSTS.has(url.hostname)) { throw new Error( - `Only the loopback interface can be probed, not ${JSON.stringify(url.hostname)}. "localhost" is excluded because it resolves through the host and can point elsewhere. ${guidance}`, + `Only 127.0.0.1 and [::1] can be probed, not ${JSON.stringify(url.hostname)}. "localhost" is excluded because it can resolve to either address, so a refusal would not say which one was tried. ${guidance}`, ); } - return url.toString(); + return url.origin; } /** @@ -354,9 +305,9 @@ export function assertLoopbackOrigin(raw: string): string { */ export function derivePreviewSurface(input: { readonly guiSurface: PreviewCapability; - readonly browserReachability: PreviewCapability; + readonly browserView: PreviewCapability; }): PreviewCapabilityStatus { - const statuses = [input.guiSurface.status, input.browserReachability.status]; + const statuses = [input.guiSurface.status, input.browserView.status]; if (statuses.includes('unsupported')) return 'unsupported'; return statuses.every((status) => status === 'verified') ? 'verified' : 'unknown'; } @@ -365,14 +316,13 @@ export function summarizePreviewPreflight(input: { readonly capabilities: readonly PreviewCapability[]; readonly surface: PreviewCapabilityStatus; readonly endpoint: PreviewEndpointStatus; - readonly ready: boolean; }): string { - if (input.ready) { - return 'A page can be shown now: this client has a usable preview surface and the endpoint answered.'; - } if (input.surface === 'unsupported') { return 'This client cannot display a page locally. The unsupported entries below say why, and that will not change at runtime — take the alternative rather than retrying.'; } + if (input.surface === 'verified' && input.endpoint === 'verified') { + return 'This client can display a page, and something is listening at the named origin. That does not prove the page you want exists there or loads — navigate to it and observe the result before reporting it shown.'; + } if (input.endpoint === 'not_checked') { return input.surface === 'verified' ? 'This client has a usable preview surface. No endpoint was named, so none was checked — an absent question, not a negative result.' @@ -381,7 +331,7 @@ export function summarizePreviewPreflight(input: { const counts = { verified: 0, unsupported: 0, unknown: 0 }; for (const capability of input.capabilities) counts[capability.status] += 1; return ( - `No verified way to show a page right now — ${counts.verified} verified, ${counts.unsupported} unsupported, ${counts.unknown} unknown. ` + + `Some of what a preview needs is unproven — ${counts.verified} verified, ${counts.unsupported} unsupported, ${counts.unknown} unknown. ` + 'Only the unsupported entries are settled boundaries; an unknown entry reports what was observed and not a cause.' ); } @@ -403,7 +353,7 @@ async function observe(signal: AbortSignal, run: () => T | Promise): Promi } } -/** The local-preview capability preflight, published as its own capability offer. */ +/** The local-preview capability preflight, published beside ArtifactPreview. */ export function buildPreviewPreflightTools( authority: PreviewPreflightAuthority, ): readonly MakaTool[] { @@ -411,9 +361,11 @@ export function buildPreviewPreflightTools( name: 'preview_preflight', displayName: 'Check local preview capabilities', description: - 'Report what this Maka client can actually preview locally BEFORE attempting it: GUI availability, which URL schemes the embedded browser accepts, whether the Artifact staging path round-trips, and whether this conversation\'s view accepts actions. ' + - 'Pass `origin` to also check a loopback endpoint you started. Each capability answers verified, unsupported, or unknown, with the observation behind it — an unknown reports what was seen and never asserts a sandbox or isolation cause. ' + - '`surface` says whether this client can display a page at all and is reported separately from `endpoint`, which is `not_checked` when no origin was passed; `ready` needs both. ' + + 'Report what this Maka client can actually preview locally BEFORE attempting it: GUI availability, which URL schemes the embedded browser accepts, and whether this conversation\'s view accepts actions. ' + + 'file:// is never navigable in the embedded browser; that is a settled boundary, not a failure to retry. ' + + 'Pass `origin` to also check whether something listens at a loopback origin you started; only `HEAD /` is sent, so the path you pass is not requested. ' + + 'Each capability answers verified, unsupported, or unknown, with the observation behind it — an unknown reports what was seen and never asserts a sandbox or isolation cause. ' + + '`surface` says whether this client can display a page at all; `endpoint` is reported separately and is `not_checked` when no origin was passed. Neither claims that a page loads. ' + 'Call this instead of inferring a boundary from a failed navigation or a refused connection.', parameters: z .object({ @@ -423,7 +375,7 @@ export function buildPreviewPreflightTools( .max(2000) .optional() .describe( - 'Optional loopback URL to probe, such as http://127.0.0.1:8765/preview.html. Only 127.0.0.1 and [::1] are accepted; no credentials, query, or fragment.', + 'Optional loopback origin to probe, such as http://127.0.0.1:8765. Only 127.0.0.1 and [::1] are accepted; no credentials, query, or fragment. Any path is ignored: the probe sends HEAD / to the origin.', ), }) .strict(), @@ -439,22 +391,17 @@ export function buildPreviewPreflightTools( await observe(abortSignal, () => authority.guiSurfaceAvailable()), ); const urlSchemes = classifyUrlSchemes(probeNavigableSchemes()); - const filesystemVisibility = classifyFilesystemVisibility( - await observe(abortSignal, () => authority.probeStagingRoot({ signal: abortSignal })), - ); // Skip the drive check when there is demonstrably no view host: asking // would throw, and an exception dressed as `unknown` would hide the // settled `unsupported` answer the caller needs. - const browserReachability = classifyBrowserReachability({ - guiSurface, - ...(guiSurface.status === 'unsupported' - ? {} - : { - drivable: await observe(abortSignal, () => + const browserView = + guiSurface.status === 'unsupported' + ? BROWSER_VIEW_WITHOUT_HOST + : classifyBrowserView( + await observe(abortSignal, () => authority.browserDrivable({ sessionId, signal: abortSignal }), ), - }), - }); + ); const loopbackEndpoint = origin === undefined ? undefined @@ -468,21 +415,19 @@ export function buildPreviewPreflightTools( const capabilities = [ guiSurface, urlSchemes, - filesystemVisibility, - browserReachability, + browserView, ...(loopbackEndpoint ? [loopbackEndpoint] : []), ]; - const surface = derivePreviewSurface({ guiSurface, browserReachability }); + const surface = derivePreviewSurface({ guiSurface, browserView }); const endpoint: PreviewEndpointStatus = loopbackEndpoint?.status ?? 'not_checked'; - const ready = surface === 'verified' && endpoint === 'verified'; + const covered = surface === 'verified' && endpoint === 'verified'; return { kind: 'preview_preflight', surface, endpoint, - ready, capabilities, - summary: summarizePreviewPreflight({ capabilities, surface, endpoint, ready }), - ...(ready ? {} : { alternative: ARTIFACT_PREVIEW_HANDOFF }), + summary: summarizePreviewPreflight({ capabilities, surface, endpoint }), + ...(covered ? {} : { alternative: ARTIFACT_PREVIEW_HANDOFF }), }; }, }; diff --git a/apps/desktop/src/main/runtime-host-artifacts-ipc-main.ts b/apps/desktop/src/main/runtime-host-artifacts-ipc-main.ts index d7ef8334949..2d7b3b0911f 100644 --- a/apps/desktop/src/main/runtime-host-artifacts-ipc-main.ts +++ b/apps/desktop/src/main/runtime-host-artifacts-ipc-main.ts @@ -55,25 +55,12 @@ type RuntimeHostAttachmentPreviewIpcDeps = Pick< const ATTACHMENT_PREVIEW_LIMIT_EXCEEDED = Symbol("attachment-preview-limit-exceeded"); -const ARTIFACT_PRESENTATION_DIR = "maka-runtime-host-artifacts"; - -/** - * Where the Open action materializes an artifact before handing it to the - * operating system. Exported so the local preview preflight probes the very - * directory this path uses, instead of a plausible-looking stand-in; omit the - * epoch to name the root shared by every Host generation. - */ -export function artifactPresentationRoot(hostEpoch?: string): string { - return hostEpoch === undefined - ? join(tmpdir(), ARTIFACT_PRESENTATION_DIR) - : join(tmpdir(), ARTIFACT_PRESENTATION_DIR, hostEpoch); -} - export function registerRuntimeHostArtifactsIpc( deps: RuntimeHostArtifactsIpcDeps, ): void { const presentationRoot = - deps.presentationRoot ?? artifactPresentationRoot(deps.client.hostEpoch); + deps.presentationRoot ?? + join(tmpdir(), "maka-runtime-host-artifacts", deps.client.hostEpoch); handleReconnectableRead( deps.ipcMain, diff --git a/apps/desktop/src/main/runtime-host-boot.ts b/apps/desktop/src/main/runtime-host-boot.ts index 21337eb79d1..90271151a53 100644 --- a/apps/desktop/src/main/runtime-host-boot.ts +++ b/apps/desktop/src/main/runtime-host-boot.ts @@ -186,14 +186,11 @@ import { createClientSettingsEffects } from "./client-settings-effects.js"; import { registerClientSettingsIpc } from "./client-settings-ipc-main.js"; import { startClientSettingsWatcher } from "./client-settings-watcher.js"; import { registerRuntimeHostGitHubCopilotIpc } from "./runtime-host-github-copilot-ipc-main.js"; -import { - artifactPresentationRoot, - registerRuntimeHostArtifactsIpc, -} from "./runtime-host-artifacts-ipc-main.js"; +import { registerRuntimeHostArtifactsIpc } from "./runtime-host-artifacts-ipc-main.js"; import { ManagedArtifactPreview } from './managed-artifact-preview.js'; import { buildManagedArtifactPreviewTools } from './managed-artifact-preview-tools.js'; -import { buildPreviewPreflightTools } from "./preview-preflight.js"; -import { createPreviewPreflightAuthority } from "./preview-preflight-probes.js"; +import { buildPreviewPreflightTools } from './preview-preflight.js'; +import { createPreviewPreflightAuthority } from './preview-preflight-probes.js'; import type { DesktopRuntimeHostClient } from "./runtime-host-client.js"; import type { DesktopRuntimeHostCandidateControls, @@ -1065,13 +1062,23 @@ const createLocalRuntimeHostManager = () => createRuntimeHostDesktopManager( { offerId: 'desktop_artifact_preview', label: 'HTML Artifact preview', - description: 'Prepare an isolated, temporary HTTP preview of a generated HTML Artifact.', - tools: buildManagedArtifactPreviewTools(async (sessionId, artifactId, signal) => { - if (!scope || !runtimeHostManager?.ownsScope(scope)) throw new Error('Preview target is unavailable'); - const target = runtimePolicyTargetsByEpoch.get(scope.targetEpoch); - if (!target?.isActive()) throw new Error('Preview target is no longer active'); - return managedArtifactPreview.prepare(scope.targetEpoch, target.client, sessionId, artifactId, signal); - }), + description: 'Check what this client can preview locally, and prepare an isolated, temporary HTTP preview of a generated HTML Artifact.', + // The preflight lives in this offer, not the Browser one, for two + // reasons. Every Browser tool is admitted against an Origin read + // from the view's current URL, so a capability check there would + // need a page already loaded and would fail with the very + // "requires an HTTP origin" error it exists to explain. And the + // route it hands off to is ArtifactPreview, so the two must be + // granted together or the handoff names a tool the caller lacks. + tools: [ + ...buildPreviewPreflightTools(createPreviewPreflightAuthority()), + ...buildManagedArtifactPreviewTools(async (sessionId, artifactId, signal) => { + if (!scope || !runtimeHostManager?.ownsScope(scope)) throw new Error('Preview target is unavailable'); + const target = runtimePolicyTargetsByEpoch.get(scope.targetEpoch); + if (!target?.isActive()) throw new Error('Preview target is no longer active'); + return managedArtifactPreview.prepare(scope.targetEpoch, target.client, sessionId, artifactId, signal); + }), + ], }, { offerId: "desktop_settings", @@ -1087,32 +1094,6 @@ const createLocalRuntimeHostManager = () => createRuntimeHostDesktopManager( "Use durable Rive workflows through this Desktop client.", tools: [riveWorkflowTool], }, - // Deliberately NOT part of the Browser offer. Every tool published - // there is admitted against an Origin resolved from the view's - // current URL, so a capability check would need a page already - // loaded — and would fail with the very "requires an HTTP origin" - // error it exists to explain. - { - offerId: "desktop_preview", - label: "Local preview", - description: - "Report what this Desktop client can actually preview locally before a preview is attempted.", - tools: buildPreviewPreflightTools( - createPreviewPreflightAuthority({ - // The generation-specific directory, resolved per call against - // the live target: reporting on any other path would describe a - // directory the Open action never touches. No live target means - // no staging root to report, which the probe states as such - // rather than inventing one. - stagingRoot: () => { - if (!scope || !runtimeHostManager?.ownsScope(scope)) return undefined; - const target = runtimePolicyTargetsByEpoch.get(scope.targetEpoch); - if (!target?.isActive()) return undefined; - return artifactPresentationRoot(target.client.hostEpoch); - }, - }), - ), - }, // One offer per MCP server keeps grant contracts server-scoped: a // server change re-prompts only that server's tools. ...[...mcpServers.keys()].sort().map((serverId) => ({ From 5cd05517f03096e338555c9487ffe4421bfcdf33 Mon Sep 17 00:00:00 2001 From: ying-hua <60057611+ying-hua@users.noreply.github.com> Date: Fri, 9 Oct 2026 20:30:53 +0800 Subject: [PATCH 3/3] fix(harness): publish the preview offer through one tested seam Build the desktop_artifact_preview tool set in buildArtifactPreviewOfferTools, so the preflight and the ArtifactPreview route it hands off to are assembled in one place a test can see. A test pins both tools in the offer and that the handoff names only a tool the same offer grants; dropping either tool from the array now fails it. Remove categoryHint and recoveryMode from preview_preflight. The tool is published as a client capability, whose descriptor carries neither, and the Runtime Host assigns both itself, so the declared `read` and `replay_safe` were never in effect and only misled a reader. Say why the headless branch stays although Desktop, the only publisher, never reaches it: GUI availability is part of the classification #5235 builds on, and it must answer for a publisher without a view host rather than throw. Generated-by: Claude Opus 5.5 via Claude Code Co-Authored-By: Claude Opus 5.5 (1M context) --- .../src/main/__tests__/preview-preflight.test.ts | 15 +++++++++++++++ .../src/main/managed-artifact-preview-tools.ts | 13 +++++++++++++ apps/desktop/src/main/preview-preflight.ts | 11 ++++++++--- apps/desktop/src/main/runtime-host-boot.ts | 13 ++++++------- 4 files changed, 42 insertions(+), 10 deletions(-) diff --git a/apps/desktop/src/main/__tests__/preview-preflight.test.ts b/apps/desktop/src/main/__tests__/preview-preflight.test.ts index 6130e080e64..7a434ed3563 100644 --- a/apps/desktop/src/main/__tests__/preview-preflight.test.ts +++ b/apps/desktop/src/main/__tests__/preview-preflight.test.ts @@ -31,6 +31,7 @@ import { after, describe, it } from 'node:test'; import type { MakaTool, MakaToolContext } from '@maka/runtime/tool-runtime'; import { parseNavigable } from '../browser/logic.js'; import { + ARTIFACT_PREVIEW_HANDOFF, assertLoopbackOrigin, buildPreviewPreflightTools, classifyBrowserView, @@ -47,6 +48,7 @@ import { createPreviewPreflightAuthority, LOOPBACK_PROBE_TIMEOUT_MS, } from '../preview-preflight-probes.js'; +import { buildArtifactPreviewOfferTools } from '../managed-artifact-preview-tools.js'; function ctx(signal?: AbortSignal): MakaToolContext { return { @@ -298,6 +300,19 @@ describe('preview preflight tool', () => { }); }); +describe('artifact preview offer', () => { + it('publishes the preflight together with the tool it hands off to', () => { + const names = buildArtifactPreviewOfferTools({ + preflight: healthyAuthority(), + prepare: () => Promise.reject(new Error('not called')), + }).map((tool) => tool.name); + assert.deepEqual(names, ['preview_preflight', 'ArtifactPreview']); + // The handoff may only name a tool this same offer grants. + const named = names.filter((name) => ARTIFACT_PREVIEW_HANDOFF.includes(name)); + assert.deepEqual(named, ['ArtifactPreview']); + }); +}); + describe('preview preflight loopback probe', () => { const servers: Server[] = []; const sockets: Socket[] = []; diff --git a/apps/desktop/src/main/managed-artifact-preview-tools.ts b/apps/desktop/src/main/managed-artifact-preview-tools.ts index 939a78c2f5c..fd6f1c528cc 100644 --- a/apps/desktop/src/main/managed-artifact-preview-tools.ts +++ b/apps/desktop/src/main/managed-artifact-preview-tools.ts @@ -20,6 +20,19 @@ import type { MakaTool } from '@maka/runtime/tool-runtime'; import { z } from 'zod'; import type { ArtifactPreviewEndpoint } from './managed-artifact-preview.js'; +import { buildPreviewPreflightTools, type PreviewPreflightAuthority } from './preview-preflight.js'; + +/** + * Every tool the `desktop_artifact_preview` offer publishes. The preflight and + * the route it hands off to share one grant unit, so a caller told to use + * ArtifactPreview is never told about a tool it was not granted. + */ +export function buildArtifactPreviewOfferTools(input: { + readonly preflight: PreviewPreflightAuthority; + readonly prepare: (sessionId: string, artifactId: string, signal: AbortSignal) => Promise; +}): readonly MakaTool[] { + return [...buildPreviewPreflightTools(input.preflight), ...buildManagedArtifactPreviewTools(input.prepare)]; +} export function buildManagedArtifactPreviewTools( prepare: (sessionId: string, artifactId: string, signal: AbortSignal) => Promise, diff --git a/apps/desktop/src/main/preview-preflight.ts b/apps/desktop/src/main/preview-preflight.ts index 3640925c926..bdab08627d3 100644 --- a/apps/desktop/src/main/preview-preflight.ts +++ b/apps/desktop/src/main/preview-preflight.ts @@ -379,8 +379,9 @@ export function buildPreviewPreflightTools( ), }) .strict(), - categoryHint: 'read', - recoveryMode: 'replay_safe', + // No categoryHint or recoveryMode: this tool is published as a client + // capability, whose descriptor carries neither, and the Runtime Host + // assigns both itself. Declaring them here would only mislead a reader. impl: async ({ origin: rawOrigin }, { sessionId, abortSignal }) => { abortSignal.throwIfAborted(); // Validate before probing so a malformed origin is an argument error the @@ -393,7 +394,11 @@ export function buildPreviewPreflightTools( const urlSchemes = classifyUrlSchemes(probeNavigableSchemes()); // Skip the drive check when there is demonstrably no view host: asking // would throw, and an exception dressed as `unknown` would hide the - // settled `unsupported` answer the caller needs. + // settled `unsupported` answer the caller needs. Desktop, the only + // publisher today, registers its host before any offer is published, so + // this branch is unreachable there; it is kept because GUI availability + // is part of the classification #5235 builds on, and that contract must + // answer correctly for a publisher without a view host rather than throw. const browserView = guiSurface.status === 'unsupported' ? BROWSER_VIEW_WITHOUT_HOST diff --git a/apps/desktop/src/main/runtime-host-boot.ts b/apps/desktop/src/main/runtime-host-boot.ts index 90271151a53..b0f37b1e518 100644 --- a/apps/desktop/src/main/runtime-host-boot.ts +++ b/apps/desktop/src/main/runtime-host-boot.ts @@ -188,8 +188,7 @@ import { startClientSettingsWatcher } from "./client-settings-watcher.js"; import { registerRuntimeHostGitHubCopilotIpc } from "./runtime-host-github-copilot-ipc-main.js"; import { registerRuntimeHostArtifactsIpc } from "./runtime-host-artifacts-ipc-main.js"; import { ManagedArtifactPreview } from './managed-artifact-preview.js'; -import { buildManagedArtifactPreviewTools } from './managed-artifact-preview-tools.js'; -import { buildPreviewPreflightTools } from './preview-preflight.js'; +import { buildArtifactPreviewOfferTools } from './managed-artifact-preview-tools.js'; import { createPreviewPreflightAuthority } from './preview-preflight-probes.js'; import type { DesktopRuntimeHostClient } from "./runtime-host-client.js"; import type { @@ -1070,15 +1069,15 @@ const createLocalRuntimeHostManager = () => createRuntimeHostDesktopManager( // "requires an HTTP origin" error it exists to explain. And the // route it hands off to is ArtifactPreview, so the two must be // granted together or the handoff names a tool the caller lacks. - tools: [ - ...buildPreviewPreflightTools(createPreviewPreflightAuthority()), - ...buildManagedArtifactPreviewTools(async (sessionId, artifactId, signal) => { + tools: buildArtifactPreviewOfferTools({ + preflight: createPreviewPreflightAuthority(), + prepare: async (sessionId, artifactId, signal) => { if (!scope || !runtimeHostManager?.ownsScope(scope)) throw new Error('Preview target is unavailable'); const target = runtimePolicyTargetsByEpoch.get(scope.targetEpoch); if (!target?.isActive()) throw new Error('Preview target is no longer active'); return managedArtifactPreview.prepare(scope.targetEpoch, target.client, sessionId, artifactId, signal); - }), - ], + }, + }), }, { offerId: "desktop_settings",