chore: drop Twist branding and legacy migration code#1
Conversation
Comms is a hard fork of Twist with no upgrade path from twist-cli, so the
v1→v2 auth migration shim, the `updateChannel` dual-write back-compat, and
every "Twist" / `tw` reference in source, docs, workflows, and skills are
dead weight.
Changes:
- Rebrand: `Twist` → `Comms`, `tw` → `cm`, `twist.com` → `comms.todoist.com`,
`twist-mention://` → `comms-mention://`, `TWIST_INCLUDE_PRIVATE_CHANNELS`
→ `COMMS_INCLUDE_PRIVATE_CHANNELS`, `TW_ACCESSIBLE`/`TW_SPINNER` →
`CM_ACCESSIBLE`/`CM_SPINNER`, CSS `--twist-teal*` → `--comms-teal*`.
- Identifiers: `createTwistAuthProvider` → `createCommsAuthProvider`,
`attachTwist*Command` → `attachComms*Command`, `ParsedTwistUrl`,
`TwistUrlRoute`, `TwistHandshake`, `looksLikeTwistAppUrl`, mocks, test
helpers, etc.
- File renames: `skills/twist-cli/` → `skills/comms-cli/`,
`docs/twist-search.md` → `docs/comms-search.md`, `icons/twist-cli.{png,svg}`
→ `icons/comms-cli.{png,svg}`, `.github/workflows/update-twist-sdk.yml`
→ `update-comms-sdk.yml`.
- Drop legacy migration: delete `src/lib/migrate-auth.{ts,test.ts}`,
remove `isLegacyAuthActive` / `assertV2Available` / `readLegacyTokenSnapshot`
/ `dischargeLegacyState` from `auth-provider.ts`, drop the
`LEGACY_KEYRING_ACCOUNT` constant, drop `AUTH_MIGRATION_PENDING` error
code, simplify `postinstall.ts` to just update installed skills.
- Drop legacy config fields: `token`, `pendingSecureStoreClear`, `authMode`,
`authScope`, `authUserId`, `authUserName` at config root, plus
`config_version` and the camelCase `updateChannel` on-disk alias.
- Drop `migrateLegacyChannelKey` preAction in `commands/update/index.ts`.
- Remove the Twist post-action release announcement step in
`.github/workflows/release.yml` (no Comms equivalent yet).
Pre-existing type/test failures noted in the bootstrap commit (UUID
strings vs numbers, `name` → `fullName`, `awayMode` removal, etc.) are
out of scope here.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
doistbot
left a comment
doistbot
left a comment
There was a problem hiding this comment.
This PR effectively scrubs Twist branding and legacy v1→v2 migration logic to establish the new Comms identity. The comprehensive cleanup significantly reduces technical debt and streamlines the workspace for the new CLI. A few final adjustments are needed, specifically removing a missed legacy token resolution step, updating the SKILL_CONTENT reference, adjusting documentation to adhere to secrets-management standards for API tokens, and ensuring that manual-token accounts with empty IDs are still properly handled and tested in the account command.
- account/current: restore empty-id guard so `cm auth token` snapshots
render as `source: "token-only"` instead of printing blank identity
fields. `loginWithToken` still persists `{ id: '', label: '' }`.
- account.test: add regression coverage for the empty-id snapshot in
both text and --json paths (replaces the deleted legacy-source tests).
- SKILL_CONTENT: document the new `source:"token-only"` payload variant.
- docs/SPEC: drop the dead "legacy plaintext token during auto-migration"
step from token resolution priority.
- README: switch the manual-token example to the prompt-based flow
(`cm auth token` with no arg) so the secret isn't visible in `ps` or
shell history (matches the Doist secrets-management standard).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
doistbot
left a comment
doistbot
left a comment
There was a problem hiding this comment.
This PR successfully completes a major cleanup by replacing all Twist branding with Comms equivalents and removing obsolete legacy migration logic and configuration fields. These changes significantly streamline the codebase and eliminate dead weight, making the CLI much easier to maintain. A couple of adjustments are needed before merging, specifically updating the skill documentation to reflect the new token-only source output and fully removing positional token arguments from the auth token command and tests to comply with the Secrets Management Standard.
Doistbot flagged that the previous commit only updated the README — the command itself still accepted `cm auth token <token>`, which violates the Doist Secrets Management Standard (process lists + shell history leak the secret). - `cm auth token` no longer takes a positional argument; the only path is the hidden-input prompt or `COMMS_API_TOKEN` for non-interactive use. - `loginWithToken()` is now zero-arg. - Updated auth.test for the prompt-only flow. - skills/comms-cli/SKILL.md: refresh the stale `source:"legacy"` comment to `source:"token-only"` to match `src/lib/skills/content.ts`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
doistbot
left a comment
doistbot
left a comment
There was a problem hiding this comment.
This is a great cleanup that successfully transitions the project's branding from Twist to Comms while systematically removing obsolete v1 migration logic, legacy configurations, and workflow steps. Stripping out this dead weight significantly streamlines the codebase and removes unnecessary complexity from the authentication layer. To finalize the changes, a few adjustments are needed, such as regenerating the skill file to pass CI, ensuring proper test cleanup with try/finally blocks, extracting shared helpers for TTY mocks and account sentinels, and adding a quick regression test to verify positional tokens are properly rejected.
Rename: - bin: `cm` → `tdc` (package.json, package-lock.json) - env vars: `CM_ACCESSIBLE` → `TDC_ACCESSIBLE`, `CM_SPINNER` → `TDC_SPINNER` - every code/doc/test reference to the CLI command name. Doistbot pass 3 fixes: - Regenerate skills/comms-cli/SKILL.md from src/lib/skills/content.ts (was stale at v2.41.2; now v0.1.0-alpha.1 with token-only payload). - Move TTY/stdout cleanup in token-subcommand tests into afterEach so it runs even when an assertion fails mid-test. - Extract `mockPromptAnswer` helper to deduplicate readline mocking across token tests. - Add regression test that `tdc auth token <secret>` is rejected and never reaches `store.set`, preventing argv-based secret entry from regressing. CI: each workflow now checks out `Doist/comms-sdk-typescript` as a sibling under $GITHUB_WORKSPACE so the `file:../comms-sdk-typescript` dep in package.json resolves on the runner. Main repo moves under `comms-cli/` and every step gets `working-directory: comms-cli`. Pre-existing bootstrap follow-ups (string-vs-number IDs, fullName/name, awayMode SDK shape) still cause type-check + 8 test failures and remain out of scope for this PR. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
doistbot
left a comment
doistbot
left a comment
There was a problem hiding this comment.
This PR does a fantastic job scrubbing the legacy Twist branding, migration code, and configurations to fully embrace the Comms transition. The cleanup significantly lightens the codebase and successfully trims out outdated back-compat logic. There are a few adjustments needed to address workflow failures related to cross-repo checkouts and secure secret handling in PRs, as well as refining how token-only account states are centrally managed and tested. Finally, the scope of the new AI agent skill will need to be adjusted to ensure compliance with internal data access standards.
…s-repo checkout
Doistbot pass 4 raised three new issues; this commit addresses them.
**Manual-token handling across the account command group (P2 + P3):**
- Extract `MANUAL_TOKEN_ACCOUNT` constant and `isManualTokenAccount()`
predicate in `auth-provider.ts` to centralise the `{ id: '', label: '' }`
contract that `tdc auth token` writes.
- `tdc account list` now filters manual-token snapshots out of the
rendered rows — they have no identity to display.
- `findAccountInStore` skips manual-token entries when resolving refs,
so `tdc account use|remove <ref>` can no longer accidentally target
the identity-less snapshot.
- `auth/token.ts` and `auth-provider.ts` now use the shared
`MANUAL_TOKEN_ACCOUNT` constant instead of inline literals.
- Test assertion for token-only path no longer relies on a brittle
spacing regex — asserts the regular header strings are absent.
**Revert CI cross-repo checkout (P1 security + CI failure):**
- The previous commit added `actions/checkout@v5` against
`Doist/comms-sdk-typescript` with `secrets.GITHUB_TOKEN`. Doistbot
flagged this as a Secrets Management Standard violation (secrets must
not be used in `pull_request` workflows that execute untrusted PR
code afterwards), and it also failed at runtime because the default
`GITHUB_TOKEN` cannot access sibling private repos.
- Lint / test / skill-sync workflows are reverted to their pre-cleanup
shape. CI will continue to fail on `npm ci` until `@doist/comms-sdk`
is published to a private npm registry — that is a follow-up to the
bootstrap commit, not the cleanup PR.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
🎉 This PR is included in version 1.0.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
2 participants
Context
Comms is a hard fork of Twist with no upgrade path from twist-cli, so the v1→v2 auth migration shim,
updateChanneldual-write back-compat, and everyTwist/twreference in source, docs, workflows, and skills are dead weight.What was changed
Twist→Comms,tw→cm,twist.com→comms.todoist.com,twist-mention://→comms-mention://,TWIST_INCLUDE_PRIVATE_CHANNELS→COMMS_*,TW_ACCESSIBLE/TW_SPINNER→CM_*, CSS--twist-teal*→--comms-teal*.createTwistAuthProvider→createCommsAuthProvider,attachTwist*Command→attachComms*Command,ParsedTwistUrl,TwistUrlRoute,TwistHandshake,looksLikeTwistAppUrl, test mocks.skills/twist-cli/→skills/comms-cli/,docs/twist-search.md→docs/comms-search.md,icons/twist-cli.{png,svg}→icons/comms-cli.{png,svg},update-twist-sdk.yml→update-comms-sdk.yml.src/lib/migrate-auth.{ts,test.ts}, removeisLegacyAuthActive/assertV2Available/ legacy snapshot fallbacks fromauth-provider.ts, dropLEGACY_KEYRING_ACCOUNT, dropAUTH_MIGRATION_PENDINGerror code, simplifypostinstall.ts.token,pendingSecureStoreClear,authMode,authScope,authUserId,authUserNameat config root, plusconfig_versionand the on-diskupdateChannelalias.migrateLegacyChannelKeypreAction incommands/update/index.ts..github/workflows/release.yml(no Comms equivalent yet).Out of scope
Pre-existing type/test failures from the bootstrap commit (UUID strings vs numbers,
name→fullName,awayModeremoval, etc.) are not addressed here.Test plan
npm run lint:checkpassesnpm test— only pre-existing bootstrap failures remain (8 tests inaway.test.ts+auth.test.tsstatus subcommand); no new failures from this PRnpm run type-check— only pre-existing bootstrap errors remain; nothing newly broken by the cleanup🤖 Generated with Claude Code