feat: add agentcore feedback command - #1321

Merged
tejaskash merged 7 commits into
mainfrom
feature/feedback-command
May 21, 2026
Merged

feat: add agentcore feedback command#1321
tejaskash merged 7 commits into
mainfrom
feature/feedback-command

Conversation

@tejaskash

Copy link
Copy Markdown
Contributor

Summary

Adds a new top-level agentcore feedback command:

  • One-shot:agentcore feedback "msg" [--screenshot path] [--json]
  • Wizard:agentcore feedback (no args) → multi-step Ink TUI

Both modes display the AWS Customer Agreement and require interactive y/N consent before submitting. Bare Enter defaults to No; non-TTY stdin is refused so the consent decision is always explicit.

The wizard walks Message → optional Screenshot → Consent → Submit, with Esc stepping back and message text preserved across declines/retries. Screenshots are validated locally (PNG/JPG/JPEG, ≤100MB) before any network call, then uploaded via presigned URL with SHA-256 checksum headers.

Submissions carry cli-version, os, node-version, and cli-mode metadata. The CLI does not attach AWS account IDs, credentials, project names, or telemetry IDs.

What's in the diff

  • src/cli/commands/feedback/ — Commander registration, action, TTY consent prompt
  • src/cli/operations/feedback/ — submission orchestrator, payload builder, presigned-URL + S3 upload + form-ingestion client
  • src/cli/tui/screens/feedback/ — Ink wizard with useFeedbackFlow hook
  • src/cli/cli.ts, src/cli/tui/copy.ts — wiring
  • README.md, docs/commands.md, docs/feedback.md — customer-facing docs

29 unit tests across operations, command, consent-prompt, and TUI hook.

Test plan

  • npm test -- feedback — 29/29 pass
  • eslint clean on all new files
  • tsc --noEmit clean on all new files
  • Manual prod submission verified end-to-end (message + screenshot, both submit and decline paths)
  • CLI consent prompt: y/yes/Y/YES accepts; n/no/bare-Enter declines; non-TTY refuses
  • TUI wizard: all phase transitions, Esc back-stepping, message preservation across consent decline
  • Plain mode and --json mode for success, declined, no-tty, validation, network error
  • Bug bash plan attached for testers (5 testers × 6 min)

@github-actionsgithub-actionsBot added agentcore-harness-reviewing AgentCore Harness review in progress size/xl PR size: XL labels May 20, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
@github-actions

github-actionsBot commented May 20, 2026

Copy link
Copy Markdown
Contributor

Package Tarball

aws-agentcore-0.14.1.tgz

How to install

gh release download pr-1321-tarball --repo aws/agentcore-cli --pattern "*.tgz" --dir /tmp/pr-tarball
npm install -g /tmp/pr-tarball/aws-agentcore-0.14.1.tgz

@tejaskash
tejaskash marked this pull request as ready for review May 20, 2026 17:23
@tejaskash
tejaskash requested a review from a teamMay 20, 2026 17:23
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

Claude Security Review: no high-confidence findings. (run)

@agentcore-cli-automationagentcore-cli-automation 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.

Thanks for the careful work on consent flow, validation, and docs — the wizard structure and cli/tui mode tagging are nicely done. A few things I'd like to see addressed before merging:

  1. Missing telemetry instrumentation. Per src/cli/telemetry/README.md, every CLI command should emit a command_run metric, but feedback doesn't. Other recent commands (e.g. deploy, remove, create) all use withCommandRunTelemetry / runCliCommand.

  2. Screenshot S3 key is fabricated client-side independent of the presigned URL. The form references a key the client invented rather than the actual upload location.

  3. TUI validates message and screenshot only after consent. Bad input forces the user to redo consent.

Inline comments below.

Comment threadsrc/cli/commands/feedback/command.tsx
Comment threadsrc/cli/operations/feedback/submit-feedback.ts Outdated
Comment threadsrc/cli/tui/screens/feedback/useFeedbackFlow.ts Outdated
@agentcore-devx-automationagentcore-devx-automationBot removed the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
@github-actionsgithub-actionsBot removed the agentcore-harness-reviewing AgentCore Harness review in progress label May 20, 2026
@github-actions

github-actionsBot commented May 20, 2026

Copy link
Copy Markdown
Contributor

Coverage Report

StatusCategoryPercentageCovered / Total
🔵Lines44.3%9531 / 21510
🔵Statements43.55%10114 / 23223
🔵Functions41.01%1655 / 4035
🔵Branches40.84%6162 / 15086
Generated in workflow #3182 for commit 91cb5da by the Vitest Coverage Report Action

@github-actionsgithub-actionsBot added size/xl PR size: XL and removed size/xl PR size: XL labels May 20, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

Claude Security Review: the review run failed before completing. See the run for details.

@agentcore-devx-automationagentcore-devx-automationBot removed the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
@tejaskash
tejaskashforce-pushed the feature/feedback-command branch from ab65255 to 94f3a55CompareMay 20, 2026 17:55
@github-actionsgithub-actionsBot added size/xl PR size: XL and removed size/xl PR size: XL labels May 20, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

Claude Security Review: no high-confidence findings. (run)

@agentcore-devx-automationagentcore-devx-automationBot removed the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
Hweinstock
Hweinstock previously approved these changes May 20, 2026

@HweinstockHweinstock left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

implementation looks good, some questions on the testing, but we can revisit as a follow-up to bug bash today.

Comment threaddocs/feedback.md
Comment threadsrc/cli/commands/feedback/__tests__/command.test.ts
Comment threadsrc/cli/commands/feedback/action.ts Outdated
Comment threadsrc/cli/tui/screens/feedback/__tests__/useFeedbackFlow.test.tsx Outdated
Comment threadsrc/cli/tui/screens/feedback/useFeedbackFlow.ts Outdated
Adds a new top-level `agentcore feedback` command that lets users send
feedback (and an optional screenshot) directly from the CLI:
agentcore feedback "your message" [--screenshot path] [--json]
agentcore feedback # multi-step wizard
Both modes display the AWS Customer Agreement and require interactive
y/N consent before submitting. Bare Enter defaults to No, and non-TTY
stdin is refused so the consent decision is always explicit.
The wizard walks through Message → optional Screenshot → Consent →
Submit, with Esc stepping back and message text preserved across
declines/retries. Screenshots are validated locally (PNG/JPG/JPEG,
≤100MB) before any network call, then uploaded via presigned URL with
SHA-256 checksum headers.
Submissions are tagged with cli-version, os, node-version, and cli-mode
metadata to help triage. The CLI does not attach AWS account IDs,
credentials, project names, or telemetry IDs.
Includes README + docs/commands.md updates and a new docs/feedback.md
covering syntax, consent text, and what not to include.
Three changes from automated review:
1. Telemetry: register `feedback` in command-run schema with `mode` +
`has_screenshot` attrs, instrument the CLI handler with
client.withCommandRun (CANCELLED on declined consent), and the TUI
submit path with withCommandRunTelemetry.
2. Screenshot S3 key: parse the actual object key from the presigned
URL path instead of fabricating one client-side. The fabricated key
could drift from Aperture's bucket layout (UTC/local date, prefix
changes) and silently produce form references to non-existent
objects.
3. TUI input validation: validate message in setMessage and screenshot
in setScreenshot, surface the error inline on the input phase via
state.inputError. Previously bad input was only caught after the
user walked through consent, forcing them to re-do consent on retry.
Exposes validateFeedbackMessage and validateScreenshotPath from the
operations module so the hook can reuse the same validators as the
submission orchestrator.
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

Claude Security Review: the review run failed before completing. See the run for details.

Three issues from bug bash:
- 2.5: Esc/Ctrl+C in the wizard didn't terminate the process. The
onExit callback unmounted the screen but Ink's stdin raw-mode
listeners kept Node alive. Fix: await render()'s waitUntilExit()
and call process.exit(0) once the wizard unmounts. Matches
invoke/command.tsx's pattern.
- 3.2: Pointing --screenshot at a directory ("/tmp") produced
"Screenshot must be one of: .png, .jpg, .jpeg" because we extension-
checked first. Fix: stat the path first; surface a directory-specific
error before the extension check.
- 3.6: Quoted paths starting with "~/" (e.g. ~/Desktop/foo.png) hit
ENOENT because Node's fs APIs don't expand tildes — only the shell
does, and quoting suppresses that. Fix: expand a leading "~" or
"~/..." to os.homedir() before stat/read.
Added unit tests for both new validator branches.
@github-actionsgithub-actionsBot added size/xl PR size: XL and removed size/xl PR size: XL labels May 20, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

Claude Security Review: no high-confidence findings. (run)

@agentcore-devx-automationagentcore-devx-automationBot removed the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
…onal
Per tester feedback: the explicit Y/N prompt felt like an extra step for
something that's already optional. Drop the screenshot-prompt phase
entirely — the wizard now goes Message → Screenshot → Consent.
The Screenshot phase is the path picker itself, with allowEmpty=true on
PathInput. Submit with a path to attach; Enter on empty input or Esc to
skip. Help text reflects this: "Enter select / skip · Esc skip".
Verified end-to-end via TUI harness: live submission id
96e2874e-3a61-4f65-8140-c144393d841e through the new flow.
@github-actionsgithub-actionsBot removed the size/xl PR size: XL label May 20, 2026
@github-actionsgithub-actionsBot added the size/xl PR size: XL label May 20, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

Claude Security Review: no high-confidence findings. (run)

@agentcore-devx-automationagentcore-devx-automationBot removed the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
Hweinstock
Hweinstock previously approved these changes May 20, 2026

@HweinstockHweinstock left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM! very minor nits and a question.

Comment threadsrc/cli/commands/feedback/__tests__/consent-prompt.test.ts
Comment threadsrc/cli/commands/feedback/action.ts Outdated
Comment threadsrc/cli/operations/feedback/aperture-client.ts
Comment threadsrc/cli/operations/feedback/aperture-client.ts
Comment threadsrc/cli/operations/feedback/submit-feedback.ts Outdated
Per Hweinstock's review:
- handleFeedback / useFeedbackFlow: replace the local
`err instanceof Error ? err : new Error(String(err))` pattern with
`toError(err)` from src/lib/errors/types.ts.
- FeedbackValidationError: extend `ValidationError` from
src/lib/errors/types.ts so the shared error classification helpers
treat it as a user error, which means telemetry's
classifyError(err)/isUserError(err) will set error_source: 'user'
on failure metrics for invalid screenshots / empty messages.
@github-actionsgithub-actionsBot added size/xl PR size: XL and removed size/xl PR size: XL labels May 20, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

Claude Security Review: the review run failed before completing. See the run for details.

@agentcore-devx-automationagentcore-devx-automationBot removed the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026

@HweinstockHweinstock left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Awesome!

@tejaskash
tejaskash merged commit 32c475d into mainMay 21, 2026
32 of 33 checks passed
@tejaskash
tejaskash deleted the feature/feedback-command branch May 21, 2026 13:35
tejaskash added a commit that referenced this pull request May 21, 2026
…works (#1327)
The /security-review slash command runs `git diff origin/HEAD...` as its
first action to enumerate the PR's changes. actions/checkout doesn't set
up the remote's symbolic HEAD ref, so that command fails with
"ambiguous argument 'origin/HEAD...': unknown revision". Claude then
loops trying variants until --max-turns 30 trips and the action exits 1.
Set origin/HEAD to the PR's base ref right after checkout so the skill's
git invocations resolve correctly. Run #26186045056 on PR #1321 was the
trigger - 30 turns spent on shell-error recovery, no findings posted.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/xlPR size: XL

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@tejaskash@Hweinstock@agentcore-cli-automation
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

feat: add agentcore feedback command - #1321

Merged
tejaskash merged 7 commits into
mainfrom
feature/feedback-command
May 21, 2026
Merged

feat: add agentcore feedback command#1321
tejaskash merged 7 commits into
mainfrom
feature/feedback-command

Conversation

@tejaskash

Copy link
Copy Markdown
Contributor

Summary

Adds a new top-level agentcore feedback command:

  • One-shot:agentcore feedback "msg" [--screenshot path] [--json]
  • Wizard:agentcore feedback (no args) → multi-step Ink TUI

Both modes display the AWS Customer Agreement and require interactive y/N consent before submitting. Bare Enter defaults to No; non-TTY stdin is refused so the consent decision is always explicit.

The wizard walks Message → optional Screenshot → Consent → Submit, with Esc stepping back and message text preserved across declines/retries. Screenshots are validated locally (PNG/JPG/JPEG, ≤100MB) before any network call, then uploaded via presigned URL with SHA-256 checksum headers.

Submissions carry cli-version, os, node-version, and cli-mode metadata. The CLI does not attach AWS account IDs, credentials, project names, or telemetry IDs.

What's in the diff

  • src/cli/commands/feedback/ — Commander registration, action, TTY consent prompt
  • src/cli/operations/feedback/ — submission orchestrator, payload builder, presigned-URL + S3 upload + form-ingestion client
  • src/cli/tui/screens/feedback/ — Ink wizard with useFeedbackFlow hook
  • src/cli/cli.ts, src/cli/tui/copy.ts — wiring
  • README.md, docs/commands.md, docs/feedback.md — customer-facing docs

29 unit tests across operations, command, consent-prompt, and TUI hook.

Test plan

  • npm test -- feedback — 29/29 pass
  • eslint clean on all new files
  • tsc --noEmit clean on all new files
  • Manual prod submission verified end-to-end (message + screenshot, both submit and decline paths)
  • CLI consent prompt: y/yes/Y/YES accepts; n/no/bare-Enter declines; non-TTY refuses
  • TUI wizard: all phase transitions, Esc back-stepping, message preservation across consent decline
  • Plain mode and --json mode for success, declined, no-tty, validation, network error
  • Bug bash plan attached for testers (5 testers × 6 min)

@github-actionsgithub-actionsBot added agentcore-harness-reviewing AgentCore Harness review in progress size/xl PR size: XL labels May 20, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
@github-actions

github-actionsBot commented May 20, 2026

Copy link
Copy Markdown
Contributor

Package Tarball

aws-agentcore-0.14.1.tgz

How to install

gh release download pr-1321-tarball --repo aws/agentcore-cli --pattern "*.tgz" --dir /tmp/pr-tarball
npm install -g /tmp/pr-tarball/aws-agentcore-0.14.1.tgz

@tejaskash
tejaskash marked this pull request as ready for review May 20, 2026 17:23
@tejaskash
tejaskash requested a review from a teamMay 20, 2026 17:23
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

Claude Security Review: no high-confidence findings. (run)

@agentcore-cli-automationagentcore-cli-automation 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.

Thanks for the careful work on consent flow, validation, and docs — the wizard structure and cli/tui mode tagging are nicely done. A few things I'd like to see addressed before merging:

  1. Missing telemetry instrumentation. Per src/cli/telemetry/README.md, every CLI command should emit a command_run metric, but feedback doesn't. Other recent commands (e.g. deploy, remove, create) all use withCommandRunTelemetry / runCliCommand.

  2. Screenshot S3 key is fabricated client-side independent of the presigned URL. The form references a key the client invented rather than the actual upload location.

  3. TUI validates message and screenshot only after consent. Bad input forces the user to redo consent.

Inline comments below.

Comment threadsrc/cli/commands/feedback/command.tsx
Comment threadsrc/cli/operations/feedback/submit-feedback.ts Outdated
Comment threadsrc/cli/tui/screens/feedback/useFeedbackFlow.ts Outdated
@agentcore-devx-automationagentcore-devx-automationBot removed the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
@github-actionsgithub-actionsBot removed the agentcore-harness-reviewing AgentCore Harness review in progress label May 20, 2026
@github-actions

github-actionsBot commented May 20, 2026

Copy link
Copy Markdown
Contributor

Coverage Report

StatusCategoryPercentageCovered / Total
🔵Lines44.3%9531 / 21510
🔵Statements43.55%10114 / 23223
🔵Functions41.01%1655 / 4035
🔵Branches40.84%6162 / 15086
Generated in workflow #3182 for commit 91cb5da by the Vitest Coverage Report Action

@github-actionsgithub-actionsBot added size/xl PR size: XL and removed size/xl PR size: XL labels May 20, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

Claude Security Review: the review run failed before completing. See the run for details.

@agentcore-devx-automationagentcore-devx-automationBot removed the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
@tejaskash
tejaskashforce-pushed the feature/feedback-command branch from ab65255 to 94f3a55CompareMay 20, 2026 17:55
@github-actionsgithub-actionsBot added size/xl PR size: XL and removed size/xl PR size: XL labels May 20, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

Claude Security Review: no high-confidence findings. (run)

@agentcore-devx-automationagentcore-devx-automationBot removed the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
Hweinstock
Hweinstock previously approved these changes May 20, 2026

@HweinstockHweinstock left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

implementation looks good, some questions on the testing, but we can revisit as a follow-up to bug bash today.

Comment threaddocs/feedback.md
Comment threadsrc/cli/commands/feedback/__tests__/command.test.ts
Comment threadsrc/cli/commands/feedback/action.ts Outdated
Comment threadsrc/cli/tui/screens/feedback/__tests__/useFeedbackFlow.test.tsx Outdated
Comment threadsrc/cli/tui/screens/feedback/useFeedbackFlow.ts Outdated
Adds a new top-level `agentcore feedback` command that lets users send
feedback (and an optional screenshot) directly from the CLI:
agentcore feedback "your message" [--screenshot path] [--json]
agentcore feedback # multi-step wizard
Both modes display the AWS Customer Agreement and require interactive
y/N consent before submitting. Bare Enter defaults to No, and non-TTY
stdin is refused so the consent decision is always explicit.
The wizard walks through Message → optional Screenshot → Consent →
Submit, with Esc stepping back and message text preserved across
declines/retries. Screenshots are validated locally (PNG/JPG/JPEG,
≤100MB) before any network call, then uploaded via presigned URL with
SHA-256 checksum headers.
Submissions are tagged with cli-version, os, node-version, and cli-mode
metadata to help triage. The CLI does not attach AWS account IDs,
credentials, project names, or telemetry IDs.
Includes README + docs/commands.md updates and a new docs/feedback.md
covering syntax, consent text, and what not to include.
Three changes from automated review:
1. Telemetry: register `feedback` in command-run schema with `mode` +
`has_screenshot` attrs, instrument the CLI handler with
client.withCommandRun (CANCELLED on declined consent), and the TUI
submit path with withCommandRunTelemetry.
2. Screenshot S3 key: parse the actual object key from the presigned
URL path instead of fabricating one client-side. The fabricated key
could drift from Aperture's bucket layout (UTC/local date, prefix
changes) and silently produce form references to non-existent
objects.
3. TUI input validation: validate message in setMessage and screenshot
in setScreenshot, surface the error inline on the input phase via
state.inputError. Previously bad input was only caught after the
user walked through consent, forcing them to re-do consent on retry.
Exposes validateFeedbackMessage and validateScreenshotPath from the
operations module so the hook can reuse the same validators as the
submission orchestrator.
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

Claude Security Review: the review run failed before completing. See the run for details.

Three issues from bug bash:
- 2.5: Esc/Ctrl+C in the wizard didn't terminate the process. The
onExit callback unmounted the screen but Ink's stdin raw-mode
listeners kept Node alive. Fix: await render()'s waitUntilExit()
and call process.exit(0) once the wizard unmounts. Matches
invoke/command.tsx's pattern.
- 3.2: Pointing --screenshot at a directory ("/tmp") produced
"Screenshot must be one of: .png, .jpg, .jpeg" because we extension-
checked first. Fix: stat the path first; surface a directory-specific
error before the extension check.
- 3.6: Quoted paths starting with "~/" (e.g. ~/Desktop/foo.png) hit
ENOENT because Node's fs APIs don't expand tildes — only the shell
does, and quoting suppresses that. Fix: expand a leading "~" or
"~/..." to os.homedir() before stat/read.
Added unit tests for both new validator branches.
@github-actionsgithub-actionsBot added size/xl PR size: XL and removed size/xl PR size: XL labels May 20, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

Claude Security Review: no high-confidence findings. (run)

@agentcore-devx-automationagentcore-devx-automationBot removed the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
…onal
Per tester feedback: the explicit Y/N prompt felt like an extra step for
something that's already optional. Drop the screenshot-prompt phase
entirely — the wizard now goes Message → Screenshot → Consent.
The Screenshot phase is the path picker itself, with allowEmpty=true on
PathInput. Submit with a path to attach; Enter on empty input or Esc to
skip. Help text reflects this: "Enter select / skip · Esc skip".
Verified end-to-end via TUI harness: live submission id
96e2874e-3a61-4f65-8140-c144393d841e through the new flow.
@github-actionsgithub-actionsBot removed the size/xl PR size: XL label May 20, 2026
@github-actionsgithub-actionsBot added the size/xl PR size: XL label May 20, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

Claude Security Review: no high-confidence findings. (run)

@agentcore-devx-automationagentcore-devx-automationBot removed the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
Hweinstock
Hweinstock previously approved these changes May 20, 2026

@HweinstockHweinstock left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM! very minor nits and a question.

Comment threadsrc/cli/commands/feedback/__tests__/consent-prompt.test.ts
Comment threadsrc/cli/commands/feedback/action.ts Outdated
Comment threadsrc/cli/operations/feedback/aperture-client.ts
Comment threadsrc/cli/operations/feedback/aperture-client.ts
Comment threadsrc/cli/operations/feedback/submit-feedback.ts Outdated
Per Hweinstock's review:
- handleFeedback / useFeedbackFlow: replace the local
`err instanceof Error ? err : new Error(String(err))` pattern with
`toError(err)` from src/lib/errors/types.ts.
- FeedbackValidationError: extend `ValidationError` from
src/lib/errors/types.ts so the shared error classification helpers
treat it as a user error, which means telemetry's
classifyError(err)/isUserError(err) will set error_source: 'user'
on failure metrics for invalid screenshots / empty messages.
@github-actionsgithub-actionsBot added size/xl PR size: XL and removed size/xl PR size: XL labels May 20, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

Claude Security Review: the review run failed before completing. See the run for details.

@agentcore-devx-automationagentcore-devx-automationBot removed the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026

@HweinstockHweinstock left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Awesome!

@tejaskash
tejaskash merged commit 32c475d into mainMay 21, 2026
32 of 33 checks passed
@tejaskash
tejaskash deleted the feature/feedback-command branch May 21, 2026 13:35
tejaskash added a commit that referenced this pull request May 21, 2026
…works (#1327)
The /security-review slash command runs `git diff origin/HEAD...` as its
first action to enumerate the PR's changes. actions/checkout doesn't set
up the remote's symbolic HEAD ref, so that command fails with
"ambiguous argument 'origin/HEAD...': unknown revision". Claude then
loops trying variants until --max-turns 30 trips and the action exits 1.
Set origin/HEAD to the PR's base ref right after checkout so the skill's
git invocations resolve correctly. Run #26186045056 on PR #1321 was the
trigger - 30 turns spent on shell-error recovery, no findings posted.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/xlPR size: XL

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@tejaskash@Hweinstock@agentcore-cli-automation
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

feat: add agentcore feedback command - #1321

Merged
tejaskash merged 7 commits into
mainfrom
feature/feedback-command
May 21, 2026
Merged

feat: add agentcore feedback command#1321
tejaskash merged 7 commits into
mainfrom
feature/feedback-command

Conversation

@tejaskash

Copy link
Copy Markdown
Contributor

Summary

Adds a new top-level agentcore feedback command:

  • One-shot:agentcore feedback "msg" [--screenshot path] [--json]
  • Wizard:agentcore feedback (no args) → multi-step Ink TUI

Both modes display the AWS Customer Agreement and require interactive y/N consent before submitting. Bare Enter defaults to No; non-TTY stdin is refused so the consent decision is always explicit.

The wizard walks Message → optional Screenshot → Consent → Submit, with Esc stepping back and message text preserved across declines/retries. Screenshots are validated locally (PNG/JPG/JPEG, ≤100MB) before any network call, then uploaded via presigned URL with SHA-256 checksum headers.

Submissions carry cli-version, os, node-version, and cli-mode metadata. The CLI does not attach AWS account IDs, credentials, project names, or telemetry IDs.

What's in the diff

  • src/cli/commands/feedback/ — Commander registration, action, TTY consent prompt
  • src/cli/operations/feedback/ — submission orchestrator, payload builder, presigned-URL + S3 upload + form-ingestion client
  • src/cli/tui/screens/feedback/ — Ink wizard with useFeedbackFlow hook
  • src/cli/cli.ts, src/cli/tui/copy.ts — wiring
  • README.md, docs/commands.md, docs/feedback.md — customer-facing docs

29 unit tests across operations, command, consent-prompt, and TUI hook.

Test plan

  • npm test -- feedback — 29/29 pass
  • eslint clean on all new files
  • tsc --noEmit clean on all new files
  • Manual prod submission verified end-to-end (message + screenshot, both submit and decline paths)
  • CLI consent prompt: y/yes/Y/YES accepts; n/no/bare-Enter declines; non-TTY refuses
  • TUI wizard: all phase transitions, Esc back-stepping, message preservation across consent decline
  • Plain mode and --json mode for success, declined, no-tty, validation, network error
  • Bug bash plan attached for testers (5 testers × 6 min)

@github-actionsgithub-actionsBot added agentcore-harness-reviewing AgentCore Harness review in progress size/xl PR size: XL labels May 20, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
@github-actions

github-actionsBot commented May 20, 2026

Copy link
Copy Markdown
Contributor

Package Tarball

aws-agentcore-0.14.1.tgz

How to install

gh release download pr-1321-tarball --repo aws/agentcore-cli --pattern "*.tgz" --dir /tmp/pr-tarball
npm install -g /tmp/pr-tarball/aws-agentcore-0.14.1.tgz

@tejaskash
tejaskash marked this pull request as ready for review May 20, 2026 17:23
@tejaskash
tejaskash requested a review from a teamMay 20, 2026 17:23
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

Claude Security Review: no high-confidence findings. (run)

@agentcore-cli-automationagentcore-cli-automation 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.

Thanks for the careful work on consent flow, validation, and docs — the wizard structure and cli/tui mode tagging are nicely done. A few things I'd like to see addressed before merging:

  1. Missing telemetry instrumentation. Per src/cli/telemetry/README.md, every CLI command should emit a command_run metric, but feedback doesn't. Other recent commands (e.g. deploy, remove, create) all use withCommandRunTelemetry / runCliCommand.

  2. Screenshot S3 key is fabricated client-side independent of the presigned URL. The form references a key the client invented rather than the actual upload location.

  3. TUI validates message and screenshot only after consent. Bad input forces the user to redo consent.

Inline comments below.

Comment threadsrc/cli/commands/feedback/command.tsx
Comment threadsrc/cli/operations/feedback/submit-feedback.ts Outdated
Comment threadsrc/cli/tui/screens/feedback/useFeedbackFlow.ts Outdated
@agentcore-devx-automationagentcore-devx-automationBot removed the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
@github-actionsgithub-actionsBot removed the agentcore-harness-reviewing AgentCore Harness review in progress label May 20, 2026
@github-actions

github-actionsBot commented May 20, 2026

Copy link
Copy Markdown
Contributor

Coverage Report

StatusCategoryPercentageCovered / Total
🔵Lines44.3%9531 / 21510
🔵Statements43.55%10114 / 23223
🔵Functions41.01%1655 / 4035
🔵Branches40.84%6162 / 15086
Generated in workflow #3182 for commit 91cb5da by the Vitest Coverage Report Action

@github-actionsgithub-actionsBot added size/xl PR size: XL and removed size/xl PR size: XL labels May 20, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

Claude Security Review: the review run failed before completing. See the run for details.

@agentcore-devx-automationagentcore-devx-automationBot removed the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
@tejaskash
tejaskashforce-pushed the feature/feedback-command branch from ab65255 to 94f3a55CompareMay 20, 2026 17:55
@github-actionsgithub-actionsBot added size/xl PR size: XL and removed size/xl PR size: XL labels May 20, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

Claude Security Review: no high-confidence findings. (run)

@agentcore-devx-automationagentcore-devx-automationBot removed the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
Hweinstock
Hweinstock previously approved these changes May 20, 2026

@HweinstockHweinstock left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

implementation looks good, some questions on the testing, but we can revisit as a follow-up to bug bash today.

Comment threaddocs/feedback.md
Comment threadsrc/cli/commands/feedback/__tests__/command.test.ts
Comment threadsrc/cli/commands/feedback/action.ts Outdated
Comment threadsrc/cli/tui/screens/feedback/__tests__/useFeedbackFlow.test.tsx Outdated
Comment threadsrc/cli/tui/screens/feedback/useFeedbackFlow.ts Outdated
Adds a new top-level `agentcore feedback` command that lets users send
feedback (and an optional screenshot) directly from the CLI:
agentcore feedback "your message" [--screenshot path] [--json]
agentcore feedback # multi-step wizard
Both modes display the AWS Customer Agreement and require interactive
y/N consent before submitting. Bare Enter defaults to No, and non-TTY
stdin is refused so the consent decision is always explicit.
The wizard walks through Message → optional Screenshot → Consent →
Submit, with Esc stepping back and message text preserved across
declines/retries. Screenshots are validated locally (PNG/JPG/JPEG,
≤100MB) before any network call, then uploaded via presigned URL with
SHA-256 checksum headers.
Submissions are tagged with cli-version, os, node-version, and cli-mode
metadata to help triage. The CLI does not attach AWS account IDs,
credentials, project names, or telemetry IDs.
Includes README + docs/commands.md updates and a new docs/feedback.md
covering syntax, consent text, and what not to include.
Three changes from automated review:
1. Telemetry: register `feedback` in command-run schema with `mode` +
`has_screenshot` attrs, instrument the CLI handler with
client.withCommandRun (CANCELLED on declined consent), and the TUI
submit path with withCommandRunTelemetry.
2. Screenshot S3 key: parse the actual object key from the presigned
URL path instead of fabricating one client-side. The fabricated key
could drift from Aperture's bucket layout (UTC/local date, prefix
changes) and silently produce form references to non-existent
objects.
3. TUI input validation: validate message in setMessage and screenshot
in setScreenshot, surface the error inline on the input phase via
state.inputError. Previously bad input was only caught after the
user walked through consent, forcing them to re-do consent on retry.
Exposes validateFeedbackMessage and validateScreenshotPath from the
operations module so the hook can reuse the same validators as the
submission orchestrator.
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

Claude Security Review: the review run failed before completing. See the run for details.

Three issues from bug bash:
- 2.5: Esc/Ctrl+C in the wizard didn't terminate the process. The
onExit callback unmounted the screen but Ink's stdin raw-mode
listeners kept Node alive. Fix: await render()'s waitUntilExit()
and call process.exit(0) once the wizard unmounts. Matches
invoke/command.tsx's pattern.
- 3.2: Pointing --screenshot at a directory ("/tmp") produced
"Screenshot must be one of: .png, .jpg, .jpeg" because we extension-
checked first. Fix: stat the path first; surface a directory-specific
error before the extension check.
- 3.6: Quoted paths starting with "~/" (e.g. ~/Desktop/foo.png) hit
ENOENT because Node's fs APIs don't expand tildes — only the shell
does, and quoting suppresses that. Fix: expand a leading "~" or
"~/..." to os.homedir() before stat/read.
Added unit tests for both new validator branches.
@github-actionsgithub-actionsBot added size/xl PR size: XL and removed size/xl PR size: XL labels May 20, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

Claude Security Review: no high-confidence findings. (run)

@agentcore-devx-automationagentcore-devx-automationBot removed the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
…onal
Per tester feedback: the explicit Y/N prompt felt like an extra step for
something that's already optional. Drop the screenshot-prompt phase
entirely — the wizard now goes Message → Screenshot → Consent.
The Screenshot phase is the path picker itself, with allowEmpty=true on
PathInput. Submit with a path to attach; Enter on empty input or Esc to
skip. Help text reflects this: "Enter select / skip · Esc skip".
Verified end-to-end via TUI harness: live submission id
96e2874e-3a61-4f65-8140-c144393d841e through the new flow.
@github-actionsgithub-actionsBot removed the size/xl PR size: XL label May 20, 2026
@github-actionsgithub-actionsBot added the size/xl PR size: XL label May 20, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

Claude Security Review: no high-confidence findings. (run)

@agentcore-devx-automationagentcore-devx-automationBot removed the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
Hweinstock
Hweinstock previously approved these changes May 20, 2026

@HweinstockHweinstock left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM! very minor nits and a question.

Comment threadsrc/cli/commands/feedback/__tests__/consent-prompt.test.ts
Comment threadsrc/cli/commands/feedback/action.ts Outdated
Comment threadsrc/cli/operations/feedback/aperture-client.ts
Comment threadsrc/cli/operations/feedback/aperture-client.ts
Comment threadsrc/cli/operations/feedback/submit-feedback.ts Outdated
Per Hweinstock's review:
- handleFeedback / useFeedbackFlow: replace the local
`err instanceof Error ? err : new Error(String(err))` pattern with
`toError(err)` from src/lib/errors/types.ts.
- FeedbackValidationError: extend `ValidationError` from
src/lib/errors/types.ts so the shared error classification helpers
treat it as a user error, which means telemetry's
classifyError(err)/isUserError(err) will set error_source: 'user'
on failure metrics for invalid screenshots / empty messages.
@github-actionsgithub-actionsBot added size/xl PR size: XL and removed size/xl PR size: XL labels May 20, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

Claude Security Review: the review run failed before completing. See the run for details.

@agentcore-devx-automationagentcore-devx-automationBot removed the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026

@HweinstockHweinstock left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Awesome!

