Skip to content

feat: add migrate urls command#38

Merged
scottlovegrove merged 2 commits into
mainfrom
feat/migrate-urls
Jun 26, 2026
Merged

feat: add migrate urls command#38
scottlovegrove merged 2 commits into
mainfrom
feat/migrate-urls

Conversation

@scottlovegrove

Copy link
Copy Markdown
Collaborator

Context

As part of the Twist→Comms migration, Twist's backend exposes POST https://twist.com/api/comms_migration/fetch_new_url, which maps an old twist.com URL to its equivalent Comms URL (useful for rewriting bookmarks/history). The Comms SDK was updated to wrap this endpoint in comms-sdk-typescript#42, adding the standalone helpers fetchNewCommsUrl / fetchNewCommsUrls.

This PR adds a tdc migrate urls command on top of those helpers.

Changes

  • New migrate command group with a urls subcommand (src/commands/migrate/). Translates twist.com URLs to their Comms equivalents.
  • Twist-token auth. The endpoint authenticates with a Twist (not Comms) token, so the command reads it from a --twist-token flag or the TWIST_AUTH_TOKEN environment variable (the flag wins) and never touches the Comms auth path. The recommended way to supply it is via the Twist CLI, if installed: --twist-token "$(tw auth token view)".
  • Input from a comma-separated argument or stdin (comma- or newline-separated).
  • Output is one line per URL in input order — old -> new on success, old ✗ <code> on failure (invalid_url / not_imported). --json / --ndjson give structured { oldUrl, newUrl } / { oldUrl, error } results. Per-URL failures don't abort the run; the command exits non-zero if any URL fails to migrate.
  • SDK bump @doist/comms-sdk 0.4.6 → 0.7.0 (first published version carrying the migration helpers). Type-check is clean across the bump.
  • Skill content (src/lib/skills/content.ts) gains a ## Migration section documenting the command and the Twist-CLI token pattern; SKILL.md regenerated. CODEBASE.md map/catalog updated.

Verification

  • npm run type-check, npm run lint:check, npm run check:skill-sync — all clean.
  • npm test — 808 passed, including 7 new tests in src/commands/migrate/migrate.test.ts.
  • Manual live run against a real channel URL (with a Twist token):
    $ tdc migrate urls "https://twist.com/a/1585/ch/820139/" --twist-token "$(tw auth token view)"
    https://twist.com/a/1585/ch/820139/ -> https://comms.todoist.com/69/ch/CW8EuRUYTPi4qCREiBf2P/
    
    Batch (good + bad URL) exits non-zero with the bad one shown as ✗ invalid_url; --json and stdin paths confirmed.

🤖 Generated with Claude Code

