diff --git a/packages/cli/src/cli.ts b/packages/cli/src/cli.ts index 45bb0a10..ec48e1e1 100644 --- a/packages/cli/src/cli.ts +++ b/packages/cli/src/cli.ts @@ -9,7 +9,6 @@ if (process.argv.includes('--no-color')) { } import { spawn } from 'node:child_process'; -import { existsSync } from 'node:fs'; import { resolve } from 'node:path'; import type { Config } from '@inkeep/open-knowledge-server'; import { Command } from 'commander'; @@ -35,7 +34,7 @@ import { shareCommand } from './commands/share/index.ts'; import { sharingCommand } from './commands/sharing/index.ts'; import { decideSingleFileTarget, - hasMarkdownExtension, + isFileishTarget, scanRootArgv, } from './commands/single-file-dispatch.ts'; import { createRealSingleFileOpenDeps, runSingleFileOpen } from './commands/single-file-open.ts'; @@ -186,7 +185,7 @@ Examples: const knownSubcommands = new Set(program.commands.map((c) => c.name())); const target = decideSingleFileTarget(scanned.operands, { knownSubcommands, - isFileish: (t) => hasMarkdownExtension(t) || existsSync(resolve(baseDir, t)), + isFileish: (t) => isFileishTarget(resolve(baseDir, t), t), }); if (target !== null) { const code = await runSingleFileOpen( diff --git a/packages/cli/src/commands/open.test.ts b/packages/cli/src/commands/open.test.ts index 4c7991fd..1600f9be 100644 --- a/packages/cli/src/commands/open.test.ts +++ b/packages/cli/src/commands/open.test.ts @@ -13,6 +13,7 @@ function makeDeps(overrides: Partial = {}): { const deps: OpenDeps = { detectBundlePath: () => null, resolveBaseUrl: () => null, + classifyName: () => 'doc', openTarget: (t) => opened.push(t), log: (m) => logs.push(m), error: (m) => errors.push(m), @@ -49,26 +50,95 @@ describe('runOpen', () => { expect(errors).toHaveLength(1); }); - test('folder takes the browser route even when a desktop bundle is present', () => { + test('folder with a desktop bundle → folder= deep link, exit 0', () => { const { deps, opened } = makeDeps({ detectBundlePath: () => '/Applications/OpenKnowledge.app', + }); + const code = runOpen('specs/foo/', { project: '/p' }, deps); + expect(code).toBe(0); + expect(opened).toEqual(['openknowledge://open?project=%2Fp&folder=specs%2Ffoo']); + }); + + test('folder, no bundle but UI running → browser folder route, exit 0', () => { + const { deps, opened } = makeDeps({ + detectBundlePath: () => null, resolveBaseUrl: () => 'http://localhost:5173', }); - const code = runOpen('specs/foo', { folder: true, project: '/p' }, deps); + const code = runOpen('specs/foo/', { project: '/p' }, deps); expect(code).toBe(0); expect(opened).toEqual(['http://localhost:5173/#/specs/foo/']); }); - test('trailing slash infers folder intent without --folder', () => { - const { deps, opened } = makeDeps({ resolveBaseUrl: () => 'http://localhost:5173' }); + test('auto-detects a folder from disk (no trailing slash needed)', () => { + const { deps, opened } = makeDeps({ + detectBundlePath: () => '/Applications/OpenKnowledge.app', + classifyName: () => 'folder', + }); + const code = runOpen('specs/foo', { project: '/p' }, deps); + expect(code).toBe(0); + expect(opened).toEqual(['openknowledge://open?project=%2Fp&folder=specs%2Ffoo']); + }); + + test('trailing slash infers folder intent even when disk classify says doc', () => { + const { deps, opened } = makeDeps({ + resolveBaseUrl: () => 'http://localhost:5173', + classifyName: () => 'doc', + }); const code = runOpen('specs/foo/', {}, deps); expect(code).toBe(0); expect(opened).toEqual(['http://localhost:5173/#/specs/foo/']); }); + test('skill with a desktop bundle → rides doc=__skill__// deep link', () => { + const { deps, opened } = makeDeps({ + detectBundlePath: () => '/Applications/OpenKnowledge.app', + }); + const code = runOpen('trip-log', { skill: true, project: '/p' }, deps); + expect(code).toBe(0); + expect(opened).toEqual([ + 'openknowledge://open?project=%2Fp&doc=__skill__%2Fproject%2Ftrip-log', + ]); + }); + + test('skill --scope global, no bundle but UI running → browser skill route', () => { + const { deps, opened } = makeDeps({ resolveBaseUrl: () => 'http://localhost:5173' }); + const code = runOpen('trip-log', { skill: true, scope: 'global', project: '/p' }, deps); + expect(code).toBe(0); + expect(opened).toEqual(['http://localhost:5173/#/__skill__/global/trip-log']); + }); + + test('skill with an invalid --scope → error, exit 1', () => { + const { deps, errors } = makeDeps({ + detectBundlePath: () => '/Applications/OpenKnowledge.app', + }); + const code = runOpen('trip-log', { skill: true, scope: 'nonsense', project: '/p' }, deps); + expect(code).toBe(1); + expect(errors).toHaveLength(1); + }); + + test('skill, neither desktop nor UI → error, exit 1, nothing opened', () => { + const { deps, opened, errors } = makeDeps(); + const code = runOpen('trip-log', { skill: true, project: '/p' }, deps); + expect(code).toBe(1); + expect(opened).toEqual([]); + expect(errors).toHaveLength(1); + }); + + test('skill name with a traversal/unsafe segment → error before any open', () => { + const { deps, opened, errors } = makeDeps({ + detectBundlePath: () => '/Applications/OpenKnowledge.app', + }); + for (const bad of ['../../etc', '/abs', 'a\\b']) { + const code = runOpen(bad, { skill: true, project: '/p' }, deps); + expect(code).toBe(1); + } + expect(opened).toEqual([]); + expect(errors.length).toBeGreaterThanOrEqual(3); + }); + test('folder with no UI running → error, exit 1', () => { const { deps, errors } = makeDeps({ resolveBaseUrl: () => null }); - const code = runOpen('specs/foo', { folder: true, project: '/p' }, deps); + const code = runOpen('specs/foo/', { project: '/p' }, deps); expect(code).toBe(1); expect(errors).toHaveLength(1); }); @@ -97,7 +167,7 @@ describe('runOpen', () => { test('rejects unsafe folder names too', () => { const { deps, opened, errors } = makeDeps({ resolveBaseUrl: () => 'http://localhost:5173' }); - const code = runOpen('specs/..', { folder: true, project: '/p' }, deps); + const code = runOpen('specs/..', { project: '/p' }, deps); expect(code).toBe(1); expect(opened).toEqual([]); expect(errors).toHaveLength(1); diff --git a/packages/cli/src/commands/open.ts b/packages/cli/src/commands/open.ts index b1945382..0eb65655 100644 --- a/packages/cli/src/commands/open.ts +++ b/packages/cli/src/commands/open.ts @@ -1,8 +1,11 @@ import { spawn as nodeSpawn } from 'node:child_process'; -import { resolve } from 'node:path'; +import { statSync } from 'node:fs'; +import { join, resolve } from 'node:path'; +import { MANAGED_ARTIFACT_SCOPES, type SkillScope } from '@inkeep/open-knowledge-core'; import { encodeDocName, encodeFolderRoute, + encodeSkillRoute, resolveLockDir, resolveUiInfo, } from '@inkeep/open-knowledge-server'; @@ -10,13 +13,15 @@ import { Command } from 'commander'; import { createRealDetectDeps, type DetectResult, detectDesktop } from './desktop-dispatch.ts'; export interface OpenOptions { - folder?: boolean; + skill?: boolean; + scope?: string; project?: string; } export interface OpenDeps { detectBundlePath: () => string | null; resolveBaseUrl: (projectDir: string) => string | null; + classifyName: (projectDir: string, name: string) => 'doc' | 'folder'; openTarget: (target: string) => void; log: (message: string) => void; error: (message: string) => void; @@ -34,6 +39,20 @@ export function createRealOpenDeps( return { detectBundlePath: () => detect().bundlePath ?? null, resolveBaseUrl: (projectDir) => resolveUiInfo({ lockDir: resolveLockDir(projectDir) }).baseUrl, + classifyName: (projectDir, name) => { + const abs = join(projectDir, name); + try { + return statSync(abs).isDirectory() ? 'folder' : 'doc'; + } catch (err) { + const code = (err as { code?: string } | null)?.code; + if (code !== 'ENOENT' && code !== 'ENOTDIR') { + process.stderr.write( + `[ok open] statSync failed for ${abs} (${code ?? 'unknown'}); treating as a doc\n`, + ); + } + return 'doc'; + } + }, openTarget: (target) => { const child = nodeSpawn('open', [target], { detached: true, @@ -47,51 +66,96 @@ export function createRealOpenDeps( }; } +function isUnsafeName(name: string): boolean { + return name.startsWith('/') || name.includes('\\') || name.split('/').includes('..'); +} + +function noTargetError(deps: OpenDeps): number { + deps.error( + 'No OpenKnowledge desktop app found and no UI is running. ' + + 'Install OK Desktop, or start a UI with `ok ui`, then retry.', + ); + return 1; +} + +function openDesktopDeepLink( + projectDir: string, + param: 'doc' | 'folder', + target: string, + deps: OpenDeps, +): void { + const deepLink = `openknowledge://open?project=${encodeURIComponent( + projectDir, + )}&${param}=${encodeURIComponent(target)}`; + deps.openTarget(deepLink); +} + export function runOpen(name: string, options: OpenOptions, deps: OpenDeps): number { const projectDir = resolve(options.project ?? process.cwd()); - const isFolder = options.folder === true || /\/+$/.test(name); const cleanName = name.replace(/\/+$/, ''); if (!cleanName) { - deps.error('Nothing to open: pass a doc path (e.g. `ok open specs/foo/SPEC`).'); + deps.error( + 'Nothing to open: pass a doc, folder, or skill name (e.g. `ok open specs/foo/SPEC`).', + ); return 1; } - if ( - cleanName.startsWith('/') || - cleanName.includes('\\') || - cleanName.split('/').includes('..') - ) { + if (isUnsafeName(cleanName)) { deps.error( `Invalid name "${cleanName}": must be a relative path with no '..' segments, leading '/', or backslashes.`, ); return 1; } - if (isFolder) { - const baseUrl = deps.resolveBaseUrl(projectDir); - if (!baseUrl) { + if (options.skill === true) { + const scope = (options.scope ?? 'project') as SkillScope; + if (!(MANAGED_ARTIFACT_SCOPES as readonly string[]).includes(scope)) { deps.error( - `No OpenKnowledge UI is running for ${projectDir}. Folder preview requires a running UI — start one with \`ok ui\`, then retry.`, + `Invalid --scope "${options.scope}": expected one of ${MANAGED_ARTIFACT_SCOPES.join(', ')}.`, ); return 1; } - const url = `${baseUrl}/#/${encodeFolderRoute(cleanName)}`; - deps.openTarget(url); - deps.log(`Opening folder ${cleanName} in your browser: ${url}`); - return 0; + const bundlePath = deps.detectBundlePath(); + if (bundlePath) { + openDesktopDeepLink(projectDir, 'doc', `__skill__/${scope}/${cleanName}`, deps); + deps.log(`Opening skill ${cleanName} (${scope}) in the OpenKnowledge desktop app.`); + return 0; + } + const baseUrl = deps.resolveBaseUrl(projectDir); + if (baseUrl) { + const url = `${baseUrl}/#/${encodeSkillRoute(scope, cleanName)}`; + deps.openTarget(url); + deps.log(`Opening skill ${cleanName} (${scope}) in your browser: ${url}`); + return 0; + } + return noTargetError(deps); } + const isFolder = /\/+$/.test(name) || deps.classifyName(projectDir, cleanName) === 'folder'; + const bundlePath = deps.detectBundlePath(); + if (isFolder) { + if (bundlePath) { + openDesktopDeepLink(projectDir, 'folder', cleanName, deps); + deps.log(`Opening folder ${cleanName} in the OpenKnowledge desktop app.`); + return 0; + } + const baseUrl = deps.resolveBaseUrl(projectDir); + if (baseUrl) { + const url = `${baseUrl}/#/${encodeFolderRoute(cleanName)}`; + deps.openTarget(url); + deps.log(`Opening folder ${cleanName} in your browser: ${url}`); + return 0; + } + return noTargetError(deps); + } + if (bundlePath) { - const deepLink = `openknowledge://open?project=${encodeURIComponent( - projectDir, - )}&doc=${encodeURIComponent(cleanName)}`; - deps.openTarget(deepLink); + openDesktopDeepLink(projectDir, 'doc', cleanName, deps); deps.log(`Opening ${cleanName} in the OpenKnowledge desktop app.`); return 0; } - const baseUrl = deps.resolveBaseUrl(projectDir); if (baseUrl) { const url = `${baseUrl}/#/${encodeDocName(cleanName)}`; @@ -99,22 +163,25 @@ export function runOpen(name: string, options: OpenOptions, deps: OpenDeps): num deps.log(`Opening ${cleanName} in your browser: ${url}`); return 0; } - - deps.error( - 'No OpenKnowledge desktop app found and no UI is running. ' + - 'Install OK Desktop, or start a UI with `ok ui`, then retry.', - ); - return 1; + return noTargetError(deps); } export function openCommand(): Command { return new Command('open') - .description('Open a doc in the OK Desktop app (folders open in the browser)') + .description( + 'Open a doc, folder, or skill in the OK Desktop app (falls back to the browser UI). ' + + 'Docs and folders are auto-detected — no flag needed.', + ) .argument( - '', - 'Extension-less doc path (e.g. specs/foo/SPEC), or a folder path with --folder', + '', + 'Doc path (specs/foo/SPEC), folder path (specs/foo or specs/foo/), or a skill name with --skill', + ) + .option('--skill', 'Open as a skill in the skill editor') + .option( + '--scope ', + `Skill scope when --skill is set: ${MANAGED_ARTIFACT_SCOPES.join(' | ')}`, + 'project', ) - .option('--folder', 'Treat as a folder and open the folder route in the browser') .option('--project ', 'Project root (defaults to the current directory)') .action((name: string, options: OpenOptions) => { process.exitCode = runOpen(name, options, createRealOpenDeps()); diff --git a/packages/cli/src/commands/single-file-dispatch.test.ts b/packages/cli/src/commands/single-file-dispatch.test.ts index 45e3e05a..b8995be9 100644 --- a/packages/cli/src/commands/single-file-dispatch.test.ts +++ b/packages/cli/src/commands/single-file-dispatch.test.ts @@ -1,7 +1,11 @@ -import { describe, expect, test } from 'bun:test'; +import { afterAll, beforeAll, describe, expect, test } from 'bun:test'; +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; import { decideSingleFileTarget, hasMarkdownExtension, + isFileishTarget, scanRootArgv, } from './single-file-dispatch.ts'; @@ -93,3 +97,31 @@ describe('hasMarkdownExtension', () => { expect(hasMarkdownExtension('a.md.txt')).toBe(false); }); }); + +describe('isFileishTarget (fs-backed predicate)', () => { + let dir: string; + beforeAll(() => { + dir = mkdtempSync(join(tmpdir(), 'ok-fileish-')); + writeFileSync(join(dir, 'note.md'), '# note'); + writeFileSync(join(dir, 'data.json'), '{}'); + mkdirSync(join(dir, 'a-folder')); + }); + afterAll(() => rmSync(dir, { recursive: true, force: true })); + + test('a markdown-extension token is fileish (even if it does not exist)', () => { + expect(isFileishTarget(join(dir, 'missing.md'), 'missing.md')).toBe(true); + }); + + test('an existing regular file is fileish', () => { + expect(isFileishTarget(join(dir, 'data.json'), 'data.json')).toBe(true); + expect(isFileishTarget(join(dir, 'note.md'), 'note.md')).toBe(true); + }); + + test('an existing DIRECTORY is NOT fileish — so `ok open ` falls through to the open command', () => { + expect(isFileishTarget(join(dir, 'a-folder'), 'a-folder')).toBe(false); + }); + + test('a non-existent non-markdown token is not fileish', () => { + expect(isFileishTarget(join(dir, 'nope'), 'nope')).toBe(false); + }); +}); diff --git a/packages/cli/src/commands/single-file-dispatch.ts b/packages/cli/src/commands/single-file-dispatch.ts index 084db877..16bbf8f2 100644 --- a/packages/cli/src/commands/single-file-dispatch.ts +++ b/packages/cli/src/commands/single-file-dispatch.ts @@ -1,3 +1,5 @@ +import { statSync } from 'node:fs'; + const VALUE_TAKING_GLOBAL_FLAGS = new Set(['--cwd', '--log-level']); export interface ScannedRootArgv { @@ -61,7 +63,22 @@ export function decideSingleFileTarget( } /** Markdown-extension test mirroring `SUPPORTED_DOC_EXTENSIONS` — the cheap half - * of the fileish predicate (the other half is an existing-file `existsSync`). */ + * of the fileish predicate (the other half is an existing-regular-file stat). */ export function hasMarkdownExtension(token: string): boolean { return /\.(md|mdx)$/i.test(token); } + +export function isFileishTarget(absPath: string, token: string): boolean { + if (hasMarkdownExtension(token)) return true; + try { + return statSync(absPath).isFile(); + } catch (err) { + const code = (err as { code?: string } | null)?.code; + if (code !== 'ENOENT' && code !== 'ENOTDIR') { + process.stderr.write( + `[ok] statSync failed for ${absPath} (${code ?? 'unknown'}); treating as non-fileish\n`, + ); + } + return false; + } +} diff --git a/packages/cli/src/mcp/server.ts b/packages/cli/src/mcp/server.ts index 52fdde02..706849c5 100644 --- a/packages/cli/src/mcp/server.ts +++ b/packages/cli/src/mcp/server.ts @@ -258,6 +258,7 @@ export async function startGlobalMcpServer( resolveCwd, config: resolveConfigForCwd, identityRef, + isDesktopTerminal: process.env.OK_DESKTOP_TERMINAL === '1', }); const transport = new StdioServerTransport(); diff --git a/packages/desktop/src/main/index.ts b/packages/desktop/src/main/index.ts index ee53a24b..db2ad3b6 100644 --- a/packages/desktop/src/main/index.ts +++ b/packages/desktop/src/main/index.ts @@ -554,6 +554,9 @@ function ensureWindowManager() { ); return win as unknown as BrowserWindowLike; }, + activateApp: () => { + if (process.platform === 'darwin') app.focus({ steal: true }); + }, forkUtility: (entry, args, opts) => { const child = utilityProcess.fork(entry, args, { ...opts, diff --git a/packages/desktop/src/main/url-scheme.test.ts b/packages/desktop/src/main/url-scheme.test.ts index 32b6d456..a3accbbb 100644 --- a/packages/desktop/src/main/url-scheme.test.ts +++ b/packages/desktop/src/main/url-scheme.test.ts @@ -8,6 +8,7 @@ describe('parseOpenKnowledgeUrl — valid inputs', () => { expect(result).toEqual({ host: 'open', project: '/abs/path', + kind: 'doc', doc: 'foo.md', }); }); @@ -19,10 +20,33 @@ describe('parseOpenKnowledgeUrl — valid inputs', () => { expect(result).toEqual({ host: 'open', project: '/abs/my path', + kind: 'doc', doc: 'foo bar.md', }); }); + test('parses a folder= deep link with kind folder', () => { + expect(parseOpenKnowledgeUrl('openknowledge://open?project=/abs&folder=specs%2Ffoo')).toEqual({ + host: 'open', + project: '/abs', + kind: 'folder', + doc: 'specs/foo', + }); + }); + + test('rejects when BOTH doc and folder are present (ambiguous)', () => { + expect(parseOpenKnowledgeUrl('openknowledge://open?project=/abs&doc=a&folder=b')).toBeNull(); + }); + + test('rejects when NEITHER doc nor folder is present', () => { + expect(parseOpenKnowledgeUrl('openknowledge://open?project=/abs')).toBeNull(); + }); + + test('applies the same traversal defense to folder= as doc=', () => { + expect(parseOpenKnowledgeUrl('openknowledge://open?project=/abs&folder=a%2F..%2Fb')).toBeNull(); + expect(parseOpenKnowledgeUrl('openknowledge://open?project=/abs&folder=%2Fabs')).toBeNull(); + }); + test('accepts flat doc-name', () => { expect(parseOpenKnowledgeUrl('openknowledge://open?project=/abs&doc=a_b-c.md')).toMatchObject({ doc: 'a_b-c.md', diff --git a/packages/desktop/src/main/url-scheme.ts b/packages/desktop/src/main/url-scheme.ts index 8a2c842b..178ae397 100644 --- a/packages/desktop/src/main/url-scheme.ts +++ b/packages/desktop/src/main/url-scheme.ts @@ -20,6 +20,7 @@ function shareTargetPath(target: ShareTarget): string { interface ParsedOpenKnowledgeUrl { readonly host: 'open'; readonly project: string; + readonly kind: 'doc' | 'folder'; readonly doc: string; } @@ -177,13 +178,17 @@ export function parseOpenKnowledgeUrl(input: string): ParsedOpenKnowledgeUrl | n const rawProject = parsed.searchParams.get('project'); const rawDoc = parsed.searchParams.get('doc'); - if (!rawProject || !rawDoc) return null; + const rawFolder = parsed.searchParams.get('folder'); + if (!rawProject) return null; + if ((rawDoc == null) === (rawFolder == null)) return null; + const kind: 'doc' | 'folder' = rawDoc != null ? 'doc' : 'folder'; + const rawTarget = (rawDoc ?? rawFolder) as string; let project: string; let doc: string; try { project = decodeURIComponent(rawProject); - doc = decodeURIComponent(rawDoc); + doc = decodeURIComponent(rawTarget); } catch { return null; } @@ -202,6 +207,7 @@ export function parseOpenKnowledgeUrl(input: string): ParsedOpenKnowledgeUrl | n return { host: 'open', project: resolve(project), + kind, doc, }; } @@ -649,11 +655,13 @@ export function registerProtocolHandler(deps: ProtocolHandlerDeps): ProtocolHand } const existing = deps.focusWindowForProject(parsed.project); if (existing) { - deps.sendDeepLink(existing, { doc: parsed.doc, kind: 'doc' }); + deps.sendDeepLink(existing, { doc: parsed.doc, kind: parsed.kind }); return; } void deps - .openProject(parsed.project, { pendingDeepLinkTarget: { kind: 'doc', path: parsed.doc } }) + .openProject(parsed.project, { + pendingDeepLinkTarget: { kind: parsed.kind, path: parsed.doc }, + }) .catch((err) => { deps.log?.warn( { err: (err as Error).message, project: parsed.project }, diff --git a/packages/desktop/src/main/window-manager.ts b/packages/desktop/src/main/window-manager.ts index b735ea1c..994e6166 100644 --- a/packages/desktop/src/main/window-manager.ts +++ b/packages/desktop/src/main/window-manager.ts @@ -41,6 +41,8 @@ export interface BrowserWindowLike { isMinimized?(): boolean; isDestroyed?(): boolean; isVisible?(): boolean; + moveTop?(): void; + isFocused?(): boolean; close?(): void; destroy?(): void; on(event: 'closed', cb: () => void): void; @@ -145,6 +147,7 @@ export interface WindowManagerDeps { showGate: ShowGateRegistry; runClean?(opts: { lockDir: string }): Promise; realpathSync?(p: string): string; + activateApp?(): void; readServerLock?(lockDir: string): ServerLockMetadataLike | null; isProcessAlive?(pid: number): boolean; hostname?(): string; @@ -232,11 +235,17 @@ export class WindowManager { focusWindowForProject(projectPath: string): BrowserWindowLike | null { const ctx = this.windowsByPath.get(this.canonicalizeKey(projectPath)); if (!ctx) return null; - const win = ctx.window; + this.bringToFront(ctx.window); + return ctx.window; + } + + private bringToFront(win: BrowserWindowLike): void { if (win.isMinimized?.()) win.restore?.(); win.show?.(); + const alreadyFrontmost = win.isFocused?.() === true; + win.moveTop?.(); win.focus(); - return win; + if (!alreadyFrontmost) this.deps.activateApp?.(); } getContextForBrowserWindow(win: BrowserWindowLike): ProjectContext | undefined { @@ -463,7 +472,7 @@ export class WindowManager { const existing = this.windowsByPath.get(canonicalKey); if (existing) { if (existing.window.isDestroyed?.() !== true) { - existing.window.focus(); + this.bringToFront(existing.window); return existing; } this.deps.log?.warn( @@ -786,7 +795,7 @@ export class WindowManager { const existing = this.windowsByPath.get(canonicalKey); if (existing) { if (existing.window.isDestroyed?.() !== true) { - existing.window.focus(); + this.bringToFront(existing.window); return existing; } this.deps.log?.warn( @@ -800,7 +809,7 @@ export class WindowManager { if (inFlight) { const ctx = await inFlight; if (ctx.window.isDestroyed?.() !== true) { - ctx.window.focus(); + this.bringToFront(ctx.window); return ctx; } return this.createEphemeralWindow(opts); diff --git a/packages/desktop/src/utility/pty-host.ts b/packages/desktop/src/utility/pty-host.ts index 4a9d5303..6b38bacd 100644 --- a/packages/desktop/src/utility/pty-host.ts +++ b/packages/desktop/src/utility/pty-host.ts @@ -142,6 +142,7 @@ export function buildShellEnv( if (stripped.has(key)) continue; out[key] = value; } + out.OK_DESKTOP_TERMINAL = '1'; return out; } diff --git a/packages/desktop/tests/main/window-manager.test.ts b/packages/desktop/tests/main/window-manager.test.ts index 4a0beb91..c05dca32 100644 --- a/packages/desktop/tests/main/window-manager.test.ts +++ b/packages/desktop/tests/main/window-manager.test.ts @@ -32,7 +32,7 @@ function makeUtility(pid: number): MockUtility { }; } -function makeWindow(opts?: { minimized?: boolean }): BrowserWindowLike & { +function makeWindow(opts?: { minimized?: boolean; focused?: boolean }): BrowserWindowLike & { fireClose: () => void; fireDomReady: () => void; fireDidFinishLoad: () => void; @@ -56,6 +56,8 @@ function makeWindow(opts?: { minimized?: boolean }): BrowserWindowLike & { minimized = false; }), isMinimized: mock(() => minimized), + moveTop: mock(() => {}), + isFocused: mock(() => opts?.focused ?? false), isDestroyed: mock(() => destroyed), isVisible: mock(() => visible), on: mock((_event: 'closed', cb: () => void) => { @@ -101,6 +103,7 @@ interface TestEnv { forkUtilityArgs: string[][]; timers: Array<{ cb: () => void; ms: number }>; killProbe: ReturnType; + activateApp: ReturnType; showGateRegistrations: ShowGateRegistration[]; deps: WindowManagerDeps; } @@ -112,6 +115,7 @@ function buildEnv(): TestEnv { const forkUtilityArgs: string[][] = []; const timers: Array<{ cb: () => void; ms: number }> = []; const killProbe = mock(() => {}); + const activateApp = mock(() => {}); const showGateRegistrations: ShowGateRegistration[] = []; const showGate: ShowGateRegistry = { register: (window, opts) => { @@ -135,6 +139,7 @@ function buildEnv(): TestEnv { forkUtilityArgs, timers, killProbe, + activateApp, showGateRegistrations, deps: { createWindow: (opts) => { @@ -157,6 +162,7 @@ function buildEnv(): TestEnv { return null; }, killProbe, + activateApp, showGate, }, }; @@ -1417,6 +1423,36 @@ describe('WindowManager.focusWindowForProject (M4 URL-scheme warm-start)', () => expect(w.focus).toHaveBeenCalled(); }); + test('brings a backgrounded window to the front: show + moveTop + focus + app steal', async () => { + const wm = new WindowManager(env.deps); + const p = wm.createProjectWindow({ projectPath: '/tmp/bg-proj' }); + env.utilities[0]?.fire({ type: 'ready', port: 51210, apiOrigin: 'http://localhost:51210' }); + const ctx = await p; + + wm.focusWindowForProject('/tmp/bg-proj'); + expect(ctx.window.show).toHaveBeenCalled(); + expect(ctx.window.moveTop).toHaveBeenCalled(); + expect(ctx.window.focus).toHaveBeenCalled(); + expect(env.activateApp).toHaveBeenCalled(); + }); + + test('skips the app-level focus steal when the window is already frontmost', async () => { + const w = makeWindow({ focused: true }); + env.deps.createWindow = () => { + env.createWindowOpts.push({ additionalArguments: [], title: '' }); + env.windows.push(w); + return w; + }; + const wm = new WindowManager(env.deps); + const p = wm.createProjectWindow({ projectPath: '/tmp/frontmost-proj' }); + env.utilities[0]?.fire({ type: 'ready', port: 51211, apiOrigin: 'http://localhost:51211' }); + await p; + + wm.focusWindowForProject('/tmp/frontmost-proj'); + expect(w.focus).toHaveBeenCalled(); + expect(env.activateApp).not.toHaveBeenCalled(); + }); + test('canonicalizes project path before lookup (resolve equivalence)', async () => { const wm = new WindowManager(env.deps); const p = wm.createProjectWindow({ projectPath: '/tmp/canon' }); diff --git a/packages/desktop/tests/utility/pty-host.test.ts b/packages/desktop/tests/utility/pty-host.test.ts index 7fbcad66..35919bb0 100644 --- a/packages/desktop/tests/utility/pty-host.test.ts +++ b/packages/desktop/tests/utility/pty-host.test.ts @@ -161,6 +161,13 @@ describe('setupPtyHost — create', () => { expect(env.OK_LOCK_KIND).toBeUndefined(); expect(env.PATH).toBe('/usr/bin'); }); + + test('marks the shell as the OK Desktop terminal (OK_DESKTOP_TERMINAL=1)', () => { + const h = makeHarness({ env: { SHELL: '/bin/zsh', OK_DESKTOP_TERMINAL: '' } }); + h.fire(CREATE()); + const env = h.spawnCalls[0]?.options.env ?? {}; + expect(env.OK_DESKTOP_TERMINAL).toBe('1'); + }); }); describe('setupPtyHost — streaming', () => { @@ -498,7 +505,7 @@ describe('setupPtyHost — incoming message validation (asIncomingMessage guard) }); describe('buildShellEnv', () => { - test('strips markers, drops undefined, preserves the rest', () => { + test('strips markers, drops undefined, preserves the rest, marks the desktop terminal', () => { const env = buildShellEnv({ PATH: '/usr/bin', HOME: '/Users/x', @@ -506,7 +513,7 @@ describe('buildShellEnv', () => { OK_LOCK_KIND: 'interactive', MAYBE: undefined, }); - expect(env).toEqual({ PATH: '/usr/bin', HOME: '/Users/x' }); + expect(env).toEqual({ PATH: '/usr/bin', HOME: '/Users/x', OK_DESKTOP_TERMINAL: '1' }); }); }); diff --git a/packages/server/assets/skills/project/SKILL.md b/packages/server/assets/skills/project/SKILL.md index d0655c1a..c06ada95 100644 --- a/packages/server/assets/skills/project/SKILL.md +++ b/packages/server/assets/skills/project/SKILL.md @@ -19,7 +19,7 @@ OpenKnowledge (OK) is a markdown-CRDT collaboration platform exposed via MCP. Th 1. **Reads:** `exec("cat …")` for one doc, `exec("ls -A …")` for a directory (folder defaults + template menu), `exec("grep …")` for literal, `search` for ranked retrieval. Native `Read` / `Grep` only on source code (`.ts` / `.py` / …), never on in-scope `.md` / `.mdx`. 2. **Writes:** `write({ document: { path, content } })` for a new or full-replace doc; `edit({ document: { path, find, replace } })` for a body find/replace; `edit({ document: { path, frontmatter } })` for a frontmatter merge-patch (`null` deletes a key). `delete({ document })` removes, `move({ from, to })` moves/renames. Body find/replace is body-only. Pass a one-line `summary` (≤80 chars, user-facing outcome) on every content write. -3. **Preview:** every read/write response carries a route-only `previewUrl` (`/#/`, no host:port). Open the browser once at session start. Don't `preview_screenshot` to confirm edits — the tool response is the confirmation. Full multi-host contract: `references/preview.md`. +3. **Preview / open a doc — determine your ONE surface FIRST (once per session).** Before opening anything, pick where to focus, stop at first match: **`OK_DESKTOP_TERMINAL` set** (check your env — `echo "$OK_DESKTOP_TERMINAL"`) → OK Desktop's own terminal → `ok open ` (switches this window) · have `preview_*` → Claude Code Desktop pane · have a URL-navigation/browser tool → Cursor/Codex → `preview_url` then navigate it · else plain CLI → `ok open `. `ok open ` opens a **doc or folder** (auto-detected — no `--folder`); add `--skill ` for a skill. The `previewUrl` field is a route id, **not** your open mechanism — don't hand it to a browser just because you saw it. Don't `preview_screenshot` to confirm edits. Full Step-0 procedure + per-surface how-to: `references/preview.md`. 4. **Workflow guides:** `workflow({ kind: 'ingest' | 'research' | 'consolidate' | 'discover' })` returns a procedural guide, not data. Use it when the work fits the layer. 5. **Direct questions:** a plain business question ("which customers…", "what did we decide about…") routes to `search` / `exec` + a cited chat answer — no "research" keyword needed. Persist only when durable + multi-doc + not already covered, and *offer* first. See `references/corpus-qa.md`. 6. **Authoring or improving a skill** ("write/make a skill", "improve this skill", "turn this into a skill"): STOP and invoke the **`open-knowledge-write-skill`** skill — it owns scope choice (project vs global), the SKILL.md contract, evaluation, and install; don't improvise from this skill. Author through the `skill` target (`write({ skill })`), never a raw `.ok/skills/…` document. **Never read, diff, or edit the installed projections under `.claude/skills/` · `.cursor/skills/` · `.codex/skills/` · `.opencode/skills/` · `.agents/skills/`** — `install` generates them and overwrites them on the next sync; they are NOT source of truth. diff --git a/packages/server/assets/skills/project/references/anti-patterns.md b/packages/server/assets/skills/project/references/anti-patterns.md index d0e50cce..13ad0003 100644 --- a/packages/server/assets/skills/project/references/anti-patterns.md +++ b/packages/server/assets/skills/project/references/anti-patterns.md @@ -14,7 +14,7 @@ | Wait for the server to tell you to open preview | Skip the session-start preview open and wait for the `attach-preview-once` hint | Open the preview browser at session start; the hint is a fallback when you didn't | | Ignore the attach hint | Skip the `warning: { action: "attach-preview-once" }` hint in write-tool responses | Open the preview when the hint fires (`preview_start`, or `preview_url`); otherwise do nothing | | Make the Claude Code Desktop preview work | Read / diagnose / edit `.claude/launch.json` (host-managed config) | Call `preview_start("open-knowledge-ui")` and nothing else; the OK lock-collision proxy bridges any port mismatch transparently | -| Open a doc in the app from the Claude Code CLI | Print the `previewUrl` / `openknowledge://` string for the user to click | Run `ok open ` — it opens the OK Desktop app (folders open in the browser), with browser fallback | +| Open a doc/folder/skill in the app from a CLI | Print the `previewUrl` / `openknowledge://` string for the user to click | Run `ok open ` (doc or folder, auto-detected; `--skill ` for a skill) — deep-links into OK Desktop, browser fallback | | Reference another doc | `` `[text](./page.md)` `` (backticked) or HTML `` | `[text](./page.md)` (raw markdown) | | Embed an image | `` (HTML), a `localhost:` / `preview_url` server URL, or hot-linked external URL | Fetch + save locally + doc-relative `![meaningful alt](./assets/images/path)` | | Write a factual claim in a KB doc | plausible prose without citation, OR inline `[source](https://URL)` | `ingest` the source first, then cite the local path per Grounding | diff --git a/packages/server/assets/skills/project/references/preview.md b/packages/server/assets/skills/project/references/preview.md index 4262d79d..920e3325 100644 --- a/packages/server/assets/skills/project/references/preview.md +++ b/packages/server/assets/skills/project/references/preview.md @@ -1,10 +1,11 @@ # Preview — full multi-host contract ## Contents +- **Step 0 — determine your ONE focus surface before opening anything (the load-bearing check)** - Re-navigation and end-of-turn discipline - `previewUrl` is a route, not a URL - The three first-class apps (Claude Code Desktop, Cursor, Codex desktop) -- Claude Code CLI track (no browser) +- Any-CLI track — Claude Code / Codex / Cursor CLI / OpenCode, no browser → `ok open` - Four attach signals - `previewUrl: null` semantics, server lifecycle, read-only mirror, no-screenshots @@ -16,7 +17,18 @@ The user watches your edits land in a live browser preview. Open it once at sess **`previewUrl` is a route, not a URL to open.** Every read response (per-doc, on `exec` / `search` / `links` rows) and every write response carries a `previewUrl` — a route fragment like `/#/specs/foo/SPEC`, with **no scheme, host, or port**. It identifies *which doc* to preview, not a URL to hand a browser by itself. Never construct or guess preview URLs. -**OK ships first-class preview support for three apps — Claude Code Desktop, Cursor, and the Codex desktop app — plus the Claude Code CLI on a separate track (below). Make the preview seamless in each.** Match on the tool you actually have (capability, not host name): if a tool can navigate to a URL, it counts as an in-app browser. The three apps map to: +### Step 0 — figure out your ONE focus surface BEFORE you open anything + +Your host doesn't change mid-session, so determine this **once**, the first time you're asked to open/show/preview a doc, and use that one surface for the rest of the session. Run the checks **top-to-bottom and stop at the first match** — earlier rungs win: + +1. **Is `OK_DESKTOP_TERMINAL` set in your environment?** You're a shell agent — actually check it (`echo "$OK_DESKTOP_TERMINAL"`, or read your env), don't assume. **Set → you're inside the OK Desktop app's own terminal.** Open a doc or folder with `ok open ` (auto-detected; `--skill ` for a skill) — it switches THIS window to the target. **Never resolve a preview URL here.** (This rung is the one agents miss most — check the env first, before anything else.) +2. **Do you have `preview_*` tools** (`preview_start` / `preview_eval`)? → **Claude Code Desktop** → drive its pane (rung details below). +3. **Do you have any URL-navigation / in-app browser tool** (Cursor `Navigate`, Codex `@Browser`, or `browser` / `open_url` / `view_url` / …)? → **Cursor / Codex desktop** → `preview_url` once, then navigate that browser to the `url`. +4. **None of the above** → you're a **plain CLI** (Claude Code CLI, Codex CLI, Cursor CLI, OpenCode, any shell agent) → `ok open ` for a doc or folder (auto-detected; `--skill ` for a skill); deep-links a separate OK Desktop window, system browser only if no desktop. + +Decide once; then every "open / show / preview this doc" this session uses that surface's mechanism. **The `previewUrl` field in tool responses is a route id, not your open mechanism** — seeing it is not a reason to hand it to a browser. The rest of this section is the per-surface "how" for whichever rung you landed on. + +**OK ships first-class preview support for three apps — Claude Code Desktop, Cursor, and the Codex desktop app — plus any CLI (Claude Code CLI, Codex CLI, Cursor CLI, OpenCode, …) on a separate `ok open` track (below). Make the preview seamless in each.** Match on the tool you actually have (capability, not host name): if a tool can navigate to a URL, it counts as an in-app browser; if nothing can, you're a CLI → `ok open`. The three apps map to: - **Claude Code Desktop — you have `preview_*` tools** (e.g. `preview_start` + `preview_eval`) → **First open of the session:** to land directly on a doc, arm it first with `preview_url({ armPaneTarget: true, document })` (or `folder`), then `preview_start("open-knowledge-ui")` — `ok ui` redirects the base-open straight to the armed route, so the pane opens on the doc, not root. Plain `preview_start` (no arm) opens at root. **Moving between docs once the pane is open: do it in one `preview_eval` step — set `window.location.hash` to the target's route fragment from the response `previewUrl`, the part from `#` on (e.g. `window.location.hash = '#/specs/foo/SPEC'`).** That drives the SPA router directly. Arm + `preview_start` only redirects a *fresh* open; it can't move an already-open pane (`preview_start` reuses the live process without reloading), so use `preview_eval` there. Don't read or edit `.claude/launch.json` — host-managed; the OK lock-collision proxy handles the UI-already-running case. If `preview_start` fails, report it; don't "fix" `launch.json`. - **Cursor / Codex desktop — no `preview_*` tool, but you have an in-app / built-in browser tool** → call `preview_url` once for the **exact** target (`document` for a doc, `folder` for a folder) and navigate your **in-app browser** straight to the returned `url`. Open that deep URL directly — never the root then navigate; omit both args only for the root. Drive the tool your host gives you: @@ -25,7 +37,13 @@ The user watches your edits land in a live browser preview. Open it once at sess - **Any other host** with a URL-navigation tool (`browser`, `view_url`, `open_url`, `web.browse`, …) → navigate it to the `url`. **This is also the fallback when a named tool above isn't present under that exact name** (hosts rename tools): match on the capability, not the name. If no URL-navigation tool exists at all, drop to the Claude Code CLI track below. - **Honor `autoOpen`** (on `preview_url`, or on `warning` for write tools). If `false`, do not open or refresh any preview UI; surface the URL only if asked. -**Claude Code CLI — a separate track (no browser).** The CLI is pure stdio: don't open or fake a browser. For an "open ``/``" request, run **`ok open `** (`--folder` for a folder) — it deep-links the doc into OK Desktop (folders open in the browser); an action you run, not a URL to print. Any other pure-stdio host with **no** URL-navigation tool is on this track too — but if you *do* have a tool that navigates to a URL, use the in-app branch above, not this track. The Codex **CLI**, **IDE extension**, and **Cloud** also live here (web search only, no localhost browser). No `ok` on PATH or no shell → `preview_url`, then `open ` in the system browser as a last resort, and say so plainly. The system browser is the fallback, never the default. +**Any CLI — a separate track (no browser).** **Claude Code CLI, Codex CLI, Cursor CLI, OpenCode — any pure-stdio / shell agent with no in-app browser tool — uses `ok open`, never a preview URL.** The rule is capability, not vendor: no tool that navigates a URL → you're on this track. + +- **`ok open `** — opens a **doc or folder** in the OK Desktop app. The type is auto-detected (a directory on disk → folder, otherwise a doc), so **`--folder` is not needed** (it stays only as an explicit override for a folder that doesn't exist on disk yet). An action you run, not a URL to print. +- **`ok open --skill [--scope project|global]`** — opens a **skill** in the skill editor (skills are addressed by name + scope, not a content path, so they need the flag). +- All three deep-link into OK Desktop when a bundle is installed, and fall back to the browser UI (`ok ui`) otherwise. No `ok` on PATH or no shell → `preview_url`, then `open ` in the system browser as a last resort, and say so plainly. The system browser is the fallback, never the default. + +(The same vendors' *desktop apps* with a built-in browser — Cursor, Codex desktop — are NOT here; they navigate their own browser per the in-app branch above. Codex **IDE extension** / **Cloud** are web-search-only → they're on this CLI track too.) **Running inside the OK Desktop built-in terminal (the `OK_DESKTOP_TERMINAL` env var is set)? Same track — `ok open ` re-targets the window you're already in** (switches that window to the doc/folder; no second window, no need to raise it since it's already frontmost). Don't resolve preview URLs in that case; `ok open` is the whole answer. **Opening or reading a file IS a preview navigation.** On any "open ``" / "read ``" request, navigate the browser to that doc's `previewUrl` route from the tool response — not a separate fetch, not a fresh system-browser launch. diff --git a/packages/server/src/index.ts b/packages/server/src/index.ts index e4676c28..61a31108 100644 --- a/packages/server/src/index.ts +++ b/packages/server/src/index.ts @@ -266,7 +266,12 @@ export { getCurrentMcpLogger, McpLogger, runWithMcpLogger } from './mcp/logger.t export { installPrettyZodErrors } from './mcp/pretty-zod-errors.ts'; export { buildExecResult, type ExecStructuredResult } from './mcp/tools/exec.ts'; export { registerAllTools } from './mcp/tools/index.ts'; -export { encodeDocName, encodeFolderRoute, resolveUiInfo } from './mcp/tools/preview-url.ts'; +export { + encodeDocName, + encodeFolderRoute, + encodeSkillRoute, + resolveUiInfo, +} from './mcp/tools/preview-url.ts'; export { createMcpHttpHandler, type McpHttpHandler, diff --git a/packages/server/src/mcp/tools/get-preview-url.test.ts b/packages/server/src/mcp/tools/get-preview-url.test.ts index f001308a..7fe7b403 100644 --- a/packages/server/src/mcp/tools/get-preview-url.test.ts +++ b/packages/server/src/mcp/tools/get-preview-url.test.ts @@ -37,6 +37,7 @@ interface EnsureDeps { offCwdResolverDeps?: OffCwdResolverDeps; ensureSingleFileSession?: (absFile: string) => Promise; resolveUserAutoOpen?: () => boolean; + isDesktopTerminal?: boolean; } function captureRegistration( @@ -125,6 +126,100 @@ describe('preview_url tool — UI running', () => { expect(readArmedPaneTarget(resolveLockDir(cwd))).toBe('#/specs/foo/'); }); + describe('isDesktopTerminal steer (OK Desktop built-in terminal)', () => { + test('document: response leads with `ok open ` and tells the agent not to navigate the URL', async () => { + const cwd = mkdtempSync(join(tmpdir(), 'ok-get-preview-url-')); + bindTestUiLock(cwd); + const handler = captureRegistration(cwd, BASE_CONFIG, { isDesktopTerminal: true }); + const result = await handler({ document: 'specs/foo/SPEC' }); + const text = result.content[0]?.text ?? ''; + expect(text).toContain('ok open specs/foo/SPEC'); + expect(text).toMatch(/Don't navigate the URL|reference only/); + expect(result.structuredContent?.running).toBe(true); + expect(result.structuredContent?.okOpenCommand).toBe('ok open specs/foo/SPEC'); + }); + + test('folder: steers to `ok open `', async () => { + const cwd = mkdtempSync(join(tmpdir(), 'ok-get-preview-url-')); + bindTestUiLock(cwd); + const handler = captureRegistration(cwd, BASE_CONFIG, { isDesktopTerminal: true }); + const result = await handler({ folder: 'specs/foo' }); + expect(result.content[0]?.text ?? '').toContain('ok open specs/foo'); + expect(result.structuredContent?.okOpenCommand).toBe('ok open specs/foo'); + }); + + test('skill default scope (project): steers to `ok open --skill` (no --scope)', async () => { + const cwd = mkdtempSync(join(tmpdir(), 'ok-get-preview-url-')); + bindTestUiLock(cwd); + const handler = captureRegistration(cwd, BASE_CONFIG, { isDesktopTerminal: true }); + const result = await handler({ skill: { name: 'trip-log' } }); + expect(result.content[0]?.text ?? '').toContain('ok open trip-log --skill'); + expect(result.content[0]?.text ?? '').not.toContain('--scope'); + expect(result.structuredContent?.okOpenCommand).toBe('ok open trip-log --skill'); + }); + + test('skill --scope global: steers to `ok open --skill --scope global`', async () => { + const cwd = mkdtempSync(join(tmpdir(), 'ok-get-preview-url-')); + bindTestUiLock(cwd); + const handler = captureRegistration(cwd, BASE_CONFIG, { isDesktopTerminal: true }); + const result = await handler({ skill: { name: 'trip-log', scope: 'global' } }); + expect(result.content[0]?.text ?? '').toContain('ok open trip-log --skill --scope global'); + expect(result.structuredContent?.okOpenCommand).toBe( + 'ok open trip-log --skill --scope global', + ); + }); + + test('no target (root): no steer — nothing to `ok open`', async () => { + const cwd = mkdtempSync(join(tmpdir(), 'ok-get-preview-url-')); + bindTestUiLock(cwd); + const handler = captureRegistration(cwd, BASE_CONFIG, { isDesktopTerminal: true }); + const result = await handler({}); + expect(result.content[0]?.text ?? '').not.toContain('ok open'); + }); + + test('NOT a desktop terminal (default): no steer — plain Preview URL', async () => { + const cwd = mkdtempSync(join(tmpdir(), 'ok-get-preview-url-')); + bindTestUiLock(cwd); + const handler = captureRegistration(cwd); + const result = await handler({ document: 'specs/foo/SPEC' }); + expect(result.content[0]?.text ?? '').not.toContain('ok open'); + expect(result.structuredContent?.okOpenCommand ?? null).toBeNull(); + }); + + test('desktop terminal + no UI running: steer + okOpenCommand still fire (ok open does not need the UI)', async () => { + const cwd = mkdtempSync(join(tmpdir(), 'ok-get-preview-url-')); + const handler = captureRegistration(cwd, BASE_CONFIG, { isDesktopTerminal: true }); + const result = await handler({ document: 'specs/foo/SPEC' }); + expect(result.structuredContent?.running).toBe(false); + expect(result.structuredContent?.okOpenCommand).toBe('ok open specs/foo/SPEC'); + expect(result.content[0]?.text ?? '').toContain('ok open specs/foo/SPEC'); + }); + + test('document with a space: okOpenCommand shell-quotes the path', async () => { + const cwd = mkdtempSync(join(tmpdir(), 'ok-get-preview-url-')); + bindTestUiLock(cwd); + const handler = captureRegistration(cwd, BASE_CONFIG, { isDesktopTerminal: true }); + const result = await handler({ document: 'notes/My Doc' }); + expect(result.structuredContent?.okOpenCommand).toBe("ok open 'notes/My Doc'"); + }); + + test('document with an embedded single quote: okOpenCommand POSIX-escapes it', async () => { + const cwd = mkdtempSync(join(tmpdir(), 'ok-get-preview-url-')); + bindTestUiLock(cwd); + const handler = captureRegistration(cwd, BASE_CONFIG, { isDesktopTerminal: true }); + const result = await handler({ document: "Q&A/what's new" }); + expect(result.structuredContent?.okOpenCommand).toBe("ok open 'Q&A/what'\\''s new'"); + }); + + test('desktop terminal + no UI + folder: okOpenCommand still fires', async () => { + const cwd = mkdtempSync(join(tmpdir(), 'ok-get-preview-url-')); + const handler = captureRegistration(cwd, BASE_CONFIG, { isDesktopTerminal: true }); + const result = await handler({ folder: 'specs/foo' }); + expect(result.structuredContent?.running).toBe(false); + expect(result.structuredContent?.okOpenCommand).toBe('ok open specs/foo'); + }); + }); + test('docName + folder together is rejected (mutually exclusive)', async () => { const cwd = mkdtempSync(join(tmpdir(), 'ok-get-preview-url-')); bindTestUiLock(cwd); diff --git a/packages/server/src/mcp/tools/get-preview-url.ts b/packages/server/src/mcp/tools/get-preview-url.ts index 5f09a01c..3949eff0 100644 --- a/packages/server/src/mcp/tools/get-preview-url.ts +++ b/packages/server/src/mcp/tools/get-preview-url.ts @@ -52,6 +52,7 @@ const DESCRIPTION = [ interface GetPreviewUrlDeps { config: ConfigOrResolver; resolveCwd: (explicit?: string) => Promise; + isDesktopTerminal?: boolean; serverUrl?: ServerUrlOrResolver; uiBindWait?: { timeoutMs?: number; pollIntervalMs?: number }; offCwdResolverDeps?: OffCwdResolverDeps; @@ -126,6 +127,13 @@ const OutputSchema = outputSchemaWithText({ .describe( 'User-scoped preview-auto-open preference (`appearance.preview.autoOpen`). When `true`, the agent should route the preview using capability-based routing (in-app browser if available, system browser as fallback). When `false`, the user is managing their own preview view (OK Desktop window, a browser tab they opened, etc.) — the agent must NOT open or refresh any preview UI, and should surface this URL only on direct user ask. Resolved fresh on every call; defaults to `true`.', ), + okOpenCommand: z + .string() + .nullable() + .optional() + .describe( + 'Machine-readable form of the desktop-terminal steer: when this MCP server runs inside OK Desktop’s built-in terminal AND a doc/folder/skill target was given, the exact `ok open …` command to run to focus it in the OK Desktop window. Prefer running it over navigating `url`. `null`/absent in every other context (navigate `url` per your host instead).', + ), }); const NO_UI_SERVER_RUNNING_MESSAGE = @@ -165,6 +173,11 @@ function isServerLive(lockDir: string): boolean { } } +function shellQuoteArg(arg: string): string { + if (/^[A-Za-z0-9._/@%+-]+$/.test(arg)) return arg; + return `'${arg.replace(/'/g, "'\\''")}'`; +} + export function register(server: ServerInstance, deps: GetPreviewUrlDeps): void { server.registerTool( 'preview_url', @@ -257,6 +270,18 @@ export function register(server: ServerInstance, deps: GetPreviewUrlDeps): void ? `#/${encodeSkillRoute(args.skill.scope ?? 'project', args.skill.name)}` : null; + const okOpenCommand = args.document + ? `ok open ${shellQuoteArg(args.document)}` + : args.folder + ? `ok open ${shellQuoteArg(args.folder)}` + : args.skill + ? `ok open ${args.skill.name} --skill${args.skill.scope === 'global' ? ' --scope global' : ''}` + : null; + const desktopTerminalSteer = + deps.isDesktopTerminal && okOpenCommand + ? `You're in the OK Desktop terminal — run \`${okOpenCommand}\` to focus this in the OK Desktop window. Don't navigate the URL below or open a browser; it's for reference only.\n\n` + : ''; + if (args.armPaneTarget && routeFragment) { try { armPaneTarget(lockDir, routeFragment); @@ -302,21 +327,23 @@ export function register(server: ServerInstance, deps: GetPreviewUrlDeps): void const hint = isServerLive(lockDir) ? NO_UI_SERVER_RUNNING_MESSAGE : `${NO_SERVER_MESSAGE}${autoStartDisabled ? AUTOSTART_DISABLED_NOTE : ''}`; - return textPlusStructured(`${hint}${armNote}`, { + return textPlusStructured(`${desktopTerminalSteer}${hint}${armNote}`, { url: null, baseUrl: null, running: false, autoOpen, + okOpenCommand: deps.isDesktopTerminal ? okOpenCommand : null, }); } const url = routeFragment ? `${baseUrl}/${routeFragment}` : baseUrl; - return textPlusStructured(`Preview URL: ${url}${armNote}`, { + return textPlusStructured(`${desktopTerminalSteer}Preview URL: ${url}${armNote}`, { url, baseUrl, running: true, autoOpen, + okOpenCommand: deps.isDesktopTerminal ? okOpenCommand : null, }); }, ); diff --git a/packages/server/src/mcp/tools/index.ts b/packages/server/src/mcp/tools/index.ts index 6b641118..37293e17 100644 --- a/packages/server/src/mcp/tools/index.ts +++ b/packages/server/src/mcp/tools/index.ts @@ -31,6 +31,7 @@ interface RegisterAllToolsOptions { config: ConfigOrResolver; identityRef?: { current: AgentIdentity }; logger?: McpLogger; + isDesktopTerminal?: boolean; } export function registerAllTools(server: ServerInstance, opts: RegisterAllToolsOptions): void { @@ -142,6 +143,7 @@ export function registerAllTools(server: ServerInstance, opts: RegisterAllToolsO config: opts.config, resolveCwd: named('preview_url'), serverUrl: opts.serverUrl, + isDesktopTerminal: opts.isDesktopTerminal, ...(opts.serverUrl ? { ensureSingleFileSession: createEnsureSingleFileSession() } : {}), }); registerConflicts(registrationServer, {