[radenkovic]

Jira: TW-7622 (epic TW-7610). Stacked on #125 (TW-7617 device sign-in); retarget to main once that merges.

What

• nylas oauth login --mcp-only (browser or --device): the --for mcp scopes without dashboard.session, so the consent screen can offer an IAM principal. There is no principal flag; the principal is chosen on the consent page.
• Never a dashboard session: an MCP-only login skips the dashboard exchange. The exchange also refuses any token carrying a principal claim before calling the server. A dashboard-capable login whose consent returned a principal is revoked and refused rather than stored as the default session.
• Credential isolation: each MCP-only credential is stored in its own keyring keys (oauth_mcp_<id>_*), beside the default session and never in place of it. The id is a hash of server, audience, org, user and principal, and a token-free index lists them. The same key replaces that credential and revokes the one it replaces; any other combination is stored alongside, so two principals or regions never overwrite each other.
• Selection: nylas mcp serve --auth oauth uses the default session when present. Otherwise it uses the only MCP-only credential, and with several it requires --credential <id>. mcp install --credential pins one and preserves it on re-install.
• Status/help: oauth status shows access mode, audience, org and principal for the default session and every MCP-only credential, never a token. Also adds oauth token --credential, oauth logout --credential / --all; config reset clears every credential.
• 503 on refresh: retries temporarily_unavailable after Retry-After (capped at 30s) with backoff, up to 3 attempts, with the same refresh token, and keeps the stored sign-in (contract §7).

Tests

• Unit and fake-server tests cover requested scopes, storage selection, two principals not overwriting each other, replacement and revocation, refusing tokens it cannot store safely, the exchange refusal, and the 503 retry and its bounds.
• Integration: TestCLI_OAuthMCPOnly_FakeServerLogin (device + --mcp-only against a fake AS: no dashboard.session requested, no exchange call, status and token --credential).
• Two deliberate code breaks (ignoring MCPOnly; disabling the retry) each failed the new tests.
• make fmt vet lint test-unit test-race security build passes. make vuln fails on the Go 1.26.5 stdlib pinned in go.mod, and fails the same way on main.
• Not tested against a live authorization server (TestOAuthAS_* skip without one).

Notes for review

• Tokens carry no authorization-id claim. Status shows the identifying fields, and the credential id is derived from them.
• With several MCP-only credentials and no default session, mcp serve exits with an error rather than choosing one.

🤖 Generated with Claude Code

[qasim-nylas]

Review of PR #127 (Claude + Codex)

Two reviewers examined this PR: Claude and Codex. We did a check of each finding against the code on 967b118. We reproduced most findings with scratch tests. Where we only read the code, we write "code reading only".

Result: the design is good. No item breaks the build or the tests. But some failure paths can lose or orphan credentials. We recommend that you fix findings 1 to 5 before merge.

The isolation design is correct:

• MCP-only tokens stay in oauth_mcp_<id>_* keys.
• All index writes occur under the cross-process lock.
• Refresh writes back to the correct credential keys.
• We found no path that puts a token with a principal into the default session or into the dashboard exchange. We examined relogin, orgs switch, EnsureFresh and refresh.

Summary of the claims

Claim	Status	Note
--mcp-only requests the MCP scopes without dashboard.session	Correct	The code refuses --for <other> and --scope with --mcp-only.
An MCP-only login never makes a dashboard session	Correct	The revocation of a refused token is best effort. If it fails, the user does not get a message.
Credential isolation, id, index, replace and revoke	Correct for the normal path	Failure paths can make the keys and the index different. Refer to findings 3, 4 and 5.
Selection in mcp serve, and install --credential	Correct as written	The selection rule has a problem. Refer to finding 2. install --auth oauth removes the pin. Refer to finding 8.
oauth status, token --credential, logout --credential / --all, config reset	Almost correct	--verify does not examine MCP-only credentials. Refer to finding 6.
503 retry: Retry-After, backoff, 3 attempts	Not fully correct	Refer to finding 7.

---

Findings

1. [Major] A semantic conflict with #126. Git does not show it. (Claude)

• Files: internal/cli/oauth/login.go, internal/cli/oauth/status.go
• git merge-tree shows text conflicts in login.go and status.go.
• There is also a conflict that git does not show. [TW-6922] fix(cli): address remaining OAuth review items (#14, #15, #21, #22, #24, N11) #126 adds writeLoginReport, and it returns before this PR's result.Credential != nil check.
• Result after a simple merge: nylas oauth login --mcp-only --json exchanges the default session for a dashboard session. This resets the user's app selection. The report says signed_in: true for an MCP-only login. The credential and the principal are not in the report.
• Recommendation: merge [TW-6922] fix(cli): address remaining OAuth review items (#14, #15, #21, #22, #24, N11) #126 first, then rebase this PR. During the rebase:
  1. Put the result.Credential != nil check before the structured-output branch. Or, make writeLoginReport skip the exchange and show the credential, audience, org and principal.
  2. Make newStatusReport list the MCP-only credentials, with access mode and principal. It must not show a token.
  3. Give logout --credential and logout --all a structured report.
  4. Make sure that WireDashboard and dashboardTokenSource from [TW-6922] fix(cli): address remaining OAuth review items (#14, #15, #21, #22, #24, N11) #126 use the default service, not a bound one.

2. [Major] mcp serve --auth oauth selects a default session that cannot reach MCP. (Claude, reproduced)

• File: internal/app/oauthlogin/credentials.go:116
• SelectMCPCredential("") always selects the default session when one exists.
• The docs in this PR tell the user to run a plain nylas oauth login for dashboard access. That session has no MCP resource.
• Result: the user has a plain default session and one MCP-only credential. mcp serve selects the default session and fails: token audience [https://dashboard-api.nylas.com] names no Nylas MCP server.
• The same problem occurs when the default session is for a different server.
• Fix: select the default session only when session.Resource != "" and the issuer is correct. If not, use the MCP-only list. Or, give the MCP-only credential id in the error message.

3. [Major] logout --credential can orphan tokens. (Claude + Codex, reproduced)

• File: internal/app/oauthlogin/credentials.go:250-256
• If bound.logoutLocked fails (for example, the keychain is locked), the code still removes the index entry.
• Scratch test result: index records=0 orphan keys=7 revokes=[].
• Result: the refresh token stays in the keyring and is not revoked. logout --credential, logout --all and config reset cannot find it again.
• Fix: remove the index entry only when logoutLocked returns nil, ErrOAuthNotLoggedIn or an issuer mismatch.

4. [Minor] A failed replace-login loses the old credential and leaks the new one. (Claude, reproduced)

• File: internal/app/oauthlogin/credentials.go:213-229
• The order is: revoke the old token, clear the keys, save the new keys, write the index.
• If the index write fails (we tested "item too large"), then:
  • the old token is already revoked;
  • the new tokens are cleared locally, but they are not revoked (revokes=[refresh-a1] only);
  • the index still lists the id, but the keys are gone.
• The storeLogin comment says "a token it refuses to keep is revoked". The code does not do this here.
• Fix:
  1. Call s.revokeTokens(ctx, tokens) when a store step fails.
  2. Remove the stale index entry.
  3. If possible, write the index before you revoke the old token.
• A crash between saveTokens and writeIndex also leaves keys without an index entry. (Code reading only.)

5. [Minor] A corrupt index makes config reset orphan all MCP credentials. (Claude, reproduced)

• File: internal/app/oauthlogin/store.go:188-193
• The code deletes the index key also when readIndex fails.
• Scratch test result: the first reset returns an error. The second reset returns nil. All oauth_mcp_<id>_* keys stay in the keyring, including the refresh tokens.
• Fix: keep the index key when the code cannot parse it. Or, remove the keys for each id that the code can read from the index.

6. [Minor] oauth status --verify does not examine MCP-only credentials. (Codex + Claude)

• File: internal/cli/oauth/status.go:69-73
• The loop calls showSession(cmd, bound, false) for each MCP-only credential. Thus --verify has no effect on them.
• Result: when only MCP-only credentials exist, oauth status --verify returns success and does not call /oauth/userinfo. A revoked credential passes. docs/COMMANDS.md says that --verify confirms that the server accepts the token.
• Fix: pass remote to showSession in the loop. Return an error when a verification fails.

7. [Minor] The 503 retry does not operate as the description says. (Claude, reproduced)

• Files: internal/domain/oauth.go:197, internal/app/oauthlogin/service.go:405,436-461
• a) IsTemporarilyUnavailable examines only StatusCode == 503. Thus the code also retries an HTML 503 from a proxy, with the same refresh token. Some proxies send a 503 after the server has already rotated the token. Then the retry can burn the token family.
  • Fix: also require Code == "temporarily_unavailable".
• b) The waits use refreshCtx. This context ignores Ctrl-C (WithoutCancel) and has one 30s deadline for all attempts.
  • With Retry-After: 20, we measured 30s and only 2 attempts. The second attempt had only 10s.
  • A late attempt with little time left has a higher risk: the client can stop after the server rotated the token.
  • Fix: during a wait, obey the caller's context (no request is active then). Give each attempt its own refreshTimeout.

8. [Minor] mcp install --auth oauth removes the pin without a message. (Claude, code reading only)

• File: internal/cli/mcp/install.go:75,99
• A re-install with an explicit --auth oauth removes a --credential X pin.
• The code does not validate --credential against ^[0-9a-f]{12}$ or against the index. Thus a typing error fails only when mcp serve starts.
• Fix: keep the pin when --credential is not given. Or, if this behavior is intended, document it in docs/COMMANDS.md. Validate the id at install time.

9. [Minor] accessMode gives too much access for a plain login. (Claude)

• File: internal/cli/oauth/status.go:140
• A plain nylas oauth login session (no resource) shows "dashboard commands (your full role) and the MCP server". But mcp serve refuses this session (finding 2).
• Fix: examine session.Resource before you show "MCP server".

---

Nits (optional)

• The credential id has 12 hex characters (48 bits). The risk of a collision is very small. But a collision replaces and revokes a different credential.
• In oauth status, if the read of the default session fails with an error that is not "not logged in", the MCP list does not show.
• The best-effort revocation of a refused principal token (dashboard login with a principal) does not tell the user when it fails.
• docs/COMMANDS.md does not document oauth status --credential. It also does not say that config reset clears the MCP-only credentials.

Items that we examined and found correct

• All files have fewer than 600 lines. The largest is service.go with 565 lines.
• All new tests have the correct build tags.
• logout --credential and --all cannot be used together.
• Retry-After handles negative, bad and very large values. The cap is 30s. An HTTP-date value uses the backoff.
• An empty principal cannot collide with a real principal, because a principal id has 21 characters.
• The id does not change after a refresh.

---

Build and test results (967b118)

Check	Result
go build ./...	Pass
go vet ./... and go vet -tags integration ./...	Pass
go test -race -count=1 on the changed packages	Pass (also with SSH variables set)
golangci-lint on the changed packages	0 issues
TestCLI_OAuthMCPOnly_FakeServerLogin (integration)	Pass

🤖 Generated with Claude Code

[qasim-nylas]

Second review of PR #127 (Claude + Codex), commit 967b118

Two reviewers examined this PR again: Claude and Codex. The PR head did not change after the first review. But #125 is now on main, and #126 has new commits. Thus this review has three goals:

• Do a check of each finding from the first review, to find false positives.
• Find problems that the first review did not find.
• Give a plan for the merge with [TW-6922] fix(cli): address remaining OAuth review items (#14, #15, #21, #22, #24, N11) #126.

Result: all nine findings from the first review are real. Both reviewers confirmed each one, and they reproduced most of them. We found five new minor problems and some nits. One new problem breaks a claim in the PR description: "the exchange also refuses any token carrying a principal claim". Refer to new finding 1.

This PR now merges into main with no text conflict. It still has text conflicts with #126 in login.go and status.go. Refer to "Merge with #126".

We recommend that you fix these items before merge:

• Findings 1 to 5 from the first review.
• New findings 1 to 3.

Check of the first review's findings

#	Verdict	Note
1. Semantic conflict with #126	Confirmed, reproduced	Claude made a simple merge of #126 and this PR, and it builds. In that merge, oauth login --mcp-only --json and --mcp-only --quiet exchange the default session for a dashboard session. The JSON says "dashboard":{"signed_in":true} and does not show the credential. No test finds this.
2. mcp serve selects a default session that cannot reach MCP	Confirmed, reproduced	TestSelectMCPCredential (credentials_test.go:256-260) tests this behavior on purpose. But the docs recommend the setup that causes the failure. Note: a check of session.Resource != "" is not sufficient. The audience must name a Nylas MCP server.
3. logout --credential can orphan tokens	Confirmed, reproduced (both)	7 keys stay in the keyring, and nothing is revoked.
4. A failed replace-login loses the old credential	Confirmed, reproduced (both)	Also: a corrupt index (credentials.go:222-225) drops a new login and does not revoke it.
5. A corrupt index makes config reset orphan credentials	Confirmed, reproduced (both)	7 keys stay after the second reset.
6. status --verify does not examine MCP-only credentials	Confirmed, reproduced	--verify passes for a credential that fails verification.
7a. The CLI retries each 503	Confirmed, reproduced (both)	The CLI retries an HTML 503 from a proxy 3 times with the same refresh token.
7b. The retry waits ignore Ctrl-C	Confirmed, reproduced	Ctrl-C has no effect. With Retry-After: 20, the CLI makes only 2 attempts in 30s. Also, the CLI holds the session lock during all of the waits.
8. mcp install --auth oauth removes the pin	Confirmed, reproduced	The CLI accepts --credential not-an-id. Also: --credential without --auth fails, also when the current install already uses OAuth.
9. accessMode gives too much access	Confirmed, reproduced

Corrections to the first review. Three statements in the first review are not correct:

• "We found no path that puts a token with a principal into the default session or into the dashboard exchange." This is not correct. Refer to new finding 1.
• "Retry-After handles ... very large values." This is not correct. Refer to the first nit.
• "All files have fewer than 600 lines." This is true for all Go files. The largest Go file is service.go with 565 lines. But docs/COMMANDS.md has 1,045 lines. It had 1,006 lines on main.

---

New findings

1. [Minor] The principal check does not refuse a token when the claims do not decode. (Claude + Codex, reproduced by both)

• Files: internal/app/oauthlogin/credentials.go:192, internal/app/dashboard/oauth_session.go:176
• Both checks run only when domain.DecodeOAuthAccessToken returns no error. If the decode fails, the CLI accepts the token.
• These tokens contain a principal, but the CLI stores them as the default session and sends them to the exchange:
  • A fractional exp, for example 4102444800.5. (Codex)
  • "principal" as a string or as an array. (Claude)
  • A valid principal object, but "permissions" as a string, for example "a b". (Claude)
• The last case comes from the new field Permissions []string (internal/domain/oauth_client.go:94). No code uses this field. But if the server sends permissions in a different shape, the decode fails for each token. Then credentialFor and mcpOnlyRecord also fail, and an MCP-only login cannot complete.
• The server still makes the final decision. Thus we do not know if a real server accepts these tokens. But the CLI check does not do what the PR description says.
• Fix:
  1. Remove the Permissions field, or change its type to json.RawMessage.
  2. Examine the principal key with a separate, tolerant decode (for example, into map[string]json.RawMessage).
  3. Refuse the token when principal is present and not null.
  4. If the claims do not decode, refuse the token. Do not accept it.
  5. Add tests for each of the token shapes above.

2. [Minor] An empty --credential value selects the default session. (Codex, reproduced)

• Files: internal/cli/oauth/token.go:26, internal/cli/oauth/helpers.go:60, internal/cli/oauth/logout.go:32,41
• A script can run nylas oauth token --credential "$id" when $id is empty. Then the CLI prints the token of the default session.
• nylas oauth logout --credential "$id" with an empty $id logs out of the default OAuth session and the dashboard session.
• An empty value also passes the check that refuses --credential together with --all.
• Fix:
  1. Use cmd.Flags().Changed("credential") to find out if the user gave the flag.
  2. If the user gave the flag, validate the id against ^[0-9a-f]{12}$ before you select or change a credential.

3. [Minor] mcp serve --auth oauth does not show the login instruction. (Claude, reproduced)

• File: internal/cli/mcp/serve.go:154-156
• On main, when no session is stored, the error tells the user: "Run 'nylas oauth login --for mcp' first".
• With this PR, the user sees only not logged in to the Nylas authorization server.
• Cause: oauthCredentials returns the selection error before buildOAuthProxy adds the instruction (serve.go:177-179).
• Fix: add the same instruction to the selection error.

4. [Minor] The integration test cannot find the regression that it must find. (Claude, reproduced)

• File: internal/cli/integration/oauth_mcp_only_test.go
• The test stores no default session. Thus an incorrect exchange fails locally, and it does not reach the fake server.
• Claude changed the code to exchange after an MCP-only login. TestCLI_OAuthMCPOnly_FakeServerLogin still passed.
• Also, the require call in the /device handler (line 39) runs on a goroutine that is not the test goroutine.
• Fix:
  1. Store a default session first.
  2. Make sure that the exchange counter and the stored default keys do not change.
  3. After the rebase on [TW-6922] fix(cli): address remaining OAuth review items (#14, #15, #21, #22, #24, N11) #126, do the same test with --json.
  4. In the handler, use t.Errorf or send the error to the test goroutine.

5. [Minor] An unreadable MCP index stops oauth status before it shows the default session. (Claude, code reading)

• File: internal/cli/oauth/status.go:55-58
• If the CLI cannot read the index, oauth status returns an error. The user does not see the default session.
• The index is a JSON array with no version. Thus a format change in a later release can break oauth status in an older release.
• Fix:
  1. Show the default session first.
  2. Then show the index error as a warning.
  3. Think about a version field in the index.

---

Nits (optional)

• Retry-After overflows before the 30s cap. (Codex, reproduced) client.go:176 multiplies before min. With Retry-After: 9223372037, the duration is negative. Then the CLI uses its short fallback wait. Fix: cap the seconds before you multiply.
• The hints for an MCP-only credential tell the user to run a plain nylas oauth login. (Claude) Refer to helpers.go:180-184 and service.go:398,478. This command makes or replaces the default session. It does not repair the MCP-only credential. The device hint at credentials.go:194 does not include --device.
• The selection does not filter credentials by the current server. (Claude) Refer to credentials.go:119-134.
• The credential id is case-sensitive for the host. (Claude + Codex) Thus API.example.com and api.example.com give two different credentials.

We also did a check of these points, and they are correct:

• The keyring key names come only from index ids or a calculated hex id. Thus --credential input cannot change a key name.
• The id is deterministic, and it does not change after a refresh.
• The dashboard token source, relogin, EnsureFresh and orgs switch use the default session only.
• The [TW-7617] Add device-code sign-in for remote CLI login and signup #125 device path sends all tokens through storeLogin. It revokes a refused token, also after Ctrl-C.
• The integration test uses only a local fake server. Thus it does not need acquireRateLimit.

---

Merge with #126

git merge-tree pr-126 pr-127 shows text conflicts in login.go and status.go. logout.go merges with no text conflict, but the result is not correct. We recommend that you merge #126 first, and then rebase this PR.

login.go

1. Keep writeLoginReport from [TW-6922] fix(cli): address remaining OAuth review items (#14, #15, #21, #22, #24, N11) #126 and loginOptionsFor from this PR.
2. Examine result.Credential != nil before the structured-output branch. The exchange must never run for an MCP-only login. This applies to text, --json, --format yaml and --quiet.
3. Add an mcp_credential object to the report: id, audience, org and principal. Never add a token.
4. For an MCP-only login, report the dashboard as not signed in, and give the reason.
5. Keep the device-prompt rule from [TW-6922] fix(cli): address remaining OAuth review items (#14, #15, #21, #22, #24, N11) #126. The URL and the code always show. In structured mode and with --quiet, they go to stderr. --quiet hides only the progress messages.
6. Add tests for --mcp-only --json and --mcp-only --quiet.

status.go

1. Do not keep the early svc.Status() return from [TW-6922] fix(cli): address remaining OAuth review items (#14, #15, #21, #22, #24, N11) #126. When no default session is stored, the report must still list the MCP-only credentials.
2. Read the index, then make the report.
3. Add the access mode, the principal and a mcp_credentials list. Never add a token.
4. Support --credential in structured mode.
5. Make --verify examine each credential. This also fixes finding 6 of the first review.
6. Document the exit code for --quiet. We recommend 0 when one or more credentials are stored.

logout.go

1. The --credential branch writes text to stdout before the report branch from [TW-6922] fix(cli): address remaining OAuth review items (#14, #15, #21, #22, #24, N11) #126. Thus --json and --quiet do not operate. Add a structured report to this branch.
2. Make --all report the MCP-only credentials that it removed.

helpers.go

• This file merges correctly. The merge result keeps only WireDashboard() from [TW-6922] fix(cli): address remaining OAuth review items (#14, #15, #21, #22, #24, N11) #126, and no init(). Make sure that WireDashboard, dashboardTokenSource and relogin continue to use the default service.

docs/COMMANDS.md

• Add the MCP fields and the --quiet exit code to the structured-output paragraph from [TW-6922] fix(cli): address remaining OAuth review items (#14, #15, #21, #22, #24, N11) #126.

---

Build and test results (967b118)

Check	Result
go build ./...	Pass
go vet ./... and go vet -tags integration ./...	Pass
golangci-lint run ./internal/... ./cmd/...	0 issues
go test -race -count=1 on the changed packages, SSH variables set	Pass
go test -race -count=1 on the changed packages, SSH variables not set	Pass
TestCLI_OAuthMCPOnly_FakeServerLogin (integration)	Pass. It needs no live credentials. It also passes with the incorrect exchange (new finding 4).
make security	Pass
Simple merge of #126 and this PR	Builds. TestStatusCmd_ListsMCPOnlyCredentialsWithoutTokens fails.

Both reviewers used scratch tests for the reproductions. We deleted the scratch tests after use.

🤖 Generated with Claude Code

[qasim-nylas]

LGTM 👍

[qasim-nylas]

LGTM 👍