feat(links): clickable URLs + file:line paths in the terminal (W1)

Terminal output URLs and file paths (e.g. src/app.ts:42) are now tappable — the
walk-away device is a phone, so this removes soft-keyboard copy gymnastics.

- public/link-paths.ts (new, pure): findPathMatches — links tokens with a '/',
  a :line suffix, or a code extension (src/app.ts:42, /abs/main.rs:10, README.md)
  while rejecting example.com / v1.2.3 / 12:34 / URL tails.
- terminal-session.ts: hardened URL handler (scheme allowlist http/https/mailto +
  window.open noopener,noreferrer, blocks javascript:/data:/file:) replacing the
  addon default; a path link provider → openPath() POSTs {file,line} to
  /open-in-editor (resolves rel paths against the OSC-7 cwd; in-flight guard).
- src/http/editor.ts: additive openFileInEditor + isGotoEditor (--goto file:line
  only for goto-capable editors; execFile, no shell). openInEditor untouched.
- src/server.ts: /open-in-editor branches on body.file vs body.path (same CSRF guard).

Deviation from the "no server change" brief: an additive openFileInEditor was
required because the existing route only opens directories, not file:line (Option A
in docs/plans/w1-clickable-links.md). Backward-compatible; src/types.ts untouched.

Verified independently: typecheck + build:web clean, 1661 tests pass (link-paths
95%+ cov). Note: xterm buffer-row indexing (getLine(n-1)/range.y=n) is asserted in
jsdom but merits a real-browser smoke check.
This commit is contained in:
Yaojia Wang
2026-07-12 19:44:40 +02:00
parent 3e49e36806
commit debf47d99e
9 changed files with 814 additions and 9 deletions

View File

@@ -9,7 +9,7 @@ import { describe, it, expect, beforeAll, afterAll } from 'vitest'
import fs from 'node:fs/promises'
import os from 'node:os'
import path from 'node:path'
import { openInEditor } from '../src/http/editor.js'
import { openInEditor, openFileInEditor, isGotoEditor } from '../src/http/editor.js'
import type { Config } from '../src/types.js'
function cfg(editorCmd: string): Config {
@@ -66,3 +66,104 @@ describe('openInEditor — launch', () => {
expect(r.status).toBe(204)
})
})
// ── W1: openFileInEditor (open a FILE at a line) ──────────────────────────────
/** Poll for a file to exist + be non-empty (the editor spawn is fire-and-forget). */
async function waitForFile(p: string, timeoutMs = 3_000): Promise<string> {
const start = Date.now()
for (;;) {
try {
const content = await fs.readFile(p, 'utf8')
if (content.length > 0) return content
} catch {
// not written yet
}
if (Date.now() - start > timeoutMs) throw new Error(`timeout waiting for ${p}`)
await new Promise((r) => setTimeout(r, 25))
}
}
describe('isGotoEditor', () => {
it('is true for the VS Code family (code/cursor/windsurf/codium)', () => {
expect(isGotoEditor('code')).toBe(true)
expect(isGotoEditor('/usr/local/bin/cursor')).toBe(true)
expect(isGotoEditor('windsurf')).toBe(true)
expect(isGotoEditor('codium')).toBe(true)
expect(isGotoEditor('code.cmd')).toBe(true) // Windows shim
})
it('is false for editors without --goto (vim/nano/true)', () => {
expect(isGotoEditor('vim')).toBe(false)
expect(isGotoEditor('nano')).toBe(false)
expect(isGotoEditor('true')).toBe(false)
})
})
describe('openFileInEditor — validation', () => {
it('rejects a missing / empty / non-string file with 400', async () => {
expect((await openFileInEditor(cfg('true'), undefined)).status).toBe(400)
expect((await openFileInEditor(cfg('true'), '')).status).toBe(400)
expect((await openFileInEditor(cfg('true'), ' ')).status).toBe(400)
expect((await openFileInEditor(cfg('true'), 123)).status).toBe(400)
})
it('rejects a relative file with 400', async () => {
expect((await openFileInEditor(cfg('true'), 'rel/a.ts')).status).toBe(400)
})
it('returns 404 for a non-existent absolute file', async () => {
const r = await openFileInEditor(cfg('true'), path.join(tmpDir, 'nope.ts'))
expect(r.status).toBe(404)
})
it('returns 400 when the path is a directory, not a file', async () => {
const r = await openFileInEditor(cfg('true'), tmpDir)
expect(r.status).toBe(400)
expect(r.error).toContain('not a file')
})
it('rejects a non-integer / zero / out-of-range / non-number line with 400', async () => {
expect((await openFileInEditor(cfg('true'), tmpFile, 1.5)).status).toBe(400)
expect((await openFileInEditor(cfg('true'), tmpFile, 0)).status).toBe(400)
expect((await openFileInEditor(cfg('true'), tmpFile, 1_000_000_001)).status).toBe(400)
expect((await openFileInEditor(cfg('true'), tmpFile, 'x')).status).toBe(400)
})
})
describe('openFileInEditor — launch', () => {
it('spawns the editor for a valid file with no line (204)', async () => {
const r = await openFileInEditor(cfg('true'), tmpFile)
expect(r.ok).toBe(true)
expect(r.status).toBe(204)
})
it('spawns the editor for a valid file + line (204)', async () => {
const r = await openFileInEditor(cfg('true'), tmpFile, 42)
expect(r.status).toBe(204)
})
it('passes `--goto <file>:<line>` for a goto-capable editor (basename code)', async () => {
const argsOut = path.join(tmpDir, 'goto-args.txt')
const recorder = path.join(tmpDir, 'code') // basename 'code' → isGotoEditor true
await fs.writeFile(recorder, `#!/bin/sh\nprintf '%s\\n' "$@" > "${argsOut}"\n`)
await fs.chmod(recorder, 0o755)
const r = await openFileInEditor(cfg(recorder), tmpFile, 42)
expect(r.status).toBe(204)
const recorded = (await waitForFile(argsOut)).trim().split('\n')
expect(recorded).toEqual(['--goto', `${tmpFile}:42`])
})
it('passes only the bare file for a non-goto editor (drops the line)', async () => {
const argsOut = path.join(tmpDir, 'bare-args.txt')
const recorder = path.join(tmpDir, 'myeditor') // basename not in the goto set
await fs.writeFile(recorder, `#!/bin/sh\nprintf '%s\\n' "$@" > "${argsOut}"\n`)
await fs.chmod(recorder, 0o755)
const r = await openFileInEditor(cfg(recorder), tmpFile, 42)
expect(r.status).toBe(204)
const recorded = (await waitForFile(argsOut)).trim().split('\n')
expect(recorded).toEqual([tmpFile])
})
})