@tejaskash
tejaskash merged commit 32c475d into mainMay 21, 2026
32 of 33 checks passed
@tejaskash
tejaskash deleted the feature/feedback-command branch May 21, 2026 13:35
tejaskash added a commit that referenced this pull request May 21, 2026
…works (#1327)
The /security-review slash command runs `git diff origin/HEAD...` as its
first action to enumerate the PR's changes. actions/checkout doesn't set
up the remote's symbolic HEAD ref, so that command fails with
"ambiguous argument 'origin/HEAD...': unknown revision". Claude then
loops trying variants until --max-turns 30 trips and the action exits 1.
Set origin/HEAD to the PR's base ref right after checkout so the skill's
git invocations resolve correctly. Run #26186045056 on PR #1321 was the
trigger - 30 turns spent on shell-error recovery, no findings posted.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/xlPR size: XL

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@tejaskash@Hweinstock@agentcore-cli-automation
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

feat: add agentcore feedback command - #1321

Merged
tejaskash merged 7 commits into
mainfrom
feature/feedback-command
May 21, 2026
Merged

feat: add agentcore feedback command#1321
tejaskash merged 7 commits into
mainfrom
feature/feedback-command

Conversation

@tejaskash

Copy link
Copy Markdown
Contributor

Summary

Adds a new top-level agentcore feedback command:

  • One-shot:agentcore feedback "msg" [--screenshot path] [--json]
  • Wizard:agentcore feedback (no args) → multi-step Ink TUI

Both modes display the AWS Customer Agreement and require interactive y/N consent before submitting. Bare Enter defaults to No; non-TTY stdin is refused so the consent decision is always explicit.

The wizard walks Message → optional Screenshot → Consent → Submit, with Esc stepping back and message text preserved across declines/retries. Screenshots are validated locally (PNG/JPG/JPEG, ≤100MB) before any network call, then uploaded via presigned URL with SHA-256 checksum headers.

Submissions carry cli-version, os, node-version, and cli-mode metadata. The CLI does not attach AWS account IDs, credentials, project names, or telemetry IDs.

What's in the diff

  • src/cli/commands/feedback/ — Commander registration, action, TTY consent prompt
  • src/cli/operations/feedback/ — submission orchestrator, payload builder, presigned-URL + S3 upload + form-ingestion client
  • src/cli/tui/screens/feedback/ — Ink wizard with useFeedbackFlow hook
  • src/cli/cli.ts, src/cli/tui/copy.ts — wiring
  • README.md, docs/commands.md, docs/feedback.md — customer-facing docs

29 unit tests across operations, command, consent-prompt, and TUI hook.

Test plan

  • npm test -- feedback — 29/29 pass
  • eslint clean on all new files
  • tsc --noEmit clean on all new files
  • Manual prod submission verified end-to-end (message + screenshot, both submit and decline paths)
  • CLI consent prompt: y/yes/Y/YES accepts; n/no/bare-Enter declines; non-TTY refuses
  • TUI wizard: all phase transitions, Esc back-stepping, message preservation across consent decline
  • Plain mode and --json mode for success, declined, no-tty, validation, network error
  • Bug bash plan attached for testers (5 testers × 6 min)

@github-actionsgithub-actionsBot added agentcore-harness-reviewing AgentCore Harness review in progress size/xl PR size: XL labels May 20, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
@github-actions

github-actionsBot commented May 20, 2026

Copy link
Copy Markdown
Contributor

Package Tarball

aws-agentcore-0.14.1.tgz

How to install

gh release download pr-1321-tarball --repo aws/agentcore-cli --pattern "*.tgz" --dir /tmp/pr-tarball
npm install -g /tmp/pr-tarball/aws-agentcore-0.14.1.tgz

@tejaskash
tejaskash marked this pull request as ready for review May 20, 2026 17:23
@tejaskash
tejaskash requested a review from a teamMay 20, 2026 17:23
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

Claude Security Review: no high-confidence findings. (run)

@agentcore-cli-automationagentcore-cli-automation 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.

Thanks for the careful work on consent flow, validation, and docs — the wizard structure and cli/tui mode tagging are nicely done. A few things I'd like to see addressed before merging:

  1. Missing telemetry instrumentation. Per src/cli/telemetry/README.md, every CLI command should emit a command_run metric, but feedback doesn't. Other recent commands (e.g. deploy, remove, create) all use withCommandRunTelemetry / runCliCommand.

  2. Screenshot S3 key is fabricated client-side independent of the presigned URL. The form references a key the client invented rather than the actual upload location.

  3. TUI validates message and screenshot only after consent. Bad input forces the user to redo consent.

Inline comments below.

Comment threadsrc/cli/commands/feedback/command.tsx
Comment threadsrc/cli/operations/feedback/submit-feedback.ts Outdated
Comment threadsrc/cli/tui/screens/feedback/useFeedbackFlow.ts Outdated
@agentcore-devx-automationagentcore-devx-automationBot removed the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
@github-actionsgithub-actionsBot removed the agentcore-harness-reviewing AgentCore Harness review in progress label May 20, 2026
@github-actions

github-actionsBot commented May 20, 2026

Copy link
Copy Markdown
Contributor

Coverage Report

StatusCategoryPercentageCovered / Total
🔵Lines44.3%9531 / 21510
🔵Statements43.55%10114 / 23223
🔵Functions41.01%1655 / 4035
🔵Branches40.84%6162 / 15086
Generated in workflow #3182 for commit 91cb5da by the Vitest Coverage Report Action

@github-actionsgithub-actionsBot added size/xl PR size: XL and removed size/xl PR size: XL labels May 20, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

Claude Security Review: the review run failed before completing. See the run for details.

@agentcore-devx-automationagentcore-devx-automationBot removed the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
@tejaskash
tejaskashforce-pushed the feature/feedback-command branch from ab65255 to 94f3a55CompareMay 20, 2026 17:55
@github-actionsgithub-actionsBot added size/xl PR size: XL and removed size/xl PR size: XL labels May 20, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

Claude Security Review: no high-confidence findings. (run)

@agentcore-devx-automationagentcore-devx-automationBot removed the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
Hweinstock
Hweinstock previously approved these changes May 20, 2026

@HweinstockHweinstock left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

implementation looks good, some questions on the testing, but we can revisit as a follow-up to bug bash today.

Comment threaddocs/feedback.md
Comment threadsrc/cli/commands/feedback/__tests__/command.test.ts
Comment threadsrc/cli/commands/feedback/action.ts Outdated
Comment threadsrc/cli/tui/screens/feedback/__tests__/useFeedbackFlow.test.tsx Outdated
Comment threadsrc/cli/tui/screens/feedback/useFeedbackFlow.ts Outdated
Adds a new top-level `agentcore feedback` command that lets users send
feedback (and an optional screenshot) directly from the CLI:
agentcore feedback "your message" [--screenshot path] [--json]
agentcore feedback # multi-step wizard
Both modes display the AWS Customer Agreement and require interactive
y/N consent before submitting. Bare Enter defaults to No, and non-TTY
stdin is refused so the consent decision is always explicit.
The wizard walks through Message → optional Screenshot → Consent →
Submit, with Esc stepping back and message text preserved across
declines/retries. Screenshots are validated locally (PNG/JPG/JPEG,
≤100MB) before any network call, then uploaded via presigned URL with
SHA-256 checksum headers.
Submissions are tagged with cli-version, os, node-version, and cli-mode
metadata to help triage. The CLI does not attach AWS account IDs,
credentials, project names, or telemetry IDs.
Includes README + docs/commands.md updates and a new docs/feedback.md
covering syntax, consent text, and what not to include.
Three changes from automated review:
1. Telemetry: register `feedback` in command-run schema with `mode` +
`has_screenshot` attrs, instrument the CLI handler with
client.withCommandRun (CANCELLED on declined consent), and the TUI
submit path with withCommandRunTelemetry.
2. Screenshot S3 key: parse the actual object key from the presigned
URL path instead of fabricating one client-side. The fabricated key
could drift from Aperture's bucket layout (UTC/local date, prefix
changes) and silently produce form references to non-existent
objects.
3. TUI input validation: validate message in setMessage and screenshot
in setScreenshot, surface the error inline on the input phase via
state.inputError. Previously bad input was only caught after the
user walked through consent, forcing them to re-do consent on retry.
Exposes validateFeedbackMessage and validateScreenshotPath from the
operations module so the hook can reuse the same validators as the
submission orchestrator.
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

Claude Security Review: the review run failed before completing. See the run for details.

Three issues from bug bash:
- 2.5: Esc/Ctrl+C in the wizard didn't terminate the process. The
onExit callback unmounted the screen but Ink's stdin raw-mode
listeners kept Node alive. Fix: await render()'s waitUntilExit()
and call process.exit(0) once the wizard unmounts. Matches
invoke/command.tsx's pattern.
- 3.2: Pointing --screenshot at a directory ("/tmp") produced
"Screenshot must be one of: .png, .jpg, .jpeg" because we extension-
checked first. Fix: stat the path first; surface a directory-specific
error before the extension check.
- 3.6: Quoted paths starting with "~/" (e.g. ~/Desktop/foo.png) hit
ENOENT because Node's fs APIs don't expand tildes — only the shell
does, and quoting suppresses that. Fix: expand a leading "~" or
"~/..." to os.homedir() before stat/read.
Added unit tests for both new validator branches.
@github-actionsgithub-actionsBot added size/xl PR size: XL and removed size/xl PR size: XL labels May 20, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

Claude Security Review: no high-confidence findings. (run)

@agentcore-devx-automationagentcore-devx-automationBot removed the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
…onal
Per tester feedback: the explicit Y/N prompt felt like an extra step for
something that's already optional. Drop the screenshot-prompt phase
entirely — the wizard now goes Message → Screenshot → Consent.
The Screenshot phase is the path picker itself, with allowEmpty=true on
PathInput. Submit with a path to attach; Enter on empty input or Esc to
skip. Help text reflects this: "Enter select / skip · Esc skip".
Verified end-to-end via TUI harness: live submission id
96e2874e-3a61-4f65-8140-c144393d841e through the new flow.
@github-actionsgithub-actionsBot removed the size/xl PR size: XL label May 20, 2026
@github-actionsgithub-actionsBot added the size/xl PR size: XL label May 20, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

Claude Security Review: no high-confidence findings. (run)

@agentcore-devx-automationagentcore-devx-automationBot removed the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
Hweinstock
Hweinstock previously approved these changes May 20, 2026

@HweinstockHweinstock left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM! very minor nits and a question.

Comment threadsrc/cli/commands/feedback/__tests__/consent-prompt.test.ts
Comment threadsrc/cli/commands/feedback/action.ts Outdated
Comment threadsrc/cli/operations/feedback/aperture-client.ts
Comment threadsrc/cli/operations/feedback/aperture-client.ts
Comment threadsrc/cli/operations/feedback/submit-feedback.ts Outdated
Per Hweinstock's review:
- handleFeedback / useFeedbackFlow: replace the local
`err instanceof Error ? err : new Error(String(err))` pattern with
`toError(err)` from src/lib/errors/types.ts.
- FeedbackValidationError: extend `ValidationError` from
src/lib/errors/types.ts so the shared error classification helpers
treat it as a user error, which means telemetry's
classifyError(err)/isUserError(err) will set error_source: 'user'
on failure metrics for invalid screenshots / empty messages.
@github-actionsgithub-actionsBot added size/xl PR size: XL and removed size/xl PR size: XL labels May 20, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

Claude Security Review: the review run failed before completing. See the run for details.

@agentcore-devx-automationagentcore-devx-automationBot removed the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026

@HweinstockHweinstock left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Awesome!

@tejaskash
tejaskash merged commit 32c475d into mainMay 21, 2026
32 of 33 checks passed
@tejaskash
tejaskash deleted the feature/feedback-command branch May 21, 2026 13:35
tejaskash added a commit that referenced this pull request May 21, 2026
…works (#1327)
The /security-review slash command runs `git diff origin/HEAD...` as its
first action to enumerate the PR's changes. actions/checkout doesn't set
up the remote's symbolic HEAD ref, so that command fails with
"ambiguous argument 'origin/HEAD...': unknown revision". Claude then
loops trying variants until --max-turns 30 trips and the action exits 1.
Set origin/HEAD to the PR's base ref right after checkout so the skill's
git invocations resolve correctly. Run #26186045056 on PR #1321 was the
trigger - 30 turns spent on shell-error recovery, no findings posted.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/xlPR size: XL

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@tejaskash@Hweinstock@agentcore-cli-automation
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

feat: add agentcore feedback command - #1321

Merged
tejaskash merged 7 commits into
mainfrom
feature/feedback-command
May 21, 2026
Merged

feat: add agentcore feedback command#1321
tejaskash merged 7 commits into
mainfrom
feature/feedback-command

Conversation

@tejaskash

Copy link
Copy Markdown
Contributor

Summary

Adds a new top-level agentcore feedback command:

  • One-shot:agentcore feedback "msg" [--screenshot path] [--json]
  • Wizard:agentcore feedback (no args) → multi-step Ink TUI

Both modes display the AWS Customer Agreement and require interactive y/N consent before submitting. Bare Enter defaults to No; non-TTY stdin is refused so the consent decision is always explicit.

The wizard walks Message → optional Screenshot → Consent → Submit, with Esc stepping back and message text preserved across declines/retries. Screenshots are validated locally (PNG/JPG/JPEG, ≤100MB) before any network call, then uploaded via presigned URL with SHA-256 checksum headers.

Submissions carry cli-version, os, node-version, and cli-mode metadata. The CLI does not attach AWS account IDs, credentials, project names, or telemetry IDs.

What's in the diff

  • src/cli/commands/feedback/ — Commander registration, action, TTY consent prompt
  • src/cli/operations/feedback/ — submission orchestrator, payload builder, presigned-URL + S3 upload + form-ingestion client
  • src/cli/tui/screens/feedback/ — Ink wizard with useFeedbackFlow hook
  • src/cli/cli.ts, src/cli/tui/copy.ts — wiring
  • README.md, docs/commands.md, docs/feedback.md — customer-facing docs

29 unit tests across operations, command, consent-prompt, and TUI hook.

Test plan

  • npm test -- feedback — 29/29 pass
  • eslint clean on all new files
  • tsc --noEmit clean on all new files
  • Manual prod submission verified end-to-end (message + screenshot, both submit and decline paths)
  • CLI consent prompt: y/yes/Y/YES accepts; n/no/bare-Enter declines; non-TTY refuses
  • TUI wizard: all phase transitions, Esc back-stepping, message preservation across consent decline
  • Plain mode and --json mode for success, declined, no-tty, validation, network error
  • Bug bash plan attached for testers (5 testers × 6 min)

@github-actionsgithub-actionsBot added agentcore-harness-reviewing AgentCore Harness review in progress size/xl PR size: XL labels May 20, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
@github-actions

github-actionsBot commented May 20, 2026

Copy link
Copy Markdown
Contributor

Package Tarball

aws-agentcore-0.14.1.tgz

How to install

gh release download pr-1321-tarball --repo aws/agentcore-cli --pattern "*.tgz" --dir /tmp/pr-tarball
npm install -g /tmp/pr-tarball/aws-agentcore-0.14.1.tgz

@tejaskash
tejaskash marked this pull request as ready for review May 20, 2026 17:23
@tejaskash
tejaskash requested a review from a teamMay 20, 2026 17:23
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

Claude Security Review: no high-confidence findings. (run)

@agentcore-cli-automationagentcore-cli-automation 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.

Thanks for the careful work on consent flow, validation, and docs — the wizard structure and cli/tui mode tagging are nicely done. A few things I'd like to see addressed before merging:

  1. Missing telemetry instrumentation. Per src/cli/telemetry/README.md, every CLI command should emit a command_run metric, but feedback doesn't. Other recent commands (e.g. deploy, remove, create) all use withCommandRunTelemetry / runCliCommand.

  2. Screenshot S3 key is fabricated client-side independent of the presigned URL. The form references a key the client invented rather than the actual upload location.

  3. TUI validates message and screenshot only after consent. Bad input forces the user to redo consent.

Inline comments below.

Comment threadsrc/cli/commands/feedback/command.tsx
Comment threadsrc/cli/operations/feedback/submit-feedback.ts Outdated
Comment threadsrc/cli/tui/screens/feedback/useFeedbackFlow.ts Outdated
@agentcore-devx-automationagentcore-devx-automationBot removed the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
@github-actionsgithub-actionsBot removed the agentcore-harness-reviewing AgentCore Harness review in progress label May 20, 2026
@github-actions

github-actionsBot commented May 20, 2026

Copy link
Copy Markdown
Contributor

Coverage Report

StatusCategoryPercentageCovered / Total
🔵Lines44.3%9531 / 21510
🔵Statements43.55%10114 / 23223
🔵Functions41.01%1655 / 4035
🔵Branches40.84%6162 / 15086
Generated in workflow #3182 for commit 91cb5da by the Vitest Coverage Report Action

@github-actionsgithub-actionsBot added size/xl PR size: XL and removed size/xl PR size: XL labels May 20, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

Claude Security Review: the review run failed before completing. See the run for details.

@agentcore-devx-automationagentcore-devx-automationBot removed the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
@tejaskash
tejaskashforce-pushed the feature/feedback-command branch from ab65255 to 94f3a55CompareMay 20, 2026 17:55
@github-actionsgithub-actionsBot added size/xl PR size: XL and removed size/xl PR size: XL labels May 20, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

Claude Security Review: no high-confidence findings. (run)

@agentcore-devx-automationagentcore-devx-automationBot removed the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
Hweinstock
Hweinstock previously approved these changes May 20, 2026

@HweinstockHweinstock left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

implementation looks good, some questions on the testing, but we can revisit as a follow-up to bug bash today.

Comment threaddocs/feedback.md
Comment threadsrc/cli/commands/feedback/__tests__/command.test.ts
Comment threadsrc/cli/commands/feedback/action.ts Outdated
Comment threadsrc/cli/tui/screens/feedback/__tests__/useFeedbackFlow.test.tsx Outdated
Comment threadsrc/cli/tui/screens/feedback/useFeedbackFlow.ts Outdated
Adds a new top-level `agentcore feedback` command that lets users send
feedback (and an optional screenshot) directly from the CLI:
agentcore feedback "your message" [--screenshot path] [--json]
agentcore feedback # multi-step wizard
Both modes display the AWS Customer Agreement and require interactive
y/N consent before submitting. Bare Enter defaults to No, and non-TTY
stdin is refused so the consent decision is always explicit.
The wizard walks through Message → optional Screenshot → Consent →
Submit, with Esc stepping back and message text preserved across
declines/retries. Screenshots are validated locally (PNG/JPG/JPEG,
≤100MB) before any network call, then uploaded via presigned URL with
SHA-256 checksum headers.
Submissions are tagged with cli-version, os, node-version, and cli-mode
metadata to help triage. The CLI does not attach AWS account IDs,
credentials, project names, or telemetry IDs.
Includes README + docs/commands.md updates and a new docs/feedback.md
covering syntax, consent text, and what not to include.
Three changes from automated review:
1. Telemetry: register `feedback` in command-run schema with `mode` +
`has_screenshot` attrs, instrument the CLI handler with
client.withCommandRun (CANCELLED on declined consent), and the TUI
submit path with withCommandRunTelemetry.
2. Screenshot S3 key: parse the actual object key from the presigned
URL path instead of fabricating one client-side. The fabricated key
could drift from Aperture's bucket layout (UTC/local date, prefix
changes) and silently produce form references to non-existent
objects.
3. TUI input validation: validate message in setMessage and screenshot
in setScreenshot, surface the error inline on the input phase via
state.inputError. Previously bad input was only caught after the
user walked through consent, forcing them to re-do consent on retry.
Exposes validateFeedbackMessage and validateScreenshotPath from the
operations module so the hook can reuse the same validators as the
submission orchestrator.
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

Claude Security Review: the review run failed before completing. See the run for details.

Three issues from bug bash:
- 2.5: Esc/Ctrl+C in the wizard didn't terminate the process. The
onExit callback unmounted the screen but Ink's stdin raw-mode
listeners kept Node alive. Fix: await render()'s waitUntilExit()
and call process.exit(0) once the wizard unmounts. Matches
invoke/command.tsx's pattern.
- 3.2: Pointing --screenshot at a directory ("/tmp") produced
"Screenshot must be one of: .png, .jpg, .jpeg" because we extension-
checked first. Fix: stat the path first; surface a directory-specific
error before the extension check.
- 3.6: Quoted paths starting with "~/" (e.g. ~/Desktop/foo.png) hit
ENOENT because Node's fs APIs don't expand tildes — only the shell
does, and quoting suppresses that. Fix: expand a leading "~" or
"~/..." to os.homedir() before stat/read.
Added unit tests for both new validator branches.
@github-actionsgithub-actionsBot added size/xl PR size: XL and removed size/xl PR size: XL labels May 20, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

Claude Security Review: no high-confidence findings. (run)

@agentcore-devx-automationagentcore-devx-automationBot removed the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
…onal
Per tester feedback: the explicit Y/N prompt felt like an extra step for
something that's already optional. Drop the screenshot-prompt phase
entirely — the wizard now goes Message → Screenshot → Consent.
The Screenshot phase is the path picker itself, with allowEmpty=true on
PathInput. Submit with a path to attach; Enter on empty input or Esc to
skip. Help text reflects this: "Enter select / skip · Esc skip".
Verified end-to-end via TUI harness: live submission id
96e2874e-3a61-4f65-8140-c144393d841e through the new flow.
@github-actionsgithub-actionsBot removed the size/xl PR size: XL label May 20, 2026
@github-actionsgithub-actionsBot added the size/xl PR size: XL label May 20, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

Claude Security Review: no high-confidence findings. (run)

@agentcore-devx-automationagentcore-devx-automationBot removed the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
Hweinstock
Hweinstock previously approved these changes May 20, 2026

@HweinstockHweinstock left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM! very minor nits and a question.

Comment threadsrc/cli/commands/feedback/__tests__/consent-prompt.test.ts
Comment threadsrc/cli/commands/feedback/action.ts Outdated
Comment threadsrc/cli/operations/feedback/aperture-client.ts
Comment threadsrc/cli/operations/feedback/aperture-client.ts
Comment threadsrc/cli/operations/feedback/submit-feedback.ts Outdated
Per Hweinstock's review:
- handleFeedback / useFeedbackFlow: replace the local
`err instanceof Error ? err : new Error(String(err))` pattern with
`toError(err)` from src/lib/errors/types.ts.
- FeedbackValidationError: extend `ValidationError` from
src/lib/errors/types.ts so the shared error classification helpers
treat it as a user error, which means telemetry's
classifyError(err)/isUserError(err) will set error_source: 'user'
on failure metrics for invalid screenshots / empty messages.
@github-actionsgithub-actionsBot added size/xl PR size: XL and removed size/xl PR size: XL labels May 20, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

Claude Security Review: the review run failed before completing. See the run for details.

@agentcore-devx-automationagentcore-devx-automationBot removed the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026

@HweinstockHweinstock left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Awesome!

