refactor: Start integrating `@doist/cli-core` by scottlovegrove · Pull Request #208 · Doist/twist-cli

scottlovegrove · 2026-05-09T19:15:42Z

Summary

First consumer-side integration of @doist/cli-core 0.9.0 into twist-cli. Mirrors todoist-cli #313 (errors / config / spinner helpers / global-args / output) and #314 (full spinner delegation via createSpinner) as a single combined PR.

The local config API stays at getConfig / setConfig / updateConfig — they're now thin wrappers around cli-core's readConfig / writeConfig / updateConfig. Zero call-site churn outside src/lib/config.ts itself.

What's wired through cli-core now

Module	Was	Now
`src/lib/errors.ts`	full local `CliError` class	local subclass over cli-core's `CliError<ErrorCode>` — keeps the historical positional `(code, message, hints?, type?)` signature so the ~hundreds of existing call sites stay untouched. `code` accepts `ErrorCode \| CliErrorCode` so cli-core's canonical codes (`CONFIG_*`, etc.) type-check at call sites without redeclaring them locally.
`src/lib/config.ts`	local path / read / strict-read / write impls using `node:fs/promises`, `node:os`, `node:path`	thin wrappers: `getConfigPath` (lazy, takes `'twist-cli'`), `getConfig` → `readConfig`, `setConfig` → `writeConfig`, `updateConfig` → `updateConfig`. Schema (`Config`, `AuthMode`, `UpdateChannel`, `UserSettings`, the `KNOWN_*_KEYS` sets, `validateConfigForDoctor`) stays. `readConfigStrict` translates cli-core's discriminated `ReadConfigStrictResult` into the same three twist `CliError`s with identical hint copy.
`src/lib/spinner.ts`	local 130-line impl with yocto-spinner, early-spinner singleton, stdout interception, adoption/release semantics, `LoadingSpinner` class, colour palette	`const spinner = createSpinner({ isDisabled: shouldDisableSpinner })` + destructured exports. The CLI-specific bit (`TW_SPINNER` opt-out + `--json` / `--ndjson` / `--no-spinner` fan-out) is wired in as `isDisabled`. `yocto-spinner` drops from direct deps (still resolved transitively via cli-core).
`src/lib/global-args.ts`	`process.env.CI` direct read	`isCI()` (also opt-out aware: `CI='false'` is treated as not-CI)
`src/lib/output.ts`	inline `JSON.stringify(_, null, 2)` / `items.map(JSON.stringify).join('\n')`	delegates serialization to `formatJson` / `formatNdjson`; entity-aware processing via `filterEntityFields` and the `_meta` cursor sentinel for paginated NDJSON stays. Paginated formatters route through the local helpers (single serialization path).

formatErrorJson deliberately keeps its inline compact JSON.stringify — it emits single-line JSON, not pretty-print, so it doesn't fit cli-core's formatJson primitive. shouldDisableSpinner likewise stays local since it bundles the TW_SPINNER env var with the global-args flag fan-out.

Behaviour change worth flagging

yocto-spinner.success() and .error() already prepend their own ✔ / ✖ glyphs. The old code added a manual ✓ / ✗ on top, which produced doubled symbols in real terminals (e.g. ✔ ✓ Done). cli-core 0.3+ drops the manual prefixes — so the success / fail line now renders as a single ✔ Done instead of ✔ ✓ Done. Surfaced upstream during the cli-core review and confirmed in cli-core's own spinner tests.

updateConfig now uses cli-core's strict-read semantics: a broken config file throws instead of being silently overwritten (the previous lenient getConfig + setConfig composition could clobber data the user might have recovered by hand-fixing the file).

vitest config

Adds server.deps.inline: ['@doist/cli-core'] to vitest.config.ts. Without it, vitest treats the real-installed cli-core as external and vi.mock('@doist/cli-core', …) / vi.doMock('node:fs/promises', …) don't reach its compiled imports, breaking the auth / config suites.

Tests

src/lib/spinner.test.ts is trimmed to a single smoke test that mocks @doist/cli-core#createSpinner and asserts the kit is built with { isDisabled: shouldDisableSpinner }. The previous 333 lines of behavioural coverage are now in cli-core's own spinner suite; the wiring assertion is the one thing that has to live here.

src/lib/config.test.ts gains two new blocks:

