diff --git a/cli-manifest.json b/cli-manifest.json index 221a656b6..cb38bb01a 100644 --- a/cli-manifest.json +++ b/cli-manifest.json @@ -4855,6 +4855,27 @@ "default": false, "required": false, "help": "Emit assistant replies as markdown" + }, + { + "name": "limit", + "type": "int", + "default": 0, + "required": false, + "help": "Max latest visible messages to return; default 0 returns all" + }, + { + "name": "max-chars", + "type": "int", + "default": 0, + "required": false, + "help": "Max characters per message; default 0 returns full text" + }, + { + "name": "timeout", + "type": "int", + "default": 15, + "required": false, + "help": "Max seconds for the read command before runtime padding" } ], "columns": [ diff --git a/clis/chatgpt/commands.test.js b/clis/chatgpt/commands.test.js index 80d773bd2..181f35b8a 100644 --- a/clis/chatgpt/commands.test.js +++ b/clis/chatgpt/commands.test.js @@ -42,4 +42,13 @@ describe('chatgpt browser command registration', () => { expect.objectContaining({ name: 'new', type: 'boolean', default: false }), ])); }); + + it('keeps read timeout-enforced while preserving full-output defaults', () => { + const read = getRegistry().get('chatgpt/read'); + expect(read.args).toEqual(expect.arrayContaining([ + expect.objectContaining({ name: 'limit', type: 'int', default: 0 }), + expect.objectContaining({ name: 'max-chars', type: 'int', default: 0 }), + expect.objectContaining({ name: 'timeout', type: 'int', default: 15 }), + ])); + }); }); diff --git a/clis/chatgpt/read.js b/clis/chatgpt/read.js index 6fad148de..215e0648e 100644 --- a/clis/chatgpt/read.js +++ b/clis/chatgpt/read.js @@ -1,12 +1,15 @@ import { cli, Strategy } from '@jackwener/opencli/registry'; -import { EmptyResultError } from '@jackwener/opencli/errors'; +import { ArgumentError, EmptyResultError } from '@jackwener/opencli/errors'; import { CHATGPT_DOMAIN, + buildChatGPTReadEmptyHint, + currentChatGPTUrl, ensureChatGPTLogin, ensureOnChatGPT, getVisibleMessages, messageHtmlToMarkdown, normalizeBooleanFlag, + requirePositiveInt, } from './utils.js'; export const readCommand = cli({ @@ -21,24 +24,48 @@ export const readCommand = cli({ navigateBefore: false, args: [ { name: 'markdown', type: 'boolean', default: false, help: 'Emit assistant replies as markdown' }, + { name: 'limit', type: 'int', default: 0, help: 'Max latest visible messages to return; default 0 returns all' }, + { name: 'max-chars', type: 'int', default: 0, help: 'Max characters per message; default 0 returns full text' }, + { name: 'timeout', type: 'int', default: 15, help: 'Max seconds for the read command before runtime padding' }, ], columns: ['Index', 'Role', 'Text'], func: async (page, kwargs) => { const wantMarkdown = normalizeBooleanFlag(kwargs.markdown, false); + const limit = Number(kwargs.limit ?? 0); + if (!Number.isInteger(limit) || limit < 0) { + throw new ArgumentError('--limit must be a non-negative integer', 'Use --limit 0 to return all visible messages.'); + } + const maxChars = Number(kwargs['max-chars'] ?? 0); + if (!Number.isInteger(maxChars) || maxChars < 0) { + throw new ArgumentError('--max-chars must be a non-negative integer', 'Use --max-chars 0 to return full message text.'); + } + requirePositiveInt( + Number(kwargs.timeout ?? 15), + 'chatgpt read --timeout', + 'Example: opencli chatgpt read --timeout 15', + ); // ensureOnChatGPT now waits for the composer selector after navigating, // so the previous standalone 2 s settle is redundant. await ensureOnChatGPT(page); await ensureChatGPTLogin(page, 'ChatGPT read requires a logged-in ChatGPT session.'); const messages = await getVisibleMessages(page); if (!messages.length) { - throw new EmptyResultError('chatgpt read', 'No visible ChatGPT messages were found in the current conversation.'); + const currentUrl = await currentChatGPTUrl(page); + throw new EmptyResultError('chatgpt read', buildChatGPTReadEmptyHint(currentUrl)); } - return messages.map((message) => ({ - Index: message.Index, - Role: message.Role, - Text: wantMarkdown && message.Role === 'Assistant' && message.Html + const selected = limit > 0 ? messages.slice(-limit) : messages; + return selected.map((message) => { + const text = wantMarkdown && message.Role === 'Assistant' && message.Html ? (messageHtmlToMarkdown(message.Html) || message.Text) - : message.Text, - })); + : message.Text; + const clipped = maxChars > 0 && text.length > maxChars + ? `${text.slice(0, maxChars)}\n\n[truncated ${text.length - maxChars} chars; rerun with --max-chars 0 for full text]` + : text; + return { + Index: message.Index, + Role: message.Role, + Text: clipped, + }; + }); }, }); diff --git a/clis/chatgpt/utils.js b/clis/chatgpt/utils.js index 422e8f1f6..22506a9b9 100644 --- a/clis/chatgpt/utils.js +++ b/clis/chatgpt/utils.js @@ -41,6 +41,26 @@ function isSameChatGPTConversation(currentUrl, expectedUrl) { || currentUrl.startsWith(`${expectedUrl}#`); } +export function isChatGPTConversationUrl(value) { + if (!value) return false; + try { + const url = new URL(String(value)); + const host = url.hostname; + if (host !== CHATGPT_DOMAIN && !host.endsWith(`.${CHATGPT_DOMAIN}`)) return false; + return /^\/c\/[^/?#]+/.test(url.pathname); + } catch { + return false; + } +} + +export function buildChatGPTReadEmptyHint(currentUrl) { + const location = currentUrl ? `Current OpenCLI ChatGPT session is at ${currentUrl}. ` : ''; + if (!isChatGPTConversationUrl(currentUrl)) { + return `${location}OpenCLI is not on a ChatGPT conversation page, so there are no visible messages to read. Run "opencli chatgpt history --limit 5" then "opencli chatgpt detail ", or use "opencli chatgpt-app read" for the ChatGPT Desktop foreground window.`; + } + return `${location}No visible ChatGPT messages were found in this conversation. The conversation may be empty, still loading, inaccessible, or the ChatGPT DOM may have changed.`; +} + function buildComposerLocatorScript() { const markerAttr = 'data-opencli-chatgpt-composer'; return ` @@ -853,6 +873,8 @@ export const __test__ = { SEND_BUTTON_LABELS, CLOSE_SIDEBAR_LABELS, buildComposerLocatorScript, + buildChatGPTReadEmptyHint, + isChatGPTConversationUrl, isSameChatGPTConversation, parseChatGPTConversationId, imageMimeFromPath, diff --git a/clis/chatgpt/utils.test.js b/clis/chatgpt/utils.test.js index 4eaa5b840..181bacd4d 100644 --- a/clis/chatgpt/utils.test.js +++ b/clis/chatgpt/utils.test.js @@ -88,6 +88,26 @@ describe('chatgpt conversation id parsing', () => { }); }); +describe('chatgpt read empty-result hints', () => { + it('distinguishes the ChatGPT home page from an empty conversation', () => { + expect(__test__.isChatGPTConversationUrl('https://chatgpt.com/')).toBe(false); + expect(__test__.isChatGPTConversationUrl('https://chatgpt.com/c/abc_123-def?model=gpt-5')).toBe(true); + + const hint = __test__.buildChatGPTReadEmptyHint('https://chatgpt.com/'); + expect(hint).toContain('Current OpenCLI ChatGPT session is at https://chatgpt.com/.'); + expect(hint).toContain('not on a ChatGPT conversation page'); + expect(hint).toContain('opencli chatgpt history --limit 5'); + expect(hint).toContain('opencli chatgpt-app read'); + }); + + it('keeps selector/loading guidance for conversation pages with no extracted messages', () => { + const hint = __test__.buildChatGPTReadEmptyHint('https://chatgpt.com/c/abc_123-def'); + expect(hint).toContain('No visible ChatGPT messages were found in this conversation'); + expect(hint).toContain('DOM may have changed'); + expect(hint).not.toContain('not on a ChatGPT conversation page'); + }); +}); + describe('chatgpt send selectors', () => { it('inlines the composer locator without returning before caller code runs', () => { const dom = new JSDOM('
', { diff --git a/src/browser/daemon-client.test.ts b/src/browser/daemon-client.test.ts index 2a65f91d1..f7e762016 100644 --- a/src/browser/daemon-client.test.ts +++ b/src/browser/daemon-client.test.ts @@ -243,4 +243,14 @@ describe('daemon-client', () => { } satisfies Partial); expect(fetchMock).toHaveBeenCalledTimes(1); }); + + it('does not retry close-window cleanup failures', async () => { + const fetchMock = vi.mocked(fetch); + const abort = new Error('aborted'); + abort.name = 'AbortError'; + fetchMock.mockRejectedValue(abort); + + await expect(sendCommand('close-window', { session: 'test', surface: 'adapter' })).rejects.toThrow('aborted'); + expect(fetchMock).toHaveBeenCalledTimes(1); + }); }); diff --git a/src/browser/daemon-client.ts b/src/browser/daemon-client.ts index 1a789a87f..0fdf06f6c 100644 --- a/src/browser/daemon-client.ts +++ b/src/browser/daemon-client.ts @@ -12,6 +12,8 @@ import { resolveProfileContextId } from './profile.js'; const DAEMON_PORT = parseInt(process.env.OPENCLI_DAEMON_PORT ?? String(DEFAULT_DAEMON_PORT), 10); const DAEMON_URL = `http://127.0.0.1:${DAEMON_PORT}`; const OPENCLI_HEADERS = { 'X-OpenCLI': '1' }; +const DEFAULT_COMMAND_REQUEST_TIMEOUT_MS = 30_000; +const CLOSE_WINDOW_REQUEST_TIMEOUT_MS = 1_500; let _idCounter = 0; @@ -176,7 +178,10 @@ async function sendCommandRaw( action: DaemonCommand['action'], params: Omit, ): Promise { - const maxRetries = 4; + const maxRetries = action === 'close-window' ? 1 : 4; + const requestTimeout = action === 'close-window' + ? CLOSE_WINDOW_REQUEST_TIMEOUT_MS + : DEFAULT_COMMAND_REQUEST_TIMEOUT_MS; for (let attempt = 1; attempt <= maxRetries; attempt++) { const id = generateId(); @@ -192,7 +197,7 @@ async function sendCommandRaw( method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify(command), - timeout: 30000, + timeout: requestTimeout, }); const result = (await res.json()) as DaemonResult; diff --git a/src/output.test.ts b/src/output.test.ts index 475199b10..6cecaf981 100644 --- a/src/output.test.ts +++ b/src/output.test.ts @@ -52,4 +52,22 @@ describe('output TTY detection', () => { expect(out).not.toContain('name: alice'); expect(out).toContain('alice'); }); + + it('auto-downgrades default TTY table for long multi-line cells', () => { + Object.defineProperty(process.stdout, 'isTTY', { value: true, writable: true }); + const text = Array.from({ length: 12 }, (_, i) => `第 ${i} 行 ` + '長文字'.repeat(120)).join('\n'); + render([{ Role: 'Assistant', Text: text }], { fmt: 'table', columns: ['Role', 'Text'] }); + const out = logSpy.mock.calls.map((c: unknown[]) => c[0]).join('\n'); + expect(out).toContain('Role: Assistant'); + expect(out).toContain('Text:'); + }); + + it('keeps explicit TTY table for long cells', () => { + Object.defineProperty(process.stdout, 'isTTY', { value: true, writable: true }); + const text = '長文字'.repeat(260); + render([{ Role: 'Assistant', Text: text }], { fmt: 'table', fmtExplicit: true, columns: ['Role', 'Text'] }); + const out = logSpy.mock.calls.map((c: unknown[]) => c[0]).join('\n'); + expect(out).not.toContain('Role: Assistant'); + expect(out).toContain('Assistant'); + }); }); diff --git a/src/output.ts b/src/output.ts index ea6ae0172..e51defc77 100644 --- a/src/output.ts +++ b/src/output.ts @@ -31,6 +31,7 @@ export function render(data: unknown, opts: RenderOptions = {}): void { // Non-TTY auto-downgrade only when format was NOT explicitly passed by user. if (!opts.fmtExplicit) { if (fmt === 'table' && !process.stdout.isTTY) fmt = 'yaml'; + if (fmt === 'table' && process.stdout.isTTY && shouldDowngradeDefaultTable(data, opts)) fmt = 'yaml'; } if (data === null || data === undefined) { console.log(data); @@ -46,6 +47,19 @@ export function render(data: unknown, opts: RenderOptions = {}): void { } } +function shouldDowngradeDefaultTable(data: unknown, opts: RenderOptions): boolean { + if (data === null || data === undefined) return false; + const rows = normalizeRows(data); + if (!rows.length) return false; + const columns = resolveColumns(rows, opts); + return rows.some(row => columns.some(column => { + const value = row[column]; + if (value === null || value === undefined) return false; + const text = String(value); + return text.length > 1000 || text.split('\n').length > 8; + })); +} + function renderTable(data: unknown, opts: RenderOptions): void { const rows = normalizeRows(data); if (!rows.length) { console.log('(no data)'); return; }