@tejaskash
tejaskash merged commit 32c475d into mainMay 21, 2026
32 of 33 checks passed
@tejaskash
tejaskash deleted the feature/feedback-command branch May 21, 2026 13:35
tejaskash added a commit that referenced this pull request May 21, 2026
…works (#1327)
The /security-review slash command runs `git diff origin/HEAD...` as its
first action to enumerate the PR's changes. actions/checkout doesn't set
up the remote's symbolic HEAD ref, so that command fails with
"ambiguous argument 'origin/HEAD...': unknown revision". Claude then
loops trying variants until --max-turns 30 trips and the action exits 1.
Set origin/HEAD to the PR's base ref right after checkout so the skill's
git invocations resolve correctly. Run #26186045056 on PR #1321 was the
trigger - 30 turns spent on shell-error recovery, no findings posted.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/xlPR size: XL

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@tejaskash@Hweinstock@agentcore-cli-automation
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

feat: add agentcore feedback command - #1321

Merged
tejaskash merged 7 commits into
mainfrom
feature/feedback-command
May 21, 2026
Merged

feat: add agentcore feedback command#1321
tejaskash merged 7 commits into
mainfrom
feature/feedback-command

Conversation

@tejaskash

Copy link
Copy Markdown
Contributor

Summary

Adds a new top-level agentcore feedback command:

  • One-shot:agentcore feedback "msg" [--screenshot path] [--json]
  • Wizard:agentcore feedback (no args) → multi-step Ink TUI

Both modes display the AWS Customer Agreement and require interactive y/N consent before submitting. Bare Enter defaults to No; non-TTY stdin is refused so the consent decision is always explicit.

The wizard walks Message → optional Screenshot → Consent → Submit, with Esc stepping back and message text preserved across declines/retries. Screenshots are validated locally (PNG/JPG/JPEG, ≤100MB) before any network call, then uploaded via presigned URL with SHA-256 checksum headers.

Submissions carry cli-version, os, node-version, and cli-mode metadata. The CLI does not attach AWS account IDs, credentials, project names, or telemetry IDs.

What's in the diff

  • src/cli/commands/feedback/ — Commander registration, action, TTY consent prompt
  • src/cli/operations/feedback/ — submission orchestrator, payload builder, presigned-URL + S3 upload + form-ingestion client
  • src/cli/tui/screens/feedback/ — Ink wizard with useFeedbackFlow hook
  • src/cli/cli.ts, src/cli/tui/copy.ts — wiring
  • README.md, docs/commands.md, docs/feedback.md — customer-facing docs

29 unit tests across operations, command, consent-prompt, and TUI hook.

Test plan

  • npm test -- feedback — 29/29 pass
  • eslint clean on all new files
  • tsc --noEmit clean on all new files
  • Manual prod submission verified end-to-end (message + screenshot, both submit and decline paths)
  • CLI consent prompt: y/yes/Y/YES accepts; n/no/bare-Enter declines; non-TTY refuses
  • TUI wizard: all phase transitions, Esc back-stepping, message preservation across consent decline
  • Plain mode and --json mode for success, declined, no-tty, validation, network error
  • Bug bash plan attached for testers (5 testers × 6 min)

@github-actionsgithub-actionsBot added agentcore-harness-reviewing AgentCore Harness review in progress size/xl PR size: XL labels May 20, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
@github-actions

github-actionsBot commented May 20, 2026

Copy link
Copy Markdown
Contributor

Package Tarball

aws-agentcore-0.14.1.tgz

How to install

gh release download pr-1321-tarball --repo aws/agentcore-cli --pattern "*.tgz" --dir /tmp/pr-tarball
npm install -g /tmp/pr-tarball/aws-agentcore-0.14.1.tgz

@tejaskash
tejaskash marked this pull request as ready for review May 20, 2026 17:23
@tejaskash
tejaskash requested a review from a teamMay 20, 2026 17:23
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

Claude Security Review: no high-confidence findings. (run)

@agentcore-cli-automationagentcore-cli-automation 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.

Thanks for the careful work on consent flow, validation, and docs — the wizard structure and cli/tui mode tagging are nicely done. A few things I'd like to see addressed before merging:

  1. Missing telemetry instrumentation. Per src/cli/telemetry/README.md, every CLI command should emit a command_run metric, but feedback doesn't. Other recent commands (e.g. deploy, remove, create) all use withCommandRunTelemetry / runCliCommand.

  2. Screenshot S3 key is fabricated client-side independent of the presigned URL. The form references a key the client invented rather than the actual upload location.

  3. TUI validates message and screenshot only after consent. Bad input forces the user to redo consent.

Inline comments below.

Comment threadsrc/cli/commands/feedback/command.tsx
Comment threadsrc/cli/operations/feedback/submit-feedback.ts Outdated
Comment threadsrc/cli/tui/screens/feedback/useFeedbackFlow.ts Outdated
@agentcore-devx-automationagentcore-devx-automationBot removed the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
@github-actionsgithub-actionsBot removed the agentcore-harness-reviewing AgentCore Harness review in progress label May 20, 2026
@github-actions

github-actionsBot commented May 20, 2026

Copy link
Copy Markdown
Contributor

Coverage Report

StatusCategoryPercentageCovered / Total
🔵Lines44.3%9531 / 21510
🔵Statements43.55%10114 / 23223
🔵Functions41.01%1655 / 4035
🔵Branches40.84%6162 / 15086
Generated in workflow #3182 for commit 91cb5da by the Vitest Coverage Report Action

@github-actionsgithub-actionsBot added size/xl PR size: XL and removed size/xl PR size: XL labels May 20, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

Claude Security Review: the review run failed before completing. See the run for details.

@agentcore-devx-automationagentcore-devx-automationBot removed the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
@tejaskash
tejaskashforce-pushed the feature/feedback-command branch from ab65255 to 94f3a55CompareMay 20, 2026 17:55
@github-actionsgithub-actionsBot added size/xl PR size: XL and removed size/xl PR size: XL labels May 20, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

Claude Security Review: no high-confidence findings. (run)

@agentcore-devx-automationagentcore-devx-automationBot removed the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
Hweinstock
Hweinstock previously approved these changes May 20, 2026

@HweinstockHweinstock left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

implementation looks good, some questions on the testing, but we can revisit as a follow-up to bug bash today.

Comment threaddocs/feedback.md
Comment threadsrc/cli/commands/feedback/__tests__/command.test.ts
Comment threadsrc/cli/commands/feedback/action.ts Outdated
Comment threadsrc/cli/tui/screens/feedback/__tests__/useFeedbackFlow.test.tsx Outdated
Comment threadsrc/cli/tui/screens/feedback/useFeedbackFlow.ts Outdated
Adds a new top-level `agentcore feedback` command that lets users send
feedback (and an optional screenshot) directly from the CLI:
agentcore feedback "your message" [--screenshot path] [--json]
agentcore feedback # multi-step wizard
Both modes display the AWS Customer Agreement and require interactive
y/N consent before submitting. Bare Enter defaults to No, and non-TTY
stdin is refused so the consent decision is always explicit.
The wizard walks through Message → optional Screenshot → Consent →
Submit, with Esc stepping back and message text preserved across
declines/retries. Screenshots are validated locally (PNG/JPG/JPEG,
≤100MB) before any network call, then uploaded via presigned URL with
SHA-256 checksum headers.
Submissions are tagged with cli-version, os, node-version, and cli-mode
metadata to help triage. The CLI does not attach AWS account IDs,
credentials, project names, or telemetry IDs.
Includes README + docs/commands.md updates and a new docs/feedback.md
covering syntax, consent text, and what not to include.
Three changes from automated review:
1. Telemetry: register `feedback` in command-run schema with `mode` +
`has_screenshot` attrs, instrument the CLI handler with
client.withCommandRun (CANCELLED on declined consent), and the TUI
submit path with withCommandRunTelemetry.
2. Screenshot S3 key: parse the actual object key from the presigned
URL path instead of fabricating one client-side. The fabricated key
could drift from Aperture's bucket layout (UTC/local date, prefix
changes) and silently produce form references to non-existent
objects.
3. TUI input validation: validate message in setMessage and screenshot
in setScreenshot, surface the error inline on the input phase via
state.inputError. Previously bad input was only caught after the
user walked through consent, forcing them to re-do consent on retry.
Exposes validateFeedbackMessage and validateScreenshotPath from the
operations module so the hook can reuse the same validators as the
submission orchestrator.
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

Claude Security Review: the review run failed before completing. See the run for details.

Three issues from bug bash:
- 2.5: Esc/Ctrl+C in the wizard didn't terminate the process. The
onExit callback unmounted the screen but Ink's stdin raw-mode
listeners kept Node alive. Fix: await render()'s waitUntilExit()
and call process.exit(0) once the wizard unmounts. Matches
invoke/command.tsx's pattern.
- 3.2: Pointing --screenshot at a directory ("/tmp") produced
"Screenshot must be one of: .png, .jpg, .jpeg" because we extension-
checked first. Fix: stat the path first; surface a directory-specific
error before the extension check.
- 3.6: Quoted paths starting with "~/" (e.g. ~/Desktop/foo.png) hit
ENOENT because Node's fs APIs don't expand tildes — only the shell
does, and quoting suppresses that. Fix: expand a leading "~" or
"~/..." to os.homedir() before stat/read.
Added unit tests for both new validator branches.
@github-actionsgithub-actionsBot added size/xl PR size: XL and removed size/xl PR size: XL labels May 20, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

Claude Security Review: no high-confidence findings. (run)

@agentcore-devx-automationagentcore-devx-automationBot removed the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
…onal
Per tester feedback: the explicit Y/N prompt felt like an extra step for
something that's already optional. Drop the screenshot-prompt phase
entirely — the wizard now goes Message → Screenshot → Consent.
The Screenshot phase is the path picker itself, with allowEmpty=true on
PathInput. Submit with a path to attach; Enter on empty input or Esc to
skip. Help text reflects this: "Enter select / skip · Esc skip".
Verified end-to-end via TUI harness: live submission id
96e2874e-3a61-4f65-8140-c144393d841e through the new flow.
@github-actionsgithub-actionsBot removed the size/xl PR size: XL label May 20, 2026
@github-actionsgithub-actionsBot added the size/xl PR size: XL label May 20, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

Claude Security Review: no high-confidence findings. (run)

@agentcore-devx-automationagentcore-devx-automationBot removed the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
Hweinstock
Hweinstock previously approved these changes May 20, 2026

@HweinstockHweinstock left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM! very minor nits and a question.

Comment threadsrc/cli/commands/feedback/__tests__/consent-prompt.test.ts
Comment threadsrc/cli/commands/feedback/action.ts Outdated
Comment threadsrc/cli/operations/feedback/aperture-client.ts
Comment threadsrc/cli/operations/feedback/aperture-client.ts
Comment threadsrc/cli/operations/feedback/submit-feedback.ts Outdated
Per Hweinstock's review:
- handleFeedback / useFeedbackFlow: replace the local
`err instanceof Error ? err : new Error(String(err))` pattern with
`toError(err)` from src/lib/errors/types.ts.
- FeedbackValidationError: extend `ValidationError` from
src/lib/errors/types.ts so the shared error classification helpers
treat it as a user error, which means telemetry's
classifyError(err)/isUserError(err) will set error_source: 'user'
on failure metrics for invalid screenshots / empty messages.
@github-actionsgithub-actionsBot added size/xl PR size: XL and removed size/xl PR size: XL labels May 20, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

Claude Security Review: the review run failed before completing. See the run for details.

@agentcore-devx-automationagentcore-devx-automationBot removed the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026

@HweinstockHweinstock left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Awesome!