View File

@@ -25,6 +25,9 @@
*/
import net from 'node:net'
import fs from 'node:fs/promises'
import os from 'node:os'
import path from 'node:path'
import { execFileSync } from 'node:child_process'
import { afterEach, beforeEach, describe, expect, it } from 'vitest'
@@ -103,6 +106,9 @@ function makeTestConfig(port: number, shellPath: string, maxPayloadBytes = 512 *
IDLE_TTL: '86400',
MAX_PAYLOAD_BYTES: String(maxPayloadBytes),
USE_TMUX: '0', // default off so most cases don't spawn/leak tmux sessions
// W1: /open-in-editor spawns EDITOR_CMD. Use the POSIX no-op so tests never
// pop a real editor (and it's a non-goto basename → bare-file argv).
EDITOR_CMD: 'true',
})
}
@@ -403,6 +409,58 @@ describe('startServer — integration', () => {
}
}, 15_000)
// ── W1: POST /open-in-editor file mode + CSRF guard ────────────────────────
// EDITOR_CMD is 'true' (see makeTestConfig) so no real editor is launched.
// These don't need a PTY — they exercise the plain HTTP route + Origin guard.
it('W1: opens a FILE at a line with a valid Origin (204)', async () => {
const dir = await fs.mkdtemp(path.join(os.tmpdir(), 'oie-file-'))
const file = path.join(dir, 'x.ts')
await fs.writeFile(file, 'hi')
try {
const res = await fetch(`http://127.0.0.1:${port}/open-in-editor`, {
method: 'POST',
headers: { 'content-type': 'application/json', Origin: `http://127.0.0.1:${port}` },
body: JSON.stringify({ file, line: 3 }),
})
expect(res.status).toBe(204)
} finally {
await fs.rm(dir, { recursive: true, force: true })
}
})
it('W1: rejects file mode from a foreign Origin (403 — CSRF guard covers the new branch)', async () => {
const res = await fetch(`http://127.0.0.1:${port}/open-in-editor`, {
method: 'POST',
headers: { 'content-type': 'application/json', Origin: 'http://evil.com' },
body: JSON.stringify({ file: '/tmp/x.ts', line: 1 }),
})
expect(res.status).toBe(403)
})
it('W1: still supports directory mode (regression — Projects panel intact)', async () => {
const dir = await fs.mkdtemp(path.join(os.tmpdir(), 'oie-dir-'))
try {
const res = await fetch(`http://127.0.0.1:${port}/open-in-editor`, {
method: 'POST',
headers: { 'content-type': 'application/json', Origin: `http://127.0.0.1:${port}` },
body: JSON.stringify({ path: dir }),
})
expect(res.status).toBe(204)
} finally {
await fs.rm(dir, { recursive: true, force: true })
}
})
it('W1: 404 for a file that does not exist (validated server-side)', async () => {
const res = await fetch(`http://127.0.0.1:${port}/open-in-editor`, {
method: 'POST',
headers: { 'content-type': 'application/json', Origin: `http://127.0.0.1:${port}` },
body: JSON.stringify({ file: '/no/such/file/here.ts', line: 1 }),
})
expect(res.status).toBe(404)
})
// ── ⑤ attach → attached → output (needs real PTY — sandbox-off) ──────────
//
// NOTE (sandbox-off): This test spawns a real shell process via node-pty.

