From 0ea6e4a15c2e605975a39daa61d9590fd4d005c1 Mon Sep 17 00:00:00 2001 From: jackwener Date: Fri, 20 Mar 2026 12:28:30 +0800 Subject: [PATCH] fix: address review findings and docs cleanup --- .github/workflows/build-extension.yml | 24 ++++- README.md | 56 +++++------ README.zh-CN.md | 56 +++++------ extension/dist/background.js | 24 ++++- extension/src/background.test.ts | 135 ++++++++++++++++++++++++++ extension/src/background.ts | 33 ++++++- src/browser.test.ts | 10 ++ src/browser/cdp.ts | 8 ++ src/browser/mcp.ts | 16 ++- src/browser/page.ts | 5 + src/build-manifest.test.ts | 36 +++++++ src/build-manifest.ts | 2 +- src/clis/bilibili/download.ts | 19 +--- src/clis/twitter/download.ts | 23 ++--- src/clis/xiaohongshu/download.ts | 6 +- src/clis/zhihu/download.ts | 6 +- src/download/index.test.ts | 16 +++ src/download/index.ts | 26 ++++- src/pipeline/executor.test.ts | 1 + src/pipeline/steps/download.ts | 31 +++--- src/types.ts | 9 ++ src/verify.ts | 67 ++++++++++++- 22 files changed, 473 insertions(+), 136 deletions(-) create mode 100644 extension/src/background.test.ts create mode 100644 src/download/index.test.ts diff --git a/.github/workflows/build-extension.yml b/.github/workflows/build-extension.yml index 301a71e..0383bfe 100644 --- a/.github/workflows/build-extension.yml +++ b/.github/workflows/build-extension.yml @@ -21,10 +21,28 @@ jobs: uses: actions/setup-node@v4 with: node-version: 20 + cache: 'npm' + cache-dependency-path: extension/package-lock.json + + - name: Install extension dependencies + run: npm ci + working-directory: extension + + - name: Build extension + run: npm run build + working-directory: extension + + - name: Prepare extension package + run: | + rm -rf extension-package + mkdir -p extension-package + cp extension/manifest.json extension-package/ + cp -R extension/dist extension-package/ + cp -R extension/icons extension-package/ - name: Create Extension ZIP run: | - cd extension + cd extension-package zip -r ../opencli-extension.zip . - name: Create Extension CRX @@ -33,11 +51,11 @@ jobs: if [ -n "${{ secrets.CRX_PRIVATE_KEY }}" ]; then echo "Found CRX_PRIVATE_KEY, signing extension..." echo "${{ secrets.CRX_PRIVATE_KEY }}" > crx-key.pem - crx3 pack extension -o opencli-extension.crx -p crx-key.pem + crx3 pack extension-package -o opencli-extension.crx -p crx-key.pem rm crx-key.pem else echo "No CRX_PRIVATE_KEY configured. Generating CRX with a temporary random key..." - crx3 pack extension -o opencli-extension.crx + crx3 pack extension-package -o opencli-extension.crx fi - name: Upload Artifacts (Action Run) diff --git a/README.md b/README.md index 94d1526..fee583d 100644 --- a/README.md +++ b/README.md @@ -115,34 +115,34 @@ Run `opencli list` for the live registry. | Site | Commands | Mode | |------|----------|------| -| **twitter** | `trending` `bookmarks` `profile` `search` `timeline` `thread` `following` `followers` `notifications` `post` `reply` `delete` `like` `article` `follow` `unfollow` `bookmark` `unbookmark` `download` `accept` `reply-dm` | πŸ” Browser | -| **reddit** | `hot` `frontpage` `popular` `search` `subreddit` `read` `user` `user-posts` `user-comments` `upvote` `save` `comment` `subscribe` `saved` `upvoted` | πŸ” Browser | -| **cursor** | `status` `send` `read` `new` `dump` `composer` `model` `extract-code` `ask` `screenshot` `history` `export` | πŸ–₯️ Desktop | -| **bilibili** | `hot` `search` `me` `favorite` `history` `feed` `subtitle` `dynamic` `ranking` `following` `user-videos` `download` | πŸ” Browser | -| **codex** | `status` `send` `read` `new` `extract-diff` `model` `ask` `screenshot` `history` `export` | πŸ–₯️ Desktop | -| **chatwise** | `status` `new` `send` `read` `ask` `model` `history` `export` `screenshot` | πŸ–₯️ Desktop | -| **notion** | `status` `search` `read` `new` `write` `sidebar` `favorites` `export` | πŸ–₯️ Desktop | -| **discord-app** | `status` `send` `read` `channels` `servers` `search` `members` | πŸ–₯️ Desktop | -| **v2ex** | `hot` `latest` `topic` `daily` `me` `notifications` | 🌐 / πŸ” | -| **xueqiu** | `feed` `hot-stock` `hot` `search` `stock` `watchlist` | πŸ” Browser | -| **antigravity** | `status` `send` `read` `new` `dump` `extract-code` `model` `watch` | πŸ–₯️ Desktop | -| **chatgpt** | `status` `new` `send` `read` `ask` | πŸ–₯️ Desktop | -| **xiaohongshu** | `search` `notifications` `feed` `me` `user` `download` | πŸ” Browser | -| **apple-podcasts** | `search` `episodes` `top` | 🌐 Public | -| **xiaoyuzhou** | `podcast` `podcast-episodes` `episode` | 🌐 Public | -| **zhihu** | `hot` `search` `question` `download` | πŸ” Browser | -| **youtube** | `search` `video` `transcript` | πŸ” Browser | -| **boss** | `search` `detail` | πŸ” Browser | -| **coupang** | `search` `add-to-cart` | πŸ” Browser | -| **bbc** | `news` | 🌐 Public | -| **ctrip** | `search` | πŸ” Browser | -| **github** | `search` | 🌐 Public | -| **hackernews** | `top` | 🌐 Public | -| **linkedin** | `search` | πŸ” Browser | -| **reuters** | `search` | πŸ” Browser | -| **smzdm** | `search` | πŸ” Browser | -| **weibo** | `hot` | πŸ” Browser | -| **yahoo-finance** | `quote` | πŸ” Browser | +| **twitter** | `trending` `bookmarks` `profile` `search` `timeline` `thread` `following` `followers` `notifications` `post` `reply` `delete` `like` `article` `follow` `unfollow` `bookmark` `unbookmark` `download` `accept` `reply-dm` | Browser | +| **reddit** | `hot` `frontpage` `popular` `search` `subreddit` `read` `user` `user-posts` `user-comments` `upvote` `save` `comment` `subscribe` `saved` `upvoted` | Browser | +| **cursor** | `status` `send` `read` `new` `dump` `composer` `model` `extract-code` `ask` `screenshot` `history` `export` | Desktop | +| **bilibili** | `hot` `search` `me` `favorite` `history` `feed` `subtitle` `dynamic` `ranking` `following` `user-videos` `download` | Browser | +| **codex** | `status` `send` `read` `new` `extract-diff` `model` `ask` `screenshot` `history` `export` | Desktop | +| **chatwise** | `status` `new` `send` `read` `ask` `model` `history` `export` `screenshot` | Desktop | +| **notion** | `status` `search` `read` `new` `write` `sidebar` `favorites` `export` | Desktop | +| **discord-app** | `status` `send` `read` `channels` `servers` `search` `members` | Desktop | +| **v2ex** | `hot` `latest` `topic` `daily` `me` `notifications` | Public / Browser | +| **xueqiu** | `feed` `hot-stock` `hot` `search` `stock` `watchlist` | Browser | +| **antigravity** | `status` `send` `read` `new` `dump` `extract-code` `model` `watch` | Desktop | +| **chatgpt** | `status` `new` `send` `read` `ask` | Desktop | +| **xiaohongshu** | `search` `notifications` `feed` `me` `user` `download` | Browser | +| **apple-podcasts** | `search` `episodes` `top` | Public | +| **xiaoyuzhou** | `podcast` `podcast-episodes` `episode` | Public | +| **zhihu** | `hot` `search` `question` `download` | Browser | +| **youtube** | `search` `video` `transcript` | Browser | +| **boss** | `search` `detail` | Browser | +| **coupang** | `search` `add-to-cart` | Browser | +| **bbc** | `news` | Public | +| **ctrip** | `search` | Browser | +| **github** | `search` | Public | +| **hackernews** | `top` | Public | +| **linkedin** | `search` | Browser | +| **reuters** | `search` | Browser | +| **smzdm** | `search` | Browser | +| **weibo** | `hot` | Browser | +| **yahoo-finance** | `quote` | Browser | ### Desktop App Adapters diff --git a/README.zh-CN.md b/README.zh-CN.md index a1b2322..e9a383a 100644 --- a/README.zh-CN.md +++ b/README.zh-CN.md @@ -116,34 +116,34 @@ npm install -g @jackwener/opencli@latest | η«™η‚Ή | 命什 | 樑式 | |------|------|------| -| **twitter** | `trending` `bookmarks` `profile` `search` `timeline` `thread` `following` `followers` `notifications` `post` `reply` `delete` `like` `article` `follow` `unfollow` `bookmark` `unbookmark` `download` `accept` `reply-dm` | πŸ” ζ΅θ§ˆε™¨ | -| **reddit** | `hot` `frontpage` `popular` `search` `subreddit` `read` `user` `user-posts` `user-comments` `upvote` `save` `comment` `subscribe` `saved` `upvoted` | πŸ” ζ΅θ§ˆε™¨ | -| **cursor** | `status` `send` `read` `new` `dump` `composer` `model` `extract-code` `ask` `screenshot` `history` `export` | πŸ–₯️ 摌青端 | -| **bilibili** | `hot` `search` `me` `favorite` `history` `feed` `subtitle` `dynamic` `ranking` `following` `user-videos` `download` | πŸ” ζ΅θ§ˆε™¨ | -| **codex** | `status` `send` `read` `new` `extract-diff` `model` `ask` `screenshot` `history` `export` | πŸ–₯️ 摌青端 | -| **chatwise** | `status` `new` `send` `read` `ask` `model` `history` `export` `screenshot` | πŸ–₯️ 摌青端 | -| **notion** | `status` `search` `read` `new` `write` `sidebar` `favorites` `export` | πŸ–₯️ 摌青端 | -| **discord-app** | `status` `send` `read` `channels` `servers` `search` `members` | πŸ–₯️ 摌青端 | -| **v2ex** | `hot` `latest` `topic` `daily` `me` `notifications` | 🌐 / πŸ” | -| **xueqiu** | `feed` `hot-stock` `hot` `search` `stock` `watchlist` | πŸ” ζ΅θ§ˆε™¨ | -| **antigravity** | `status` `send` `read` `new` `dump` `extract-code` `model` `watch` | πŸ–₯️ 摌青端 | -| **chatgpt** | `status` `new` `send` `read` `ask` | πŸ–₯️ 摌青端 | -| **xiaohongshu** | `search` `notifications` `feed` `me` `user` `download` | πŸ” ζ΅θ§ˆε™¨ | -| **apple-podcasts** | `search` `episodes` `top` | 🌐 ε…¬εΌ€ | -| **xiaoyuzhou** | `podcast` `podcast-episodes` `episode` | 🌐 ε…¬εΌ€ | -| **zhihu** | `hot` `search` `question` `download` | πŸ” ζ΅θ§ˆε™¨ | -| **youtube** | `search` `video` `transcript` | πŸ” ζ΅θ§ˆε™¨ | -| **boss** | `search` `detail` | πŸ” ζ΅θ§ˆε™¨ | -| **coupang** | `search` `add-to-cart` | πŸ” ζ΅θ§ˆε™¨ | -| **bbc** | `news` | 🌐 ε…¬ε…± API | -| **ctrip** | `search` | πŸ” ζ΅θ§ˆε™¨ | -| **github** | `search` | 🌐 ε…¬ε…± API | -| **hackernews** | `top` | 🌐 ε…¬ε…± API | -| **linkedin** | `search` | πŸ” ζ΅θ§ˆε™¨ | -| **reuters** | `search` | πŸ” ζ΅θ§ˆε™¨ | -| **smzdm** | `search` | πŸ” ζ΅θ§ˆε™¨ | -| **weibo** | `hot` | πŸ” ζ΅θ§ˆε™¨ | -| **yahoo-finance** | `quote` | πŸ” ζ΅θ§ˆε™¨ | +| **twitter** | `trending` `bookmarks` `profile` `search` `timeline` `thread` `following` `followers` `notifications` `post` `reply` `delete` `like` `article` `follow` `unfollow` `bookmark` `unbookmark` `download` `accept` `reply-dm` | ζ΅θ§ˆε™¨ | +| **reddit** | `hot` `frontpage` `popular` `search` `subreddit` `read` `user` `user-posts` `user-comments` `upvote` `save` `comment` `subscribe` `saved` `upvoted` | ζ΅θ§ˆε™¨ | +| **cursor** | `status` `send` `read` `new` `dump` `composer` `model` `extract-code` `ask` `screenshot` `history` `export` | 摌青端 | +| **bilibili** | `hot` `search` `me` `favorite` `history` `feed` `subtitle` `dynamic` `ranking` `following` `user-videos` `download` | ζ΅θ§ˆε™¨ | +| **codex** | `status` `send` `read` `new` `extract-diff` `model` `ask` `screenshot` `history` `export` | 摌青端 | +| **chatwise** | `status` `new` `send` `read` `ask` `model` `history` `export` `screenshot` | 摌青端 | +| **notion** | `status` `search` `read` `new` `write` `sidebar` `favorites` `export` | 摌青端 | +| **discord-app** | `status` `send` `read` `channels` `servers` `search` `members` | 摌青端 | +| **v2ex** | `hot` `latest` `topic` `daily` `me` `notifications` | ε…¬εΌ€ / ζ΅θ§ˆε™¨ | +| **xueqiu** | `feed` `hot-stock` `hot` `search` `stock` `watchlist` | ζ΅θ§ˆε™¨ | +| **antigravity** | `status` `send` `read` `new` `dump` `extract-code` `model` `watch` | 摌青端 | +| **chatgpt** | `status` `new` `send` `read` `ask` | 摌青端 | +| **xiaohongshu** | `search` `notifications` `feed` `me` `user` `download` | ζ΅θ§ˆε™¨ | +| **apple-podcasts** | `search` `episodes` `top` | ε…¬εΌ€ | +| **xiaoyuzhou** | `podcast` `podcast-episodes` `episode` | ε…¬εΌ€ | +| **zhihu** | `hot` `search` `question` `download` | ζ΅θ§ˆε™¨ | +| **youtube** | `search` `video` `transcript` | ζ΅θ§ˆε™¨ | +| **boss** | `search` `detail` | ζ΅θ§ˆε™¨ | +| **coupang** | `search` `add-to-cart` | ζ΅θ§ˆε™¨ | +| **bbc** | `news` | ε…¬ε…± API | +| **ctrip** | `search` | ζ΅θ§ˆε™¨ | +| **github** | `search` | ε…¬ε…± API | +| **hackernews** | `top` | ε…¬ε…± API | +| **linkedin** | `search` | ζ΅θ§ˆε™¨ | +| **reuters** | `search` | ζ΅θ§ˆε™¨ | +| **smzdm** | `search` | ζ΅θ§ˆε™¨ | +| **weibo** | `hot` | ζ΅θ§ˆε™¨ | +| **yahoo-finance** | `quote` | ζ΅θ§ˆε™¨ | ### ζ‘Œι’εΊ”η”¨ι€‚ι…ε™¨ diff --git a/extension/dist/background.js b/extension/dist/background.js index b260716..7a34be1 100644 --- a/extension/dist/background.js +++ b/extension/dist/background.js @@ -267,6 +267,19 @@ async function resolveTabId(tabId) { if (!newTab.id) throw new Error("Failed to create tab in automation window"); return newTab.id; } +async function listAutomationTabs() { + if (automationWindowId === null) return []; + try { + return await chrome.tabs.query({ windowId: automationWindowId }); + } catch { + automationWindowId = null; + return []; + } +} +async function listAutomationWebTabs() { + const tabs = await listAutomationTabs(); + return tabs.filter((tab) => isWebUrl(tab.url)); +} async function handleExec(cmd) { if (!cmd.code) return { id: cmd.id, ok: false, error: "Missing code" }; const tabId = await resolveTabId(cmd.tabId); @@ -306,8 +319,8 @@ async function handleNavigate(cmd) { async function handleTabs(cmd) { switch (cmd.op) { case "list": { - const tabs = await chrome.tabs.query({}); - const data = tabs.filter((t) => isWebUrl(t.url)).map((t, i) => ({ + const tabs = await listAutomationWebTabs(); + const data = tabs.map((t, i) => ({ index: i, tabId: t.id, url: t.url, @@ -317,12 +330,13 @@ async function handleTabs(cmd) { return { id: cmd.id, ok: true, data }; } case "new": { - const tab = await chrome.tabs.create({ url: cmd.url, active: true }); + const windowId = await getAutomationWindow(); + const tab = await chrome.tabs.create({ windowId, url: cmd.url ?? "about:blank", active: true }); return { id: cmd.id, ok: true, data: { tabId: tab.id, url: tab.url } }; } case "close": { if (cmd.index !== void 0) { - const tabs = await chrome.tabs.query({}); + const tabs = await listAutomationWebTabs(); const target = tabs[cmd.index]; if (!target?.id) return { id: cmd.id, ok: false, error: `Tab index ${cmd.index} not found` }; await chrome.tabs.remove(target.id); @@ -341,7 +355,7 @@ async function handleTabs(cmd) { await chrome.tabs.update(cmd.tabId, { active: true }); return { id: cmd.id, ok: true, data: { selected: cmd.tabId } }; } - const tabs = await chrome.tabs.query({}); + const tabs = await listAutomationWebTabs(); const target = tabs[cmd.index]; if (!target?.id) return { id: cmd.id, ok: false, error: `Tab index ${cmd.index} not found` }; await chrome.tabs.update(target.id, { active: true }); diff --git a/extension/src/background.test.ts b/extension/src/background.test.ts new file mode 100644 index 0000000..483e80f --- /dev/null +++ b/extension/src/background.test.ts @@ -0,0 +1,135 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +type Listener void> = { addListener: (fn: T) => void }; + +type MockTab = { + id: number; + windowId: number; + url?: string; + title?: string; + active?: boolean; + status?: string; +}; + +class MockWebSocket { + static OPEN = 1; + static CONNECTING = 0; + readyState = MockWebSocket.CONNECTING; + onopen: (() => void) | null = null; + onmessage: ((event: { data: string }) => void) | null = null; + onclose: (() => void) | null = null; + onerror: (() => void) | null = null; + + constructor(_url: string) {} + send(_data: string): void {} + close(): void { + this.onclose?.(); + } +} + +function createChromeMock() { + let nextTabId = 10; + const tabs: MockTab[] = [ + { id: 1, windowId: 1, url: 'https://automation.example', title: 'automation', active: true, status: 'complete' }, + { id: 2, windowId: 2, url: 'https://user.example', title: 'user', active: true, status: 'complete' }, + { id: 3, windowId: 1, url: 'chrome://extensions', title: 'chrome', active: false, status: 'complete' }, + ]; + + const query = vi.fn(async (queryInfo: { windowId?: number } = {}) => { + return tabs.filter((tab) => queryInfo.windowId === undefined || tab.windowId === queryInfo.windowId); + }); + const create = vi.fn(async ({ windowId, url, active }: { windowId?: number; url?: string; active?: boolean }) => { + const tab: MockTab = { + id: nextTabId++, + windowId: windowId ?? 999, + url, + title: url ?? 'blank', + active: !!active, + status: 'complete', + }; + tabs.push(tab); + return tab; + }); + const update = vi.fn(async (tabId: number, updates: { active?: boolean; url?: string }) => { + const tab = tabs.find((entry) => entry.id === tabId); + if (!tab) throw new Error(`Unknown tab ${tabId}`); + if (updates.active !== undefined) tab.active = updates.active; + if (updates.url !== undefined) tab.url = updates.url; + return tab; + }); + + const chrome = { + tabs: { + query, + create, + update, + remove: vi.fn(async (_tabId: number) => {}), + get: vi.fn(async (tabId: number) => { + const tab = tabs.find((entry) => entry.id === tabId); + if (!tab) throw new Error(`Unknown tab ${tabId}`); + return tab; + }), + onUpdated: { addListener: vi.fn(), removeListener: vi.fn() } as Listener<(id: number, info: chrome.tabs.TabChangeInfo) => void>, + }, + windows: { + get: vi.fn(async (windowId: number) => ({ id: windowId })), + create: vi.fn(async ({ url, focused, width, height, type }: any) => ({ id: 1, url, focused, width, height, type })), + remove: vi.fn(async (_windowId: number) => {}), + onRemoved: { addListener: vi.fn() } as Listener<(windowId: number) => void>, + }, + alarms: { + create: vi.fn(), + onAlarm: { addListener: vi.fn() } as Listener<(alarm: { name: string }) => void>, + }, + runtime: { + onInstalled: { addListener: vi.fn() } as Listener<() => void>, + onStartup: { addListener: vi.fn() } as Listener<() => void>, + }, + cookies: { + getAll: vi.fn(async () => []), + }, + }; + + return { chrome, tabs, query, create, update }; +} + +describe('background tab isolation', () => { + beforeEach(() => { + vi.resetModules(); + vi.stubGlobal('WebSocket', MockWebSocket); + }); + + it('lists only automation-window web tabs', async () => { + const { chrome } = createChromeMock(); + vi.stubGlobal('chrome', chrome); + + const mod = await import('./background'); + mod.__test__.setAutomationWindowId(1); + + const result = await mod.__test__.handleTabs({ id: '1', action: 'tabs', op: 'list' }); + + expect(result.ok).toBe(true); + expect(result.data).toEqual([ + { + index: 0, + tabId: 1, + url: 'https://automation.example', + title: 'automation', + active: true, + }, + ]); + }); + + it('creates new tabs inside the automation window', async () => { + const { chrome, create } = createChromeMock(); + vi.stubGlobal('chrome', chrome); + + const mod = await import('./background'); + mod.__test__.setAutomationWindowId(1); + + const result = await mod.__test__.handleTabs({ id: '2', action: 'tabs', op: 'new', url: 'https://new.example' }); + + expect(result.ok).toBe(true); + expect(create).toHaveBeenCalledWith({ windowId: 1, url: 'https://new.example', active: true }); + }); +}); diff --git a/extension/src/background.ts b/extension/src/background.ts index 218a9ab..c8b2e69 100644 --- a/extension/src/background.ts +++ b/extension/src/background.ts @@ -234,6 +234,21 @@ async function resolveTabId(tabId?: number): Promise { return newTab.id; } +async function listAutomationTabs(): Promise { + if (automationWindowId === null) return []; + try { + return await chrome.tabs.query({ windowId: automationWindowId }); + } catch { + automationWindowId = null; + return []; + } +} + +async function listAutomationWebTabs(): Promise { + const tabs = await listAutomationTabs(); + return tabs.filter((tab) => isWebUrl(tab.url)); +} + async function handleExec(cmd: Command): Promise { if (!cmd.code) return { id: cmd.id, ok: false, error: 'Missing code' }; const tabId = await resolveTabId(cmd.tabId); @@ -278,9 +293,8 @@ async function handleNavigate(cmd: Command): Promise { async function handleTabs(cmd: Command): Promise { switch (cmd.op) { case 'list': { - const tabs = await chrome.tabs.query({}); + const tabs = await listAutomationWebTabs(); const data = tabs - .filter((t) => isWebUrl(t.url)) .map((t, i) => ({ index: i, tabId: t.id, @@ -291,12 +305,13 @@ async function handleTabs(cmd: Command): Promise { return { id: cmd.id, ok: true, data }; } case 'new': { - const tab = await chrome.tabs.create({ url: cmd.url, active: true }); + const windowId = await getAutomationWindow(); + const tab = await chrome.tabs.create({ windowId, url: cmd.url ?? 'about:blank', active: true }); return { id: cmd.id, ok: true, data: { tabId: tab.id, url: tab.url } }; } case 'close': { if (cmd.index !== undefined) { - const tabs = await chrome.tabs.query({}); + const tabs = await listAutomationWebTabs(); const target = tabs[cmd.index]; if (!target?.id) return { id: cmd.id, ok: false, error: `Tab index ${cmd.index} not found` }; await chrome.tabs.remove(target.id); @@ -315,7 +330,7 @@ async function handleTabs(cmd: Command): Promise { await chrome.tabs.update(cmd.tabId, { active: true }); return { id: cmd.id, ok: true, data: { selected: cmd.tabId } }; } - const tabs = await chrome.tabs.query({}); + const tabs = await listAutomationWebTabs(); const target = tabs[cmd.index!]; if (!target?.id) return { id: cmd.id, ok: false, error: `Tab index ${cmd.index} not found` }; await chrome.tabs.update(target.id, { active: true }); @@ -368,3 +383,11 @@ async function handleCloseWindow(cmd: Command): Promise { } return { id: cmd.id, ok: true, data: { closed: true } }; } + +export const __test__ = { + handleTabs, + getAutomationWindowId: () => automationWindowId, + setAutomationWindowId: (windowId: number | null) => { + automationWindowId = windowId; + }, +}; diff --git a/src/browser.test.ts b/src/browser.test.ts index 6fea348..d3fc173 100644 --- a/src/browser.test.ts +++ b/src/browser.test.ts @@ -1,5 +1,6 @@ import { afterEach, describe, it, expect, vi } from 'vitest'; import { BrowserBridge, __test__ } from './browser/index.js'; +import * as daemonClient from './browser/daemon-client.js'; describe('browser helpers', () => { it('extracts tab entries from string snapshots', () => { @@ -122,4 +123,13 @@ describe('BrowserBridge state', () => { await expect(mcp.connect()).rejects.toThrow('Session is closing'); }); + + it('fails fast when daemon is running but extension is disconnected', async () => { + vi.spyOn(daemonClient, 'isExtensionConnected').mockResolvedValue(false); + vi.spyOn(daemonClient, 'isDaemonRunning').mockResolvedValue(true); + + const mcp = new BrowserBridge(); + + await expect(mcp.connect()).rejects.toThrow('Browser Extension is not connected'); + }); }); diff --git a/src/browser/cdp.ts b/src/browser/cdp.ts index d193ba5..83d6eb3 100644 --- a/src/browser/cdp.ts +++ b/src/browser/cdp.ts @@ -113,6 +113,14 @@ class CDPPage implements IPage { return result.result?.value; } + async getCookies(opts: { domain?: string; url?: string } = {}): Promise { + const result = await this.bridge.send('Network.getCookies', opts.url ? { urls: [opts.url] } : {}); + const cookies = Array.isArray(result?.cookies) ? result.cookies : []; + return opts.domain + ? cookies.filter((cookie: any) => typeof cookie.domain === 'string' && cookie.domain.includes(opts.domain!)) + : cookies; + } + async snapshot(opts?: any): Promise { throw new Error('Method not implemented.'); } diff --git a/src/browser/mcp.ts b/src/browser/mcp.ts index e78093e..a42a744 100644 --- a/src/browser/mcp.ts +++ b/src/browser/mcp.ts @@ -35,7 +35,7 @@ export class BrowserBridge { this._state = 'connecting'; try { - await this._ensureDaemon(); + await this._ensureDaemon(opts.timeout); this._page = new Page(); this._state = 'connected'; return this._page; @@ -54,8 +54,16 @@ export class BrowserBridge { this._state = 'closed'; } - private async _ensureDaemon(): Promise { - if (await isDaemonRunning()) return; + private async _ensureDaemon(timeoutSeconds?: number): Promise { + const timeoutMs = Math.max(1, timeoutSeconds ?? Math.ceil(DAEMON_SPAWN_TIMEOUT / 1000)) * 1000; + + if (await isExtensionConnected()) return; + if (await isDaemonRunning()) { + throw new Error( + 'Daemon is running but the Browser Extension is not connected.\n' + + 'Please install and enable the opencli Browser Bridge extension in Chrome.', + ); + } // Find daemon relative to this file β€” works for both: // npx tsx src/main.ts β†’ src/browser/mcp.ts β†’ src/daemon.ts @@ -85,7 +93,7 @@ export class BrowserBridge { this._daemonProc.unref(); // Wait for daemon to be ready AND extension to connect - const deadline = Date.now() + DAEMON_SPAWN_TIMEOUT; + const deadline = Date.now() + timeoutMs; while (Date.now() < deadline) { await new Promise(resolve => setTimeout(resolve, 300)); if (await isExtensionConnected()) return; diff --git a/src/browser/page.ts b/src/browser/page.ts index 700588f..a1623d0 100644 --- a/src/browser/page.ts +++ b/src/browser/page.ts @@ -52,6 +52,11 @@ export class Page implements IPage { return sendCommand('exec', { code, ...this._tabOpt() }); } + async getCookies(opts: { domain?: string; url?: string } = {}): Promise { + const result = await sendCommand('cookies', opts); + return Array.isArray(result) ? result : []; + } + async snapshot(opts: { interactive?: boolean; compact?: boolean; maxDepth?: number; raw?: boolean } = {}): Promise { const maxDepth = Math.max(1, Math.min(Number(opts.maxDepth) || 50, 200)); const code = ` diff --git a/src/build-manifest.test.ts b/src/build-manifest.test.ts index d2adae7..b4eabfe 100644 --- a/src/build-manifest.test.ts +++ b/src/build-manifest.test.ts @@ -25,4 +25,40 @@ describe('parseTsArgsBlock', () => { }, ]); }); + + it('keeps hyphenated arg names from TS adapters', () => { + const args = parseTsArgsBlock(` + { + name: 'tweet-url', + help: 'Single tweet URL to download', + }, + { + name: 'download-images', + type: 'boolean', + default: false, + help: 'Download images locally', + }, + `); + + expect(args).toEqual([ + { + name: 'tweet-url', + type: 'str', + default: undefined, + required: false, + positional: undefined, + help: 'Single tweet URL to download', + choices: undefined, + }, + { + name: 'download-images', + type: 'boolean', + default: false, + required: false, + positional: undefined, + help: 'Download images locally', + choices: undefined, + }, + ]); + }); }); diff --git a/src/build-manifest.ts b/src/build-manifest.ts index 1fcaf98..ea9e657 100644 --- a/src/build-manifest.ts +++ b/src/build-manifest.ts @@ -114,7 +114,7 @@ export function parseTsArgsBlock(argsBlock: string): ManifestEntry['args'] { let cursor = 0; while (cursor < argsBlock.length) { - const nameMatch = argsBlock.slice(cursor).match(/\{\s*name\s*:\s*['"`](\w+)['"`]/); + const nameMatch = argsBlock.slice(cursor).match(/\{\s*name\s*:\s*['"`]([^'"`]+)['"`]/); if (!nameMatch || nameMatch.index === undefined) break; const objectStart = cursor + nameMatch.index; diff --git a/src/clis/bilibili/download.ts b/src/clis/bilibili/download.ts index 9b2759c..e4ca929 100644 --- a/src/clis/bilibili/download.ts +++ b/src/clis/bilibili/download.ts @@ -17,6 +17,7 @@ import { sanitizeFilename, getTempDir, exportCookiesToNetscape, + formatCookieHeader, } from '../../download/index.js'; import { DownloadProgressTracker, formatBytes } from '../../download/progress.js'; @@ -63,30 +64,18 @@ cli({ const title = sanitizeFilename(data?.title || 'video'); // Extract cookies for authenticated downloads - const cookieString = await page.evaluate(`(() => document.cookie)()`); + const cookies = await page.getCookies({ domain: 'bilibili.com' }); + const cookieString = formatCookieHeader(cookies); // Create output directory fs.mkdirSync(output, { recursive: true }); // Export cookies to Netscape format for yt-dlp let cookiesFile: string | undefined; - if (typeof cookieString === 'string' && cookieString) { + if (cookies.length > 0) { const tempDir = getTempDir(); fs.mkdirSync(tempDir, { recursive: true }); cookiesFile = path.join(tempDir, `bilibili_cookies_${Date.now()}.txt`); - - const cookies = cookieString.split(';').map((c) => { - const [name, ...rest] = c.trim().split('='); - return { - name: name || '', - value: rest.join('=') || '', - domain: '.bilibili.com', - path: '/', - secure: true, - httpOnly: false, - }; - }).filter((c) => c.name); - exportCookiesToNetscape(cookies, cookiesFile); } diff --git a/src/clis/twitter/download.ts b/src/clis/twitter/download.ts index a32d0a6..308430a 100644 --- a/src/clis/twitter/download.ts +++ b/src/clis/twitter/download.ts @@ -16,6 +16,7 @@ import { sanitizeFilename, getTempDir, exportCookiesToNetscape, + formatCookieHeader, } from '../../download/index.js'; import { DownloadProgressTracker, formatBytes } from '../../download/progress.js'; @@ -109,7 +110,8 @@ cli({ } // Extract cookies - const cookieString = await page.evaluate(`(() => document.cookie)()`); + const cookies = await page.getCookies({ domain: 'x.com' }); + const cookieString = formatCookieHeader(cookies); // Create output directory const outputDir = tweetUrl @@ -119,23 +121,10 @@ cli({ // Export cookies for yt-dlp let cookiesFile: string | undefined; - if (typeof cookieString === 'string' && cookieString) { + if (cookies.length > 0) { const tempDir = getTempDir(); fs.mkdirSync(tempDir, { recursive: true }); cookiesFile = path.join(tempDir, `twitter_cookies_${Date.now()}.txt`); - - const cookies = cookieString.split(';').map((c) => { - const [name, ...rest] = c.trim().split('='); - return { - name: name || '', - value: rest.join('=') || '', - domain: '.x.com', - path: '/', - secure: true, - httpOnly: false, - }; - }).filter((c) => c.name); - exportCookiesToNetscape(cookies, cookiesFile); } @@ -173,7 +162,7 @@ cli({ } else if (media.type === 'image') { // Direct HTTP download for images result = await httpDownload(media.url, destPath, { - cookies: typeof cookieString === 'string' ? cookieString : '', + cookies: cookieString, timeout: 30000, onProgress: (received, total) => { if (progressBar) progressBar.update(received, total); @@ -182,7 +171,7 @@ cli({ } else { // Direct HTTP download for direct video URLs result = await httpDownload(media.url, destPath, { - cookies: typeof cookieString === 'string' ? cookieString : '', + cookies: cookieString, timeout: 60000, onProgress: (received, total) => { if (progressBar) progressBar.update(received, total); diff --git a/src/clis/xiaohongshu/download.ts b/src/clis/xiaohongshu/download.ts index 890010c..bb6d89f 100644 --- a/src/clis/xiaohongshu/download.ts +++ b/src/clis/xiaohongshu/download.ts @@ -11,7 +11,7 @@ import { cli, Strategy } from '../../registry.js'; import { httpDownload, sanitizeFilename, - detectContentType, + formatCookieHeader, } from '../../download/index.js'; import { DownloadProgressTracker, formatBytes } from '../../download/progress.js'; @@ -114,7 +114,7 @@ cli({ } // Extract cookies for authenticated downloads - const cookies = await page.evaluate(`(() => document.cookie)()`); + const cookies = formatCookieHeader(await page.getCookies({ domain: 'xiaohongshu.com' })); // Create output directory const outputDir = path.join(output, noteId); @@ -134,7 +134,7 @@ cli({ try { const result = await httpDownload(media.url, destPath, { - cookies: typeof cookies === 'string' ? cookies : '', + cookies, timeout: 60000, onProgress: (received, total) => { if (progressBar) progressBar.update(received, total); diff --git a/src/clis/zhihu/download.ts b/src/clis/zhihu/download.ts index 967f6f9..ef7a024 100644 --- a/src/clis/zhihu/download.ts +++ b/src/clis/zhihu/download.ts @@ -8,7 +8,7 @@ import * as fs from 'node:fs'; import * as path from 'node:path'; import { cli, Strategy } from '../../registry.js'; -import { sanitizeFilename, httpDownload } from '../../download/index.js'; +import { sanitizeFilename, httpDownload, formatCookieHeader } from '../../download/index.js'; import { formatBytes } from '../../download/progress.js'; /** @@ -178,7 +178,7 @@ cli({ const imagesDir = path.join(output, 'images'); fs.mkdirSync(imagesDir, { recursive: true }); - const cookies = await page.evaluate(`(() => document.cookie)()`); + const cookies = formatCookieHeader(await page.getCookies({ domain: 'zhihu.com' })); for (let i = 0; i < data.images.length; i++) { const imgUrl = data.images[i]; @@ -188,7 +188,7 @@ cli({ try { await httpDownload(imgUrl, imgPath, { - cookies: typeof cookies === 'string' ? cookies : '', + cookies, timeout: 30000, }); diff --git a/src/download/index.test.ts b/src/download/index.test.ts new file mode 100644 index 0000000..56dc2b9 --- /dev/null +++ b/src/download/index.test.ts @@ -0,0 +1,16 @@ +import { describe, expect, it } from 'vitest'; +import { formatCookieHeader, resolveRedirectUrl } from './index.js'; + +describe('download helpers', () => { + it('resolves relative redirects against the original URL', () => { + expect(resolveRedirectUrl('https://example.com/a/file', '/cdn/file.bin')).toBe('https://example.com/cdn/file.bin'); + expect(resolveRedirectUrl('https://example.com/a/file', '../next')).toBe('https://example.com/next'); + }); + + it('formats browser cookies into a Cookie header', () => { + expect(formatCookieHeader([ + { name: 'sid', value: 'abc', domain: 'example.com' }, + { name: 'ct0', value: 'def', domain: 'example.com' }, + ])).toBe('sid=abc; ct0=def'); + }); +}); diff --git a/src/download/index.ts b/src/download/index.ts index 8253795..1105d0c 100644 --- a/src/download/index.ts +++ b/src/download/index.ts @@ -26,6 +26,16 @@ export interface YtdlpOptions { onProgress?: (percent: number) => void; } +export interface BrowserCookie { + name: string; + value: string; + domain: string; + path?: string; + secure?: boolean; + httpOnly?: boolean; + expirationDate?: number; +} + /** * Check if yt-dlp is available in PATH. */ @@ -142,14 +152,14 @@ export async function httpDownload( // Handle redirects if (response.statusCode && response.statusCode >= 300 && response.statusCode < 400 && response.headers.location) { file.close(); - fs.unlinkSync(tempPath); - httpDownload(response.headers.location, destPath, options).then(resolve); + if (fs.existsSync(tempPath)) fs.unlinkSync(tempPath); + httpDownload(resolveRedirectUrl(url, response.headers.location), destPath, options).then(resolve); return; } if (response.statusCode !== 200) { file.close(); - fs.unlinkSync(tempPath); + if (fs.existsSync(tempPath)) fs.unlinkSync(tempPath); resolve({ success: false, size: 0, error: `HTTP ${response.statusCode}` }); return; } @@ -187,11 +197,15 @@ export async function httpDownload( }); } +export function resolveRedirectUrl(currentUrl: string, location: string): string { + return new URL(location, currentUrl).toString(); +} + /** * Export cookies to Netscape format for yt-dlp. */ export function exportCookiesToNetscape( - cookies: Array<{ name: string; value: string; domain: string; path?: string; secure?: boolean; httpOnly?: boolean }>, + cookies: BrowserCookie[], filePath: string, ): void { const lines = [ @@ -214,6 +228,10 @@ export function exportCookiesToNetscape( fs.writeFileSync(filePath, lines.join('\n')); } +export function formatCookieHeader(cookies: BrowserCookie[]): string { + return cookies.map((cookie) => `${cookie.name}=${cookie.value}`).join('; '); +} + /** * Download video using yt-dlp. */ diff --git a/src/pipeline/executor.test.ts b/src/pipeline/executor.test.ts index dcd1b67..7dfddd9 100644 --- a/src/pipeline/executor.test.ts +++ b/src/pipeline/executor.test.ts @@ -11,6 +11,7 @@ function createMockPage(overrides: Partial = {}): IPage { return { goto: vi.fn(), evaluate: vi.fn().mockResolvedValue(null), + getCookies: vi.fn().mockResolvedValue([]), snapshot: vi.fn().mockResolvedValue(''), click: vi.fn(), typeText: vi.fn(), diff --git a/src/pipeline/steps/download.ts b/src/pipeline/steps/download.ts index 15453a6..86ecc13 100644 --- a/src/pipeline/steps/download.ts +++ b/src/pipeline/steps/download.ts @@ -23,6 +23,7 @@ import { generateFilename, exportCookiesToNetscape, getTempDir, + formatCookieHeader, } from '../../download/index.js'; import { DownloadProgressTracker, formatBytes } from '../../download/progress.js'; @@ -62,9 +63,8 @@ async function mapConcurrent( */ async function extractBrowserCookies(page: IPage, domain?: string): Promise { try { - // Use browser evaluate to get document.cookie - const cookieString = await page.evaluate(`(() => document.cookie)()`); - return typeof cookieString === 'string' ? cookieString : ''; + const cookies = await page.getCookies(domain ? { domain } : {}); + return formatCookieHeader(cookies); } catch { return ''; } @@ -78,20 +78,17 @@ async function extractCookiesArray( domain: string, ): Promise> { try { - const cookieString = await extractBrowserCookies(page); - if (!cookieString) return []; - - return cookieString.split(';').map((c) => { - const [name, ...rest] = c.trim().split('='); - return { - name: name || '', - value: rest.join('=') || '', - domain, - path: '/', - secure: true, - httpOnly: false, - }; - }).filter((c) => c.name); + const cookies = await page.getCookies({ domain }); + return cookies + .filter((cookie) => cookie.name) + .map((cookie) => ({ + name: cookie.name, + value: cookie.value, + domain: cookie.domain, + path: cookie.path ?? '/', + secure: cookie.secure ?? false, + httpOnly: cookie.httpOnly ?? false, + })); } catch { return []; } diff --git a/src/types.ts b/src/types.ts index 5ee5af6..3e807e3 100644 --- a/src/types.ts +++ b/src/types.ts @@ -8,6 +8,15 @@ export interface IPage { goto(url: string): Promise; evaluate(js: string): Promise; + getCookies(opts?: { domain?: string; url?: string }): Promise>; snapshot(opts?: { interactive?: boolean; compact?: boolean; maxDepth?: number; raw?: boolean }): Promise; click(ref: string): Promise; typeText(ref: string, text: string): Promise; diff --git a/src/verify.ts b/src/verify.ts index da4b970..f7fba7e 100644 --- a/src/verify.ts +++ b/src/verify.ts @@ -7,6 +7,9 @@ */ import { validateClisWithTarget, renderValidationReport, type ValidationReport } from './validate.js'; +import { spawn } from 'node:child_process'; +import * as fs from 'node:fs'; +import * as path from 'node:path'; export interface VerifyOptions { builtinClis: string; @@ -18,15 +21,73 @@ export interface VerifyOptions { export interface VerifyReport { ok: boolean; validation: ValidationReport; - smoke: null; + smoke: null | { + requested: boolean; + executed: boolean; + ok: boolean; + summary: string; + }; } export async function verifyClis(opts: VerifyOptions): Promise { const report = validateClisWithTarget([opts.builtinClis, opts.userClis], opts.target); - return { ok: report.ok, validation: report, smoke: null }; + let smoke: VerifyReport['smoke'] = null; + if (opts.smoke) { + smoke = await runSmokeTests(opts.builtinClis); + } + return { ok: report.ok && (smoke?.ok ?? true), validation: report, smoke }; } export function renderVerifyReport(report: VerifyReport): string { - return renderValidationReport(report.validation); + const base = renderValidationReport(report.validation); + if (!report.smoke) return base; + const status = report.smoke.ok ? 'PASS' : 'FAIL'; + const mode = report.smoke.executed ? 'executed' : 'skipped'; + return `${base}\nSmoke: ${status} (${mode}) β€” ${report.smoke.summary}`; } +async function runSmokeTests(builtinClis: string): Promise> { + const projectRoot = path.resolve(builtinClis, '..', '..'); + const smokeDir = path.join(projectRoot, 'tests', 'smoke'); + + if (!fs.existsSync(smokeDir)) { + return { + requested: true, + executed: false, + ok: false, + summary: 'Smoke tests are unavailable in this package/environment.', + }; + } + + const npx = process.platform === 'win32' ? 'npx.cmd' : 'npx'; + return new Promise((resolve) => { + const child = spawn(npx, ['vitest', 'run', 'tests/smoke/', '--reporter=dot'], { + cwd: projectRoot, + env: { ...process.env }, + stdio: ['ignore', 'pipe', 'pipe'], + }); + + let stderr = ''; + child.stderr.on('data', (chunk) => { + stderr += chunk.toString(); + }); + + child.on('error', (error) => { + resolve({ + requested: true, + executed: false, + ok: false, + summary: `Failed to start smoke tests: ${error.message}`, + }); + }); + + child.on('close', (code) => { + resolve({ + requested: true, + executed: true, + ok: code === 0, + summary: code === 0 ? 'tests/smoke passed' : (stderr.trim() || `vitest exited with code ${code}`), + }); + }); + }); +}