Skip to content

feat: sync charts commands, OAuth device flow, and agent output mode - #3

Closed
ekim-amplitude wants to merge 1 commit into
mainfrom
sync/20260722
Closed

feat: sync charts commands, OAuth device flow, and agent output mode#3
ekim-amplitude wants to merge 1 commit into
mainfrom
sync/20260722

Conversation

@ekim-amplitude

@ekim-amplitude ekim-amplitude commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Sync monorepo developer-cli after prod charts validation (MCP-528 complete)
  • Adds amp charts list/get/query, OAuth device flow, and agent TTY/JSON output
  • Updates bundled OpenAPI + generated CLI manifest/scopes

Test plan

  • pnpm test (294 tests)
  • pnpm test:typescript
  • pnpm build + pack
  • Merge → release-please release PR → npm beta

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, and query call 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 / poll emit JSON status envelopes (secrets stay in pending.json), bounded polling with --timeout, and --force when retargeting a profile to another region. Bare amp auth login runs the interactive device flow at a TTY and login start when piped/non-interactive. Profile creation no longer requires --profile (implicit default) and public docs emphasize --region us|eu instead of --env / --base-url. Logout can cancel in-flight logins and clears pending state with --all.

Supporting changes: persistent install device_id in state.json sent as Amp-Device-Id, OAuth requests get User-Agent, EU PAT links use app.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.

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>

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

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.

Create PR

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.

Comment thread src/scopes.ts
...MCP_BASE_SCOPES,
]),
]
.filter((scope) => !LOGIN_SCOPE_EXCLUSIONS.has(scope))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit f8b43a3. Configure here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant