Skip to content

feat: add registerUpdateCommand to ./commands subpath#9

Merged
scottlovegrove merged 3 commits into
mainfrom
scottl/extract-update-command
May 9, 2026
Merged

feat: add registerUpdateCommand to ./commands subpath#9
scottlovegrove merged 3 commits into
mainfrom
scottl/extract-update-command

Conversation

@scottlovegrove
Copy link
Copy Markdown
Collaborator

Summary

  • Extract update + update switch from todoist/twist/outline into @doist/cli-core/commands as registerUpdateCommand, parameterised by package name, current version, and config path.
  • Anchor CoreConfig + UpdateChannel at the root config module so future config-owning extractions (auth, token storage) extend a single shape instead of redeclaring per CLI.
  • Throw CliError (INVALID_FLAGS, UPDATE_CHECK_FAILED, UPDATE_INSTALL_FAILED, plus broken-config translation via BROKEN_CONFIG_STATE_TO_CODE); both subcommands respect --json / --ndjson through ViewOptions.

Channel set fixed to 'stable' / 'pre-release' (npm tags latest / next). Pure semver helpers (parseVersion, compareVersions, isNewer, getInstallTag, fetchLatestVersion, getConfiguredUpdateChannel) are also exported. Skill-sync hook stays per-CLI via npm postinstall — separate concern; todoist / twist / outline wiring tracked as follow-up after the cli-core release.

Test plan

  • npm test — 182 tests pass (34 new in src/commands/update.test.ts covering semver helpers, --channel, --check, install flow, error paths, switch persist + broken-config translation, JSON / NDJSON envelopes)
  • npm run type-check clean
  • npm run check (oxlint + oxfmt) clean
  • npm run build clean
  • Smoke: npm pack cli-core, install into a todoist working copy, swap registerUpdateCommand in, exercise td update --check [--json] and td update switch --pre-release against a real config

🤖 Generated with Claude Code

Extract the `update` and `update switch` commands shared by todoist-cli,
twist-cli, and outline-cli into `@doist/cli-core/commands`, parameterised
by package name, current version, and config path. Channel set fixed to
`'stable'` / `'pre-release'` (npm tags `latest` / `next`). Both
subcommands respect `--json` / `--ndjson` via `ViewOptions`. Errors
surface as `CliError` (`INVALID_FLAGS`, `UPDATE_CHECK_FAILED`,
`UPDATE_INSTALL_FAILED`, plus broken-config translation through
`BROKEN_CONFIG_STATE_TO_CODE`).

Also introduce `CoreConfig` and `UpdateChannel` at the root config module
so future config-owning extractions (auth, token storage) can extend the
same shape rather than redeclaring it per CLI.

Pure semver helpers (`parseVersion`, `compareVersions`, `isNewer`,
`getInstallTag`, `fetchLatestVersion`, `getConfiguredUpdateChannel`) are
also exported for ad-hoc use.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Member

@doistbot doistbot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR successfully extracts the update and update switch subcommands into the core library alongside new shared configuration and semver helpers. Centralizing this logic is a great step toward reducing duplication and ensuring consistent update behavior across all Doist CLIs. A few adjustments are needed regarding cross-platform process execution, package manager detection for global installs, and stdout buffer deadlocks, alongside some smaller refinements for semver parsing, strict config validation, and test coverage.

Share FeedbackReview Logs

Comment thread src/commands/update.ts Outdated
Comment thread src/commands/update.ts Outdated
Comment thread src/commands/update.ts Outdated
Comment thread src/commands/update.ts Outdated
Comment thread src/commands/update.ts Outdated
Comment thread src/commands/update.ts Outdated
Comment thread src/commands/update.ts Outdated
Comment thread src/commands/update.ts
Comment thread src/commands/update.test.ts
Comment thread src/commands/update.test.ts Outdated
scottlovegrove and others added 2 commits May 9, 2026 12:15
Parametrise channel reporting, machine-output envelopes, install-spawn
shapes, changelog-tip stable/pre-release, registry error paths, switch
persistence, and INVALID_FLAGS cases. Flatten the EACCES + non-zero-exit
assertions to `rejects.toMatchObject` to drop the `promise.catch` dance,
collapse `mockSpawnSuccess` into `mockSpawnExit(0)`, and generalise the
permission-error helper to `mockSpawnError(error)`. Same coverage, ~37%
fewer lines (448 → ~280).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
P1 — bugs that would break installs:
- Spawn `npm`/`pnpm` with `shell: process.platform === 'win32'` so the
  `.cmd` shims resolve on Windows.
- Switch stdio to `['ignore', 'ignore', 'pipe']` so a chatty install
  can't deadlock by filling an unread stdout buffer; stderr stays piped
  for the CliError hint.
- Use the new `readConfigOrThrow` for the channel read and validate the
  `update_channel` value (`INVALID_UPDATE_CHANNEL`) so a broken or
  garbage config no longer silently degrades to `'stable'`.
- Widen pnpm detection beyond `npm_execpath` (which is unset for
  globally-installed CLIs run from the shell) to also inspect
  `process.argv[1]` and `process.execPath`.

P2 — substantive:
- `update --check` now reports `Downgrade available` when current >
  latest instead of falsely claiming "Already up to date" before then
  performing a downgrade in the install flow.
- `parseVersion` strips `+build` metadata per semver §10 and throws on
  inputs that lack a numeric `major.minor.patch`, so `compareVersions`
  can no longer be fed a typed-but-NaN result.
- Request the abbreviated registry payload via
  `Accept: application/vnd.npm.install-v1+json` (~50× smaller than the
  default manifest).
- Promote the strict-read/CliError translation into shared
  `readConfigOrThrow` and `updateConfigOrThrow` helpers in
  `src/config.ts` and consume them from update — switch now does a
  single read pass instead of pre-checking + re-reading.

P3 — refactors:
- Extract `formatChannel(channel)` and `runWithSpinner(text, op)` to
  DRY the chalk-coloured channel rendering and the optional-spinner
  branching.

Tests:
- Cover `parseVersion` build-metadata + NaN rejection,
  `INVALID_UPDATE_CHANNEL`, `--check` downgrade reporting, and the
  parent-parsed `--json switch` path that the `optsWithGlobals()`
  workaround was added for.
- Verify `withSpinner` is threaded around both the fetch and install
  ops with the right text/colour.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@scottlovegrove scottlovegrove merged commit 17c6dc7 into main May 9, 2026
3 checks passed
@scottlovegrove scottlovegrove deleted the scottl/extract-update-command branch May 9, 2026 11:30
doist-release-bot Bot added a commit that referenced this pull request May 9, 2026
## [0.7.0](v0.6.0...v0.7.0) (2026-05-09)

### Features

* add registerUpdateCommand to ./commands subpath ([#9](#9)) ([17c6dc7](17c6dc7))
@doist-release-bot
Copy link
Copy Markdown
Contributor

🎉 This PR is included in version 0.7.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants