feat: Add Todoist OAuth for Comms auth#15
Merged
Conversation
Add Todoist OAuth as the CLI auth flow for Comms, including Comms scopes, public DCR client registration, PKCE token exchange, refresh handling, persisted OAuth metadata, and resource-aware Comms API/search clients.
doistbot
reviewed
Jun 4, 2026
doistbot
left a comment
doistbot
left a comment
Member
There was a problem hiding this comment.
Great work Amir on integrating the Todoist OAuth flow into the Comms CLI 😊.
Few things worth tightening:
- The local storage of delegated OAuth tokens conflicts with the internal AI tools standard; this flow needs to be moved behind an approved server-side service.
- The
tdc doctorandtdc auth statuscommands currently bypass token refresh or stored resources, causing valid staging/local sessions to appear broken. - Type the provider handshake explicitly instead of using a generic record, and ensure the refresh logic requires the complete set of OAuth metadata to avoid silent fallbacks.
- Add test coverage for the
getCommsClient()resource wiring and the refresh fallback scenario where the token response omits thescope.
I also included a few optional follow-up notes in the details below.
Optional follow-up notes (2)
- [P3] src/commands/auth/login.ts:24:
getConfig()andstore.active(account.id)are independent I/O, but this path waits for them serially. On keyring-backed setups that adds avoidable latency to every successful OAuth login. Fetching them in parallel (for example with separate promises orPromise.allSettled) would keep the new workspace-reset step from stretching login time. - [P3] src/commands/auth/login.ts:44: This always does
workspaces.getWorkspaces()once login succeeds, even when there is no storedcurrentWorkspace. That adds a full extra API round-trip to fresh OAuth logins just to prefill workspace state, whilegetCurrentWorkspaceId()already resolves and persists a workspace lazily on first real use. If the main goal is clearing stale selections, consider skipping this lookup whencurrentWorkspaceis unset.
Keep the OAuth test suite focused on DCR, token exchange, refresh, Comms resource handling, workspace reset behavior, and diagnostics.
Use refreshed account snapshots and stored Comms resources when auth status and doctor validate OAuth tokens.
Member
Author
|
Posted by Claude Code/Codex on behalf of Amir: Addressed the concrete PR-level points in follow-up commits:
The server-side token-storage point is out of scope for this CLI PR. This PR keeps local OS credential storage plus config fallback, matching the existing CLI auth model. Not requesting another Doistbot round per Amir's direction. |
Refresh doctor probes online, fail closed on partial OAuth metadata, and cover resource-aware shared client creation.
…lled OAuth Bump @doist/cli-core to 0.25.0 and replace comms-cli's bespoke DCR registration / PKCE authorize / token exchange / refresh implementation with a composition of cli-core's createDcrProvider, now that it supports the three things comms-cli needed: the RFC 8707 resource indicator, refresh-token support, and loadClient/saveClient client caching. auth-provider.ts shrinks ~507 lines net. What stays Comms-specific: - endpoint + resource resolution from COMMS_BASE_URL / COMMS_AS_URL, - the config client cache wired through loadClient/saveClient (keyed on redirectUri), - recording the server-granted scope (an exchangeCode wrapper stashes it on the handshake for validate; refreshToken rebuilds the account from it) — using the scope cli-core now surfaces on ExchangeResult, - the token-response diagnostics surfaced when Comms rejects a token, - validate: getSessionUser probe + authMode derivation, - the COMMS_API_TOKEN token-store override. The refresh handshake is reconstructed from the stored account (clientId + resource/authBaseUrl + identity fields) since cli-core doesn't persist it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Member
Author
|
@scottlovegrove I’ll merge this in, I think I resolved all of the issues. if something is off, let me know and I’ll do a follow-up. |
doist-release-bot Bot
added a commit
that referenced
this pull request
Jun 5, 2026
## [1.5.0](v1.4.0...v1.5.0) (2026-06-05) ### Features * Add Todoist OAuth for Comms auth ([#15](#15)) ([cd64fa7](cd64fa7))
Contributor
|
🎉 This PR is included in version 1.5.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
Adds Todoist OAuth as the Comms CLI auth flow.
What was changed
resourcesupport--full-accessfor delete/admin scopesTesting
npm run check:skill-syncnpm run type-checknpm testnpm run lint:checkdoistbot review staged(one local pass, concrete findings addressed)