@tejaskash
tejaskash merged commit 32c475d into mainMay 21, 2026
32 of 33 checks passed
@tejaskash
tejaskash deleted the feature/feedback-command branch May 21, 2026 13:35
tejaskash added a commit that referenced this pull request May 21, 2026
…works (#1327)
The /security-review slash command runs `git diff origin/HEAD...` as its
first action to enumerate the PR's changes. actions/checkout doesn't set
up the remote's symbolic HEAD ref, so that command fails with
"ambiguous argument 'origin/HEAD...': unknown revision". Claude then
loops trying variants until --max-turns 30 trips and the action exits 1.
Set origin/HEAD to the PR's base ref right after checkout so the skill's
git invocations resolve correctly. Run #26186045056 on PR #1321 was the
trigger - 30 turns spent on shell-error recovery, no findings posted.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/xlPR size: XL

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@tejaskash@Hweinstock@agentcore-cli-automation
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

feat: add agentcore feedback command - #1321

Merged
tejaskash merged 7 commits into
mainfrom
feature/feedback-command
May 21, 2026
Merged

feat: add agentcore feedback command#1321
tejaskash merged 7 commits into
mainfrom
feature/feedback-command

Conversation

@tejaskash

Copy link
Copy Markdown
Contributor

Summary

Adds a new top-level agentcore feedback command:

  • One-shot:agentcore feedback "msg" [--screenshot path] [--json]
  • Wizard:agentcore feedback (no args) → multi-step Ink TUI

Both modes display the AWS Customer Agreement and require interactive y/N consent before submitting. Bare Enter defaults to No; non-TTY stdin is refused so the consent decision is always explicit.

The wizard walks Message → optional Screenshot → Consent → Submit, with Esc stepping back and message text preserved across declines/retries. Screenshots are validated locally (PNG/JPG/JPEG, ≤100MB) before any network call, then uploaded via presigned URL with SHA-256 checksum headers.

Submissions carry cli-version, os, node-version, and cli-mode metadata. The CLI does not attach AWS account IDs, credentials, project names, or telemetry IDs.

What's in the diff

  • src/cli/commands/feedback/ — Commander registration, action, TTY consent prompt
  • src/cli/operations/feedback/ — submission orchestrator, payload builder, presigned-URL + S3 upload + form-ingestion client
  • src/cli/tui/screens/feedback/ — Ink wizard with useFeedbackFlow hook
  • src/cli/cli.ts, src/cli/tui/copy.ts — wiring
  • README.md, docs/commands.md, docs/feedback.md — customer-facing docs

29 unit tests across operations, command, consent-prompt, and TUI hook.

Test plan

  • npm test -- feedback — 29/29 pass
  • eslint clean on all new files
  • tsc --noEmit clean on all new files
  • Manual prod submission verified end-to-end (message + screenshot, both submit and decline paths)
  • CLI consent prompt: y/yes/Y/YES accepts; n/no/bare-Enter declines; non-TTY refuses
  • TUI wizard: all phase transitions, Esc back-stepping, message preservation across consent decline
  • Plain mode and --json mode for success, declined, no-tty, validation, network error
  • Bug bash plan attached for testers (5 testers × 6 min)

@github-actionsgithub-actionsBot added agentcore-harness-reviewing AgentCore Harness review in progress size/xl PR size: XL labels May 20, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
@github-actions

github-actionsBot commented May 20, 2026

Copy link
Copy Markdown
Contributor

Package Tarball

aws-agentcore-0.14.1.tgz

How to install

gh release download pr-1321-tarball --repo aws/agentcore-cli --pattern "*.tgz" --dir /tmp/pr-tarball
npm install -g /tmp/pr-tarball/aws-agentcore-0.14.1.tgz

@tejaskash
tejaskash marked this pull request as ready for review May 20, 2026 17:23
@tejaskash
tejaskash requested a review from a teamMay 20, 2026 17:23
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

Claude Security Review: no high-confidence findings. (run)

@agentcore-cli-automationagentcore-cli-automation 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.

Thanks for the careful work on consent flow, validation, and docs — the wizard structure and cli/tui mode tagging are nicely done. A few things I'd like to see addressed before merging:

  1. Missing telemetry instrumentation. Per src/cli/telemetry/README.md, every CLI command should emit a command_run metric, but feedback doesn't. Other recent commands (e.g. deploy, remove, create) all use withCommandRunTelemetry / runCliCommand.

  2. Screenshot S3 key is fabricated client-side independent of the presigned URL. The form references a key the client invented rather than the actual upload location.

  3. TUI validates message and screenshot only after consent. Bad input forces the user to redo consent.

Inline comments below.

Comment threadsrc/cli/commands/feedback/command.tsx
Comment threadsrc/cli/operations/feedback/submit-feedback.ts Outdated
Comment threadsrc/cli/tui/screens/feedback/useFeedbackFlow.ts Outdated
@agentcore-devx-automationagentcore-devx-automationBot removed the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
@github-actionsgithub-actionsBot removed the agentcore-harness-reviewing AgentCore Harness review in progress label May 20, 2026
@github-actions

github-actionsBot commented May 20, 2026

Copy link
Copy Markdown
Contributor

Coverage Report

StatusCategoryPercentageCovered / Total
🔵Lines44.3%9531 / 21510
🔵Statements43.55%10114 / 23223
🔵Functions41.01%1655 / 4035
🔵Branches40.84%6162 / 15086
Generated in workflow #3182 for commit 91cb5da by the Vitest Coverage Report Action

@github-actionsgithub-actionsBot added size/xl PR size: XL and removed size/xl PR size: XL labels May 20, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

Claude Security Review: the review run failed before completing. See the run for details.

@agentcore-devx-automationagentcore-devx-automationBot removed the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
@tejaskash
tejaskashforce-pushed the feature/feedback-command branch from ab65255 to 94f3a55CompareMay 20, 2026 17:55
@github-actionsgithub-actionsBot added size/xl PR size: XL and removed size/xl PR size: XL labels May 20, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

Claude Security Review: no high-confidence findings. (run)

@agentcore-devx-automationagentcore-devx-automationBot removed the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
Hweinstock
Hweinstock previously approved these changes May 20, 2026

@HweinstockHweinstock left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

implementation looks good, some questions on the testing, but we can revisit as a follow-up to bug bash today.

Comment threaddocs/feedback.md
Comment threadsrc/cli/commands/feedback/__tests__/command.test.ts
Comment threadsrc/cli/commands/feedback/action.ts Outdated
Comment threadsrc/cli/tui/screens/feedback/__tests__/useFeedbackFlow.test.tsx Outdated
Comment threadsrc/cli/tui/screens/feedback/useFeedbackFlow.ts Outdated
Adds a new top-level `agentcore feedback` command that lets users send
feedback (and an optional screenshot) directly from the CLI:
agentcore feedback "your message" [--screenshot path] [--json]
agentcore feedback # multi-step wizard
Both modes display the AWS Customer Agreement and require interactive
y/N consent before submitting. Bare Enter defaults to No, and non-TTY
stdin is refused so the consent decision is always explicit.
The wizard walks through Message → optional Screenshot → Consent →
Submit, with Esc stepping back and message text preserved across
declines/retries. Screenshots are validated locally (PNG/JPG/JPEG,
≤100MB) before any network call, then uploaded via presigned URL with
SHA-256 checksum headers.
Submissions are tagged with cli-version, os, node-version, and cli-mode
metadata to help triage. The CLI does not attach AWS account IDs,
credentials, project names, or telemetry IDs.
Includes README + docs/commands.md updates and a new docs/feedback.md
covering syntax, consent text, and what not to include.
Three changes from automated review:
1. Telemetry: register `feedback` in command-run schema with `mode` +
`has_screenshot` attrs, instrument the CLI handler with
client.withCommandRun (CANCELLED on declined consent), and the TUI
submit path with withCommandRunTelemetry.
2. Screenshot S3 key: parse the actual object key from the presigned
URL path instead of fabricating one client-side. The fabricated key
could drift from Aperture's bucket layout (UTC/local date, prefix
changes) and silently produce form references to non-existent
objects.
3. TUI input validation: validate message in setMessage and screenshot
in setScreenshot, surface the error inline on the input phase via
state.inputError. Previously bad input was only caught after the
user walked through consent, forcing them to re-do consent on retry.
Exposes validateFeedbackMessage and validateScreenshotPath from the
operations module so the hook can reuse the same validators as the
submission orchestrator.
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

Claude Security Review: the review run failed before completing. See the run for details.

Three issues from bug bash:
- 2.5: Esc/Ctrl+C in the wizard didn't terminate the process. The
onExit callback unmounted the screen but Ink's stdin raw-mode
listeners kept Node alive. Fix: await render()'s waitUntilExit()
and call process.exit(0) once the wizard unmounts. Matches
invoke/command.tsx's pattern.
- 3.2: Pointing --screenshot at a directory ("/tmp") produced
"Screenshot must be one of: .png, .jpg, .jpeg" because we extension-
checked first. Fix: stat the path first; surface a directory-specific
error before the extension check.
- 3.6: Quoted paths starting with "~/" (e.g. ~/Desktop/foo.png) hit
ENOENT because Node's fs APIs don't expand tildes — only the shell
does, and quoting suppresses that. Fix: expand a leading "~" or
"~/..." to os.homedir() before stat/read.
Added unit tests for both new validator branches.
@github-actionsgithub-actionsBot added size/xl PR size: XL and removed size/xl PR size: XL labels May 20, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

Claude Security Review: no high-confidence findings. (run)

@agentcore-devx-automationagentcore-devx-automationBot removed the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
…onal
Per tester feedback: the explicit Y/N prompt felt like an extra step for
something that's already optional. Drop the screenshot-prompt phase
entirely — the wizard now goes Message → Screenshot → Consent.
The Screenshot phase is the path picker itself, with allowEmpty=true on
PathInput. Submit with a path to attach; Enter on empty input or Esc to
skip. Help text reflects this: "Enter select / skip · Esc skip".
Verified end-to-end via TUI harness: live submission id
96e2874e-3a61-4f65-8140-c144393d841e through the new flow.
@github-actionsgithub-actionsBot removed the size/xl PR size: XL label May 20, 2026
@github-actionsgithub-actionsBot added the size/xl PR size: XL label May 20, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

Claude Security Review: no high-confidence findings. (run)

@agentcore-devx-automationagentcore-devx-automationBot removed the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
Hweinstock
Hweinstock previously approved these changes May 20, 2026

@HweinstockHweinstock left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM! very minor nits and a question.

Comment threadsrc/cli/commands/feedback/__tests__/consent-prompt.test.ts
Comment threadsrc/cli/commands/feedback/action.ts Outdated
Comment threadsrc/cli/operations/feedback/aperture-client.ts
Comment threadsrc/cli/operations/feedback/aperture-client.ts
Comment threadsrc/cli/operations/feedback/submit-feedback.ts Outdated
Per Hweinstock's review:
- handleFeedback / useFeedbackFlow: replace the local
`err instanceof Error ? err : new Error(String(err))` pattern with
`toError(err)` from src/lib/errors/types.ts.
- FeedbackValidationError: extend `ValidationError` from
src/lib/errors/types.ts so the shared error classification helpers
treat it as a user error, which means telemetry's
classifyError(err)/isUserError(err) will set error_source: 'user'
on failure metrics for invalid screenshots / empty messages.
@github-actionsgithub-actionsBot added size/xl PR size: XL and removed size/xl PR size: XL labels May 20, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

Claude Security Review: the review run failed before completing. See the run for details.

@agentcore-devx-automationagentcore-devx-automationBot removed the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026

@HweinstockHweinstock left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Awesome!

@tejaskash
tejaskash merged commit 32c475d into mainMay 21, 2026
32 of 33 checks passed
@tejaskash
tejaskash deleted the feature/feedback-command branch May 21, 2026 13:35
tejaskash added a commit that referenced this pull request May 21, 2026
…works (#1327)
The /security-review slash command runs `git diff origin/HEAD...` as its
first action to enumerate the PR's changes. actions/checkout doesn't set
up the remote's symbolic HEAD ref, so that command fails with
"ambiguous argument 'origin/HEAD...': unknown revision". Claude then
loops trying variants until --max-turns 30 trips and the action exits 1.
Set origin/HEAD to the PR's base ref right after checkout so the skill's
git invocations resolve correctly. Run #26186045056 on PR #1321 was the
trigger - 30 turns spent on shell-error recovery, no findings posted.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/xlPR size: XL

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@tejaskash@Hweinstock@agentcore-cli-automation
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

feat: add agentcore feedback command - #1321

Merged
tejaskash merged 7 commits into
mainfrom
feature/feedback-command
May 21, 2026
Merged

feat: add agentcore feedback command#1321
tejaskash merged 7 commits into
mainfrom
feature/feedback-command

Conversation

@tejaskash

Copy link
Copy Markdown
Contributor

Summary

Adds a new top-level agentcore feedback command:

  • One-shot:agentcore feedback "msg" [--screenshot path] [--json]
  • Wizard:agentcore feedback (no args) → multi-step Ink TUI

Both modes display the AWS Customer Agreement and require interactive y/N consent before submitting. Bare Enter defaults to No; non-TTY stdin is refused so the consent decision is always explicit.

The wizard walks Message → optional Screenshot → Consent → Submit, with Esc stepping back and message text preserved across declines/retries. Screenshots are validated locally (PNG/JPG/JPEG, ≤100MB) before any network call, then uploaded via presigned URL with SHA-256 checksum headers.

Submissions carry cli-version, os, node-version, and cli-mode metadata. The CLI does not attach AWS account IDs, credentials, project names, or telemetry IDs.

What's in the diff

  • src/cli/commands/feedback/ — Commander registration, action, TTY consent prompt
  • src/cli/operations/feedback/ — submission orchestrator, payload builder, presigned-URL + S3 upload + form-ingestion client
  • src/cli/tui/screens/feedback/ — Ink wizard with useFeedbackFlow hook
  • src/cli/cli.ts, src/cli/tui/copy.ts — wiring
  • README.md, docs/commands.md, docs/feedback.md — customer-facing docs

29 unit tests across operations, command, consent-prompt, and TUI hook.

Test plan

  • npm test -- feedback — 29/29 pass
  • eslint clean on all new files
  • tsc --noEmit clean on all new files
  • Manual prod submission verified end-to-end (message + screenshot, both submit and decline paths)
  • CLI consent prompt: y/yes/Y/YES accepts; n/no/bare-Enter declines; non-TTY refuses
  • TUI wizard: all phase transitions, Esc back-stepping, message preservation across consent decline
  • Plain mode and --json mode for success, declined, no-tty, validation, network error
  • Bug bash plan attached for testers (5 testers × 6 min)

@github-actionsgithub-actionsBot added agentcore-harness-reviewing AgentCore Harness review in progress size/xl PR size: XL labels May 20, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
@github-actions

github-actionsBot commented May 20, 2026

Copy link
Copy Markdown
Contributor

Package Tarball

aws-agentcore-0.14.1.tgz

How to install

gh release download pr-1321-tarball --repo aws/agentcore-cli --pattern "*.tgz" --dir /tmp/pr-tarball
npm install -g /tmp/pr-tarball/aws-agentcore-0.14.1.tgz

@tejaskash
tejaskash marked this pull request as ready for review May 20, 2026 17:23
@tejaskash
tejaskash requested a review from a teamMay 20, 2026 17:23
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

Claude Security Review: no high-confidence findings. (run)

@agentcore-cli-automationagentcore-cli-automation 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.

Thanks for the careful work on consent flow, validation, and docs — the wizard structure and cli/tui mode tagging are nicely done. A few things I'd like to see addressed before merging:

  1. Missing telemetry instrumentation. Per src/cli/telemetry/README.md, every CLI command should emit a command_run metric, but feedback doesn't. Other recent commands (e.g. deploy, remove, create) all use withCommandRunTelemetry / runCliCommand.

  2. Screenshot S3 key is fabricated client-side independent of the presigned URL. The form references a key the client invented rather than the actual upload location.

  3. TUI validates message and screenshot only after consent. Bad input forces the user to redo consent.

Inline comments below.

Comment threadsrc/cli/commands/feedback/command.tsx
Comment threadsrc/cli/operations/feedback/submit-feedback.ts Outdated
Comment threadsrc/cli/tui/screens/feedback/useFeedbackFlow.ts Outdated
@agentcore-devx-automationagentcore-devx-automationBot removed the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
@github-actionsgithub-actionsBot removed the agentcore-harness-reviewing AgentCore Harness review in progress label May 20, 2026
@github-actions

github-actionsBot commented May 20, 2026

Copy link
Copy Markdown
Contributor

Coverage Report

StatusCategoryPercentageCovered / Total
🔵Lines44.3%9531 / 21510
🔵Statements43.55%10114 / 23223
🔵Functions41.01%1655 / 4035
🔵Branches40.84%6162 / 15086
Generated in workflow #3182 for commit 91cb5da by the Vitest Coverage Report Action

@github-actionsgithub-actionsBot added size/xl PR size: XL and removed size/xl PR size: XL labels May 20, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

Claude Security Review: the review run failed before completing. See the run for details.

@agentcore-devx-automationagentcore-devx-automationBot removed the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
@tejaskash
tejaskashforce-pushed the feature/feedback-command branch from ab65255 to 94f3a55CompareMay 20, 2026 17:55
@github-actionsgithub-actionsBot added size/xl PR size: XL and removed size/xl PR size: XL labels May 20, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

Claude Security Review: no high-confidence findings. (run)

@agentcore-devx-automationagentcore-devx-automationBot removed the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
Hweinstock
Hweinstock previously approved these changes May 20, 2026

@HweinstockHweinstock left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

implementation looks good, some questions on the testing, but we can revisit as a follow-up to bug bash today.

Comment threaddocs/feedback.md
Comment threadsrc/cli/commands/feedback/__tests__/command.test.ts
Comment threadsrc/cli/commands/feedback/action.ts Outdated
Comment threadsrc/cli/tui/screens/feedback/__tests__/useFeedbackFlow.test.tsx Outdated
Comment threadsrc/cli/tui/screens/feedback/useFeedbackFlow.ts Outdated
Adds a new top-level `agentcore feedback` command that lets users send
feedback (and an optional screenshot) directly from the CLI:
agentcore feedback "your message" [--screenshot path] [--json]
agentcore feedback # multi-step wizard
Both modes display the AWS Customer Agreement and require interactive
y/N consent before submitting. Bare Enter defaults to No, and non-TTY
stdin is refused so the consent decision is always explicit.
The wizard walks through Message → optional Screenshot → Consent →
Submit, with Esc stepping back and message text preserved across
declines/retries. Screenshots are validated locally (PNG/JPG/JPEG,
≤100MB) before any network call, then uploaded via presigned URL with
SHA-256 checksum headers.
Submissions are tagged with cli-version, os, node-version, and cli-mode
metadata to help triage. The CLI does not attach AWS account IDs,
credentials, project names, or telemetry IDs.
Includes README + docs/commands.md updates and a new docs/feedback.md
covering syntax, consent text, and what not to include.
Three changes from automated review:
1. Telemetry: register `feedback` in command-run schema with `mode` +
`has_screenshot` attrs, instrument the CLI handler with
client.withCommandRun (CANCELLED on declined consent), and the TUI
submit path with withCommandRunTelemetry.
2. Screenshot S3 key: parse the actual object key from the presigned
URL path instead of fabricating one client-side. The fabricated key
could drift from Aperture's bucket layout (UTC/local date, prefix
changes) and silently produce form references to non-existent
objects.
3. TUI input validation: validate message in setMessage and screenshot
in setScreenshot, surface the error inline on the input phase via
state.inputError. Previously bad input was only caught after the
user walked through consent, forcing them to re-do consent on retry.
Exposes validateFeedbackMessage and validateScreenshotPath from the
operations module so the hook can reuse the same validators as the
submission orchestrator.
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

Claude Security Review: the review run failed before completing. See the run for details.

Three issues from bug bash:
- 2.5: Esc/Ctrl+C in the wizard didn't terminate the process. The
onExit callback unmounted the screen but Ink's stdin raw-mode
listeners kept Node alive. Fix: await render()'s waitUntilExit()
and call process.exit(0) once the wizard unmounts. Matches
invoke/command.tsx's pattern.
- 3.2: Pointing --screenshot at a directory ("/tmp") produced
"Screenshot must be one of: .png, .jpg, .jpeg" because we extension-
checked first. Fix: stat the path first; surface a directory-specific
error before the extension check.
- 3.6: Quoted paths starting with "~/" (e.g. ~/Desktop/foo.png) hit
ENOENT because Node's fs APIs don't expand tildes — only the shell
does, and quoting suppresses that. Fix: expand a leading "~" or
"~/..." to os.homedir() before stat/read.
Added unit tests for both new validator branches.
@github-actionsgithub-actionsBot added size/xl PR size: XL and removed size/xl PR size: XL labels May 20, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

Claude Security Review: no high-confidence findings. (run)

@agentcore-devx-automationagentcore-devx-automationBot removed the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
…onal
Per tester feedback: the explicit Y/N prompt felt like an extra step for
something that's already optional. Drop the screenshot-prompt phase
entirely — the wizard now goes Message → Screenshot → Consent.
The Screenshot phase is the path picker itself, with allowEmpty=true on
PathInput. Submit with a path to attach; Enter on empty input or Esc to
skip. Help text reflects this: "Enter select / skip · Esc skip".
Verified end-to-end via TUI harness: live submission id
96e2874e-3a61-4f65-8140-c144393d841e through the new flow.
@github-actionsgithub-actionsBot removed the size/xl PR size: XL label May 20, 2026
@github-actionsgithub-actionsBot added the size/xl PR size: XL label May 20, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

Claude Security Review: no high-confidence findings. (run)

@agentcore-devx-automationagentcore-devx-automationBot removed the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
Hweinstock
Hweinstock previously approved these changes May 20, 2026

@HweinstockHweinstock left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM! very minor nits and a question.

Comment threadsrc/cli/commands/feedback/__tests__/consent-prompt.test.ts
Comment threadsrc/cli/commands/feedback/action.ts Outdated
Comment threadsrc/cli/operations/feedback/aperture-client.ts
Comment threadsrc/cli/operations/feedback/aperture-client.ts
Comment threadsrc/cli/operations/feedback/submit-feedback.ts Outdated
Per Hweinstock's review:
- handleFeedback / useFeedbackFlow: replace the local
`err instanceof Error ? err : new Error(String(err))` pattern with
`toError(err)` from src/lib/errors/types.ts.
- FeedbackValidationError: extend `ValidationError` from
src/lib/errors/types.ts so the shared error classification helpers
treat it as a user error, which means telemetry's
classifyError(err)/isUserError(err) will set error_source: 'user'
on failure metrics for invalid screenshots / empty messages.
@github-actionsgithub-actionsBot added size/xl PR size: XL and removed size/xl PR size: XL labels May 20, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

Claude Security Review: the review run failed before completing. See the run for details.

@agentcore-devx-automationagentcore-devx-automationBot removed the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026

@HweinstockHweinstock left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Awesome!

@tejaskash
tejaskash merged commit 32c475d into mainMay 21, 2026
32 of 33 checks passed
@tejaskash
tejaskash deleted the feature/feedback-command branch May 21, 2026 13:35
tejaskash added a commit that referenced this pull request May 21, 2026
…works (#1327)
The /security-review slash command runs `git diff origin/HEAD...` as its
first action to enumerate the PR's changes. actions/checkout doesn't set
up the remote's symbolic HEAD ref, so that command fails with
"ambiguous argument 'origin/HEAD...': unknown revision". Claude then
loops trying variants until --max-turns 30 trips and the action exits 1.
Set origin/HEAD to the PR's base ref right after checkout so the skill's
git invocations resolve correctly. Run #26186045056 on PR #1321 was the
trigger - 30 turns spent on shell-error recovery, no findings posted.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/xlPR size: XL

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@tejaskash@Hweinstock@agentcore-cli-automation