From 6dbebaf7423e30fa7c425ead570d1b5eafa0f0ec Mon Sep 17 00:00:00 2001 From: jariy17 Date: Mon, 31 Aug 2026 19:09:11 +0000 Subject: [PATCH 1/9] feat(feedback): port the feedback command to the refactor architecture Adds `agentcore feedback [--screenshot ] [--yes]`, which submits to the Aperture public feedback API. Consent for the AWS Customer Agreement uses the project/remove imperative pattern: a readline y/N prompt on a TTY, --yes to accept non-interactively, and a hard failure (not a silent submit) when neither a TTY nor --yes is present. Screenshot attachments go presign -> S3 PUT (SHA256 checksum + NOT_SCANNED tag) -> form POST, referencing the object key parsed from the presigned URL. - src/core/feedback.tsx: FeedbackClient (injected fetch) + ApertureError (ERROR_SOURCE.SERVICE) + payload/validation ported from the pre-refactor CLI - src/handlers/feedback/: leaf handler with inline consent + types + flow tests - wired onto Core, CoreClient, the root handler, and TestCoreClient --- src/core/feedback.tsx | 326 ++++++++++++++++++++++++ src/core/index.tsx | 4 + src/handlers/feedback/feedback.test.tsx | 156 ++++++++++++ src/handlers/feedback/index.tsx | 77 ++++++ src/handlers/feedback/types.tsx | 26 ++ src/handlers/index.tsx | 2 + src/handlers/types.tsx | 2 + src/testing/TestCoreClient.tsx | 36 +++ 8 files changed, 629 insertions(+) create mode 100644 src/core/feedback.tsx create mode 100644 src/handlers/feedback/feedback.test.tsx create mode 100644 src/handlers/feedback/index.tsx create mode 100644 src/handlers/feedback/types.tsx diff --git a/src/core/feedback.tsx b/src/core/feedback.tsx new file mode 100644 index 000000000..b2c3c24b9 --- /dev/null +++ b/src/core/feedback.tsx @@ -0,0 +1,326 @@ +import { createHash } from "node:crypto"; +import { stat, readFile } from "node:fs/promises"; +import * as os from "node:os"; +import * as path from "node:path"; +import { AgentCoreCLIError, ERROR_SOURCE, InputValidationError } from "../errors"; +import { PACKAGE_VERSION } from "../constants"; +import type { AwsClients, CoreFetch, CoreOptions } from "./types"; +import type { + CoreFeedbackClient, + FeedbackSubmissionResult, + SubmitFeedbackInput, +} from "../handlers/feedback/types"; + +// Aperture public feedback API. These are commercial-partition (.aws.dev) endpoints +// with no partition variant, so feedback is unavailable in GovCloud/China — carried +// over from the pre-refactor CLI, flagged here rather than silently. +const INGESTION_URL = "https://ingestion.aperture-public-api.feedback.console.aws.dev/form"; +const PRESIGN_URL = + "https://presignedurl.aperture-public-api.feedback.console.aws.dev/presignedurl"; +const FORM_CATEGORY = "AgentCore"; +const FORM_NAME = "CLI"; +const FORM_VERSION = "0.1.0"; +const LOCALE = "en_US"; +const REFERENCE = "agentcore-cli"; +const MESSAGE_QUESTION = "What feedback do you have for the AgentCore CLI"; +const ATTACHMENT_QUESTION = "Attachments"; +const MESSAGE_MAX_LENGTH = 1000; +const MAX_SCREENSHOT_BYTES = 100 * 1024 * 1024; +const ALLOWED_SCREENSHOT_EXTENSIONS = [".png", ".jpg", ".jpeg"] as const; + +// Rendered by the feedback command's consent prompt before every submission. +export const CONSENT_TEXT = + "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."; + +// Extends the CLI error hierarchy (the pre-refactor ApertureError extended plain +// Error, so telemetry classified it as unknown) so failures record error_source=service. +export class ApertureError extends AgentCoreCLIError { + constructor( + message: string, + readonly status?: number, + readonly body?: string, + ) { + super(message, { source: ERROR_SOURCE.SERVICE, name: "ApertureError" }); + } +} + +interface LoadedScreenshot { + buffer: Uint8Array; + fileName: string; + contentType: string; + sha256Base64: string; + size: number; +} + +interface ApertureCustomerResponse { + question: string; + pii: boolean; + response: + | { responseType: "textArea"; responseValue: string } + | { responseType: "fileUpload"; responseValue: string[] }; +} + +interface ApertureFormPayload { + category: string; + name: string; + version: string; + locale: string; + reference: string; + location: string; + customerResponses: ApertureCustomerResponse[]; + metadataList: { key: string; value: string }[]; +} + +export class FeedbackClient implements CoreFeedbackClient { + constructor( + private readonly clients: AwsClients, + private readonly fetch: CoreFetch, + ) {} + + async submitFeedback( + input: SubmitFeedbackInput, + _options: CoreOptions, + ): Promise { + const message = input.message.trim(); + if (!message) { + throw new InputValidationError("Feedback message cannot be empty."); + } + if (message.length > MESSAGE_MAX_LENGTH) { + throw new InputValidationError( + `Feedback message must be ${MESSAGE_MAX_LENGTH} characters or fewer.`, + ); + } + + const userAgent = `AgentCoreCLI/${PACKAGE_VERSION} (${process.platform} ${os.release()}; node/${process.version})`; + + let screenshotReference: string | undefined; + if (input.screenshot) { + const file = await this.loadScreenshot(input.screenshot.path); + const presignedUrl = await this.fetchPresignedUrl( + { + category: FORM_CATEGORY, + name: FORM_NAME, + version: FORM_VERSION, + fileName: file.fileName, + fileSize: file.size, + uploadFileSHA256: file.sha256Base64, + }, + userAgent, + ); + await this.uploadFileToS3( + presignedUrl, + file.buffer, + file.contentType, + file.sha256Base64, + userAgent, + ); + screenshotReference = objectKeyFromPresignedUrl(presignedUrl); + } + + const payload = buildFeedbackPayload({ message, screenshotReference }); + const response = await this.submitForm(payload, userAgent); + return { + id: response.id, + timestamp: response.timestamp, + reference: response.reference, + }; + } + + // Aperture returns the presigned URL as a plain-text body (not JSON). + private async fetchPresignedUrl( + request: { + category: string; + name: string; + version: string; + fileName: string; + fileSize: number; + uploadFileSHA256: string; + }, + userAgent: string, + ): Promise { + const response = await this.fetch(PRESIGN_URL, { + method: "POST", + headers: { "content-type": "application/json", "user-agent": userAgent }, + body: JSON.stringify(request), + }); + if (!response.ok) { + throw new ApertureError( + `Failed to fetch screenshot upload URL (HTTP ${response.status}).`, + response.status, + await readBody(response), + ); + } + return (await response.text()).trim(); + } + + // Aperture's bucket policy requires the SHA-256 checksum headers and a tag + // marking the object as not yet AV-scanned; omitting either is rejected. + private async uploadFileToS3( + presignedUrl: string, + fileBuffer: Uint8Array, + contentType: string, + base64Sha256: string, + userAgent: string, + ): Promise { + const response = await this.fetch(presignedUrl, { + method: "PUT", + headers: { + "content-type": contentType, + "x-amz-checksum-algorithm": "SHA256", + "x-amz-checksum-sha256": base64Sha256, + "x-amz-tagging": "scanstatus=NOT_SCANNED", + "user-agent": userAgent, + }, + body: fileBuffer, + }); + if (!response.ok) { + throw new ApertureError( + `Failed to upload screenshot (HTTP ${response.status}).`, + response.status, + await readBody(response), + ); + } + } + + private async submitForm( + payload: ApertureFormPayload, + userAgent: string, + ): Promise { + const response = await this.fetch(INGESTION_URL, { + method: "POST", + headers: { "content-type": "application/json", "user-agent": userAgent }, + body: JSON.stringify(payload), + }); + if (!response.ok) { + const body = await readBody(response); + throw new ApertureError(mapStatusToMessage(response.status, body), response.status, body); + } + return (await response.json()) as FeedbackSubmissionResult; + } + + private async loadScreenshot(rawFilePath: string): Promise { + const filePath = expandTilde(rawFilePath); + + let stats: Awaited>; + try { + stats = await stat(filePath); + } catch (err) { + throw new InputValidationError( + `Could not read screenshot at ${filePath}: ${err instanceof Error ? err.message : String(err)}`, + ); + } + if (stats.isDirectory()) { + throw new InputValidationError(`Screenshot path is a directory, not a file: ${filePath}`); + } + if (!stats.isFile()) { + throw new InputValidationError(`Screenshot path is not a regular file: ${filePath}`); + } + + const ext = path.extname(filePath).toLowerCase(); + if ( + !ALLOWED_SCREENSHOT_EXTENSIONS.includes(ext as (typeof ALLOWED_SCREENSHOT_EXTENSIONS)[number]) + ) { + throw new InputValidationError( + `Screenshot must be one of: ${ALLOWED_SCREENSHOT_EXTENSIONS.join(", ")}.`, + ); + } + + let buffer: Buffer; + try { + buffer = await readFile(filePath); + } catch (err) { + throw new InputValidationError( + `Could not read screenshot at ${filePath}: ${err instanceof Error ? err.message : String(err)}`, + ); + } + if (buffer.byteLength > MAX_SCREENSHOT_BYTES) { + const sizeMb = (buffer.byteLength / (1024 * 1024)).toFixed(1); + throw new InputValidationError(`Screenshot is ${sizeMb} MB; maximum allowed size is 100 MB.`); + } + + return { + buffer: new Uint8Array(buffer), + fileName: path.basename(filePath), + contentType: ext === ".png" ? "image/png" : "image/jpeg", + sha256Base64: createHash("sha256").update(buffer).digest("base64"), + size: buffer.byteLength, + }; + } +} + +// Expand a leading ~ / ~/... to $HOME. Node's fs APIs don't expand tildes (the +// shell normally does), so a quoted path like "~/shot.png" would otherwise ENOENT. +function expandTilde(filePath: string): string { + if (filePath === "~") return os.homedir(); + if (filePath.startsWith("~/")) return path.join(os.homedir(), filePath.slice(2)); + return filePath; +} + +// The presigned URL's path IS the S3 object key the form must reference; +// fabricating one client-side risks pointing at a nonexistent object if +// Aperture's bucket layout or region shifts. +function objectKeyFromPresignedUrl(presignedUrl: string): string { + return decodeURIComponent(new URL(presignedUrl).pathname.replace(/^\/+/, "")); +} + +function buildFeedbackPayload(input: { + message: string; + screenshotReference?: string; +}): ApertureFormPayload { + const customerResponses: ApertureCustomerResponse[] = [ + { + question: MESSAGE_QUESTION, + pii: false, + response: { responseType: "textArea", responseValue: input.message }, + }, + ]; + if (input.screenshotReference) { + customerResponses.push({ + question: ATTACHMENT_QUESTION, + pii: true, + response: { responseType: "fileUpload", responseValue: [input.screenshotReference] }, + }); + } + + // Aperture rejects unknown metadata keys with HTTP 400; only cli-version and os + // are registered in the form template, so node version + mode ride in `location`. + return { + category: FORM_CATEGORY, + name: FORM_NAME, + version: FORM_VERSION, + locale: LOCALE, + reference: REFERENCE, + location: `agentcore-cli@${PACKAGE_VERSION} (${process.platform}; node ${process.version}; cli)`, + customerResponses, + metadataList: [ + { key: "cli-version", value: PACKAGE_VERSION }, + { key: "os", value: `${process.platform} ${os.release()}` }, + ], + }; +} + +function mapStatusToMessage(status: number, body: string): string { + switch (status) { + case 400: + return `Feedback service rejected the submission (HTTP 400). ${body || "Form payload may be malformed."}`; + case 412: + return "Feedback service is missing required headers (HTTP 412)."; + case 417: + return "Feedback service rejected the request content type (HTTP 417)."; + case 500: + return "Feedback service returned an internal error (HTTP 500). Please try again later."; + default: + return `Feedback service returned HTTP ${status}.`; + } +} + +async function readBody(response: Response): Promise { + try { + return await response.text(); + } catch { + return ""; + } +} diff --git a/src/core/index.tsx b/src/core/index.tsx index 5a6bab9fb..95060bcfe 100644 --- a/src/core/index.tsx +++ b/src/core/index.tsx @@ -3,6 +3,7 @@ import { BedrockAgentCoreClient } from "@aws-sdk/client-bedrock-agentcore"; import { IAMClient } from "@aws-sdk/client-iam"; import { CloudWatchLogsClient } from "@aws-sdk/client-cloudwatch-logs"; import { EvalClient } from "./eval"; +import { FeedbackClient } from "./feedback"; import { GatewayClient } from "./gateway"; import { HarnessClient } from "./harness"; import { IdentityClient } from "./identity"; @@ -72,6 +73,7 @@ export class CoreClient implements AwsClients { readonly runtime: RuntimeClient; readonly gateway: GatewayClient; readonly eval: EvalClient; + readonly feedback: FeedbackClient; readonly observability: ObservabilityClient; readonly projectManager: ProjectManager; @@ -86,6 +88,8 @@ export class CoreClient implements AwsClients { const fetch = config.fetch ?? globalThis.fetch; this.runtime = new RuntimeClient(this, fetch, this.logger.child({ module: "runtime" })); this.gateway = new GatewayClient(this, fetch, this.logger.child({ module: "gateway" })); + // Feedback posts to the Aperture public API via the injected fetch, outside the SDK seam. + this.feedback = new FeedbackClient(this, fetch); // EvalClient shares the injected fetch: dataset content is served from a // presigned S3 URL, outside the SDK seam the other operations use. The logger // is used for batch-evaluation result-log diagnostics. diff --git a/src/handlers/feedback/feedback.test.tsx b/src/handlers/feedback/feedback.test.tsx new file mode 100644 index 000000000..144785a65 --- /dev/null +++ b/src/handlers/feedback/feedback.test.tsx @@ -0,0 +1,156 @@ +import { test, expect, describe } from "bun:test"; +import { CoreClient } from "../../core"; +import { createRootHandler } from "../index"; +import { + createSilentLogger, + fixtureFactories, + TestGlobalConfigAccessor, + testIO, + type TestIOOptions, +} from "../../testing"; +import { InputValidationError, UserCancellationError } from "../../errors"; +import { join } from "node:path"; +import { mkdtemp, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; + +// Command-flow tests for `agentcore feedback`. The Aperture POST/PUT calls go +// through an injected `fetch` stub (feedback is the one Core path outside the SDK +// seam), so these run offline and assert the exact HTTP the client makes. + +const REGION = "us-east-1"; +const FIXTURES = join(import.meta.dir, "__fixtures__"); + +interface Recorded { + url: string; + method: string; + headers: Record; + body: unknown; +} + +// stubFetch answers the three Aperture calls (presign POST, S3 PUT, form POST) +// and records each so tests can assert the request shape. +function stubFetch(calls: Recorded[]) { + const presignedUrl = + "https://aperture-bucket.s3.us-east-1.amazonaws.com/us-east-1/AgentCore/CLI/0.1.0/13052026/abc-123.png?X-Amz-Signature=sig"; + return (async (input: Parameters[0], init?: Parameters[1]) => { + const url = String(input); + calls.push({ + url, + method: init?.method ?? "GET", + headers: (init?.headers as Record) ?? {}, + body: init?.body, + }); + if (url.includes("/presignedurl")) { + return new Response(presignedUrl, { status: 200 }); + } + if (url.includes("/form")) { + return new Response( + JSON.stringify({ + id: "11111111-2222-3333-4444-555555555555", + timestamp: "2026-08-31T00:00:00Z", + reference: "agentcore-cli", + }), + { status: 200, headers: { "content-type": "application/json" } }, + ); + } + return new Response(null, { status: 200 }); // the S3 PUT + }) as unknown as typeof fetch; +} + +async function run( + args: string[], + opts: { io?: TestIOOptions; calls?: Recorded[] } = {}, +): Promise<{ stdout: string; stderr: string }> { + const factories = fixtureFactories(FIXTURES); + const core = new CoreClient({ + ...factories, + logger: createSilentLogger(), + fetch: stubFetch(opts.calls ?? []), + }); + const io = testIO(opts.io); + const root = createRootHandler(core, { + io: io.io, + logger: createSilentLogger(), + globalConfigAccessor: new TestGlobalConfigAccessor(), + }); + await root.route(["node", "agentcore", "feedback", ...args, "--region", REGION]); + return { stdout: io.stdout(), stderr: io.stderr() }; +} + +describe("feedback", () => { + test("--yes --json submits and prints the result envelope", async () => { + const calls: Recorded[] = []; + const { stdout } = await run(["great tool", "--yes", "--json"], { calls }); + const parsed = JSON.parse(stdout); + expect(parsed.success).toBe(true); + expect(parsed.id).toBe("11111111-2222-3333-4444-555555555555"); + // Text-only: exactly one call, the form POST. + expect(calls).toHaveLength(1); + expect(calls[0]!.url).toContain("/form"); + expect(calls[0]!.method).toBe("POST"); + }); + + test("prompts on a TTY and submits on 'y'", async () => { + const calls: Recorded[] = []; + const { stderr } = await run(["nice cli"], { io: { isTTY: true, stdin: "y\n" }, calls }); + expect(stderr).toContain("AWS Customer Agreement"); + expect(stderr).toContain("Submit feedback? (y/N)"); + expect(calls).toHaveLength(1); + }); + + test("declining the prompt cancels without submitting", async () => { + const calls: Recorded[] = []; + await expect( + run(["nope"], { io: { isTTY: true, stdin: "n\n" }, calls }), + ).rejects.toBeInstanceOf(UserCancellationError); + expect(calls).toHaveLength(0); + }); + + test("non-interactive without --yes fails and does not submit", async () => { + const calls: Recorded[] = []; + const removal = run(["headless"], { calls }); + await expect(removal).rejects.toBeInstanceOf(InputValidationError); + await expect(run(["headless"], { calls: [] })).rejects.toThrow(/--yes/); + expect(calls).toHaveLength(0); + }); + + test("an empty message is rejected before any network call", async () => { + const calls: Recorded[] = []; + await expect(run([" ", "--yes"], { calls })).rejects.toThrow(/cannot be empty/); + expect(calls).toHaveLength(0); + }); + + test("a message over 1000 chars is rejected", async () => { + const calls: Recorded[] = []; + await expect(run(["x".repeat(1001), "--yes"], { calls })).rejects.toThrow(/1000 characters/); + expect(calls).toHaveLength(0); + }); + + test("a screenshot drives presign -> S3 PUT (checksum + tag) -> form POST", async () => { + const dir = await mkdtemp(join(tmpdir(), "agentcore-fb-")); + const shot = join(dir, "shot.png"); + await writeFile(shot, Buffer.from("iVBORw0KGgoAAAANSUhEUgAA", "base64")); + + const calls: Recorded[] = []; + const { stdout } = await run(["with shot", "--screenshot", shot, "--yes", "--json"], { calls }); + expect(JSON.parse(stdout).success).toBe(true); + + expect( + calls.map( + (c) => + `${c.method} ${c.url.includes("/presignedurl") ? "presign" : c.url.includes("/form") ? "form" : "s3"}`, + ), + ).toEqual(["POST presign", "PUT s3", "POST form"]); + const put = calls[1]!; + expect(put.headers["x-amz-checksum-algorithm"]).toBe("SHA256"); + expect(put.headers["x-amz-tagging"]).toBe("scanstatus=NOT_SCANNED"); + // The form references the exact object key parsed from the presigned URL path. + const form = JSON.parse(String(calls[2]!.body)); + const attachment = form.customerResponses.find( + (r: { response: { responseType: string } }) => r.response.responseType === "fileUpload", + ); + expect(attachment.response.responseValue).toEqual([ + "us-east-1/AgentCore/CLI/0.1.0/13052026/abc-123.png", + ]); + }); +}); diff --git a/src/handlers/feedback/index.tsx b/src/handlers/feedback/index.tsx new file mode 100644 index 000000000..1bcfacc5b --- /dev/null +++ b/src/handlers/feedback/index.tsx @@ -0,0 +1,77 @@ +import { createInterface } from "node:readline/promises"; +import z from "zod"; +import { argument, createHandler, flag } from "../../router"; +import { JsonRendererKey } from "../../tui"; +import { InputValidationError, UserCancellationError } from "../../errors"; +import { coreOptsFromCtx } from "../utils.tsx"; +import { JsonKey } from "../keys.tsx"; +import { CONSENT_TEXT } from "../../core/feedback"; +import type { Core } from "../types.tsx"; +import type { AppIO } from "../../io"; + +export const createFeedbackHandler = (core: Core, io: AppIO) => + createHandler({ + name: "feedback", + description: "Send feedback about the AgentCore CLI to the team.", + arguments: [argument("message", "the feedback message to send", z.string().max(1000))], + flags: [ + flag( + "screenshot", + "path to a PNG or JPG screenshot to attach (max 100MB)", + z.string().optional(), + ), + flag( + "yes", + "accept the AWS Customer Agreement and skip the consent prompt", + z.boolean().default(false), + ), + ], + handle: async (ctx, flags, args) => { + await confirmConsent(io, ctx.require(JsonKey), flags.yes); + + const result = await core.feedback.submitFeedback( + { + message: args["message"], + screenshot: flags["screenshot"] ? { path: flags["screenshot"] } : undefined, + }, + coreOptsFromCtx(ctx), + ); + + ctx.require(JsonRendererKey).renderJson({ success: true, ...result }); + }, + }); + +// Mirrors project/remove's confirmRemoveAll: --yes bypasses the prompt, a +// non-interactive session (or --json) fails rather than submitting without +// consent, and a decline (or SIGINT) raises UserCancellationError. +async function confirmConsent(io: AppIO, jsonOutput: boolean, confirmed: boolean): Promise { + if (confirmed) return; + const canPrompt = !jsonOutput && io.stdin.isTTY && io.stdout.isTTY && io.stderr.isTTY; + if (!canPrompt) { + throw new InputValidationError( + "submitting feedback requires accepting the AWS Customer Agreement; re-run with --yes to confirm non-interactively", + ); + } + if (!(await promptForConsent(io))) { + throw new UserCancellationError(); + } +} + +async function promptForConsent(io: AppIO): Promise { + // Prompt on stderr so --json / piped stdout stays a clean machine-readable stream. + const readline = createInterface({ input: io.stdin, output: io.stderr }); + try { + const cancelled = new Promise((_resolve, reject) => { + const cancel = () => reject(new UserCancellationError()); + readline.once("SIGINT", cancel); + readline.once("close", cancel); + }); + const answer = await Promise.race([ + readline.question(`\n${CONSENT_TEXT}\n\nSubmit feedback? (y/N) `), + cancelled, + ]); + return /^(?:y|yes)$/i.test(answer.trim()); + } finally { + readline.close(); + } +} diff --git a/src/handlers/feedback/types.tsx b/src/handlers/feedback/types.tsx new file mode 100644 index 000000000..f5536cdd9 --- /dev/null +++ b/src/handlers/feedback/types.tsx @@ -0,0 +1,26 @@ +import type { CoreOptions } from "../../core/types"; + +export interface ScreenshotInput { + path: string; +} + +export interface SubmitFeedbackInput { + message: string; + screenshot?: ScreenshotInput; +} + +export interface FeedbackSubmissionResult { + id: string; + timestamp: string; + reference: string; +} + +// Consumer-defined interface (dependency inversion): the handler depends on this, +// src/core/feedback.tsx implements it. Message/screenshot validation happens inside +// submitFeedback so the one code path guards every caller. +export interface CoreFeedbackClient { + submitFeedback( + input: SubmitFeedbackInput, + options: CoreOptions, + ): Promise; +} diff --git a/src/handlers/index.tsx b/src/handlers/index.tsx index a2ff49e03..cd03e75f2 100644 --- a/src/handlers/index.tsx +++ b/src/handlers/index.tsx @@ -1,5 +1,6 @@ import { Router } from "../router"; import { createEvalHandler } from "./eval/index.tsx"; +import { createFeedbackHandler } from "./feedback/index.tsx"; import { createGatewayHandler } from "./gateway/index.tsx"; import { createHarnessHandler } from "./harness/index.tsx"; import { createIdentityHandler } from "./identity/index.tsx"; @@ -53,6 +54,7 @@ export function createRootHandler(core: Core, config: RootHandlerConfig): Router root.handler(createMemoryHandler(core, io)); root.handler(createGatewayHandler(core, io)); root.handler(createEvalHandler(core, io)); + root.handler(createFeedbackHandler(core, io)); root.handler(createConfigHandler()); root.handler(createProjectHandler({ core, io })); diff --git a/src/handlers/types.tsx b/src/handlers/types.tsx index 3d76827b8..ccf802421 100644 --- a/src/handlers/types.tsx +++ b/src/handlers/types.tsx @@ -1,4 +1,5 @@ import type { CoreEvalClient } from "./eval/types.tsx"; +import type { CoreFeedbackClient } from "./feedback/types.tsx"; import type { CoreGatewayClient } from "./gateway/types.tsx"; import type { CoreHarnessClient } from "./harness/types.tsx"; import type { CoreIdentityClient } from "./identity/types.tsx"; @@ -15,6 +16,7 @@ export interface Core { runtime: CoreRuntimeClient; gateway: CoreGatewayClient; eval: CoreEvalClient; + feedback: CoreFeedbackClient; observability: CoreObservabilityClient; projectManager: ProjectManager; /** Describes a Bedrock Agent + alias for `--type import`. */ diff --git a/src/testing/TestCoreClient.tsx b/src/testing/TestCoreClient.tsx index a6da20a9e..b4833c57b 100644 --- a/src/testing/TestCoreClient.tsx +++ b/src/testing/TestCoreClient.tsx @@ -126,6 +126,11 @@ import type { UpdateOauth2CredentialProviderInput, } from "../handlers/identity/types"; import type { CoreMemoryClient } from "../handlers/memory/types"; +import type { + CoreFeedbackClient, + FeedbackSubmissionResult, + SubmitFeedbackInput, +} from "../handlers/feedback/types"; import type { CoreObservabilityClient, CoreRuntimeClient, @@ -2297,6 +2302,36 @@ export class TestObservabilityClient implements CoreObservabilityClient { } } +// TestFeedbackClient is the feedback sub-client of TestCoreClient. +export class TestFeedbackClient implements CoreFeedbackClient { + readonly calls: RecordedCall[] = []; + private response: FeedbackSubmissionResult = { + id: "feedback-test-id", + timestamp: "2026-01-01T00:00:00Z", + reference: "agentcore-cli", + }; + private error?: Error; + + setSubmitResponse(response: FeedbackSubmissionResult): this { + this.response = response; + return this; + } + + setError(error: Error | undefined): this { + this.error = error; + return this; + } + + async submitFeedback( + input: SubmitFeedbackInput, + options: CoreOptions, + ): Promise { + this.calls.push({ method: "submitFeedback", args: [input, options] }); + if (this.error) throw this.error; + return this.response; + } +} + // TestCoreClient implements the Core contract with fully controllable sub-clients. export class TestCoreClient implements Core { readonly harness = new TestHarnessClient(); @@ -2305,6 +2340,7 @@ export class TestCoreClient implements Core { readonly runtime = new TestRuntimeClient(); readonly gateway = new TestGatewayClient(); readonly eval = new TestEvalClient(); + readonly feedback = new TestFeedbackClient(); readonly observability = new TestObservabilityClient(); readonly projectManager: ProjectManager; From 2d80c457906f737f8ab01002fd55bbaabd8fd9db Mon Sep 17 00:00:00 2001 From: jariy17 Date: Mon, 31 Aug 2026 19:56:43 +0000 Subject: [PATCH 2/9] fix(feedback): make core the single message-length validator The argument schema z.string().max(1000) double-validated the raw (untrimmed) message and fired a generic zod error before core's friendlier, trim-aware 'must be 1000 characters or fewer' guard could run. Drop the arg constraint so core.submitFeedback is the one code path that validates, matching the intent noted in feedback/types.tsx. --- src/handlers/feedback/index.tsx | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/handlers/feedback/index.tsx b/src/handlers/feedback/index.tsx index 1bcfacc5b..01bbe1e0c 100644 --- a/src/handlers/feedback/index.tsx +++ b/src/handlers/feedback/index.tsx @@ -13,7 +13,9 @@ export const createFeedbackHandler = (core: Core, io: AppIO) => createHandler({ name: "feedback", description: "Send feedback about the AgentCore CLI to the team.", - arguments: [argument("message", "the feedback message to send", z.string().max(1000))], + // Length/empty validation lives solely in core.submitFeedback so one code path + // guards every caller; the arg is unconstrained here beyond being a string. + arguments: [argument("message", "the feedback message to send", z.string())], flags: [ flag( "screenshot", From 0ecd641f4a0b076e63d20dd754475b8e0dccdd74 Mon Sep 17 00:00:00 2001 From: jariy17 Date: Mon, 31 Aug 2026 19:57:01 +0000 Subject: [PATCH 3/9] fix(feedback): reject oversized screenshots from stat before reading The 100MB cap was checked on buffer.byteLength after readFile loaded the whole file, so a multi-GB file was read entirely into memory just to be rejected. Check stats.size before readFile instead. --- src/core/feedback.tsx | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/core/feedback.tsx b/src/core/feedback.tsx index b2c3c24b9..b85ef323e 100644 --- a/src/core/feedback.tsx +++ b/src/core/feedback.tsx @@ -218,6 +218,12 @@ export class FeedbackClient implements CoreFeedbackClient { if (!stats.isFile()) { throw new InputValidationError(`Screenshot path is not a regular file: ${filePath}`); } + // Reject oversized files from stat before readFile, so a hostile/huge file + // is never loaded into memory just to be rejected. + if (stats.size > MAX_SCREENSHOT_BYTES) { + const sizeMb = (stats.size / (1024 * 1024)).toFixed(1); + throw new InputValidationError(`Screenshot is ${sizeMb} MB; maximum allowed size is 100 MB.`); + } const ext = path.extname(filePath).toLowerCase(); if ( @@ -236,11 +242,6 @@ export class FeedbackClient implements CoreFeedbackClient { `Could not read screenshot at ${filePath}: ${err instanceof Error ? err.message : String(err)}`, ); } - if (buffer.byteLength > MAX_SCREENSHOT_BYTES) { - const sizeMb = (buffer.byteLength / (1024 * 1024)).toFixed(1); - throw new InputValidationError(`Screenshot is ${sizeMb} MB; maximum allowed size is 100 MB.`); - } - return { buffer: new Uint8Array(buffer), fileName: path.basename(filePath), From 04e24ea64c16bbd1325f8fa3fb89f100f0919b23 Mon Sep 17 00:00:00 2001 From: jariy17 Date: Mon, 31 Aug 2026 19:57:24 +0000 Subject: [PATCH 4/9] refactor(feedback): drop unused AwsClients dependency from FeedbackClient FeedbackClient only uses the injected fetch (Aperture is outside the SDK seam), so the stored AwsClients param was dead. Take only CoreFetch and update the CoreClient construction site. --- src/core/feedback.tsx | 9 ++++----- src/core/index.tsx | 2 +- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/core/feedback.tsx b/src/core/feedback.tsx index b85ef323e..c747dfe38 100644 --- a/src/core/feedback.tsx +++ b/src/core/feedback.tsx @@ -4,7 +4,7 @@ import * as os from "node:os"; import * as path from "node:path"; import { AgentCoreCLIError, ERROR_SOURCE, InputValidationError } from "../errors"; import { PACKAGE_VERSION } from "../constants"; -import type { AwsClients, CoreFetch, CoreOptions } from "./types"; +import type { CoreFetch, CoreOptions } from "./types"; import type { CoreFeedbackClient, FeedbackSubmissionResult, @@ -75,10 +75,9 @@ interface ApertureFormPayload { } export class FeedbackClient implements CoreFeedbackClient { - constructor( - private readonly clients: AwsClients, - private readonly fetch: CoreFetch, - ) {} + // Feedback posts to the Aperture public API via the injected fetch only; it makes + // no AWS SDK calls, so it does not take the AwsClients aggregate its siblings do. + constructor(private readonly fetch: CoreFetch) {} async submitFeedback( input: SubmitFeedbackInput, diff --git a/src/core/index.tsx b/src/core/index.tsx index 95060bcfe..4492d02ea 100644 --- a/src/core/index.tsx +++ b/src/core/index.tsx @@ -89,7 +89,7 @@ export class CoreClient implements AwsClients { this.runtime = new RuntimeClient(this, fetch, this.logger.child({ module: "runtime" })); this.gateway = new GatewayClient(this, fetch, this.logger.child({ module: "gateway" })); // Feedback posts to the Aperture public API via the injected fetch, outside the SDK seam. - this.feedback = new FeedbackClient(this, fetch); + this.feedback = new FeedbackClient(fetch); // EvalClient shares the injected fetch: dataset content is served from a // presigned S3 URL, outside the SDK seam the other operations use. The logger // is used for batch-evaluation result-log diagnostics. From 8f53b7b13f599f424ae24bf92971ea58c0137677 Mon Sep 17 00:00:00 2001 From: jariy17 Date: Mon, 31 Aug 2026 19:57:36 +0000 Subject: [PATCH 5/9] fix(feedback): classify a non-URL presign body as ApertureError If Aperture returns a 2xx presign body that isn't a URL, new URL() threw a bare TypeError that mapped to an internal-source error. Wrap it in ApertureError so telemetry attributes the failure to the service. --- src/core/feedback.tsx | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/core/feedback.tsx b/src/core/feedback.tsx index c747dfe38..136ccd822 100644 --- a/src/core/feedback.tsx +++ b/src/core/feedback.tsx @@ -263,7 +263,13 @@ function expandTilde(filePath: string): string { // fabricating one client-side risks pointing at a nonexistent object if // Aperture's bucket layout or region shifts. function objectKeyFromPresignedUrl(presignedUrl: string): string { - return decodeURIComponent(new URL(presignedUrl).pathname.replace(/^\/+/, "")); + try { + return decodeURIComponent(new URL(presignedUrl).pathname.replace(/^\/+/, "")); + } catch { + // A 2xx presign body that isn't a URL is a service fault, not a bare TypeError — + // classify it so telemetry attributes it to the service, not internal. + throw new ApertureError("Feedback service returned an invalid screenshot upload URL."); + } } function buildFeedbackPayload(input: { From b980b90afa81aa0d0149d9b8912b1bfdf2d7693c Mon Sep 17 00:00:00 2001 From: jariy17 Date: Mon, 31 Aug 2026 19:57:50 +0000 Subject: [PATCH 6/9] fix(feedback): reject an explicitly-empty --screenshot value --screenshot "" was falsy so it silently submitted with no attachment, unlike every other bad screenshot value which errors. Reject a present-but-blank path with an InputValidationError. --- src/handlers/feedback/index.tsx | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/handlers/feedback/index.tsx b/src/handlers/feedback/index.tsx index 01bbe1e0c..d2f4ddaba 100644 --- a/src/handlers/feedback/index.tsx +++ b/src/handlers/feedback/index.tsx @@ -29,12 +29,19 @@ export const createFeedbackHandler = (core: Core, io: AppIO) => ), ], handle: async (ctx, flags, args) => { + // An explicitly-empty --screenshot "" is a mistake, not "no screenshot": + // reject it rather than silently submitting without an attachment. + const screenshotPath = flags["screenshot"]; + if (screenshotPath !== undefined && screenshotPath.trim() === "") { + throw new InputValidationError("--screenshot requires a file path"); + } + await confirmConsent(io, ctx.require(JsonKey), flags.yes); const result = await core.feedback.submitFeedback( { message: args["message"], - screenshot: flags["screenshot"] ? { path: flags["screenshot"] } : undefined, + screenshot: screenshotPath ? { path: screenshotPath } : undefined, }, coreOptsFromCtx(ctx), ); From 311de1b5e032e8934a0c847eb126994e3958122e Mon Sep 17 00:00:00 2001 From: jariy17 Date: Mon, 31 Aug 2026 20:01:20 +0000 Subject: [PATCH 7/9] chore(feedback): drop redundant comment at the FeedbackClient wiring The rationale now lives on the FeedbackClient constructor in core/feedback.tsx. --- src/core/index.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/src/core/index.tsx b/src/core/index.tsx index 4492d02ea..e561664b7 100644 --- a/src/core/index.tsx +++ b/src/core/index.tsx @@ -88,7 +88,6 @@ export class CoreClient implements AwsClients { const fetch = config.fetch ?? globalThis.fetch; this.runtime = new RuntimeClient(this, fetch, this.logger.child({ module: "runtime" })); this.gateway = new GatewayClient(this, fetch, this.logger.child({ module: "gateway" })); - // Feedback posts to the Aperture public API via the injected fetch, outside the SDK seam. this.feedback = new FeedbackClient(fetch); // EvalClient shares the injected fetch: dataset content is served from a // presigned S3 URL, outside the SDK seam the other operations use. The logger From 6d051c00eca450530c349407d18342a21d9de513 Mon Sep 17 00:00:00 2001 From: jariy17 Date: Mon, 31 Aug 2026 20:05:21 +0000 Subject: [PATCH 8/9] test(feedback): replace unit test with golden fixture test Follows the batch-evaluation pattern: golden-backed happy paths (text-only and screenshot presign->S3 PUT->form, recorded against Aperture, replayed offline) plus rejects.toThrow validation/consent cases (non-TTY without --yes, decline, empty message, >1000 chars, empty --screenshot). Each submit test uses its own fixtureFetch subdir since the fetch fixture key is method+path only and both POST to /form. The presign response fixture has its X-Amz-* query stripped so no signed URL is committed; replay keys on the stable object path. --- src/handlers/feedback/__fixtures__/shot.png | Bin 0 -> 69 bytes .../submit-screenshot.golden.json | 6 + .../Fetch.499b0768d488bbcf.json | 5 + .../Fetch.92c60dabfb9be7f2.json | 5 + .../Fetch.f7c65f1fef88f718.json | 5 + .../__fixtures__/submit-text.golden.json | 6 + .../submit-text/Fetch.92c60dabfb9be7f2.json | 5 + .../feedback/feedback.fixture.test.tsx | 118 +++++++++++++ src/handlers/feedback/feedback.test.tsx | 156 ------------------ 9 files changed, 150 insertions(+), 156 deletions(-) create mode 100644 src/handlers/feedback/__fixtures__/shot.png create mode 100644 src/handlers/feedback/__fixtures__/submit-screenshot.golden.json create mode 100644 src/handlers/feedback/__fixtures__/submit-screenshot/Fetch.499b0768d488bbcf.json create mode 100644 src/handlers/feedback/__fixtures__/submit-screenshot/Fetch.92c60dabfb9be7f2.json create mode 100644 src/handlers/feedback/__fixtures__/submit-screenshot/Fetch.f7c65f1fef88f718.json create mode 100644 src/handlers/feedback/__fixtures__/submit-text.golden.json create mode 100644 src/handlers/feedback/__fixtures__/submit-text/Fetch.92c60dabfb9be7f2.json create mode 100644 src/handlers/feedback/feedback.fixture.test.tsx delete mode 100644 src/handlers/feedback/feedback.test.tsx diff --git a/src/handlers/feedback/__fixtures__/shot.png b/src/handlers/feedback/__fixtures__/shot.png new file mode 100644 index 0000000000000000000000000000000000000000..875245de1db9e1bf24138d7d97a8b8df252915e6 GIT binary patch literal 69 zcmeAS@N?(olHy`uVBq!ia0vp^j3CUx0wlM}@Gt=>Zci7-kcwN$DL>9LFfcPRGMxN3 R={-=K!PC{xWt~$(69Acj4;KIc literal 0 HcmV?d00001 diff --git a/src/handlers/feedback/__fixtures__/submit-screenshot.golden.json b/src/handlers/feedback/__fixtures__/submit-screenshot.golden.json new file mode 100644 index 000000000..a72012723 --- /dev/null +++ b/src/handlers/feedback/__fixtures__/submit-screenshot.golden.json @@ -0,0 +1,6 @@ +{ + "success": true, + "id": "9385c2bc-e013-4c31-a5c4-e430f221037a", + "timestamp": "2026-08-31T20:04:07.165808755Z", + "reference": "agentcore-cli" +} \ No newline at end of file diff --git a/src/handlers/feedback/__fixtures__/submit-screenshot/Fetch.499b0768d488bbcf.json b/src/handlers/feedback/__fixtures__/submit-screenshot/Fetch.499b0768d488bbcf.json new file mode 100644 index 000000000..0e218e8c7 --- /dev/null +++ b/src/handlers/feedback/__fixtures__/submit-screenshot/Fetch.499b0768d488bbcf.json @@ -0,0 +1,5 @@ +{ + "status": 200, + "statusText": "OK", + "body": "" +} \ No newline at end of file diff --git a/src/handlers/feedback/__fixtures__/submit-screenshot/Fetch.92c60dabfb9be7f2.json b/src/handlers/feedback/__fixtures__/submit-screenshot/Fetch.92c60dabfb9be7f2.json new file mode 100644 index 000000000..676884db3 --- /dev/null +++ b/src/handlers/feedback/__fixtures__/submit-screenshot/Fetch.92c60dabfb9be7f2.json @@ -0,0 +1,5 @@ +{ + "status": 200, + "statusText": "OK", + "body": "{\"reference\":\"agentcore-cli\",\"id\":\"9385c2bc-e013-4c31-a5c4-e430f221037a\",\"timestamp\":\"2026-08-31T20:04:07.165808755Z\"}" +} \ No newline at end of file diff --git a/src/handlers/feedback/__fixtures__/submit-screenshot/Fetch.f7c65f1fef88f718.json b/src/handlers/feedback/__fixtures__/submit-screenshot/Fetch.f7c65f1fef88f718.json new file mode 100644 index 000000000..7c0298d14 --- /dev/null +++ b/src/handlers/feedback/__fixtures__/submit-screenshot/Fetch.f7c65f1fef88f718.json @@ -0,0 +1,5 @@ +{ + "status": 200, + "statusText": "OK", + "body": "https://aperture-forms-uploaded-files-prod-us-east-1.s3.amazonaws.com/us-east-1/AgentCore/CLI/0.1.0/31082026/3066c98d-4cda-42d5-ad1e-b00e8f69b96f.png" +} diff --git a/src/handlers/feedback/__fixtures__/submit-text.golden.json b/src/handlers/feedback/__fixtures__/submit-text.golden.json new file mode 100644 index 000000000..ecd3e34af --- /dev/null +++ b/src/handlers/feedback/__fixtures__/submit-text.golden.json @@ -0,0 +1,6 @@ +{ + "success": true, + "id": "395e1470-9d77-40a8-af81-9b1181bca976", + "timestamp": "2026-08-31T20:03:36.685002013Z", + "reference": "agentcore-cli" +} \ No newline at end of file diff --git a/src/handlers/feedback/__fixtures__/submit-text/Fetch.92c60dabfb9be7f2.json b/src/handlers/feedback/__fixtures__/submit-text/Fetch.92c60dabfb9be7f2.json new file mode 100644 index 000000000..f82879966 --- /dev/null +++ b/src/handlers/feedback/__fixtures__/submit-text/Fetch.92c60dabfb9be7f2.json @@ -0,0 +1,5 @@ +{ + "status": 200, + "statusText": "OK", + "body": "{\"reference\":\"agentcore-cli\",\"id\":\"395e1470-9d77-40a8-af81-9b1181bca976\",\"timestamp\":\"2026-08-31T20:03:36.685002013Z\"}" +} \ No newline at end of file diff --git a/src/handlers/feedback/feedback.fixture.test.tsx b/src/handlers/feedback/feedback.fixture.test.tsx new file mode 100644 index 000000000..728850dea --- /dev/null +++ b/src/handlers/feedback/feedback.fixture.test.tsx @@ -0,0 +1,118 @@ +import { describe, expect, test } from "bun:test"; +import { join } from "node:path"; +import { CoreClient } from "../../core"; +import { createRootHandler } from "../index"; +import { + createSilentLogger, + fixtureFactories, + fixtureFetch, + matchGolden, + TestGlobalConfigAccessor, + testIO, + type TestIOOptions, +} from "../../testing"; +import { UserCancellationError } from "../../errors"; + +// Record with: RECORD=1 bun test src/handlers/feedback/feedback.fixture.test.tsx +// +// The two "submits …" tests are WRITES: a record run posts a REAL feedback +// submission to the Aperture public API (and, for the screenshot case, uploads +// shot.png through a real presigned S3 PUT) — there is no undo, same as the +// batch-evaluation evaluate/simulate fixtures that submit real jobs. After a +// record run, strip the X-Amz-* query from the recorded presign Fetch fixture +// so no signed URL is committed (the object-path key it replays on is unchanged). +// Every other run replays the committed fixtures offline. +// +// Aperture is the one Core path outside the AWS SDK `.send()` seam, so it is +// driven through the injected `fetch` (fixtureFetch) rather than the SDK +// factories. Each submit test uses its own fixture subdir because fixtureFetch +// keys on method+path only, and both submits POST to the same /form path. +const REGION = "us-east-1"; +const FIXTURES = join(import.meta.dir, "__fixtures__"); +const SHOT = join(FIXTURES, "shot.png"); + +function createFixtureCore(fetchDir: string): CoreClient { + const { createControlClient, createDataClient, createIamClient, createLogsClient } = + fixtureFactories(FIXTURES); + return new CoreClient({ + createControlClient, + createDataClient, + createIamClient, + createLogsClient, + logger: createSilentLogger(), + fetch: fixtureFetch(join(FIXTURES, fetchDir)), + }); +} + +// run drives the real router (parsing → consent → handler → CoreClient → +// Aperture fetch) against the fixture-backed clients and returns captured IO. +async function run( + args: string[], + opts: { fetchDir?: string; io?: TestIOOptions } = {}, +): Promise<{ stdout: string; stderr: string }> { + const io = testIO(opts.io); + const root = createRootHandler(createFixtureCore(opts.fetchDir ?? "unused"), { + io: io.io, + logger: createSilentLogger(), + globalConfigAccessor: new TestGlobalConfigAccessor(), + }); + await root.route(["node", "agentcore", "feedback", ...args, "--region", REGION]); + return { stdout: io.stdout(), stderr: io.stderr() }; +} + +describe("feedback (fixture-backed)", () => { + test("submits text-only feedback and prints the result envelope", async () => { + const { stdout } = await run( + ["[agentcore-cli golden fixture] text submit — please ignore", "--yes", "--json"], + { fetchDir: "submit-text" }, + ); + + matchGolden(FIXTURES, "submit-text.golden.json", stdout); + const result = JSON.parse(stdout); + expect(result.success).toBe(true); + expect(typeof result.id).toBe("string"); + expect(result.reference).toBe("agentcore-cli"); + }, 120_000); + + test("submits feedback with a screenshot (presign → S3 PUT → form)", async () => { + const { stdout } = await run( + [ + "[agentcore-cli golden fixture] screenshot submit — please ignore", + "--screenshot", + SHOT, + "--yes", + "--json", + ], + { fetchDir: "submit-screenshot" }, + ); + + matchGolden(FIXTURES, "submit-screenshot.golden.json", stdout); + expect(JSON.parse(stdout).success).toBe(true); + }, 120_000); + + // ── validation / consent errors (no network, no fixtures — like batch-evaluation's not-found) ── + + test("without --yes and without a TTY it fails rather than submitting", async () => { + await expect(run(["headless", "--json"])).rejects.toThrow(/--yes/); + }); + + test("declining the consent prompt cancels", async () => { + await expect(run(["no thanks"], { io: { isTTY: true, stdin: "n\n" } })).rejects.toBeInstanceOf( + UserCancellationError, + ); + }); + + test("an empty message is rejected", async () => { + await expect(run([" ", "--yes"])).rejects.toThrow(/cannot be empty/); + }); + + test("a message over 1000 characters is rejected", async () => { + await expect(run(["x".repeat(1001), "--yes"])).rejects.toThrow(/1000 characters/); + }); + + test("an explicitly-empty --screenshot is rejected", async () => { + await expect(run(["msg", "--screenshot", "", "--yes"])).rejects.toThrow( + /--screenshot requires a file path/, + ); + }); +}); diff --git a/src/handlers/feedback/feedback.test.tsx b/src/handlers/feedback/feedback.test.tsx deleted file mode 100644 index 144785a65..000000000 --- a/src/handlers/feedback/feedback.test.tsx +++ /dev/null @@ -1,156 +0,0 @@ -import { test, expect, describe } from "bun:test"; -import { CoreClient } from "../../core"; -import { createRootHandler } from "../index"; -import { - createSilentLogger, - fixtureFactories, - TestGlobalConfigAccessor, - testIO, - type TestIOOptions, -} from "../../testing"; -import { InputValidationError, UserCancellationError } from "../../errors"; -import { join } from "node:path"; -import { mkdtemp, writeFile } from "node:fs/promises"; -import { tmpdir } from "node:os"; - -// Command-flow tests for `agentcore feedback`. The Aperture POST/PUT calls go -// through an injected `fetch` stub (feedback is the one Core path outside the SDK -// seam), so these run offline and assert the exact HTTP the client makes. - -const REGION = "us-east-1"; -const FIXTURES = join(import.meta.dir, "__fixtures__"); - -interface Recorded { - url: string; - method: string; - headers: Record; - body: unknown; -} - -// stubFetch answers the three Aperture calls (presign POST, S3 PUT, form POST) -// and records each so tests can assert the request shape. -function stubFetch(calls: Recorded[]) { - const presignedUrl = - "https://aperture-bucket.s3.us-east-1.amazonaws.com/us-east-1/AgentCore/CLI/0.1.0/13052026/abc-123.png?X-Amz-Signature=sig"; - return (async (input: Parameters[0], init?: Parameters[1]) => { - const url = String(input); - calls.push({ - url, - method: init?.method ?? "GET", - headers: (init?.headers as Record) ?? {}, - body: init?.body, - }); - if (url.includes("/presignedurl")) { - return new Response(presignedUrl, { status: 200 }); - } - if (url.includes("/form")) { - return new Response( - JSON.stringify({ - id: "11111111-2222-3333-4444-555555555555", - timestamp: "2026-08-31T00:00:00Z", - reference: "agentcore-cli", - }), - { status: 200, headers: { "content-type": "application/json" } }, - ); - } - return new Response(null, { status: 200 }); // the S3 PUT - }) as unknown as typeof fetch; -} - -async function run( - args: string[], - opts: { io?: TestIOOptions; calls?: Recorded[] } = {}, -): Promise<{ stdout: string; stderr: string }> { - const factories = fixtureFactories(FIXTURES); - const core = new CoreClient({ - ...factories, - logger: createSilentLogger(), - fetch: stubFetch(opts.calls ?? []), - }); - const io = testIO(opts.io); - const root = createRootHandler(core, { - io: io.io, - logger: createSilentLogger(), - globalConfigAccessor: new TestGlobalConfigAccessor(), - }); - await root.route(["node", "agentcore", "feedback", ...args, "--region", REGION]); - return { stdout: io.stdout(), stderr: io.stderr() }; -} - -describe("feedback", () => { - test("--yes --json submits and prints the result envelope", async () => { - const calls: Recorded[] = []; - const { stdout } = await run(["great tool", "--yes", "--json"], { calls }); - const parsed = JSON.parse(stdout); - expect(parsed.success).toBe(true); - expect(parsed.id).toBe("11111111-2222-3333-4444-555555555555"); - // Text-only: exactly one call, the form POST. - expect(calls).toHaveLength(1); - expect(calls[0]!.url).toContain("/form"); - expect(calls[0]!.method).toBe("POST"); - }); - - test("prompts on a TTY and submits on 'y'", async () => { - const calls: Recorded[] = []; - const { stderr } = await run(["nice cli"], { io: { isTTY: true, stdin: "y\n" }, calls }); - expect(stderr).toContain("AWS Customer Agreement"); - expect(stderr).toContain("Submit feedback? (y/N)"); - expect(calls).toHaveLength(1); - }); - - test("declining the prompt cancels without submitting", async () => { - const calls: Recorded[] = []; - await expect( - run(["nope"], { io: { isTTY: true, stdin: "n\n" }, calls }), - ).rejects.toBeInstanceOf(UserCancellationError); - expect(calls).toHaveLength(0); - }); - - test("non-interactive without --yes fails and does not submit", async () => { - const calls: Recorded[] = []; - const removal = run(["headless"], { calls }); - await expect(removal).rejects.toBeInstanceOf(InputValidationError); - await expect(run(["headless"], { calls: [] })).rejects.toThrow(/--yes/); - expect(calls).toHaveLength(0); - }); - - test("an empty message is rejected before any network call", async () => { - const calls: Recorded[] = []; - await expect(run([" ", "--yes"], { calls })).rejects.toThrow(/cannot be empty/); - expect(calls).toHaveLength(0); - }); - - test("a message over 1000 chars is rejected", async () => { - const calls: Recorded[] = []; - await expect(run(["x".repeat(1001), "--yes"], { calls })).rejects.toThrow(/1000 characters/); - expect(calls).toHaveLength(0); - }); - - test("a screenshot drives presign -> S3 PUT (checksum + tag) -> form POST", async () => { - const dir = await mkdtemp(join(tmpdir(), "agentcore-fb-")); - const shot = join(dir, "shot.png"); - await writeFile(shot, Buffer.from("iVBORw0KGgoAAAANSUhEUgAA", "base64")); - - const calls: Recorded[] = []; - const { stdout } = await run(["with shot", "--screenshot", shot, "--yes", "--json"], { calls }); - expect(JSON.parse(stdout).success).toBe(true); - - expect( - calls.map( - (c) => - `${c.method} ${c.url.includes("/presignedurl") ? "presign" : c.url.includes("/form") ? "form" : "s3"}`, - ), - ).toEqual(["POST presign", "PUT s3", "POST form"]); - const put = calls[1]!; - expect(put.headers["x-amz-checksum-algorithm"]).toBe("SHA256"); - expect(put.headers["x-amz-tagging"]).toBe("scanstatus=NOT_SCANNED"); - // The form references the exact object key parsed from the presigned URL path. - const form = JSON.parse(String(calls[2]!.body)); - const attachment = form.customerResponses.find( - (r: { response: { responseType: string } }) => r.response.responseType === "fileUpload", - ); - expect(attachment.response.responseValue).toEqual([ - "us-east-1/AgentCore/CLI/0.1.0/13052026/abc-123.png", - ]); - }); -}); From c25a04ca68922888def1460e443d4c01ecf9f32b Mon Sep 17 00:00:00 2001 From: jariy17 Date: Mon, 31 Aug 2026 20:14:59 +0000 Subject: [PATCH 9/9] test(feedback): add feedback to the root command-tree assertion The feedback command was registered on the root handler in PR #2149 but root.test.tsx's expected subcommand list was not updated, so 'builds the agentcore command tree with its subcommands' failed in CI. Add 'feedback' in its registration position (after eval). --- src/handlers/root.test.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/src/handlers/root.test.tsx b/src/handlers/root.test.tsx index b3f4e3386..63f5dfc1c 100644 --- a/src/handlers/root.test.tsx +++ b/src/handlers/root.test.tsx @@ -17,6 +17,7 @@ describe("createRootHandler", () => { "memory", "gateway", "eval", + "feedback", "config", "project", ]);