Skip to content

refactor(config): migrate updateChannel → update_channel (read seam)#211

Merged
scottlovegrove merged 2 commits into
mainfrom
scottl/cli-core-3
May 11, 2026
Merged

refactor(config): migrate updateChannel → update_channel (read seam)#211
scottlovegrove merged 2 commits into
mainfrom
scottl/cli-core-3

Conversation

@scottlovegrove
Copy link
Copy Markdown
Collaborator

Summary

Prerequisite for a follow-up PR that delegates tw update to @doist/cli-core/commands (mirror of todoist-cli#321). cli-core's registerUpdateCommand reads/writes the config key update_channel (snake_case, hardcoded). Twist's schema is camelCase (updateChannel). This PR aligns the one field cli-core requires, with transparent migration of existing user configs.

  • Rename Config.updateChannelConfig.update_channel. The only snake_case key in an otherwise camelCase schema; the inconsistency is intentionally scoped to this one field — alternative is a translation layer at every callsite.
  • Add migrateLegacyKeys helper that renames updateChannelupdate_channel at the read seam (getConfig, readConfigStrict). In-memory Config is always canonical; on-disk file gets rewritten on the next config update.
  • updateConfig is now a read-merge-write wrapper rather than delegating to cli-core's atomic variant, so legacy keys are dropped from disk on the next mutation rather than lingering across updates.
  • validateConfigForDoctor accepts both keys: update_channel validated as before; updateChannel emits a soft warning that it will be auto-migrated. Both stay in KNOWN_CONFIG_KEYS so doctor doesn't flag a legacy file as having "unrecognized" keys.

Behaviour

  • User with new config (update_channel): no change.
  • User with legacy config (updateChannel): every read transparently renames in memory. First config write (tw update switch, tw auth login, etc.) persists the canonical key and drops the legacy one. No user action required.
  • `tw doctor` against legacy config: emits a soft warning "updateChannel is a legacy key — will be migrated to update_channel automatically on next config write". Still PASSes; doesn't error.

Test plan

  • `npm run type-check` clean
  • `npm run lint:check` clean
  • `npm test` — 593 tests pass (was 586; +7 migration cases)
  • `npm run build`
  • Smoke: `tw update --channel` against default config reports `stable`
  • Manual: write legacy `{ "updateChannel": "pre-release" }` to `~/.config/twist-cli/config.json`, run `tw update --channel` → reports `pre-release`. Run `tw update switch --stable` → on-disk file now has `update_channel: 'stable'`, no `updateChannel` key.
  • Manual: `tw doctor` against the legacy config shows the soft warning but doesn't error.

🤖 Generated with Claude Code

cli-core's `registerUpdateCommand` reads/writes the config key
`update_channel` (snake_case, hardcoded). Aligning twist's config schema
to that key is a prerequisite for a follow-up PR that delegates the
update command to cli-core.

- Rename `Config.updateChannel` → `Config.update_channel`. This is the
  only snake_case key in an otherwise camelCase schema; the inconsistency
  is contained to the one field cli-core requires.
- Add a `migrateLegacyKeys` helper that renames `updateChannel` →
  `update_channel` at the read seam (`getConfig`, `readConfigStrict`).
  In-memory `Config` is always canonical; on-disk file gets rewritten on
  the next config update.
- `updateConfig` is now a read-merge-write wrapper rather than delegating
  to cli-core's atomic variant, so legacy keys are dropped from disk on
  the next mutation rather than lingering across updates.
- `validateConfigForDoctor` accepts both keys: `update_channel` validated
  as before; `updateChannel` emits a soft warning that it will be
  auto-migrated. Both stay in `KNOWN_CONFIG_KEYS` so doctor doesn't flag
  a legacy file as having "unrecognized" keys.
- Update consumers: `src/lib/update.ts` reads `update_channel`;
  `src/commands/update/switch.ts` writes `update_channel`;
  `src/commands/config/view.ts` reads `update_channel`.

Tests: 7 new cases covering migration through `getConfig` /
`readConfigStrict`, validator behaviour on both keys, and `updateConfig`
writing canonical shape from a legacy on-disk file. Existing fixtures
across config / doctor / update tests renamed to snake_case.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@scottlovegrove scottlovegrove marked this pull request as ready for review May 10, 2026 21:09
@scottlovegrove scottlovegrove self-assigned this May 10, 2026
@scottlovegrove scottlovegrove added the 👀 Show PR PR must be reviewed before or after merging label May 10, 2026
@doistbot doistbot requested a review from pawelgrimm May 10, 2026 21:09
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 updates the configuration schema to align the updateChannel key with @doist/cli-core's snake_case requirement and introduces migration handling for legacy configs. While this effectively prepares the CLI for delegating update commands and includes helpful diagnostic warnings, there are a few areas to refine regarding persistence and backward compatibility. Specifically, there are opportunities to maintain atomic writes to avoid race conditions, type-guard manual edits, validate legacy values during doctor checks, keep the public API in camelCase, expand test coverage, and implement a dual-write phase to support older CLI versions.

Share FeedbackReview Logs

Comment thread src/lib/config.ts Outdated
Comment thread src/lib/config.ts Outdated
Comment thread src/lib/config.ts Outdated
Comment thread src/lib/config.ts Outdated
Comment thread src/commands/doctor.test.ts
Comment thread src/lib/config.ts Outdated
- Keep `Config.updateChannel` (camelCase) as the in-memory shape; move
  the snake_case `update_channel` to a pure persistence concern. Adds
  `fromDiskShape` (read seam: legacy `updateChannel` and canonical
  `update_channel` both accepted; canonical wins if both present) and
  `toDiskShape` (write seam: dual-writes both keys when the channel is
  set). Stops the disk format from leaking through every caller.
- Restore atomic `updateConfig` via cli-core's `updateConfigOrThrow`
  with translation applied to the partial. Drops the lost-update race
  between concurrent `tw` processes.
- Dual-write `updateChannel` + `update_channel` to disk so older twist
  builds (still reading `updateChannel`) keep working through the
  version-overlap window. cli-core's update command will read the
  snake_case key; the camelCase write is dropped in a later release.
- Guard `fromDiskShape` against non-object inputs (`null`, primitive,
  array) — a manually-edited JSON file with a top-level non-object now
  returns `{}` instead of crashing on `in` checks.
- `validateConfigForDoctor` validates the legacy `updateChannel` value
  against `stable | pre-release` (was only emitting a migration warning,
  silently passing invalid values). Both legacy and canonical keys
  carry the same shape check.
- Add command-level doctor tests for a legacy-only on-disk config:
  channel is read correctly, the legacy key is not flagged as
  unrecognized, invalid legacy values are flagged.

593 → 598 tests (5 net new across seam/dual-write/guard cases).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@scottlovegrove scottlovegrove merged commit 8802b46 into main May 11, 2026
4 checks passed
@scottlovegrove scottlovegrove deleted the scottl/cli-core-3 branch May 11, 2026 08:57
scottlovegrove added a commit that referenced this pull request May 11, 2026
- src/commands/update/index.ts now attaches a commander preAction hook
  that migrates a legacy-only `updateChannel` on disk to the canonical
  `update_channel` before cli-core's action runs. Older twist builds
  persisted the channel under `updateChannel`; cli-core reads the file
  directly (bypassing twist's getConfig translation seam), so without
  this bridge a user who had set `pre-release` on a pre-#211 build and
  hasn't written config since would silently revert to `stable` on
  their first `tw update`. Hook propagates to `tw update switch` too.
- Drop the duplicated valid-channel set in src/lib/update.ts. Export
  `UPDATE_CHANNELS` from src/lib/config.ts (was private) and import it
  from the single source of truth. Doctor, config validation, and the
  update read path now share one constant.
- Rework src/commands/update/update.test.ts to be hermetic. Each test
  mocks `getConfigPath` to a fresh temp file so the developer's real
  config can't leak into the run. Adds:
    - migration test: legacy-only `updateChannel` on disk gets
      `update_channel` written before cli-core reads it
    - invalid-value test: `update_channel: 'beta'` surfaces as
      `INVALID_UPDATE_CHANNEL` (new behaviour from cli-core)
    - hermetic smoke that doesn't pin cli-core's exact registry URL —
      asserts only the package name + dist-tag are present.
- src/lib/skills/content.ts adds `--ndjson` examples on both
  subcommands; skills/twist-cli/SKILL.md regenerated.

574 → 576 tests (4 update wrapper tests replace the smoke pair).

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

@pawelgrimm pawelgrimm left a comment

Choose a reason for hiding this comment

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

🚀

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

Labels

👀 Show PR PR must be reviewed before or after merging

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants