feat: sync charts commands, OAuth device flow, and agent output mode - #3
Closed
ekim-amplitude wants to merge 1 commit into
Closed
feat: sync charts commands, OAuth device flow, and agent output mode#3ekim-amplitude wants to merge 1 commit into
ekim-amplitude wants to merge 1 commit into
Conversation
Mirror latest monorepo developer-cli after prod charts validation, including analytics chart list/get/query, OAuth login start/poll, and TTY/JSON agent output. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Default login skips analytics scope
- Removed the read:analytics login scope exclusion so DEFAULT_SCOPES now includes the scope required by the new chart commands.
Or push these changes by commenting:
@cursor push 8bb485b6da
Preview (8bb485b6da)
diff --git a/src/scopes.test.ts b/src/scopes.test.ts
--- a/src/scopes.test.ts
+++ b/src/scopes.test.ts
@@ -5,7 +5,7 @@
describe('DEFAULT_SCOPES', () => {
it('is the manifest scope union plus the legacy mcp scopes, sorted', () => {
expect(DEFAULT_SCOPES).toBe(
- 'mcp:read mcp:write read:flags read:projects read:taxonomy write:flags write:taxonomy',
+ 'mcp:read mcp:write read:analytics read:flags read:projects read:taxonomy write:flags write:taxonomy',
);
});
@@ -15,7 +15,7 @@
expect(scopes).toContain('mcp:write');
});
- it('excludes read:analytics until the OAuth client registers it', () => {
- expect(DEFAULT_SCOPES.split(' ')).not.toContain('read:analytics');
+ it('includes read:analytics for chart commands', () => {
+ expect(DEFAULT_SCOPES.split(' ')).toContain('read:analytics');
});
});
diff --git a/src/scopes.ts b/src/scopes.ts
--- a/src/scopes.ts
+++ b/src/scopes.ts
@@ -6,13 +6,6 @@
import { MCP_BASE_SCOPES } from './generated/scopes';
/**
- * Scopes declared in the OpenAPI manifest but not yet registered on the
- * device-flow OAuth client. Requesting these during login makes Hydra reject
- * the whole authorization. Remove entries here once MCP-396 registers them.
- */
-const LOGIN_SCOPE_EXCLUSIONS = new Set(['read:analytics']);
-
-/**
* The scope set a login requests when no `--scope` is given. The granular
* `read:`/`write:` scopes come from the generated manifest, so the default
* tracks the command surface automatically; the MCP scopes are appended.
@@ -24,6 +17,5 @@
...MCP_BASE_SCOPES,
]),
]
- .filter((scope) => !LOGIN_SCOPE_EXCLUSIONS.has(scope))
.sort()
.join(' ');You can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit f8b43a3. Configure here.
| ...MCP_BASE_SCOPES, | ||
| ]), | ||
| ] | ||
| .filter((scope) => !LOGIN_SCOPE_EXCLUSIONS.has(scope)) |
There was a problem hiding this comment.
Default login skips analytics scope
Medium Severity
This PR adds amp charts list/get/query, which require read:analytics, but default device login still filters that scope out of DEFAULT_SCOPES. Users who follow documented amp auth login --region us cannot call the new chart commands until they use another credential path that includes analytics.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit f8b43a3. Configure here.
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
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.



Summary
amp charts list/get/query, OAuth device flow, and agent TTY/JSON outputTest plan
Note
High Risk
Touches OAuth device flow, credential/profile resolution, and logout semantics—security- and UX-critical paths—with a large diff; chart query is new surface area but mostly spec/manifest-driven.
Overview
Syncs the bundled OpenAPI spec and generated manifest so
amp charts list,get, andquerycall the new Analytics endpoints (read:analytics), including list pagination/sort docs and normalized query results for supported chart types.Auth is reworked for agents and simpler defaults: new
amp auth login start/pollemit JSON status envelopes (secrets stay inpending.json), bounded polling with--timeout, and--forcewhen retargeting a profile to another region. Bareamp auth loginruns the interactive device flow at a TTY andlogin startwhen piped/non-interactive. Profile creation no longer requires--profile(implicitdefault) and public docs emphasize--region us|euinstead of--env/--base-url. Logout can cancel in-flight logins and clears pending state with--all.Supporting changes: persistent install
device_idinstate.jsonsent asAmp-Device-Id, OAuth requests getUser-Agent, EU PAT links useapp.eu.amplitude.com, and API error hints point agents at start/poll/PAT flows.Reviewed by Cursor Bugbot for commit f8b43a3. Bugbot is set up for automated code reviews on this repo. Configure here.