feat(auth): support multiple accounts via --profile#35
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ffed0314c6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e2142932ce
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 295d45571a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
lox
left a comment
There was a problem hiding this comment.
Generally looks good, with the exception of the global vs state passed in via main and passed to where it's needed.
|
Agreed. I removed the package-global active profile and pass the resolved profile through the Kong-injected command context instead. Token store, MCP client, and official API config paths now receive the profile explicitly. CI is green. |
New package resolves the active notion-cli profile from --profile flag, NOTION_CLI_PROFILE env, settings.json default_profile, and legacy top-level credentials, and exposes helpers for per-profile token/config paths. Includes a resolver that returns ErrNoProfile when no profile is selected and no legacy top-level files exist, so callers can fail up front instead of silently running unauthenticated. Profile names must match ^[a-z0-9][a-z0-9_-]*\$ so they stay safe to embed in filesystem paths.
Accept a profile in NewFileTokenStore and config.Load/Save so named profiles store credentials under ~/.config/notion-cli/<profile>/ while the implicit default profile keeps using the legacy top-level paths. Existing single-account installs read and write the same files as before. Adds round-trip tests that two profiles can hold their own tokens and configs without colliding.
Add a persistent --profile flag (and NOTION_CLI_PROFILE env) on the root command, resolve it once in main, and thread it through every token store and config load via internal/cli.ActiveProfile. auth status gains a Profile line that names the active profile and where the selection came from (flag, env, settings, or implicit default) so users can verify which account the CLI is hitting. When no profile is selected and no legacy top-level credentials exist, notion-cli now fails up front with "No profile specified. Pass --profile <name> or set NOTION_CLI_PROFILE." instead of silently treating the caller as unauthenticated.
Add a Profiles section to the README explaining --profile, NOTION_CLI_PROFILE, settings.json default_profile, resolution precedence, the no-profile error, name validation, and on-disk layout. Mirror the short form in the bundled notion skill and add NOTION_CLI_PROFILE to the environment variables table.
…file Headless callers with only NOTION_ACCESS_TOKEN no longer trip the "No profile specified" gate; profile resolution is only fatal when there is no access token to authenticate with. Also route the MCP client token store through the active profile. Before this, every GetClient call constructed mcp.NewClient() without profile context, so it always opened the default profile's store; a run with --profile <name> could refresh one profile and then authenticate with another, or fail as unauthenticated when only the named profile had credentials.
Drop the env:"NOTION_CLI_PROFILE" tag from CLI.Profile. Kong was merging the flag and env values into one field, which meant profile.Resolve always saw a non-empty flagValue and classified env-selected profiles as SourceFlag; auth status would then report "from --profile flag" when the user had only set the environment variable. profile.Resolve already handles NOTION_CLI_PROFILE in its fallback chain, so main passes the bare flag value and the resolver records SourceEnv correctly.
e938f4c to
d3727b1
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d3727b1d4f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@lox I’ll be using and dogfooding this new feature locally on my fork for a few more days, may update PR along the way with improvements/fixes. If you want to be efficient might be best to re-review it then, I can post something to lyk when I think its ready |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 719e0816c3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
`auth status` and `auth list` previously reported `OAuth: expired` whenever the access token was past its expiry, even though the README already promises the CLI auto-refreshes on use when a refresh token is on file. The "expired" line was cosmetic only; commands kept working, but it caused agents and humans to react to a non-problem. Collapse the recoverable case into `valid`, and surface the truly broken case as `login_required` so the actionable distinction is preserved. Update the bundled skill's "Auth preflight" tip and auth command annotations accordingly. JSON shape: `oauth_status` now reports `valid` | `login_required` | `missing` instead of `valid` | `expired` | `missing`. The `authenticated` boolean continues to track whether the next command will succeed without re-login, which is now actually accurate.
💡 Codex Reviewnotion-cli/internal/config/config.go Lines 103 to 106 in b01b357
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
`PathForProfile` only returns a profile's config-file path, but it delegated to `PathsForProfile`, which always computed the default profile's legacy token path via `os.UserHomeDir()`. That made config-only flows (`auth api status/setup` and similar) fail in environments where HOME is unavailable, even though they only need `config.json` and previously resolved via `os.UserConfigDir()` alone. Extract a `profileBaseDir` helper that depends only on `ConfigDir()` so config-path resolution no longer transitively requires HOME. Token path resolution still uses `legacyDefaultTokenPath` for the default profile, since that path is genuinely under `~/.config/notion-cli` and cannot be served from `os.UserConfigDir()` on every platform. Reported by Codex review on PR lox#35.
`PathForProfile` only returns a profile's config-file path, but it delegated to `PathsForProfile`, which always computed the default profile's legacy token path via `os.UserHomeDir()`. That made config-only flows (`auth api status/setup` and similar) fail in environments where HOME is unavailable, even though they only need `config.json` and previously resolved via `os.UserConfigDir()` alone. Extract a `profileBaseDir` helper that depends only on `ConfigDir()` so config-path resolution no longer transitively requires HOME. Token path resolution still uses `legacyDefaultTokenPath` for the default profile, since that path is genuinely under `~/.config/notion-cli` and cannot be served from `os.UserConfigDir()` on every platform. Reported by Codex review on PR lox#35.
💡 Codex ReviewLines 524 to 525 in 5bdef6f
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
`auth status --json` and `auth list --json` shipped with divergent key names for the same fields: status used `expires_at`/`has_token` while list used `oauth_expires_at`/`has_oauth_token`. Status also omitted `active`, `has_api_token`, and `config_path` despite having the data. Standardize on the list/struct key names and add the missing fields so a single-profile inspection and a rows-of-the-list-format inspection return the same shape (plus the existing `authenticated` synthetic at the top level).
💡 Codex Reviewnotion-cli/internal/config/config.go Lines 171 to 187 in 5cdab59
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Two follow-on fixes from Codex review of the multi-profile work: - inspectProfileStatus always populated OAuthExpiresAt from the stored token, even when the access token field was empty. An interrupted OAuth login can leave token.json holding only a client_id, so status output would report `oauth_status: missing` alongside a misleading `oauth_expires_at: 0001-01-01T00:00:00Z`. Only set the expiry when a real access token is present. - ResolveProfile accepted Windows reserved device basenames (con, prn, aux, nul, com1-9, lpt1-9, plus the same with extensions) which then flow into file paths under `~/.config/notion-cli/profiles/<name>/`. Allowing them lets a profile authenticate on macOS or Linux but fail the moment Windows tries to create the directory. Reject reserved basenames during validation.
Why
Using
notion-clicurrently across multiple Notion workspaces is not supported, or would involve swapping~/.config/notion-cli/{token,config}.jsonby hand or re-runningauth loginevery time. Adding a--profileflag and named profiles to config, with zero migration for existing single-account installs and full backwards compatibility.Closes #30.
Examples
Summary
--profileflag andNOTION_CLI_PROFILEenv scope the OAuth token and official API config to a named account~/.config/notion-cli/<profile>/{token,config}.json; the implicit default profile keeps using the existing top-level~/.config/notion-cli/{token,config}.json, so current setups need no migration~/.config/notion-cli/settings.jsonaccepts{"default_profile": "<name>"}; precedence is--profile>NOTION_CLI_PROFILE>settings.json> implicit top-level default{token,config}.json, nosettings.json, no flag, no env), fail up front with "No profile specified. Pass --profile or set NOTION_CLI_PROFILE." instead of silently treating the caller as unauthenticatedNotes
^[a-z0-9][a-z0-9_-]*$(lowercase ASCII; digit- or letter-led;_and-allowed). Empty, whitespace, or path-unsafe names are rejected up front with a specific error.auth statusalways shows the resolved profile on its own line, including where it came from (--profileflag,NOTION_CLI_PROFILE,settings.json, or implicit default), so users never have to guess which account the CLI is hitting.~/.config/notion-cli/{token,config}.jsonexists, every command errors with✗ No profile specified. Pass --profile <name> or set NOTION_CLI_PROFILE.. I actually find this useful, and personally I intentionally remove the root-level files to require--profileeach time, which forces agents to be more intentional and not read or write the wrong workspace.