@scottlovegrove scottlovegrove self-assigned this Jun 26, 2026
@scottlovegrove scottlovegrove added the 👀 Show PR PR must be reviewed before or after merging label Jun 26, 2026
Add a `migrate` command group with a `urls` subcommand that translates old
twist.com URLs to their Comms equivalents via Twist's
`comms_migration/fetch_new_url` endpoint, wrapped by the SDK helpers
`fetchNewCommsUrls`/`fetchNewCommsUrl` (comms-sdk-typescript#42).

The endpoint authenticates with a Twist (not Comms) token, so the command
reads it from a `--twist-token` flag or the `TWIST_AUTH_TOKEN` environment
variable (the flag wins) and never touches the Comms auth path. The skill
recommends supplying it via the Twist CLI: `--twist-token "$(tw auth token view)"`.

URLs come from a comma-separated argument or stdin. Output is one line per URL
(`old -> new`, or `old  ✗ <code>` on failure); `--json`/`--ndjson` give
structured results. Per-URL failures don't abort the run, and the command exits
non-zero if any URL fails to migrate.

Bumps @doist/comms-sdk 0.4.6 -> 0.7.0 for the migration helpers.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@doistbot doistbot left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This PR adds a tdc migrate urls command that translates Twist URLs to their Comms equivalents via the new SDK migration helpers, with Twist-token auth, stdin/argument input, and text/JSON/NDJSON output.

Few things worth tightening:

  • Missing spinner: The fetchNewCommsUrls call bypasses the shared client's spinner middleware, so wrap it explicitly with withSpinner from ../../lib/spinner.js to give users progress feedback as required by the repo guidelines.
  • Stdin can hang: When [urls] is omitted the command unconditionally waits on readStdinToEnd(), which blocks until EOF and can hang in non-interactive/CI environments with an open but empty stdin. Adopt the same "wait for first byte, then read to end" pattern used elsewhere before committing to a full read.
  • Error code stability: The fallback that sets error.code to error.message turns unexpected failures into arbitrary prose and forces message-text matching. Use a stable machine-readable code (e.g. API_ERROR/UNKNOWN_ERROR) and keep the human-readable text in message.
  • Secret handling & discoverability: --twist-token exposes the token in process listings, which conflicts with the Doist secrets-management standard; consider making TWIST_AUTH_TOKEN the documented primary method and de-emphasizing the flag. Also add tdc migrate to the skill's top-level quick-reference list so agents discover it, and add test coverage for the --ndjson output path and TWIST_BASE_URL forwarding branch—both are easily reachable with the existing harness.

I also included a few optional follow-up notes in the details below.

Optional follow-up notes (2)
  • P3 src/lib/skills/content.ts:349: This adds a new top-level command, but the skill’s setup/quick-reference block above still doesn’t mention tdc migrate. SKILL_CONTENT is the source of truth for installed agent skills, so agents that rely on that top command list won’t discover the new entry. Please add tdc migrate there as well.
  • P3 src/commands/migrate/migrate.test.ts:138: The --ndjson output path is untested. The production code branches to formatNdjson(output) when --ndjson is passed (distinct from --json), and other commands in the codebase (e.g. groups.test.ts) include NDJSON integration assertions. Adding a test with --ndjson — verifying the output parses as newline-delimited JSON with the expected { oldUrl, newUrl } / { oldUrl, error } shapes — would close this gap using the existing harness (same createProgram + captureConsole pattern as the --json test).

Share FeedbackReview Logs

Comment thread src/commands/migrate/urls.ts Outdated
Comment thread src/commands/migrate/urls.ts Outdated
Comment thread src/commands/migrate/urls.ts Outdated
Comment thread src/commands/migrate/urls.ts Outdated
Comment thread src/commands/migrate/urls.ts
Comment thread src/commands/migrate/index.ts Outdated
- Wrap the fetchNewCommsUrls call in withSpinner for progress feedback
- Read stdin via readStdin (wait-for-first-byte) so an open but empty stdin
  raises MISSING_CONTENT instead of hanging in CI/spawned children
- Use a stable API_ERROR code when the endpoint returns no error code, keeping
  the human-readable text in message
- Iterate results directly for text output; only materialise the simplified
  shape in the --json/--ndjson branches
- Make TWIST_AUTH_TOKEN the documented primary token method and de-emphasise the
  --twist-token flag (a CLI flag exposes the token in process listings), per the
  Doist secrets-management standard; the flag still works and overrides the env var
- Add tdc migrate urls to the skill's top-level quick-reference list
- Add test coverage for --ndjson output, TWIST_BASE_URL forwarding, and the
  API_ERROR fallback

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@scottlovegrove scottlovegrove requested a review from amix June 26, 2026 17:36
@scottlovegrove scottlovegrove merged commit 99697c0 into main Jun 26, 2026
5 checks passed
@scottlovegrove scottlovegrove deleted the feat/migrate-urls branch June 26, 2026 17:36
doist-release-bot Bot added a commit that referenced this pull request Jun 26, 2026
## [1.9.0](v1.8.0...v1.9.0) (2026-06-26)

### Features

* add migrate urls command ([#38](#38)) ([99697c0](99697c0))
@doist-release-bot

Copy link
Copy Markdown
Contributor

🎉 This PR is included in version 1.9.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@amix amix left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM 🚀

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

Labels

released 👀 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