123
test/link-paths.test.ts Normal file
View File

@@ -0,0 +1,123 @@
/**
* test/link-paths.test.ts — pure findPathMatches matcher (W1).
*
* Node environment (no DOM): the matcher is deliberately DOM-free so it can be
* unit-tested directly and reused by later diff/preview views.
*/
import { describe, it, expect } from 'vitest'
import { findPathMatches, CODE_EXT } from '../public/link-paths.js'
describe('findPathMatches — file:line paths', () => {
it('matches a src/app.ts:42 path with correct 1-based range', () => {
const line = 'see src/app.ts:42 for'
const matches = findPathMatches(line)
expect(matches).toHaveLength(1)
const m = matches[0]!
expect(m.text).toBe('src/app.ts:42')
expect(m.path).toBe('src/app.ts')
expect(m.line).toBe(42)
expect(m.column).toBeUndefined()
// 'see ' = 4 chars → startX = 5 (1-based), endX inclusive = 5 + 13 - 1 = 17.
expect(m.startX).toBe(5)
expect(m.endX).toBe(17)
expect(line.slice(m.startX - 1, m.endX)).toBe('src/app.ts:42')
})
it('parses :line:col into line + column', () => {
const matches = findPathMatches('./a/b.tsx:10:5')
expect(matches).toHaveLength(1)
const m = matches[0]!
expect(m.path).toBe('./a/b.tsx')
expect(m.line).toBe(10)
expect(m.column).toBe(5)
})
it('matches a bare filename with an allowlisted extension (README.md)', () => {
const matches = findPathMatches('open README.md please')
expect(matches).toHaveLength(1)
expect(matches[0]!.path).toBe('README.md')
expect(matches[0]!.line).toBeUndefined()
})
it('matches main.rs:10 via its :line suffix', () => {
const matches = findPathMatches('at main.rs:10')
expect(matches).toHaveLength(1)
expect(matches[0]!.path).toBe('main.rs')
expect(matches[0]!.line).toBe(10)
})
it('matches an absolute path with a leading slash', () => {
const matches = findPathMatches('Error at /home/u/app.ts:42 now')
expect(matches).toHaveLength(1)
const m = matches[0]!
expect(m.path).toBe('/home/u/app.ts')
expect(m.line).toBe(42)
})
})
describe('findPathMatches — non-paths are rejected', () => {
it('does not match a bare domain (example.com)', () => {
expect(findPathMatches('visit example.com today')).toHaveLength(0)
})
it('does not match a version string (v1.2.3)', () => {
expect(findPathMatches('version v1.2.3 released')).toHaveLength(0)
})
it('does not match a foo.bar token (unknown extension, no slash/line)', () => {
expect(findPathMatches('the foo.bar thing')).toHaveLength(0)
})
it('does not match a bare timestamp (12:34)', () => {
expect(findPathMatches('at 12:34 today')).toHaveLength(0)
})
})
describe('findPathMatches — URL guard', () => {
it('does not double-link the path tail of an http URL (WebLinksAddon owns it)', () => {
expect(findPathMatches('https://host/path.html')).toHaveLength(0)
})
it('does not match a path glued to a URL host (a.ts inside http://x/a.ts)', () => {
expect(findPathMatches('http://example.com/src/a.ts')).toHaveLength(0)
})
})
describe('findPathMatches — multiple + relative paths', () => {
it('returns two disjoint matches for two paths on one line', () => {
const matches = findPathMatches('edit src/a.ts:1 and lib/b.rs:2')
expect(matches).toHaveLength(2)
expect(matches[0]!.path).toBe('src/a.ts')
expect(matches[1]!.path).toBe('lib/b.rs')
// Ranges are disjoint and ordered.
expect(matches[0]!.endX).toBeLessThan(matches[1]!.startX)
})
it('keeps a ../ prefix in the path portion', () => {
const matches = findPathMatches('see ../lib/util.ts:7')
expect(matches).toHaveLength(1)
expect(matches[0]!.path).toBe('../lib/util.ts')
expect(matches[0]!.line).toBe(7)
})
it('matches a nested config.test.ts filename (multi-dot) with the final extension', () => {
const matches = findPathMatches('run test/config.test.ts')
expect(matches).toHaveLength(1)
expect(matches[0]!.path).toBe('test/config.test.ts')
})
it('returns [] for a line with no paths', () => {
expect(findPathMatches('just some plain words here')).toEqual([])
})
})
describe('CODE_EXT allowlist', () => {
it('contains common code extensions and not arbitrary ones', () => {
expect(CODE_EXT.has('ts')).toBe(true)
expect(CODE_EXT.has('md')).toBe(true)
expect(CODE_EXT.has('rs')).toBe(true)
expect(CODE_EXT.has('com')).toBe(false)
expect(CODE_EXT.has('bar')).toBe(false)
})
})

View File

@@ -10,12 +10,47 @@
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'
// Captures the handler passed to WebLinksAddon so a test can assert the hardened
// activation handler is wired (W1). vi.hoisted so the mock factory can see it.
const webLinksState = vi.hoisted(
() => ({ handler: undefined as ((e: MouseEvent, uri: string) => void) | undefined }),
)
// A link registered via the (mocked) provider — the shape terminal-session builds.
interface TestLink {
text: string
range: { start: { x: number; y: number }; end: { x: number; y: number } }
activate: (event: MouseEvent, text: string) => void
}
interface TestLinkProvider {
provideLinks: (bufferLineNumber: number, callback: (links: TestLink[] | undefined) => void) => void
}
// ── Stub xterm + addons (heavy DOM/canvas deps we don't need here) ────────────
class FakeTerminal {
options: Record<string, unknown> = {}
cols = 80
rows = 24
parser = { registerOscHandler: vi.fn() }
/** Text the fake buffer returns for any getLine() (drives the link provider). */
lineText = ''
/** The link provider registered by TerminalSession (W1). */
captured: TestLinkProvider | undefined = undefined
/** OSC handlers registered by TerminalSession, keyed by code (7 = cwd). */
oscHandlers: Record<number, (data: string) => boolean> = {}
parser = {
registerOscHandler: vi.fn((code: number, handler: (data: string) => boolean) => {
this.oscHandlers[code] = handler
}),
}
buffer = {
active: {
getLine: (_y: number) => ({ translateToString: (_trim?: boolean) => this.lineText }),
},
}
registerLinkProvider = vi.fn((p: TestLinkProvider) => {
this.captured = p
return { dispose: vi.fn() }
})
loadAddon = vi.fn()
open = vi.fn()
write = vi.fn()
@@ -45,7 +80,13 @@ vi.mock('@xterm/addon-search', () => ({
clearDecorations = vi.fn()
},
}))
vi.mock('@xterm/addon-web-links', () => ({ WebLinksAddon: class {} }))
vi.mock('@xterm/addon-web-links', () => ({
WebLinksAddon: class {
constructor(handler?: (e: MouseEvent, uri: string) => void) {
webLinksState.handler = handler
}
},
}))
// ── Mock WebSocket ────────────────────────────────────────────────────────────
class MockWebSocket {
@@ -109,7 +150,20 @@ afterEach(() => {
vi.useRealTimers()
})
const { TerminalSession } = await import('../public/terminal-session.js')
const { TerminalSession, openWebLink } = await import('../public/terminal-session.js')
function term(s: unknown): FakeTerminal {
return (s as unknown as { term: FakeTerminal }).term
}
/** Drive the captured link provider for one buffer row, returning its links. */
function provideLinks(t: FakeTerminal, bufferLineNumber = 1): TestLink[] | undefined {
let result: TestLink[] | undefined
t.captured!.provideLinks(bufferLineNumber, (links) => {
result = links
})
return result
}
function setLocation(protocol: string, host: string): void {
vi.stubGlobal('location', { protocol, host })
@@ -611,3 +665,168 @@ describe('approve(mode?) — B4 permission-mode relay', () => {
expect(s.pendingApproval).toBe(false)
})
})
// ── W1 clickable file paths (custom link provider) ────────────────────────────
describe('clickable file paths (W1)', () => {
beforeEach(() => setLocation('http:', 'lan:3000'))
it('registers a link provider on construction', () => {
const s = new TerminalSession({ sessionId: null, onSessionId: vi.fn() })
expect(term(s).registerLinkProvider).toHaveBeenCalledOnce()
expect(term(s).captured).toBeTruthy()
})
it('provideLinks yields one link matching findPathMatches (text + 1-based range)', () => {
const s = new TerminalSession({ sessionId: null, onSessionId: vi.fn() })
const t = term(s)
t.lineText = 'see src/app.ts:42 for'
const links = provideLinks(t, 1)!
expect(links).toHaveLength(1)
expect(links[0]!.text).toBe('src/app.ts:42')
expect(links[0]!.range).toEqual({ start: { x: 5, y: 1 }, end: { x: 17, y: 1 } })
})
it('provideLinks returns undefined for a line with no paths', () => {
const s = new TerminalSession({ sessionId: null, onSessionId: vi.fn() })
const t = term(s)
t.lineText = 'just some plain words'
expect(provideLinks(t, 1)).toBeUndefined()
})
it('provideLinks returns undefined when the buffer row is missing', () => {
const s = new TerminalSession({ sessionId: null, onSessionId: vi.fn() })
const t = term(s)
t.buffer.active.getLine = () => undefined as never
expect(provideLinks(t, 999)).toBeUndefined()
})
it('activate() POSTs {file,line} for an absolute path (no cwd needed)', () => {
const fetchMock = vi.fn(async (_u: string, _o: RequestInit) => ({ ok: true }))
vi.stubGlobal('fetch', fetchMock)
const s = new TerminalSession({ sessionId: null, onSessionId: vi.fn() })
const t = term(s)
t.lineText = 'open /home/u/app.ts:42 now'
const links = provideLinks(t, 1)!
links[0]!.activate({} as MouseEvent, links[0]!.text)
expect(fetchMock).toHaveBeenCalledOnce()
const call = fetchMock.mock.calls[0]!
expect(call[0]).toBe('/open-in-editor')
expect(call[1].method).toBe('POST')
expect(JSON.parse(String(call[1].body))).toEqual({ file: '/home/u/app.ts', line: 42 })
})
it('resolves a relative path against the OSC-7 cwd', () => {
const fetchMock = vi.fn(async (_u: string, _o: RequestInit) => ({ ok: true }))
vi.stubGlobal('fetch', fetchMock)
const s = new TerminalSession({ sessionId: null, onSessionId: vi.fn() })
const t = term(s)
t.oscHandlers[7]!('file:///work/here') // set cwd via the real OSC-7 handler
t.lineText = 'edit src/app.ts:42'
const links = provideLinks(t, 1)!
links[0]!.activate({} as MouseEvent, links[0]!.text)
expect(JSON.parse(String(fetchMock.mock.calls[0]![1].body))).toEqual({
file: '/work/here/src/app.ts',
line: 42,
})
})
it('collapses ../ against the cwd', () => {
const fetchMock = vi.fn(async (_u: string, _o: RequestInit) => ({ ok: true }))
vi.stubGlobal('fetch', fetchMock)
const s = new TerminalSession({ sessionId: null, onSessionId: vi.fn() })
const t = term(s)
t.oscHandlers[7]!('file:///work/here')
t.lineText = 'see ../lib/util.ts:7'
const links = provideLinks(t, 1)!
links[0]!.activate({} as MouseEvent, links[0]!.text)
expect(JSON.parse(String(fetchMock.mock.calls[0]![1].body))).toEqual({
file: '/work/lib/util.ts',
line: 7,
})
})
it('relative path with null cwd → no fetch, writes a status line', () => {
const fetchMock = vi.fn(async () => ({ ok: true }))
vi.stubGlobal('fetch', fetchMock)
const s = new TerminalSession({ sessionId: null, onSessionId: vi.fn() })
const t = term(s)
t.lineText = 'edit src/app.ts:42'
const links = provideLinks(t, 1)!
t.write.mockClear()
links[0]!.activate({} as MouseEvent, links[0]!.text)
expect(fetchMock).not.toHaveBeenCalled()
expect(t.write).toHaveBeenCalled()
expect(String(t.write.mock.calls[0]![0])).toContain('working dir unknown')
})
it('in-flight guard: two rapid activate calls → one fetch', () => {
const fetchMock = vi.fn(() => new Promise<Response>(() => {})) // never resolves
vi.stubGlobal('fetch', fetchMock)
const s = new TerminalSession({ sessionId: null, onSessionId: vi.fn() })
const t = term(s)
t.lineText = 'open /a/b.ts:1'
const links = provideLinks(t, 1)!
links[0]!.activate({} as MouseEvent, links[0]!.text)
links[0]!.activate({} as MouseEvent, links[0]!.text)
expect(fetchMock).toHaveBeenCalledOnce()
})
it('writes a status line when the server rejects the open (non-ok)', async () => {
const fetchMock = vi.fn(async () => ({ ok: false, status: 404 }))
vi.stubGlobal('fetch', fetchMock)
const s = new TerminalSession({ sessionId: null, onSessionId: vi.fn() })
const t = term(s)
t.lineText = 'open /home/u/app.ts:42'
const links = provideLinks(t, 1)!
t.write.mockClear()
links[0]!.activate({} as MouseEvent, links[0]!.text)
await new Promise((r) => setTimeout(r, 0)) // flush the fetch().then() chain
expect(t.write).toHaveBeenCalled()
expect(String(t.write.mock.calls[0]![0])).toContain('404')
})
it('disposes the link provider on dispose()', () => {
const s = new TerminalSession({ sessionId: null, onSessionId: vi.fn() })
const disposable = term(s).registerLinkProvider.mock.results[0]!.value as {
dispose: ReturnType<typeof vi.fn>
}
s.dispose()
expect(disposable.dispose).toHaveBeenCalled()
})
})
// ── W1 URL activation hardening ───────────────────────────────────────────────
describe('openWebLink (W1 URL hardening)', () => {
it('passes a handler function to WebLinksAddon (not the default activation)', () => {
new TerminalSession({ sessionId: null, onSessionId: vi.fn() })
expect(typeof webLinksState.handler).toBe('function')
})
it('opens http/https/mailto with noopener,noreferrer', () => {
const openSpy = vi.fn()
vi.stubGlobal('open', openSpy)
openWebLink('https://example.com/x')
expect(openSpy).toHaveBeenCalledWith('https://example.com/x', '_blank', 'noopener,noreferrer')
openWebLink('http://lan:3000')
openWebLink('mailto:a@b.com')
expect(openSpy).toHaveBeenCalledTimes(3)
})
it('blocks javascript: / data: / file: URIs (no window.open)', () => {
const openSpy = vi.fn()
vi.stubGlobal('open', openSpy)
openWebLink('javascript:alert(1)')
openWebLink('data:text/html,<script>alert(1)</script>')
openWebLink('file:///etc/passwd')
expect(openSpy).not.toHaveBeenCalled()
})
it('ignores an unparseable URI without throwing', () => {
const openSpy = vi.fn()
vi.stubGlobal('open', openSpy)
expect(() => openWebLink('not a url')).not.toThrow()
expect(openSpy).not.toHaveBeenCalled()
})
})