Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 87
feat: add agentcore feedback command#1321
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
04b8141ffbb0fe884a1941f387d7ee4920b90dbe0c91cb5daFile filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,55 @@ | ||
| # Feedback | ||
| Send feedback about the AgentCore CLI directly from your terminal. | ||
| ## When to use | ||
| | Use `agentcore feedback` | Use [GitHub Issues](https://github.com/aws/agentcore-cli/issues) instead | | ||
| | -------------------------------------- | ------------------------------------------------------------------------ | | ||
| | Quick comments, suggestions, papercuts | Bugs that need a conversation or repro steps | | ||
| | First impressions, onboarding friction | Regressions you want to track | | ||
| | Sharing a screenshot of confusing UX | Feature requests you want to discuss publicly | | ||
| ## Syntax | ||
| ```bash | ||
| # One-shot | ||
| agentcore feedback "your message" [--screenshot path/to/file.png] [--json] | ||
| # Multi-step wizard | ||
| agentcore feedback | ||
| ``` | ||
| The wizard walks through: message → optional screenshot → consent → submit. Press `Esc` to step back one phase. | ||
| ## Screenshots | ||
| - Allowed types: `.png`, `.jpg`, `.jpeg` | ||
| - Maximum size: 100 MB | ||
| ## Consent | ||
| Every submission requires interactive consent. The CLI displays: | ||
| > All feedback submissions, including any uploaded text and images, are subject to the AWS Customer Agreement | ||
| > (https://aws.amazon.com/agreement/). By submitting feedback, you agree that your submissions constitute "Suggestions" | ||
| > as defined in the AWS Customer Agreement. | ||
| Bare `Enter` defaults to **No**. The command refuses to submit when stdin is not a TTY (e.g. piped input, CI). There is | ||
| no flag that bypasses the prompt. | ||
| ## What not to include | ||
| Do not paste credentials, secrets, account IDs, or customer data into the message, and do not attach screenshots that | ||
| show those values. | ||
| ## Output | ||
| Plain mode prints a confirmation line on success. JSON mode (`--json`) prints a single line: | ||
| ```json | ||
| { "success": true, "id": "<uuid>", "timestamp": "<iso8601>", "reference": "<reference>" } | ||
| ``` | ||
| On failure the CLI exits with code 1 and prints a human-readable error, or `{"success": false, "error": "..."}` when | ||
| `--json` is set. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,163 @@ | ||
| import { TelemetryClient } from '../../../telemetry/client'; | ||
| import { TelemetryClientAccessor } from '../../../telemetry/client-accessor'; | ||
| import { InMemorySink } from '../../../telemetry/sinks/in-memory-sink'; | ||
| import { registerFeedback } from '../command'; | ||
| import { Command } from '@commander-js/extra-typings'; | ||
| import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; | ||
| const mockHandleFeedback = vi.fn(); | ||
| const mockRender = vi.fn(); | ||
| const mockRequireTTY = vi.fn(); | ||
| vi.mock('../action', () => ({ | ||
| handleFeedback: (...args: unknown[]) => mockHandleFeedback(...args), | ||
| })); | ||
| vi.mock('../../../tui/guards/tty', () => ({ | ||
| requireTTY: () => mockRequireTTY(), | ||
| })); | ||
| vi.mock('../../../tui/screens/feedback', () => ({ | ||
Hweinstock marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| FeedbackScreen: () => null, | ||
| })); | ||
| vi.mock('ink', () => ({ | ||
| render: (...args: unknown[]) => { | ||
| mockRender(...args); | ||
| return { | ||
| clear: vi.fn(), | ||
| unmount: vi.fn(), | ||
| waitUntilExit: () => Promise.resolve(), | ||
| }; | ||
| }, | ||
| Text: 'Text', | ||
| Box: 'Box', | ||
| })); | ||
| const submittedOutcome = { | ||
| kind: 'submitted' as const, | ||
| result: { id: 'sub-1', timestamp: '2026-05-13T18:00:00Z', reference: 'S3' }, | ||
| }; | ||
| describe('registerFeedback', () => { | ||
| let program: Command; | ||
| let sink: InMemorySink; | ||
| let mockExit: ReturnType<typeof vi.spyOn>; | ||
| let mockLog: ReturnType<typeof vi.spyOn>; | ||
| let mockError: ReturnType<typeof vi.spyOn>; | ||
| beforeEach(() => { | ||
| program = new Command(); | ||
| program.exitOverride(); | ||
| registerFeedback(program); | ||
| sink = new InMemorySink(); | ||
| vi.spyOn(TelemetryClientAccessor, 'get').mockResolvedValue(new TelemetryClient(sink)); | ||
| mockExit = vi.spyOn(process, 'exit').mockImplementation(() => { | ||
| throw new Error('process.exit'); | ||
| }); | ||
| mockLog = vi.spyOn(console, 'log').mockImplementation(() => undefined); | ||
| mockError = vi.spyOn(console, 'error').mockImplementation(() => undefined); | ||
| }); | ||
| afterEach(() => { | ||
| vi.restoreAllMocks(); | ||
| vi.clearAllMocks(); | ||
| }); | ||
| it('registers a top-level feedback command', () => { | ||
| const cmd = program.commands.find(c => c.name() === 'feedback'); | ||
| expect(cmd).toBeDefined(); | ||
| }); | ||
| it('emits success JSON when --json is supplied with a message', async () => { | ||
| mockHandleFeedback.mockResolvedValue(submittedOutcome); | ||
| await expect(program.parseAsync(['feedback', 'looks good', '--json'], { from: 'user' })).rejects.toThrow( | ||
| 'process.exit' | ||
| ); | ||
| expect(mockHandleFeedback).toHaveBeenCalledWith('looks good', expect.objectContaining({ json: true })); | ||
| expect(mockExit).toHaveBeenCalledWith(0); | ||
| const output = JSON.parse(mockLog.mock.calls[0]?.[0] as string); | ||
| expect(output).toEqual({ | ||
| success: true, | ||
| id: 'sub-1', | ||
| timestamp: '2026-05-13T18:00:00Z', | ||
| reference: 'S3', | ||
| }); | ||
| expect(sink.metrics).toHaveLength(1); | ||
| expect(sink.metrics[0]!.attrs).toMatchObject({ | ||
| command: 'feedback', | ||
| exit_reason: 'success', | ||
| mode: 'cli', | ||
| has_screenshot: 'false', | ||
| }); | ||
| }); | ||
| it('reports a TTY error when consent cannot be confirmed and exits 1', async () => { | ||
| mockHandleFeedback.mockResolvedValue({ kind: 'no-tty' }); | ||
| await expect(program.parseAsync(['feedback', 'msg'], { from: 'user' })).rejects.toThrow('process.exit'); | ||
| expect(mockError).toHaveBeenCalledWith(expect.stringContaining('consent must be confirmed interactively')); | ||
| expect(mockExit).toHaveBeenCalledWith(1); | ||
| }); | ||
| it('prints a friendly cancellation message when the user declines consent', async () => { | ||
| mockHandleFeedback.mockResolvedValue({ kind: 'declined' }); | ||
| await expect(program.parseAsync(['feedback', 'msg'], { from: 'user' })).rejects.toThrow('process.exit'); | ||
| expect(mockLog).toHaveBeenCalledWith(expect.stringContaining('Feedback cancelled.')); | ||
| expect(mockExit).toHaveBeenCalledWith(0); | ||
| }); | ||
| it('reports submission errors with exit 1 in plain mode', async () => { | ||
| mockHandleFeedback.mockResolvedValue({ kind: 'error', error: 'HTTP 500' }); | ||
| await expect(program.parseAsync(['feedback', 'msg'], { from: 'user' })).rejects.toThrow('process.exit'); | ||
| expect(mockExit).toHaveBeenCalledWith(1); | ||
| expect(mockError).toHaveBeenCalledWith(expect.stringContaining('HTTP 500')); | ||
| expect(sink.metrics).toHaveLength(1); | ||
| expect(sink.metrics[0]!.attrs).toMatchObject({ | ||
| command: 'feedback', | ||
| exit_reason: 'failure', | ||
| mode: 'cli', | ||
| has_screenshot: 'false', | ||
| }); | ||
| }); | ||
| it('emits a JSON error envelope on submission failure when --json is set', async () => { | ||
| mockHandleFeedback.mockResolvedValue({ kind: 'error', error: 'HTTP 500' }); | ||
| await expect(program.parseAsync(['feedback', 'msg', '--json'], { from: 'user' })).rejects.toThrow('process.exit'); | ||
| const output = JSON.parse(mockLog.mock.calls[0]?.[0] as string); | ||
| expect(output).toEqual({ success: false, error: 'HTTP 500' }); | ||
| expect(mockExit).toHaveBeenCalledWith(1); | ||
| }); | ||
| it('refuses --json when no message is supplied', async () => { | ||
| await expect(program.parseAsync(['feedback', '--json'], { from: 'user' })).rejects.toThrow('process.exit'); | ||
| expect(mockError).toHaveBeenCalledWith(expect.stringContaining('--json requires a feedback message')); | ||
| expect(mockExit).toHaveBeenCalledWith(1); | ||
| expect(mockHandleFeedback).not.toHaveBeenCalled(); | ||
| }); | ||
| it('hands off to the TUI when no message argument is provided, then exits cleanly', async () => { | ||
| await expect(program.parseAsync(['feedback'], { from: 'user' })).rejects.toThrow('process.exit'); | ||
| expect(mockRequireTTY).toHaveBeenCalled(); | ||
| expect(mockRender).toHaveBeenCalled(); | ||
| expect(mockHandleFeedback).not.toHaveBeenCalled(); | ||
| // After the wizard unmounts we must terminate the Node process; otherwise | ||
| // Ink's stdin raw-mode listeners keep the process alive. | ||
| expect(mockExit).toHaveBeenCalledWith(0); | ||
| }); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,52 @@ | ||
| import { promptForConsent } from '../consent-prompt'; | ||
| import { PassThrough } from 'node:stream'; | ||
| import { describe, expect, it } from 'vitest'; | ||
| function makeTtyStdin(input: string): NodeJS.ReadableStream & { isTTY?: boolean } { | ||
| const stream = new PassThrough() as PassThrough & { isTTY?: boolean }; | ||
| stream.isTTY = true; | ||
| stream.end(input); | ||
| return stream; | ||
| } | ||
| describe('promptForConsent', () => { | ||
tejaskash marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| it('returns no-tty when stdin is not a TTY', async () => { | ||
| const stdin = new PassThrough() as PassThrough & { isTTY?: boolean }; | ||
| stdin.isTTY = false; | ||
| const stdout = new PassThrough(); | ||
| const result = await promptForConsent({ stdin, stdout }); | ||
| expect(result).toEqual({ accepted: false, reason: 'no-tty' }); | ||
| }); | ||
| it('accepts when the user types y', async () => { | ||
| const stdin = makeTtyStdin('y\n'); | ||
| const stdout = new PassThrough(); | ||
| stdout.resume(); | ||
| const result = await promptForConsent({ stdin, stdout }); | ||
| expect(result.accepted).toBe(true); | ||
| }); | ||
| it('declines on bare Enter (defaults to No)', async () => { | ||
| const stdin = makeTtyStdin('\n'); | ||
| const stdout = new PassThrough(); | ||
| stdout.resume(); | ||
| const result = await promptForConsent({ stdin, stdout }); | ||
| expect(result).toEqual({ accepted: false, reason: 'declined' }); | ||
| }); | ||
| it('declines when the user types n', async () => { | ||
| const stdin = makeTtyStdin('n\n'); | ||
| const stdout = new PassThrough(); | ||
| stdout.resume(); | ||
| const result = await promptForConsent({ stdin, stdout }); | ||
| expect(result).toEqual({ accepted: false, reason: 'declined' }); | ||
| }); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| import { toError } from '../../../lib/errors/types'; | ||
| import { submitFeedback } from '../../operations/feedback'; | ||
| import type { FeedbackSubmissionResult } from '../../operations/feedback'; | ||
| import { promptForConsent } from './consent-prompt'; | ||
| import type { FeedbackOptions } from './types'; | ||
| export type FeedbackOutcome = | ||
| | { kind: 'submitted'; result: FeedbackSubmissionResult } | ||
| | { kind: 'declined' } | ||
| | { kind: 'no-tty' } | ||
| | { kind: 'error'; error: Error }; | ||
| export async function handleFeedback(message: string, options: FeedbackOptions): Promise<FeedbackOutcome> { | ||
| const consent = await promptForConsent(); | ||
| if (!consent.accepted) { | ||
| return consent.reason === 'no-tty' ? { kind: 'no-tty' } : { kind: 'declined' }; | ||
| } | ||
| try { | ||
| const result = await submitFeedback({ | ||
| message, | ||
| screenshot: options.screenshot ? { path: options.screenshot } : undefined, | ||
| mode: 'cli', | ||
| }); | ||
| return { kind: 'submitted', result }; | ||
| } catch (err) { | ||
| return { kind: 'error', error: toError(err) }; | ||
| } | ||
| } |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.