readConfigStrict wrapper — covers all five state→CliError translations (missing, present, read-failed, invalid-json, invalid-shape).
thin config wrappers — proves getConfigPath, getConfig, setConfig, updateConfig each forward to cli-core with the resolved 'twist-cli' app-name path. A wrong app name would silently redirect every config read/write — these tests are the tripwire.

src/lib/global-args.test.ts adds a regression locking in cli-core's isCI() opt-out: CI='false' no longer disables the spinner.

Test plan

npm run type-check
npm run lint:check — 0 warnings, 0 errors
npm test — 579 / 579 pass
npm run build
npm ls yocto-spinner confirms cli-core still resolves it transitively
Manually exercise tw auth status, tw config view, tw doctor against a real config to confirm parity at the wrapper boundary
Run a long-running command (tw inbox) in a TTY to confirm the spinner still appears, the early spinner is adopted, and no doubled ✔ ✓ glyphs on success
CI=1 tw inbox, tw inbox --json, TW_SPINNER=false tw inbox to confirm spinner suppression via isCI() and shouldDisableSpinner

🤖 Generated with Claude Code

Wire twist-cli's foundational primitives through @doist/cli-core, mirroring todoist-cli #313 (errors, config, spinner helpers, global-args, output) + #314 (full spinner delegation via createSpinner). Also rename the local config API getConfig/setConfig -> readConfig/writeConfig so the vocabulary matches cli-core. - errors.ts now subclasses cli-core's CliError, preserving the positional (code, message, hints?, type?) ctor used across the CLI's hundreds of call sites. - config.ts delegates getConfigPath/readConfig/readConfigStrict/ writeConfig to cli-core; readConfigStrict's discriminated result is translated to twist's existing CONFIG_READ_FAILED / CONFIG_INVALID_JSON / CONFIG_INVALID_SHAPE CliErrors with the same hint copy. - auth.ts call sites renamed; auth's private writeConfig helper (delete-when-empty for cleanup flows) renamed to persistOrClearConfig to avoid clashing with the new public writeConfig. - view.ts switches CONFIG_PATH constant -> getConfigPath() fn. - global-args.ts uses cli-core's isCI() (also opt-out aware) in shouldDisableSpinner. - output.ts delegates base JSON/NDJSON serialization to cli-core while keeping twist's entity-aware filterEntityFields wrapping. - spinner.ts is now a thin re-export of createSpinner({ isDisabled: shouldDisableSpinner }); the LoadingSpinner class, early-spinner singleton, stdout interception, and adoption semantics all live in cli-core. yocto-spinner drops out of direct deps. - vitest.config.ts inlines @doist/cli-core so vi.mock and vi.doMock reach its compiled imports. - Test mocks updated; spinner.test.ts trimmed to wiring smoke tests (deep behaviour covered in cli-core's own suite). Behaviour note: cli-core's spinner no longer prepends a manual '✓ ' / '✗ ' on top of yocto-spinner's '✔' / '✖', so success/fail lines now render as '✔ Done' instead of '✔ ✓ Done'. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Reverts the rename portion of the previous commit. The local config API stays at getConfig / setConfig / updateConfig — they just delegate to cli-core's readConfig / writeConfig under the hood. Smaller blast radius across the codebase, no call-site churn for future readers. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

The smoke tests duplicated cli-core's spinner suite, and the only twist-specific wiring (TW_SPINNER → shouldDisableSpinner → isDisabled) is already covered by global-args.test.ts. The createSpinner({ isDisabled }) hookup is type-checked at compile time. Also drops the resetEarlySpinner re-export, which was only exposed for the deleted suite. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

doistbot

This PR cleanly integrates @doist/cli-core 0.9.0 into twist-cli, adopting the central utilities for errors, configuration, output formatting, and spinners while keeping existing call sites intact. Centralizing these foundations significantly reduces local boilerplate and aligns the project more closely with our standard tooling ecosystem. A few minor adjustments are needed to fully wire up the updateConfig delegate, streamline paginated JSON/NDJSON formatting to avoid unnecessary memory allocations, and ensure test coverage is preserved for the spinner, CI environment flags, and configuration path resolution.

_{Share Feedback • Review Logs}

- config.ts: updateConfig now actually delegates to cli-core's updateConfig (was claimed in the PR description but the local body still composed getConfig + setConfig). Uses cli-core's strict-read semantics so a broken config file throws instead of being silently overwritten. - output.ts: route formatPaginatedJson through the local formatJson (was calling formatJsonCore directly, leaving a duplicate serialization path). formatPaginatedNdjson similarly uses local formatNdjson and appends the cursor trailer via string concat instead of [...results, trailer], avoiding the O(n) array copy on every paginated response. - global-args.test.ts: add a regression test that CI='false' does NOT disable the spinner, exercising the cli-core isCI() opt-out path that the swap unlocked. - config.test.ts: add a 'thin config wrappers' block proving getConfigPath, getConfig, setConfig, and updateConfig forward to cli-core with the 'twist-cli' app name. A wrong app name would silently redirect every config read/write — these tests are the tripwire. - spinner.test.ts: re-add a single smoke test that mocks createSpinner and asserts it's invoked with shouldDisableSpinner. global-args.test.ts only tests shouldDisableSpinner in isolation; this test is the only thing catching a broken createSpinner({ isDisabled }) handoff. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

## Summary First consumer-side integration of [@doist/cli-core](https://www.npmjs.com/package/@doist/cli-core) 0.9.0 into outline-cli. Mirrors twist-cli [#208](Doist/twist-cli#208) (errors / config / spinner / output / markdown helpers) on this codebase. PR 1 of a 3-PR stack: 1. **This PR** — cli-core wiring (errors, config consolidation, spinner, output, markdown) 2. Next — `global-args` + `ViewOptions` + empty-output test helper (mirror of twist-cli #209) 3. Next — delegate `ol update` to `@doist/cli-core/commands` (mirror of twist-cli #214) The migration step from twist-cli #211 (`updateChannel` → `update_channel`) is not needed: outline-cli already stores the channel under `update_channel`. ## What's wired through cli-core now | Module | Was | Now | | --- | --- | --- | | `src/lib/errors.ts` | n/a (plain `Error`s) | new module — local `CliError` subclass over cli-core's `CliError<ErrorCode>`. Keeps the historical positional `(code, message, hints?, type?)` signature. `code` accepts `ErrorCode \| CliErrorCode` so cli-core's canonical codes type-check at call sites. | | `src/lib/config.ts` | n/a — config reads were split across `auth.ts` (api_token / base_url / oauth_client_id) and `update-config.ts` (update_channel), both writing the same file with their own sync `fs` calls | new module — thin wrappers over cli-core's `readConfig` / `writeConfig` / `updateConfig` with `'outline-cli'` app name. Single source of truth for the config schema. | | `src/lib/auth.ts` | sync `fs` reads + writes, racing `update-config.ts` on the same file | async wrappers over `config.ts`. Public API preserved (`getApiToken` / `getBaseUrl` / `getOAuthClientId` / `getTokenSource` / `saveConfig` / `clearConfig`) — now returns promises. | | `src/lib/update-config.ts` | independent sync `fs` reader/writer hitting the same config file | async wrappers over `config.ts`. | | `src/lib/spinner.ts` | local 70-line impl with yocto-spinner, manual `chalk.green('✓ …')` / `chalk.red('✗ …')` doubling, ad-hoc `shouldDisableSpinner` | `const spinner = createSpinner({ isDisabled: shouldDisableSpinner })` + destructured exports. The CLI-specific bit (`OL_SPINNER` opt-out + `--json` / `--ndjson` / `--no-spinner` fan-out, `CI` opt-out aware: `CI='false'` is treated as not-CI) is wired in as `isDisabled`. | | `src/lib/output.ts` | inline `JSON.stringify(_, null, 2)` / one log per ndjson item | delegates serialization to cli-core's `formatJson` / `formatNdjson`. `formatError` and the essential-keys filtering layer stay local. | | `src/lib/markdown.ts` | `marked@15` + `marked-terminal@7` with sync `marked.parse()` | cli-core's `preloadMarkdown` / `renderMarkdown` (async). `renderMarkdown` call site in `document.ts` now awaits. | ## Dependency bumps - Add `@doist/cli-core` 0.9.0 - `marked` 15.0.12 → 18.0.3 (cli-core peer requires `>=18`) - Replace `marked-terminal` 7.3.0 (caps at `marked@<16`) with `marked-terminal-renderer` 2.2.0 — same fork that cli-core itself uses, no marked version cap - Drop `yocto-spinner` from direct deps (still resolved transitively via cli-core) - `vitest` 4.0.18 → 4.1.5 (cli-core peer requires `>=4.1`) ## Behaviour changes worth flagging - `yocto-spinner.success()` and `.error()` already prepend their own `✔` / `✖` glyphs. The old `spinner.ts` added a manual `chalk.green('✓ ')` / `chalk.red('✗ ')` on top, which produced doubled symbols in real terminals (e.g. `✔ ✓ Done`). cli-core 0.3+ drops the manual prefixes — so the success / fail line now renders as a single `✔ Done` instead of `✔ ✓ Done`. - `updateConfig` now uses cli-core's strict-read semantics: a broken config file throws instead of being silently overwritten (the previous lenient `getUpdateChannel` + `setUpdateChannel` composition could clobber data the user might have recovered by hand-fixing the file). - `auth.ts` and `update-config.ts` no longer race on writes — both go through the single `config.ts` wrapper. - `CI='false'` is now honoured as an opt-out (the spinner runs). ## vitest config Adds `server.deps.inline: ['@doist/cli-core']` to a new `vitest.config.ts`. Without it, vitest treats the real-installed cli-core as external and `vi.mock('@doist/cli-core', …)` doesn't reach its compiled imports. ## Tests - `src/__tests__/spinner.test.ts` trimmed to a single smoke test that mocks `@doist/cli-core#createSpinner` and asserts the kit is built with a function `isDisabled`. The previous ~200 lines of behavioural coverage now live in cli-core's own spinner suite. - `src/__tests__/auth.test.ts` updated for the async API; same coverage. - `src/__tests__/output.test.ts` adjusts the ndjson expectation to match cli-core's `formatNdjson` (single `console.log` with `\n`-joined entries rather than one log per item). - `src/__tests__/commands.test.ts` mocks `auth.js` with async functions; the document-get rendering assertion drops the `# ` prefix (marked-terminal-renderer renders headings differently from the old marked-terminal). ## Test plan - [x] `npm run type-check` - [x] `npm run lint:check` — 0 warnings, 0 errors - [x] `npm test` — **118 / 118** pass - [x] `npm run build` - [x] `npm run check:skill-sync` - [x] `npm ls yocto-spinner` confirms cli-core still resolves it transitively - [ ] Manually exercise `ol auth status`, `ol auth login`, `ol update --channel` against a real config - [ ] Run a long-running command (`ol search foo`) in a TTY to confirm the spinner appears and **no doubled `✔ ✓` glyphs** on success - [ ] `CI=1 ol search foo`, `ol search foo --json`, `OL_SPINNER=false ol search foo` to confirm spinner suppression 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

doist-release-bot · 2026-05-13T13:05:24Z

🎉 This PR is included in version 2.36.4 🎉

The release is available on:

Your semantic-release bot 📦🚀

scottlovegrove and others added 3 commits May 9, 2026 20:14

scottlovegrove marked this pull request as ready for review May 9, 2026 19:33

doistbot requested a review from Bloomca May 9, 2026 19:33

scottlovegrove self-assigned this May 9, 2026

scottlovegrove added the 👀 Show PR PR must be reviewed before or after merging label May 9, 2026

scottlovegrove changed the title ~~refactor: integrate @doist/cli-core 0.9.0~~ refactor: Start integrating @doist/cli-core May 9, 2026

doistbot reviewed May 9, 2026

View reviewed changes

Comment thread src/lib/config.ts

Comment thread src/lib/output.ts Outdated

Comment thread src/lib/output.ts Outdated

Comment thread src/lib/spinner.test.ts

Comment thread src/lib/global-args.ts

Comment thread src/lib/config.test.ts

scottlovegrove merged commit 0a44baf into main May 9, 2026
5 checks passed

scottlovegrove deleted the scottl/cli-core branch May 9, 2026 19:50

scottlovegrove mentioned this pull request May 11, 2026

refactor: Start integrating @doist/cli-core Doist/outline-cli#66

Merged

9 tasks

doist-release-bot Bot added the released label May 13, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: Start integrating `@doist/cli-core`#208

refactor: Start integrating `@doist/cli-core`#208
scottlovegrove merged 4 commits into
mainfrom
scottl/cli-core

scottlovegrove commented May 9, 2026 •

edited

Loading

Uh oh!

doistbot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

doist-release-bot Bot commented May 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

scottlovegrove commented May 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

What's wired through cli-core now

Behaviour change worth flagging

vitest config

Tests

Test plan

Uh oh!

doistbot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

doist-release-bot Bot commented May 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

scottlovegrove commented May 9, 2026 •

edited

Loading