Skip to content

feat(export): surface export notes inline in the CLI and TUI#1676

Merged
padmak30 merged 3 commits into
mainfrom
feat/export-notes-prominent
Jul 1, 2026
Merged

feat(export): surface export notes inline in the CLI and TUI#1676
padmak30 merged 3 commits into
mainfrom
feat/export-notes-prominent

Conversation

@padmak30

Copy link
Copy Markdown
Contributor

Description

Export notes — manual follow-up items the exporter emits (e.g. "browser tool requires a Container build", "verify the path skill exists in the image", "malformed S3 skill URI") — were only written to app/<agent>/EXPORT_NOTES.md and surfaced by a single passive line in the CLI output. They were easy to miss, which defeats their purpose.

This makes them prominent on the export success path, on both surfaces:

  • CLI: a yellow ⚠ N export note(s) requiring manual follow-up: block listing each note's category + (multi-line) message, with a pointer to EXPORT_NOTES.md. When there are none: No manual follow-up required.
  • TUI: the same notes rendered on the export success screen, above the next-steps menu.
  • --json: the notes are returned as a structured notes[] array (each { category, message }) via serializeResult, so scripts/CI can consume them programmatically.

handleExportHarness now returns notes: ExportNote[] in its success result (single source for all three surfaces). EXPORT_NOTES.md is still written unchanged.

Related Issue

Type of Change

  • New feature

Testing

  • npm run typecheck (clean)
  • npm run lint (clean)
  • npm run test:unit — export unit suites pass (150)
  • Verified live locally on both surfaces: a browser-tool CodeZip export shows the note block (CLI + TUI via the TUI harness); a plain harness shows "No manual follow-up required"; --json includes the notes[] array.

Checklist

  • I have added/maintained tests covering the change
  • My changes generate no new warnings

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Export notes (manual follow-up items like "browser tool needs a Container build"
or "verify the path skill exists in the image") were only written to
app/<agent>/EXPORT_NOTES.md and referenced by a single passive line, so they were
easy to miss.

Return the notes from handleExportHarness and render them prominently on success:
- CLI: a yellow "N export note(s) requiring manual follow-up" block listing each
  note's category + message, with a pointer to EXPORT_NOTES.md; a "no manual
  follow-up required" line when there are none.
- TUI: the same notes shown on the export success screen above the next-steps menu.
- --json: notes are included as a structured `notes[]` array (each {category,
  message}) via serializeResult, so scripts/CI can consume them.

EXPORT_NOTES.md is still written unchanged. Verified live on both surfaces (a
browser-on-CodeZip export shows the note; a plain harness shows none) and via --json.
@padmak30 padmak30 requested a review from a team June 30, 2026 21:14
@github-actions github-actions Bot added the size/s PR size: S label Jun 30, 2026
@github-actions github-actions Bot added the agentcore-harness-reviewing AgentCore Harness review in progress label Jun 30, 2026
@agentcore-devx-automation agentcore-devx-automation Bot added the claude-security-reviewing Claude Code /security-review in progress label Jun 30, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Package Tarball

aws-agentcore-0.21.1.tgz

How to install

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

@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

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

@agentcore-devx-automation agentcore-devx-automation Bot removed the claude-security-reviewing Claude Code /security-review in progress label Jun 30, 2026

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

LGTM — small, focused change. Plumbing already-collected exportNotes through handleExportHarness's success result and rendering them on the three surfaces (CLI block, TUI success screen, --json via serializeResult) is a clear win in discoverability over the old single-line pointer.

A few observations, none blocking:

  • Telemetry already covers this via the existing notes_count attribute on export.harness — no instrumentation gap.
  • No new mocking introduced; the existing harness-action.test.ts continues to cover the note-builder functions.
  • The new notes: ExportNote[] field on the --json success payload becomes an implicit public contract for scripted consumers. Worth keeping ExportNote's shape ({ category, message }) stable going forward, or adding a brief schema note somewhere if --json outputs are documented elsewhere.
  • The CLI/TUI rendering of the notes themselves isn't covered by a test (just manual verification per the PR description). The formatting is simple and the underlying builders are tested, so I don't think it's necessary to block on this, but a small assertion that result.notes flows through to stdout (or a TUI snapshot) would harden the contract if you want to add it.

@github-actions github-actions Bot removed the agentcore-harness-reviewing AgentCore Harness review in progress label Jun 30, 2026
@github-actions

github-actions Bot commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Coverage Report

Status Category Percentage Covered / Total
🔵 Lines 38.05% 14104 / 37066
🔵 Statements 37.34% 15021 / 40226
🔵 Functions 32.69% 2426 / 7419
🔵 Branches 31.93% 9385 / 29391
Generated in workflow #3952 for commit 4177aa4 by the Vitest Coverage Report Action

Addresses the PR review note that the CLI/TUI note rendering had no unit
coverage (only manual verification). Extract the note-formatting into a pure
`formatExportNotes(notes, notesFileHint)` helper that returns tone-tagged lines
(`warn`/`dim`), and have both the CLI (tone → ANSI) and the TUI export success
screen (tone → Ink color) render from it. This makes the output testable without
spawning the command and keeps the two surfaces' wording in sync (also removes
the earlier duplicated rendering logic).

Add format-export-notes.test.ts covering the no-notes line, singular/plural
header, category + multi-line message expansion, and the EXPORT_NOTES.md hint.
@github-actions github-actions Bot added size/m PR size: M and removed size/s PR size: S labels Jul 1, 2026
@agentcore-devx-automation agentcore-devx-automation Bot added the claude-security-reviewing Claude Code /security-review in progress label Jul 1, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

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

@agentcore-devx-automation agentcore-devx-automation Bot removed the claude-security-reviewing Claude Code /security-review in progress label Jul 1, 2026
The CLI `--json` branch emits `JSON.stringify(serializeResult(result))`, so the
export success result's `notes` array is a public contract for scripted
consumers. Add cases asserting serializeResult preserves the notes array, that
it survives a JSON round-trip as structured { category, message } objects, and
that an empty notes list serializes to `[]` (not undefined).
@github-actions github-actions Bot added size/m PR size: M and removed size/m PR size: M labels Jul 1, 2026
@agentcore-devx-automation agentcore-devx-automation Bot added the claude-security-reviewing Claude Code /security-review in progress label Jul 1, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

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

@agentcore-devx-automation agentcore-devx-automation Bot removed the claude-security-reviewing Claude Code /security-review in progress label Jul 1, 2026
@padmak30 padmak30 merged commit 2953d61 into main Jul 1, 2026
70 of 71 checks passed
@padmak30 padmak30 deleted the feat/export-notes-prominent branch July 1, 2026 16:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/m PR size: M

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants