From cf9c59237935651404e14eca066a085c436cce9e Mon Sep 17 00:00:00 2001 From: Scott Lovegrove Date: Mon, 11 May 2026 15:17:57 +0100 Subject: [PATCH 1/4] refactor: delegate global-args + ViewOptions + empty-test helper to @doist/cli-core MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Mirrors twist-cli #209 on top of outline's cli-core 0.9.0 baseline. Both CLIs now share one source of truth for global-flag parsing, env-var gating, the `ViewOptions` shape, and the empty-machine-output test contract. | Module | Was | Now | | --- | --- | --- | | `src/lib/global-args.ts` | n/a | new module — `createGlobalArgsStore()` + `createAccessibleGate({ envVar: 'OL_ACCESSIBLE' })` + `createSpinnerGate({ envVar: 'OL_SPINNER' })`. | | `src/lib/spinner.ts` | inline `shouldDisableSpinner` reading `OL_SPINNER`/`CI`/argv | imports `shouldDisableSpinner` from `global-args.js`. | | `src/lib/output.ts` | local `OutputOptions = { json?, ndjson?, full? }` | `CoreViewOptions & { full?, raw? }`. Adds `printEmpty` wrapper around cli-core's `printEmpty`. | | `src/commands/document.ts` | `ol document list` printed nothing on an empty result | calls `printEmpty('No documents found.', outputOpts)` so machine consumers get `[]\n` / nothing for `--json` / `--ndjson` and humans get the message. | | `src/__tests__/empty-output.test.ts` | n/a | new — uses `describeEmptyMachineOutput` from `@doist/cli-core/testing` to lock in the `ol document list` empty contract. | ## Behaviour worth flagging - `--accessible` flag and `OL_ACCESSIBLE=1` env var are now recognised globally (no consumer reads `isAccessible` yet — wired for parity with twist + todoist). - `ol document list` against an empty result now prints `No documents found.` in human mode (was silent). `--json` / `--ndjson` behaviour matches the canonical empty contract. ## Test plan - [x] `npm run type-check` - [x] `npm run lint:check` — 0 warnings, 0 errors - [x] `npm test` — **127 / 127** pass - [x] `npm run build` - [x] `npm run check:skill-sync` - [ ] Smoke (gates): - `OL_SPINNER=false ol search foo`, `ol --no-spinner search foo` → spinner off - `CI=1 ol search foo` → spinner off; `CI=false ol search foo` → spinner ON - `OL_ACCESSIBLE=1 ol search foo`, `ol --accessible search foo` → no behaviour change yet (no consumer) - [ ] Smoke (empty list contract): - `ol document list --collection ` → `No documents found.` - `ol document list --collection --json` → `[]` - `ol document list --collection --ndjson | wc -c` → `0` Co-Authored-By: Claude Opus 4.7 (1M context) --- src/__tests__/empty-output.test.ts | 28 ++++++++++++++++++++++++++++ src/__tests__/output.test.ts | 1 + src/commands/document.ts | 9 +++++++-- src/lib/global-args.ts | 16 ++++++++++++++++ src/lib/output.ts | 24 ++++++++++++++++++++---- src/lib/spinner.ts | 13 +------------ 6 files changed, 73 insertions(+), 18 deletions(-) create mode 100644 src/__tests__/empty-output.test.ts create mode 100644 src/lib/global-args.ts diff --git a/src/__tests__/empty-output.test.ts b/src/__tests__/empty-output.test.ts new file mode 100644 index 0000000..feb01bc --- /dev/null +++ b/src/__tests__/empty-output.test.ts @@ -0,0 +1,28 @@ +import { describeEmptyMachineOutput } from '@doist/cli-core/testing' +import { Command } from 'commander' +import { vi } from 'vitest' + +vi.mock('../lib/auth.js', () => ({ + getApiToken: async () => 'test-token', + getBaseUrl: async () => 'https://test.outline.com', + getOAuthClientId: async () => undefined, + getTokenSource: async () => 'config' as const, + saveConfig: vi.fn(), + clearConfig: vi.fn(), +})) + +vi.mock('../lib/api.js', () => ({ + apiRequest: vi.fn().mockResolvedValue({ data: [], pagination: undefined }), +})) + +describeEmptyMachineOutput('ol document list', { + setup: () => {}, + run: async (extraArgs) => { + const { registerDocumentCommand } = await import('../commands/document.js') + const program = new Command() + program.exitOverride() + registerDocumentCommand(program) + await program.parseAsync(['node', 'ol', 'document', 'list', ...extraArgs]) + }, + humanMessage: 'No documents found.', +}) diff --git a/src/__tests__/output.test.ts b/src/__tests__/output.test.ts index 1db5a4a..75dc19d 100644 --- a/src/__tests__/output.test.ts +++ b/src/__tests__/output.test.ts @@ -53,6 +53,7 @@ describe('output', () => { json: true, ndjson: false, full: true, + raw: false, }) }) diff --git a/src/commands/document.ts b/src/commands/document.ts index dce30ed..f374847 100644 --- a/src/commands/document.ts +++ b/src/commands/document.ts @@ -5,7 +5,7 @@ import open from 'open' import { apiRequest } from '../lib/api.js' import { getBaseUrl } from '../lib/auth.js' import { renderMarkdown } from '../lib/markdown.js' -import { formatError, getOutputOptions, outputItem, outputList } from '../lib/output.js' +import { formatError, getOutputOptions, outputItem, outputList, printEmpty } from '../lib/output.js' import { resolveCollectionId, resolveDocumentId, resolveDocumentRef } from '../lib/refs.js' interface Document { @@ -79,7 +79,12 @@ export function registerDocumentCommand(program: Command): void { const { data, pagination } = await apiRequest('documents.list', body) - outputList(data, formatDoc, essentialKeys, getOutputOptions(opts), pagination) + const outputOpts = getOutputOptions(opts) + if (data.length === 0) { + printEmpty('No documents found.', outputOpts) + return + } + outputList(data, formatDoc, essentialKeys, outputOpts, pagination) }) doc.command('get ') diff --git a/src/lib/global-args.ts b/src/lib/global-args.ts new file mode 100644 index 0000000..6f7e55d --- /dev/null +++ b/src/lib/global-args.ts @@ -0,0 +1,16 @@ +import { createAccessibleGate, createGlobalArgsStore, createSpinnerGate } from '@doist/cli-core' + +const store = createGlobalArgsStore() + +export const getGlobalArgs = store.get +export const resetGlobalArgs = store.reset + +export const isAccessible = createAccessibleGate({ + envVar: 'OL_ACCESSIBLE', + getArgs: store.get, +}) + +export const shouldDisableSpinner = createSpinnerGate({ + envVar: 'OL_SPINNER', + getArgs: store.get, +}) diff --git a/src/lib/output.ts b/src/lib/output.ts index 54e4a64..1bd9bf1 100644 --- a/src/lib/output.ts +++ b/src/lib/output.ts @@ -1,11 +1,15 @@ -import { formatJson, formatNdjson } from '@doist/cli-core' +import { + formatJson, + formatNdjson, + printEmpty as corePrintEmpty, + type ViewOptions, +} from '@doist/cli-core' import chalk from 'chalk' import type { Pagination } from './api.js' -interface OutputOptions { - json?: boolean - ndjson?: boolean +export type OutputOptions = ViewOptions & { full?: boolean + raw?: boolean } export function getOutputOptions(opts: Record): OutputOptions { @@ -13,9 +17,21 @@ export function getOutputOptions(opts: Record): OutputOptions { json: Boolean(opts.json), ndjson: Boolean(opts.ndjson), full: Boolean(opts.full), + raw: Boolean(opts.raw), } } +/** + * Wraps cli-core's `printEmpty` so list commands can call a single helper + * for the empty-state branch and stay consistent with the canonical contract: + * --json → `[]\n` + * --ndjson → no output + * neither → human message + */ +export function printEmpty(message: string, opts: OutputOptions = {}): void { + corePrintEmpty({ options: opts, message }) +} + export function outputItem( item: T, humanFormatter: (item: T) => string, diff --git a/src/lib/spinner.ts b/src/lib/spinner.ts index bc7fee7..08d89ff 100644 --- a/src/lib/spinner.ts +++ b/src/lib/spinner.ts @@ -1,19 +1,8 @@ import { createSpinner } from '@doist/cli-core' +import { shouldDisableSpinner } from './global-args.js' export type { SpinnerColor, SpinnerOptions } from '@doist/cli-core' -function shouldDisableSpinner(): boolean { - if (process.env.OL_SPINNER === 'false') return true - if (process.env.CI && process.env.CI !== 'false') return true - - const args = process.argv - if (args.includes('--json') || args.includes('--ndjson') || args.includes('--no-spinner')) { - return true - } - - return false -} - const spinner = createSpinner({ isDisabled: shouldDisableSpinner }) export const { LoadingSpinner, withSpinner, startEarlySpinner, stopEarlySpinner } = spinner From 5cfcda7eef134689eaa9673660ff5f03b62b3ad2 Mon Sep 17 00:00:00 2001 From: Scott Lovegrove Date: Mon, 11 May 2026 15:40:12 +0100 Subject: [PATCH 2/4] review: address PR #67 feedback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - index.ts: register `--accessible` as a Commander root option so the gate has a corresponding CLI flag (cli-core's `parseGlobalArgs` reads argv directly but Commander rejects unknown options). - output.ts: `outputList` accepts an optional `emptyMessage` and routes through `printEmpty` internally — no per-command `if (data.length === 0)` duplication. - output.ts: drop `raw` from shared `OutputOptions`. It's a `document get` rendering flag, not a generic view option; read locally in `document.ts`. - document.ts: list call site uses the new `emptyMessage` param. - spinner test: cover the new disable triggers from `createSpinnerGate` — `--progress-jsonl`, `--progress-jsonl=path`, `--verbose`, `-v`. - vitest.config: exclude `.claude/worktrees/**` so parallel agent worktrees don't pollute the local test run. Co-Authored-By: Claude Opus 4.7 (1M context) --- .claude/worktrees/agent-afbab9cdd70deaebf | 1 + src/__tests__/output.test.ts | 1 - src/__tests__/spinner.test.ts | 20 ++++++++++++++++++++ src/commands/document.ts | 16 +++++++++------- src/index.ts | 1 + src/lib/output.ts | 8 ++++++-- vitest.config.ts | 1 + 7 files changed, 38 insertions(+), 10 deletions(-) create mode 160000 .claude/worktrees/agent-afbab9cdd70deaebf diff --git a/.claude/worktrees/agent-afbab9cdd70deaebf b/.claude/worktrees/agent-afbab9cdd70deaebf new file mode 160000 index 0000000..e1e4f64 --- /dev/null +++ b/.claude/worktrees/agent-afbab9cdd70deaebf @@ -0,0 +1 @@ +Subproject commit e1e4f644510c59286399b35b6b02b3cc90b5b23e diff --git a/src/__tests__/output.test.ts b/src/__tests__/output.test.ts index 75dc19d..1db5a4a 100644 --- a/src/__tests__/output.test.ts +++ b/src/__tests__/output.test.ts @@ -53,7 +53,6 @@ describe('output', () => { json: true, ndjson: false, full: true, - raw: false, }) }) diff --git a/src/__tests__/spinner.test.ts b/src/__tests__/spinner.test.ts index 7ce9c77..889db0a 100644 --- a/src/__tests__/spinner.test.ts +++ b/src/__tests__/spinner.test.ts @@ -79,4 +79,24 @@ describe('spinner wiring', () => { process.argv = ['node', 'ol', 'auth', 'status', '--no-spinner'] expect((await loadIsDisabled())()).toBe(true) }) + + it('disables when --progress-jsonl is in argv', async () => { + process.argv = ['node', 'ol', 'search', 'foo', '--progress-jsonl'] + expect((await loadIsDisabled())()).toBe(true) + }) + + it('disables when --progress-jsonl=path is in argv', async () => { + process.argv = ['node', 'ol', 'search', 'foo', '--progress-jsonl=/tmp/p.jsonl'] + expect((await loadIsDisabled())()).toBe(true) + }) + + it('disables when --verbose is in argv', async () => { + process.argv = ['node', 'ol', 'search', 'foo', '--verbose'] + expect((await loadIsDisabled())()).toBe(true) + }) + + it('disables when -v short flag is in argv', async () => { + process.argv = ['node', 'ol', 'search', 'foo', '-v'] + expect((await loadIsDisabled())()).toBe(true) + }) }) diff --git a/src/commands/document.ts b/src/commands/document.ts index f374847..d3404ee 100644 --- a/src/commands/document.ts +++ b/src/commands/document.ts @@ -5,7 +5,7 @@ import open from 'open' import { apiRequest } from '../lib/api.js' import { getBaseUrl } from '../lib/auth.js' import { renderMarkdown } from '../lib/markdown.js' -import { formatError, getOutputOptions, outputItem, outputList, printEmpty } from '../lib/output.js' +import { formatError, getOutputOptions, outputItem, outputList } from '../lib/output.js' import { resolveCollectionId, resolveDocumentId, resolveDocumentRef } from '../lib/refs.js' interface Document { @@ -79,12 +79,14 @@ export function registerDocumentCommand(program: Command): void { const { data, pagination } = await apiRequest('documents.list', body) - const outputOpts = getOutputOptions(opts) - if (data.length === 0) { - printEmpty('No documents found.', outputOpts) - return - } - outputList(data, formatDoc, essentialKeys, outputOpts, pagination) + outputList( + data, + formatDoc, + essentialKeys, + getOutputOptions(opts), + pagination, + 'No documents found.', + ) }) doc.command('get ') diff --git a/src/index.ts b/src/index.ts index e908b71..45d253f 100644 --- a/src/index.ts +++ b/src/index.ts @@ -15,6 +15,7 @@ program .version('0.1.0') .description('CLI for the Outline wiki/knowledge base API') .option('--no-spinner', 'Disable loading animations') + .option('--accessible', 'Render output in screen-reader-friendly mode') .addHelpText( 'after', ` diff --git a/src/lib/output.ts b/src/lib/output.ts index 1bd9bf1..bd17310 100644 --- a/src/lib/output.ts +++ b/src/lib/output.ts @@ -9,7 +9,6 @@ import type { Pagination } from './api.js' export type OutputOptions = ViewOptions & { full?: boolean - raw?: boolean } export function getOutputOptions(opts: Record): OutputOptions { @@ -17,7 +16,6 @@ export function getOutputOptions(opts: Record): OutputOptions { json: Boolean(opts.json), ndjson: Boolean(opts.ndjson), full: Boolean(opts.full), - raw: Boolean(opts.raw), } } @@ -57,7 +55,13 @@ export function outputList( essentialKeys?: (keyof T)[], opts: OutputOptions = {}, pagination?: Pagination, + emptyMessage?: string, ): void { + if (items.length === 0 && emptyMessage !== undefined) { + printEmpty(emptyMessage, opts) + return + } + const project = (item: T) => (opts.full || !essentialKeys ? item : pick(item, essentialKeys)) if (opts.ndjson) { diff --git a/vitest.config.ts b/vitest.config.ts index 3f166f1..e807403 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -2,6 +2,7 @@ import { defineConfig } from 'vitest/config' export default defineConfig({ test: { + exclude: ['**/node_modules/**', '**/dist/**', '**/.claude/worktrees/**'], server: { deps: { inline: ['@doist/cli-core'], From 5e7ab2b717bf8c9b5c2d24f79ec098448b70278d Mon Sep 17 00:00:00 2001 From: Scott Lovegrove Date: Mon, 11 May 2026 15:40:31 +0100 Subject: [PATCH 3/4] chore: untrack accidental worktree submodule + ignore worktrees dir Co-Authored-By: Claude Opus 4.7 (1M context) --- .claude/worktrees/agent-afbab9cdd70deaebf | 1 - .gitignore | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) delete mode 160000 .claude/worktrees/agent-afbab9cdd70deaebf diff --git a/.claude/worktrees/agent-afbab9cdd70deaebf b/.claude/worktrees/agent-afbab9cdd70deaebf deleted file mode 160000 index e1e4f64..0000000 --- a/.claude/worktrees/agent-afbab9cdd70deaebf +++ /dev/null @@ -1 +0,0 @@ -Subproject commit e1e4f644510c59286399b35b6b02b3cc90b5b23e diff --git a/.gitignore b/.gitignore index fd8e4b0..1ca2268 100644 --- a/.gitignore +++ b/.gitignore @@ -2,3 +2,4 @@ node_modules/ dist/ .claude/settings.local.json .claude/scheduled_tasks.lock +.claude/worktrees/ From 412cd89eacdb18dea0e1e936424576f387f7a4fc Mon Sep 17 00:00:00 2001 From: Scott Lovegrove Date: Mon, 11 May 2026 15:49:18 +0100 Subject: [PATCH 4/4] review: drop printEmpty wrapper + revert worktree workarounds - output.ts: list commands route through cli-core's `printEmpty` directly via `outputList`'s `emptyMessage` param. The local wrapper only re-shaped the args and added no value. - .gitignore + vitest.config.ts: revert the `.claude/worktrees/` ignore and exclude rules from 5cfcda7 / 5e7ab2b. Those were workarounds for an agent worktree mistakenly created inside the repo; agent worktrees should live under `~/worktrees` and never under the project root. Co-Authored-By: Claude Opus 4.7 (1M context) --- .gitignore | 1 - src/lib/output.ts | 20 ++------------------ vitest.config.ts | 1 - 3 files changed, 2 insertions(+), 20 deletions(-) diff --git a/.gitignore b/.gitignore index 1ca2268..fd8e4b0 100644 --- a/.gitignore +++ b/.gitignore @@ -2,4 +2,3 @@ node_modules/ dist/ .claude/settings.local.json .claude/scheduled_tasks.lock -.claude/worktrees/ diff --git a/src/lib/output.ts b/src/lib/output.ts index bd17310..c0a90c6 100644 --- a/src/lib/output.ts +++ b/src/lib/output.ts @@ -1,9 +1,4 @@ -import { - formatJson, - formatNdjson, - printEmpty as corePrintEmpty, - type ViewOptions, -} from '@doist/cli-core' +import { formatJson, formatNdjson, printEmpty, type ViewOptions } from '@doist/cli-core' import chalk from 'chalk' import type { Pagination } from './api.js' @@ -19,17 +14,6 @@ export function getOutputOptions(opts: Record): OutputOptions { } } -/** - * Wraps cli-core's `printEmpty` so list commands can call a single helper - * for the empty-state branch and stay consistent with the canonical contract: - * --json → `[]\n` - * --ndjson → no output - * neither → human message - */ -export function printEmpty(message: string, opts: OutputOptions = {}): void { - corePrintEmpty({ options: opts, message }) -} - export function outputItem( item: T, humanFormatter: (item: T) => string, @@ -58,7 +42,7 @@ export function outputList( emptyMessage?: string, ): void { if (items.length === 0 && emptyMessage !== undefined) { - printEmpty(emptyMessage, opts) + printEmpty({ options: opts, message: emptyMessage }) return } diff --git a/vitest.config.ts b/vitest.config.ts index e807403..3f166f1 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -2,7 +2,6 @@ import { defineConfig } from 'vitest/config' export default defineConfig({ test: { - exclude: ['**/node_modules/**', '**/dist/**', '**/.claude/worktrees/**'], server: { deps: { inline: ['@doist/cli-core'],