fix: create global entrypoint for tui - #1365

Merged
Hweinstock merged 12 commits into
aws:mainfrom
Hweinstock:fix/tui-telemetry-init
May 27, 2026
Merged

fix: create global entrypoint for tui#1365
Hweinstock merged 12 commits into
aws:mainfrom
Hweinstock:fix/tui-telemetry-init

Conversation

@Hweinstock

@HweinstockHweinstock commented May 22, 2026

Copy link
Copy Markdown
Contributor

Description

Previously, commands like agentcore add, deploy, etc. rendered TUI screens inline via direct render() calls. This made it difficult to distinguish CLI vs TUI code paths, caused telemetry to be mislabeled as mode="cli" for interactive flows, and meant bare agentcore never initialized or shut down the telemetry client.

This PR creates a unified renderTUI() entrypoint that owns the telemetry lifecycle and replaces all inline render() calls. As part of this:

  • Extracted rendering logic into src/cli/tui/render.ts and shared notices into src/cli/notices.ts to break circular imports
  • Migrated add, deploy, create, remove, invoke to use renderTUI()
  • Made TelemetryClientAccessor.init() handle re-initialization safely
  • Preserved existing behavior (diffMode, isInteractive, actionOnBack)

This refactor fixes#895.

Type of Change

  • Bug fix
  • New feature

Testing

  • Typecheck, lint, unit tests pass
  • Manual E2E testing of affected commands

basic-flows.mov

Manually tested flows:

  • back on inline tui exists instead of going to help.
  • successful flow on tui exists instead of going back to help.
  • root level agentcore works as expected.
  • errors still propagate correctly in inline TUI.
  • telemetry is now emitted for both inline TUI and full screen.

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

@github-actionsgithub-actionsBot added the size/m PR size: M label May 22, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 22, 2026
@github-actionsgithub-actionsBot added the agentcore-harness-reviewing AgentCore Harness review in progress label May 22, 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 22, 2026
@github-actions

github-actionsBot commented May 22, 2026

Copy link
Copy Markdown
Contributor

Package Tarball

aws-agentcore-0.15.0.tgz

How to install

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

@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 unifying the TUI entrypoint — the RenderTUIOptions shape and InitialRoute typing are nice. I have a few concerns that I think need to be addressed before this merges.

Summary of issues

  1. Telemetry regression for invoke TUI: The PR removes the withCommandRunTelemetry('invoke', …) wrapper that previously wrapped the TUI invoke flow. No replacement was added, so cli.command_run (with has_stream/has_session_id/auth_type/agent_protocol/duration/exit_reason) is no longer emitted for agentcore invoke interactive mode. See inline comment.

  2. Behavior change: TUI flows no longer auto-exit on success when launched from CLI commands. Previously agentcore add, agentcore remove, etc. rendered their flows with isInteractive={false}, which makes screens like AddFlow, RemoveAllScreen, RemoveFlow, AddSuccessScreen, AddMemoryFlow, AddEvaluatorFlow, AddOnlineEvalFlow, AddIdentityFlow auto-exit after success (see e.g. AddFlow.tsx:206, RemoveAllScreen.tsx:30). Going through App now means they always receive isInteractive={true}, so the user has to press escape to leave after every successful run. See inline comment.

  3. Double TelemetryClientAccessor.init() for command-routed paths. When agentcore add (or any of the migrated commands) runs, main() calls TelemetryClientAccessor.init(args[0], 'cli') (cli.ts:271) and then the action handler calls renderTUI() which calls TelemetryClientAccessor.init(initialRoute.name, 'tui') (cli.ts:127). The first client (mode=cli) is overwritten in the singleton and never shutdown()'d. See inline comment.

  4. Circular import.src/cli/cli.ts imports registerInvoke/registerAdd/etc. from ./commands/*, and those command files now import renderTUI from '../../cli'. It works today because renderTUI is only referenced inside async action handlers, but it's fragile. See inline comment.

Items 1 and 3 are the most important — 1 is a clear telemetry regression and 3 partially undoes the telemetry-mode fix this PR is trying to land.

Comment threadsrc/cli/commands/invoke/command.tsx Outdated
Comment threadsrc/cli/tui/App.tsx Outdated
Comment threadsrc/cli/cli.ts Outdated
Comment threadsrc/cli/commands/invoke/command.tsx Outdated
@github-actionsgithub-actionsBot added size/l PR size: L and removed agentcore-harness-reviewing AgentCore Harness review in progress size/m PR size: M labels May 22, 2026
@HweinstockHweinstock reopened this May 22, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 22, 2026
@github-actionsgithub-actionsBot added size/l PR size: L agentcore-harness-reviewing AgentCore Harness review in progress and removed size/l PR size: L labels May 22, 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 22, 2026

@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 cleanup — the unified renderTUI() entrypoint is a nice improvement and the recent commits address most of the earlier review feedback (double init, isInteractive threading, render-tui module extraction).

Flagging two new issues below; both involve information being silently dropped on the way through renderTUI. The other pre-existing review comments still appear open but their underlying concerns look addressed in the latest commits — happy to leave those for the original reviewer to confirm.

Comment threadsrc/cli/commands/deploy/command.tsx
Comment threadsrc/cli/tui/screens/invoke/useInvokeFlow.ts Outdated
@github-actionsgithub-actionsBot removed the agentcore-harness-reviewing AgentCore Harness review in progress label May 22, 2026
@Hweinstock
Hweinstockforce-pushed the fix/tui-telemetry-init branch from 9456078 to bce4cf9CompareMay 22, 2026 03:32
@github-actionsgithub-actionsBot added size/l PR size: L and removed size/l PR size: L labels May 22, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 22, 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 22, 2026
@Hweinstock
Hweinstockforce-pushed the fix/tui-telemetry-init branch from bce4cf9 to 8d0f3b3CompareMay 22, 2026 03:33
Comment threadsrc/cli/commands/remove/command.tsx Outdated
const project = await configIO.readProjectSpec().catch(() => undefined);
const firstProtocol = project?.runtimes?.[0]?.protocol ?? 'unknown';

const result = await withCommandRunTelemetry(

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.

When i tested this PR locally, i noticed that the value in the emitted telemetry was always 1 (likely how long it took to load the config).

Before this change, the value was the actual duration of my entire invoke session.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Discussed in person, this is to be consistent with dev, and to avoid tying the duration to arbitrary user sessions.

Comment threadsrc/cli/tui/render.ts Outdated
process.stdout.write(SHOW_CURSOR);
}

await TelemetryClientAccessor.shutdown();

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.

wouldnt shutting down TelemetryClientAccessor here make it so if telemetry is emitted during a post-exit action (i.e. launchBrowserDev) then it would stop working?

@HweinstockHweinstockMay 26, 2026

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Yes, good call, this is important for when we add telemetry for the browser actions. I've swapped it to flush before the blocking process, then the shutdown should be handled by the global one in the finally block, if it reaches there. Shutdown is "best-effort" so its fine if it doesn't.

@Hweinstock
Hweinstockforce-pushed the fix/tui-telemetry-init branch from 7f6cf1a to 9bad38dCompareMay 26, 2026 17:36
@github-actionsgithub-actionsBot added size/l PR size: L and removed size/l PR size: L labels May 26, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 26, 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 26, 2026
Comment threadsrc/cli/commands/invoke/command.tsx Outdated
}
enterAltScreen: false,
actionOnBack: 'exit',
isInteractive: false,

@avi-alpertavi-alpertMay 26, 2026

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.

why was isInteractive switched to false here?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

good question. I made this change thinking that isInteractive=true meant exit after first success (as it does for other commands), and I didn't want the tui to exit early. However, turns out its just dead code:

isInteractive: _isInteractive,
. I think we can remove it as a followup.

@github-actionsgithub-actionsBot removed the size/l PR size: L label May 26, 2026
@github-actionsgithub-actionsBot added the size/l PR size: L label May 26, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 26, 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 26, 2026
@github-actionsgithub-actionsBot added size/l PR size: L and removed size/l PR size: L labels May 26, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 26, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

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

avi-alpert
avi-alpert previously approved these changes May 26, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

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

# Conflicts:
#	src/cli/cli.ts
#	src/cli/commands/invoke/command.tsx
#	src/cli/tui/screens/invoke/useInvokeFlow.ts
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

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

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

Labels

size/lPR size: L

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: duplicate header on entrypoint

3 participants

@Hweinstock@avi-alpert@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

fix: create global entrypoint for tui - #1365

Merged
Hweinstock merged 12 commits into
aws:mainfrom
Hweinstock:fix/tui-telemetry-init
May 27, 2026
Merged

fix: create global entrypoint for tui#1365
Hweinstock merged 12 commits into
aws:mainfrom
Hweinstock:fix/tui-telemetry-init

Conversation

@Hweinstock

@HweinstockHweinstock commented May 22, 2026

Copy link
Copy Markdown
Contributor

Description

Previously, commands like agentcore add, deploy, etc. rendered TUI screens inline via direct render() calls. This made it difficult to distinguish CLI vs TUI code paths, caused telemetry to be mislabeled as mode="cli" for interactive flows, and meant bare agentcore never initialized or shut down the telemetry client.

This PR creates a unified renderTUI() entrypoint that owns the telemetry lifecycle and replaces all inline render() calls. As part of this:

  • Extracted rendering logic into src/cli/tui/render.ts and shared notices into src/cli/notices.ts to break circular imports
  • Migrated add, deploy, create, remove, invoke to use renderTUI()
  • Made TelemetryClientAccessor.init() handle re-initialization safely
  • Preserved existing behavior (diffMode, isInteractive, actionOnBack)

This refactor fixes#895.

Type of Change

  • Bug fix
  • New feature

Testing

  • Typecheck, lint, unit tests pass
  • Manual E2E testing of affected commands

basic-flows.mov

Manually tested flows:

  • back on inline tui exists instead of going to help.
  • successful flow on tui exists instead of going back to help.
  • root level agentcore works as expected.
  • errors still propagate correctly in inline TUI.
  • telemetry is now emitted for both inline TUI and full screen.

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

@github-actionsgithub-actionsBot added the size/m PR size: M label May 22, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 22, 2026
@github-actionsgithub-actionsBot added the agentcore-harness-reviewing AgentCore Harness review in progress label May 22, 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 22, 2026
@github-actions

github-actionsBot commented May 22, 2026

Copy link
Copy Markdown
Contributor

Package Tarball

aws-agentcore-0.15.0.tgz

How to install

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

@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 unifying the TUI entrypoint — the RenderTUIOptions shape and InitialRoute typing are nice. I have a few concerns that I think need to be addressed before this merges.

Summary of issues

  1. Telemetry regression for invoke TUI: The PR removes the withCommandRunTelemetry('invoke', …) wrapper that previously wrapped the TUI invoke flow. No replacement was added, so cli.command_run (with has_stream/has_session_id/auth_type/agent_protocol/duration/exit_reason) is no longer emitted for agentcore invoke interactive mode. See inline comment.

  2. Behavior change: TUI flows no longer auto-exit on success when launched from CLI commands. Previously agentcore add, agentcore remove, etc. rendered their flows with isInteractive={false}, which makes screens like AddFlow, RemoveAllScreen, RemoveFlow, AddSuccessScreen, AddMemoryFlow, AddEvaluatorFlow, AddOnlineEvalFlow, AddIdentityFlow auto-exit after success (see e.g. AddFlow.tsx:206, RemoveAllScreen.tsx:30). Going through App now means they always receive isInteractive={true}, so the user has to press escape to leave after every successful run. See inline comment.

  3. Double TelemetryClientAccessor.init() for command-routed paths. When agentcore add (or any of the migrated commands) runs, main() calls TelemetryClientAccessor.init(args[0], 'cli') (cli.ts:271) and then the action handler calls renderTUI() which calls TelemetryClientAccessor.init(initialRoute.name, 'tui') (cli.ts:127). The first client (mode=cli) is overwritten in the singleton and never shutdown()'d. See inline comment.

  4. Circular import.src/cli/cli.ts imports registerInvoke/registerAdd/etc. from ./commands/*, and those command files now import renderTUI from '../../cli'. It works today because renderTUI is only referenced inside async action handlers, but it's fragile. See inline comment.

Items 1 and 3 are the most important — 1 is a clear telemetry regression and 3 partially undoes the telemetry-mode fix this PR is trying to land.

Comment threadsrc/cli/commands/invoke/command.tsx Outdated
Comment threadsrc/cli/tui/App.tsx Outdated
Comment threadsrc/cli/cli.ts Outdated
Comment threadsrc/cli/commands/invoke/command.tsx Outdated
@github-actionsgithub-actionsBot added size/l PR size: L and removed agentcore-harness-reviewing AgentCore Harness review in progress size/m PR size: M labels May 22, 2026
@HweinstockHweinstock reopened this May 22, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 22, 2026
@github-actionsgithub-actionsBot added size/l PR size: L agentcore-harness-reviewing AgentCore Harness review in progress and removed size/l PR size: L labels May 22, 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 22, 2026

@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 cleanup — the unified renderTUI() entrypoint is a nice improvement and the recent commits address most of the earlier review feedback (double init, isInteractive threading, render-tui module extraction).

Flagging two new issues below; both involve information being silently dropped on the way through renderTUI. The other pre-existing review comments still appear open but their underlying concerns look addressed in the latest commits — happy to leave those for the original reviewer to confirm.

Comment threadsrc/cli/commands/deploy/command.tsx
Comment threadsrc/cli/tui/screens/invoke/useInvokeFlow.ts Outdated
@github-actionsgithub-actionsBot removed the agentcore-harness-reviewing AgentCore Harness review in progress label May 22, 2026
@Hweinstock
Hweinstockforce-pushed the fix/tui-telemetry-init branch from 9456078 to bce4cf9CompareMay 22, 2026 03:32
@github-actionsgithub-actionsBot added size/l PR size: L and removed size/l PR size: L labels May 22, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 22, 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 22, 2026
@Hweinstock
Hweinstockforce-pushed the fix/tui-telemetry-init branch from bce4cf9 to 8d0f3b3CompareMay 22, 2026 03:33
Comment threadsrc/cli/commands/remove/command.tsx Outdated
const project = await configIO.readProjectSpec().catch(() => undefined);
const firstProtocol = project?.runtimes?.[0]?.protocol ?? 'unknown';

const result = await withCommandRunTelemetry(

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.

When i tested this PR locally, i noticed that the value in the emitted telemetry was always 1 (likely how long it took to load the config).

Before this change, the value was the actual duration of my entire invoke session.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Discussed in person, this is to be consistent with dev, and to avoid tying the duration to arbitrary user sessions.

Comment threadsrc/cli/tui/render.ts Outdated
process.stdout.write(SHOW_CURSOR);
}

await TelemetryClientAccessor.shutdown();

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.

wouldnt shutting down TelemetryClientAccessor here make it so if telemetry is emitted during a post-exit action (i.e. launchBrowserDev) then it would stop working?

@HweinstockHweinstockMay 26, 2026

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Yes, good call, this is important for when we add telemetry for the browser actions. I've swapped it to flush before the blocking process, then the shutdown should be handled by the global one in the finally block, if it reaches there. Shutdown is "best-effort" so its fine if it doesn't.

@Hweinstock
Hweinstockforce-pushed the fix/tui-telemetry-init branch from 7f6cf1a to 9bad38dCompareMay 26, 2026 17:36
@github-actionsgithub-actionsBot added size/l PR size: L and removed size/l PR size: L labels May 26, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 26, 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 26, 2026
Comment threadsrc/cli/commands/invoke/command.tsx Outdated
}
enterAltScreen: false,
actionOnBack: 'exit',
isInteractive: false,

@avi-alpertavi-alpertMay 26, 2026

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.

why was isInteractive switched to false here?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

good question. I made this change thinking that isInteractive=true meant exit after first success (as it does for other commands), and I didn't want the tui to exit early. However, turns out its just dead code:

isInteractive: _isInteractive,
. I think we can remove it as a followup.

@github-actionsgithub-actionsBot removed the size/l PR size: L label May 26, 2026
@github-actionsgithub-actionsBot added the size/l PR size: L label May 26, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 26, 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 26, 2026
@github-actionsgithub-actionsBot added size/l PR size: L and removed size/l PR size: L labels May 26, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 26, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

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

avi-alpert
avi-alpert previously approved these changes May 26, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

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

# Conflicts:
#	src/cli/cli.ts
#	src/cli/commands/invoke/command.tsx
#	src/cli/tui/screens/invoke/useInvokeFlow.ts
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

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

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

Labels

size/lPR size: L

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: duplicate header on entrypoint

3 participants

@Hweinstock@avi-alpert@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

fix: create global entrypoint for tui - #1365

Merged
Hweinstock merged 12 commits into
aws:mainfrom
Hweinstock:fix/tui-telemetry-init
May 27, 2026
Merged

fix: create global entrypoint for tui#1365
Hweinstock merged 12 commits into
aws:mainfrom
Hweinstock:fix/tui-telemetry-init

Conversation

@Hweinstock

@HweinstockHweinstock commented May 22, 2026

Copy link
Copy Markdown
Contributor

Description

Previously, commands like agentcore add, deploy, etc. rendered TUI screens inline via direct render() calls. This made it difficult to distinguish CLI vs TUI code paths, caused telemetry to be mislabeled as mode="cli" for interactive flows, and meant bare agentcore never initialized or shut down the telemetry client.

This PR creates a unified renderTUI() entrypoint that owns the telemetry lifecycle and replaces all inline render() calls. As part of this:

  • Extracted rendering logic into src/cli/tui/render.ts and shared notices into src/cli/notices.ts to break circular imports
  • Migrated add, deploy, create, remove, invoke to use renderTUI()
  • Made TelemetryClientAccessor.init() handle re-initialization safely
  • Preserved existing behavior (diffMode, isInteractive, actionOnBack)

This refactor fixes#895.

Type of Change

  • Bug fix
  • New feature

Testing

  • Typecheck, lint, unit tests pass
  • Manual E2E testing of affected commands

basic-flows.mov

Manually tested flows:

  • back on inline tui exists instead of going to help.
  • successful flow on tui exists instead of going back to help.
  • root level agentcore works as expected.
  • errors still propagate correctly in inline TUI.
  • telemetry is now emitted for both inline TUI and full screen.

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

@github-actionsgithub-actionsBot added the size/m PR size: M label May 22, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 22, 2026
@github-actionsgithub-actionsBot added the agentcore-harness-reviewing AgentCore Harness review in progress label May 22, 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 22, 2026
@github-actions

github-actionsBot commented May 22, 2026

Copy link
Copy Markdown
Contributor

Package Tarball

aws-agentcore-0.15.0.tgz

How to install

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

@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 unifying the TUI entrypoint — the RenderTUIOptions shape and InitialRoute typing are nice. I have a few concerns that I think need to be addressed before this merges.

Summary of issues

  1. Telemetry regression for invoke TUI: The PR removes the withCommandRunTelemetry('invoke', …) wrapper that previously wrapped the TUI invoke flow. No replacement was added, so cli.command_run (with has_stream/has_session_id/auth_type/agent_protocol/duration/exit_reason) is no longer emitted for agentcore invoke interactive mode. See inline comment.

  2. Behavior change: TUI flows no longer auto-exit on success when launched from CLI commands. Previously agentcore add, agentcore remove, etc. rendered their flows with isInteractive={false}, which makes screens like AddFlow, RemoveAllScreen, RemoveFlow, AddSuccessScreen, AddMemoryFlow, AddEvaluatorFlow, AddOnlineEvalFlow, AddIdentityFlow auto-exit after success (see e.g. AddFlow.tsx:206, RemoveAllScreen.tsx:30). Going through App now means they always receive isInteractive={true}, so the user has to press escape to leave after every successful run. See inline comment.

  3. Double TelemetryClientAccessor.init() for command-routed paths. When agentcore add (or any of the migrated commands) runs, main() calls TelemetryClientAccessor.init(args[0], 'cli') (cli.ts:271) and then the action handler calls renderTUI() which calls TelemetryClientAccessor.init(initialRoute.name, 'tui') (cli.ts:127). The first client (mode=cli) is overwritten in the singleton and never shutdown()'d. See inline comment.

  4. Circular import.src/cli/cli.ts imports registerInvoke/registerAdd/etc. from ./commands/*, and those command files now import renderTUI from '../../cli'. It works today because renderTUI is only referenced inside async action handlers, but it's fragile. See inline comment.

Items 1 and 3 are the most important — 1 is a clear telemetry regression and 3 partially undoes the telemetry-mode fix this PR is trying to land.

Comment threadsrc/cli/commands/invoke/command.tsx Outdated
Comment threadsrc/cli/tui/App.tsx Outdated
Comment threadsrc/cli/cli.ts Outdated
Comment threadsrc/cli/commands/invoke/command.tsx Outdated
@github-actionsgithub-actionsBot added size/l PR size: L and removed agentcore-harness-reviewing AgentCore Harness review in progress size/m PR size: M labels May 22, 2026
@HweinstockHweinstock reopened this May 22, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 22, 2026
@github-actionsgithub-actionsBot added size/l PR size: L agentcore-harness-reviewing AgentCore Harness review in progress and removed size/l PR size: L labels May 22, 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 22, 2026

@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 cleanup — the unified renderTUI() entrypoint is a nice improvement and the recent commits address most of the earlier review feedback (double init, isInteractive threading, render-tui module extraction).

Flagging two new issues below; both involve information being silently dropped on the way through renderTUI. The other pre-existing review comments still appear open but their underlying concerns look addressed in the latest commits — happy to leave those for the original reviewer to confirm.

Comment threadsrc/cli/commands/deploy/command.tsx
Comment threadsrc/cli/tui/screens/invoke/useInvokeFlow.ts Outdated
@github-actionsgithub-actionsBot removed the agentcore-harness-reviewing AgentCore Harness review in progress label May 22, 2026
@Hweinstock
Hweinstockforce-pushed the fix/tui-telemetry-init branch from 9456078 to bce4cf9CompareMay 22, 2026 03:32
@github-actionsgithub-actionsBot added size/l PR size: L and removed size/l PR size: L labels May 22, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 22, 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 22, 2026
@Hweinstock
Hweinstockforce-pushed the fix/tui-telemetry-init branch from bce4cf9 to 8d0f3b3CompareMay 22, 2026 03:33
Comment threadsrc/cli/commands/remove/command.tsx Outdated
const project = await configIO.readProjectSpec().catch(() => undefined);
const firstProtocol = project?.runtimes?.[0]?.protocol ?? 'unknown';

const result = await withCommandRunTelemetry(

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.

When i tested this PR locally, i noticed that the value in the emitted telemetry was always 1 (likely how long it took to load the config).

Before this change, the value was the actual duration of my entire invoke session.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Discussed in person, this is to be consistent with dev, and to avoid tying the duration to arbitrary user sessions.

Comment threadsrc/cli/tui/render.ts Outdated
process.stdout.write(SHOW_CURSOR);
}

await TelemetryClientAccessor.shutdown();

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.

wouldnt shutting down TelemetryClientAccessor here make it so if telemetry is emitted during a post-exit action (i.e. launchBrowserDev) then it would stop working?

@HweinstockHweinstockMay 26, 2026

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Yes, good call, this is important for when we add telemetry for the browser actions. I've swapped it to flush before the blocking process, then the shutdown should be handled by the global one in the finally block, if it reaches there. Shutdown is "best-effort" so its fine if it doesn't.

@Hweinstock
Hweinstockforce-pushed the fix/tui-telemetry-init branch from 7f6cf1a to 9bad38dCompareMay 26, 2026 17:36
@github-actionsgithub-actionsBot added size/l PR size: L and removed size/l PR size: L labels May 26, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 26, 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 26, 2026
Comment threadsrc/cli/commands/invoke/command.tsx Outdated
}
enterAltScreen: false,
actionOnBack: 'exit',
isInteractive: false,

@avi-alpertavi-alpertMay 26, 2026

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.

why was isInteractive switched to false here?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

good question. I made this change thinking that isInteractive=true meant exit after first success (as it does for other commands), and I didn't want the tui to exit early. However, turns out its just dead code:

isInteractive: _isInteractive,
. I think we can remove it as a followup.

@github-actionsgithub-actionsBot removed the size/l PR size: L label May 26, 2026
@github-actionsgithub-actionsBot added the size/l PR size: L label May 26, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 26, 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 26, 2026
@github-actionsgithub-actionsBot added size/l PR size: L and removed size/l PR size: L labels May 26, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 26, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

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

avi-alpert
avi-alpert previously approved these changes May 26, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

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

# Conflicts:
#	src/cli/cli.ts
#	src/cli/commands/invoke/command.tsx
#	src/cli/tui/screens/invoke/useInvokeFlow.ts
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

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

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

Labels

size/lPR size: L

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: duplicate header on entrypoint

3 participants

@Hweinstock@avi-alpert@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

fix: create global entrypoint for tui - #1365

Merged
Hweinstock merged 12 commits into
aws:mainfrom
Hweinstock:fix/tui-telemetry-init
May 27, 2026
Merged

fix: create global entrypoint for tui#1365
Hweinstock merged 12 commits into
aws:mainfrom
Hweinstock:fix/tui-telemetry-init

Conversation

@Hweinstock

@HweinstockHweinstock commented May 22, 2026

Copy link
Copy Markdown
Contributor

Description

Previously, commands like agentcore add, deploy, etc. rendered TUI screens inline via direct render() calls. This made it difficult to distinguish CLI vs TUI code paths, caused telemetry to be mislabeled as mode="cli" for interactive flows, and meant bare agentcore never initialized or shut down the telemetry client.

This PR creates a unified renderTUI() entrypoint that owns the telemetry lifecycle and replaces all inline render() calls. As part of this:

  • Extracted rendering logic into src/cli/tui/render.ts and shared notices into src/cli/notices.ts to break circular imports
  • Migrated add, deploy, create, remove, invoke to use renderTUI()
  • Made TelemetryClientAccessor.init() handle re-initialization safely
  • Preserved existing behavior (diffMode, isInteractive, actionOnBack)

This refactor fixes#895.

Type of Change

  • Bug fix
  • New feature

Testing

  • Typecheck, lint, unit tests pass
  • Manual E2E testing of affected commands

basic-flows.mov

Manually tested flows:

  • back on inline tui exists instead of going to help.
  • successful flow on tui exists instead of going back to help.
  • root level agentcore works as expected.
  • errors still propagate correctly in inline TUI.
  • telemetry is now emitted for both inline TUI and full screen.

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

@github-actionsgithub-actionsBot added the size/m PR size: M label May 22, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 22, 2026
@github-actionsgithub-actionsBot added the agentcore-harness-reviewing AgentCore Harness review in progress label May 22, 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 22, 2026
@github-actions

github-actionsBot commented May 22, 2026

Copy link
Copy Markdown
Contributor

Package Tarball

aws-agentcore-0.15.0.tgz

How to install

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

@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 unifying the TUI entrypoint — the RenderTUIOptions shape and InitialRoute typing are nice. I have a few concerns that I think need to be addressed before this merges.

Summary of issues

  1. Telemetry regression for invoke TUI: The PR removes the withCommandRunTelemetry('invoke', …) wrapper that previously wrapped the TUI invoke flow. No replacement was added, so cli.command_run (with has_stream/has_session_id/auth_type/agent_protocol/duration/exit_reason) is no longer emitted for agentcore invoke interactive mode. See inline comment.

  2. Behavior change: TUI flows no longer auto-exit on success when launched from CLI commands. Previously agentcore add, agentcore remove, etc. rendered their flows with isInteractive={false}, which makes screens like AddFlow, RemoveAllScreen, RemoveFlow, AddSuccessScreen, AddMemoryFlow, AddEvaluatorFlow, AddOnlineEvalFlow, AddIdentityFlow auto-exit after success (see e.g. AddFlow.tsx:206, RemoveAllScreen.tsx:30). Going through App now means they always receive isInteractive={true}, so the user has to press escape to leave after every successful run. See inline comment.

  3. Double TelemetryClientAccessor.init() for command-routed paths. When agentcore add (or any of the migrated commands) runs, main() calls TelemetryClientAccessor.init(args[0], 'cli') (cli.ts:271) and then the action handler calls renderTUI() which calls TelemetryClientAccessor.init(initialRoute.name, 'tui') (cli.ts:127). The first client (mode=cli) is overwritten in the singleton and never shutdown()'d. See inline comment.

  4. Circular import.src/cli/cli.ts imports registerInvoke/registerAdd/etc. from ./commands/*, and those command files now import renderTUI from '../../cli'. It works today because renderTUI is only referenced inside async action handlers, but it's fragile. See inline comment.

Items 1 and 3 are the most important — 1 is a clear telemetry regression and 3 partially undoes the telemetry-mode fix this PR is trying to land.

Comment threadsrc/cli/commands/invoke/command.tsx Outdated
Comment threadsrc/cli/tui/App.tsx Outdated
Comment threadsrc/cli/cli.ts Outdated
Comment threadsrc/cli/commands/invoke/command.tsx Outdated
@github-actionsgithub-actionsBot added size/l PR size: L and removed agentcore-harness-reviewing AgentCore Harness review in progress size/m PR size: M labels May 22, 2026
@HweinstockHweinstock reopened this May 22, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 22, 2026
@github-actionsgithub-actionsBot added size/l PR size: L agentcore-harness-reviewing AgentCore Harness review in progress and removed size/l PR size: L labels May 22, 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 22, 2026

@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 cleanup — the unified renderTUI() entrypoint is a nice improvement and the recent commits address most of the earlier review feedback (double init, isInteractive threading, render-tui module extraction).

Flagging two new issues below; both involve information being silently dropped on the way through renderTUI. The other pre-existing review comments still appear open but their underlying concerns look addressed in the latest commits — happy to leave those for the original reviewer to confirm.

Comment threadsrc/cli/commands/deploy/command.tsx
Comment threadsrc/cli/tui/screens/invoke/useInvokeFlow.ts Outdated
@github-actionsgithub-actionsBot removed the agentcore-harness-reviewing AgentCore Harness review in progress label May 22, 2026
@Hweinstock
Hweinstockforce-pushed the fix/tui-telemetry-init branch from 9456078 to bce4cf9CompareMay 22, 2026 03:32
@github-actionsgithub-actionsBot added size/l PR size: L and removed size/l PR size: L labels May 22, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 22, 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 22, 2026
@Hweinstock
Hweinstockforce-pushed the fix/tui-telemetry-init branch from bce4cf9 to 8d0f3b3CompareMay 22, 2026 03:33
Comment threadsrc/cli/commands/remove/command.tsx Outdated
const project = await configIO.readProjectSpec().catch(() => undefined);
const firstProtocol = project?.runtimes?.[0]?.protocol ?? 'unknown';

const result = await withCommandRunTelemetry(

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.

When i tested this PR locally, i noticed that the value in the emitted telemetry was always 1 (likely how long it took to load the config).

Before this change, the value was the actual duration of my entire invoke session.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Discussed in person, this is to be consistent with dev, and to avoid tying the duration to arbitrary user sessions.

Comment threadsrc/cli/tui/render.ts Outdated
process.stdout.write(SHOW_CURSOR);
}

await TelemetryClientAccessor.shutdown();

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.

wouldnt shutting down TelemetryClientAccessor here make it so if telemetry is emitted during a post-exit action (i.e. launchBrowserDev) then it would stop working?

@HweinstockHweinstockMay 26, 2026

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Yes, good call, this is important for when we add telemetry for the browser actions. I've swapped it to flush before the blocking process, then the shutdown should be handled by the global one in the finally block, if it reaches there. Shutdown is "best-effort" so its fine if it doesn't.

@Hweinstock
Hweinstockforce-pushed the fix/tui-telemetry-init branch from 7f6cf1a to 9bad38dCompareMay 26, 2026 17:36
@github-actionsgithub-actionsBot added size/l PR size: L and removed size/l PR size: L labels May 26, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 26, 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 26, 2026
Comment threadsrc/cli/commands/invoke/command.tsx Outdated
}
enterAltScreen: false,
actionOnBack: 'exit',
isInteractive: false,

@avi-alpertavi-alpertMay 26, 2026

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.

why was isInteractive switched to false here?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

good question. I made this change thinking that isInteractive=true meant exit after first success (as it does for other commands), and I didn't want the tui to exit early. However, turns out its just dead code:

isInteractive: _isInteractive,
. I think we can remove it as a followup.

@github-actionsgithub-actionsBot removed the size/l PR size: L label May 26, 2026
@github-actionsgithub-actionsBot added the size/l PR size: L label May 26, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 26, 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 26, 2026
@github-actionsgithub-actionsBot added size/l PR size: L and removed size/l PR size: L labels May 26, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 26, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

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

avi-alpert
avi-alpert previously approved these changes May 26, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

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

# Conflicts:
#	src/cli/cli.ts
#	src/cli/commands/invoke/command.tsx
#	src/cli/tui/screens/invoke/useInvokeFlow.ts
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

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

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

Labels

size/lPR size: L

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: duplicate header on entrypoint

3 participants

@Hweinstock@avi-alpert@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

fix: create global entrypoint for tui - #1365

Merged
Hweinstock merged 12 commits into
aws:mainfrom
Hweinstock:fix/tui-telemetry-init
May 27, 2026
Merged

fix: create global entrypoint for tui#1365
Hweinstock merged 12 commits into
aws:mainfrom
Hweinstock:fix/tui-telemetry-init

Conversation

@Hweinstock

@HweinstockHweinstock commented May 22, 2026

Copy link
Copy Markdown
Contributor

Description

Previously, commands like agentcore add, deploy, etc. rendered TUI screens inline via direct render() calls. This made it difficult to distinguish CLI vs TUI code paths, caused telemetry to be mislabeled as mode="cli" for interactive flows, and meant bare agentcore never initialized or shut down the telemetry client.

This PR creates a unified renderTUI() entrypoint that owns the telemetry lifecycle and replaces all inline render() calls. As part of this:

  • Extracted rendering logic into src/cli/tui/render.ts and shared notices into src/cli/notices.ts to break circular imports
  • Migrated add, deploy, create, remove, invoke to use renderTUI()
  • Made TelemetryClientAccessor.init() handle re-initialization safely
  • Preserved existing behavior (diffMode, isInteractive, actionOnBack)

This refactor fixes#895.

Type of Change

  • Bug fix
  • New feature

Testing

  • Typecheck, lint, unit tests pass
  • Manual E2E testing of affected commands

basic-flows.mov

Manually tested flows:

  • back on inline tui exists instead of going to help.
  • successful flow on tui exists instead of going back to help.
  • root level agentcore works as expected.
  • errors still propagate correctly in inline TUI.
  • telemetry is now emitted for both inline TUI and full screen.

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

@github-actionsgithub-actionsBot added the size/m PR size: M label May 22, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 22, 2026
@github-actionsgithub-actionsBot added the agentcore-harness-reviewing AgentCore Harness review in progress label May 22, 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 22, 2026
@github-actions

github-actionsBot commented May 22, 2026

Copy link
Copy Markdown
Contributor

Package Tarball

aws-agentcore-0.15.0.tgz

How to install

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

@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 unifying the TUI entrypoint — the RenderTUIOptions shape and InitialRoute typing are nice. I have a few concerns that I think need to be addressed before this merges.

Summary of issues

  1. Telemetry regression for invoke TUI: The PR removes the withCommandRunTelemetry('invoke', …) wrapper that previously wrapped the TUI invoke flow. No replacement was added, so cli.command_run (with has_stream/has_session_id/auth_type/agent_protocol/duration/exit_reason) is no longer emitted for agentcore invoke interactive mode. See inline comment.

  2. Behavior change: TUI flows no longer auto-exit on success when launched from CLI commands. Previously agentcore add, agentcore remove, etc. rendered their flows with isInteractive={false}, which makes screens like AddFlow, RemoveAllScreen, RemoveFlow, AddSuccessScreen, AddMemoryFlow, AddEvaluatorFlow, AddOnlineEvalFlow, AddIdentityFlow auto-exit after success (see e.g. AddFlow.tsx:206, RemoveAllScreen.tsx:30). Going through App now means they always receive isInteractive={true}, so the user has to press escape to leave after every successful run. See inline comment.

  3. Double TelemetryClientAccessor.init() for command-routed paths. When agentcore add (or any of the migrated commands) runs, main() calls TelemetryClientAccessor.init(args[0], 'cli') (cli.ts:271) and then the action handler calls renderTUI() which calls TelemetryClientAccessor.init(initialRoute.name, 'tui') (cli.ts:127). The first client (mode=cli) is overwritten in the singleton and never shutdown()'d. See inline comment.

  4. Circular import.src/cli/cli.ts imports registerInvoke/registerAdd/etc. from ./commands/*, and those command files now import renderTUI from '../../cli'. It works today because renderTUI is only referenced inside async action handlers, but it's fragile. See inline comment.

Items 1 and 3 are the most important — 1 is a clear telemetry regression and 3 partially undoes the telemetry-mode fix this PR is trying to land.

Comment threadsrc/cli/commands/invoke/command.tsx Outdated
Comment threadsrc/cli/tui/App.tsx Outdated
Comment threadsrc/cli/cli.ts Outdated
Comment threadsrc/cli/commands/invoke/command.tsx Outdated
@github-actionsgithub-actionsBot added size/l PR size: L and removed agentcore-harness-reviewing AgentCore Harness review in progress size/m PR size: M labels May 22, 2026
@HweinstockHweinstock reopened this May 22, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 22, 2026
@github-actionsgithub-actionsBot added size/l PR size: L agentcore-harness-reviewing AgentCore Harness review in progress and removed size/l PR size: L labels May 22, 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 22, 2026

@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 cleanup — the unified renderTUI() entrypoint is a nice improvement and the recent commits address most of the earlier review feedback (double init, isInteractive threading, render-tui module extraction).

Flagging two new issues below; both involve information being silently dropped on the way through renderTUI. The other pre-existing review comments still appear open but their underlying concerns look addressed in the latest commits — happy to leave those for the original reviewer to confirm.

Comment threadsrc/cli/commands/deploy/command.tsx
Comment threadsrc/cli/tui/screens/invoke/useInvokeFlow.ts Outdated
@github-actionsgithub-actionsBot removed the agentcore-harness-reviewing AgentCore Harness review in progress label May 22, 2026
@Hweinstock
Hweinstockforce-pushed the fix/tui-telemetry-init branch from 9456078 to bce4cf9CompareMay 22, 2026 03:32
@github-actionsgithub-actionsBot added size/l PR size: L and removed size/l PR size: L labels May 22, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 22, 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 22, 2026
@Hweinstock
Hweinstockforce-pushed the fix/tui-telemetry-init branch from bce4cf9 to 8d0f3b3CompareMay 22, 2026 03:33
Comment threadsrc/cli/commands/remove/command.tsx Outdated
const project = await configIO.readProjectSpec().catch(() => undefined);
const firstProtocol = project?.runtimes?.[0]?.protocol ?? 'unknown';

const result = await withCommandRunTelemetry(

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.

When i tested this PR locally, i noticed that the value in the emitted telemetry was always 1 (likely how long it took to load the config).

Before this change, the value was the actual duration of my entire invoke session.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Discussed in person, this is to be consistent with dev, and to avoid tying the duration to arbitrary user sessions.

Comment threadsrc/cli/tui/render.ts Outdated
process.stdout.write(SHOW_CURSOR);
}

await TelemetryClientAccessor.shutdown();

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.

wouldnt shutting down TelemetryClientAccessor here make it so if telemetry is emitted during a post-exit action (i.e. launchBrowserDev) then it would stop working?

@HweinstockHweinstockMay 26, 2026

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Yes, good call, this is important for when we add telemetry for the browser actions. I've swapped it to flush before the blocking process, then the shutdown should be handled by the global one in the finally block, if it reaches there. Shutdown is "best-effort" so its fine if it doesn't.

@Hweinstock
Hweinstockforce-pushed the fix/tui-telemetry-init branch from 7f6cf1a to 9bad38dCompareMay 26, 2026 17:36
@github-actionsgithub-actionsBot added size/l PR size: L and removed size/l PR size: L labels May 26, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 26, 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 26, 2026
Comment threadsrc/cli/commands/invoke/command.tsx Outdated
}
enterAltScreen: false,
actionOnBack: 'exit',
isInteractive: false,

@avi-alpertavi-alpertMay 26, 2026

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.

why was isInteractive switched to false here?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

good question. I made this change thinking that isInteractive=true meant exit after first success (as it does for other commands), and I didn't want the tui to exit early. However, turns out its just dead code:

isInteractive: _isInteractive,
. I think we can remove it as a followup.

@github-actionsgithub-actionsBot removed the size/l PR size: L label May 26, 2026
@github-actionsgithub-actionsBot added the size/l PR size: L label May 26, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 26, 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 26, 2026
@github-actionsgithub-actionsBot added size/l PR size: L and removed size/l PR size: L labels May 26, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 26, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

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

avi-alpert
avi-alpert previously approved these changes May 26, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

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

# Conflicts:
#	src/cli/cli.ts
#	src/cli/commands/invoke/command.tsx
#	src/cli/tui/screens/invoke/useInvokeFlow.ts
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

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

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

Labels

size/lPR size: L

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: duplicate header on entrypoint

3 participants

@Hweinstock@avi-alpert@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

fix: create global entrypoint for tui - #1365

Merged
Hweinstock merged 12 commits into
aws:mainfrom
Hweinstock:fix/tui-telemetry-init
May 27, 2026
Merged

fix: create global entrypoint for tui#1365
Hweinstock merged 12 commits into
aws:mainfrom
Hweinstock:fix/tui-telemetry-init

Conversation

@Hweinstock

@HweinstockHweinstock commented May 22, 2026

Copy link
Copy Markdown
Contributor

Description

Previously, commands like agentcore add, deploy, etc. rendered TUI screens inline via direct render() calls. This made it difficult to distinguish CLI vs TUI code paths, caused telemetry to be mislabeled as mode="cli" for interactive flows, and meant bare agentcore never initialized or shut down the telemetry client.

This PR creates a unified renderTUI() entrypoint that owns the telemetry lifecycle and replaces all inline render() calls. As part of this:

  • Extracted rendering logic into src/cli/tui/render.ts and shared notices into src/cli/notices.ts to break circular imports
  • Migrated add, deploy, create, remove, invoke to use renderTUI()
  • Made TelemetryClientAccessor.init() handle re-initialization safely
  • Preserved existing behavior (diffMode, isInteractive, actionOnBack)

This refactor fixes#895.

Type of Change

  • Bug fix
  • New feature

Testing

  • Typecheck, lint, unit tests pass
  • Manual E2E testing of affected commands

basic-flows.mov

Manually tested flows:

  • back on inline tui exists instead of going to help.
  • successful flow on tui exists instead of going back to help.
  • root level agentcore works as expected.
  • errors still propagate correctly in inline TUI.
  • telemetry is now emitted for both inline TUI and full screen.

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

@github-actionsgithub-actionsBot added the size/m PR size: M label May 22, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 22, 2026
@github-actionsgithub-actionsBot added the agentcore-harness-reviewing AgentCore Harness review in progress label May 22, 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 22, 2026
@github-actions

github-actionsBot commented May 22, 2026

Copy link
Copy Markdown
Contributor

Package Tarball

aws-agentcore-0.15.0.tgz

How to install

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

@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 unifying the TUI entrypoint — the RenderTUIOptions shape and InitialRoute typing are nice. I have a few concerns that I think need to be addressed before this merges.

Summary of issues

  1. Telemetry regression for invoke TUI: The PR removes the withCommandRunTelemetry('invoke', …) wrapper that previously wrapped the TUI invoke flow. No replacement was added, so cli.command_run (with has_stream/has_session_id/auth_type/agent_protocol/duration/exit_reason) is no longer emitted for agentcore invoke interactive mode. See inline comment.

  2. Behavior change: TUI flows no longer auto-exit on success when launched from CLI commands. Previously agentcore add, agentcore remove, etc. rendered their flows with isInteractive={false}, which makes screens like AddFlow, RemoveAllScreen, RemoveFlow, AddSuccessScreen, AddMemoryFlow, AddEvaluatorFlow, AddOnlineEvalFlow, AddIdentityFlow auto-exit after success (see e.g. AddFlow.tsx:206, RemoveAllScreen.tsx:30). Going through App now means they always receive isInteractive={true}, so the user has to press escape to leave after every successful run. See inline comment.

  3. Double TelemetryClientAccessor.init() for command-routed paths. When agentcore add (or any of the migrated commands) runs, main() calls TelemetryClientAccessor.init(args[0], 'cli') (cli.ts:271) and then the action handler calls renderTUI() which calls TelemetryClientAccessor.init(initialRoute.name, 'tui') (cli.ts:127). The first client (mode=cli) is overwritten in the singleton and never shutdown()'d. See inline comment.

  4. Circular import.src/cli/cli.ts imports registerInvoke/registerAdd/etc. from ./commands/*, and those command files now import renderTUI from '../../cli'. It works today because renderTUI is only referenced inside async action handlers, but it's fragile. See inline comment.

Items 1 and 3 are the most important — 1 is a clear telemetry regression and 3 partially undoes the telemetry-mode fix this PR is trying to land.

Comment threadsrc/cli/commands/invoke/command.tsx Outdated
Comment threadsrc/cli/tui/App.tsx Outdated
Comment threadsrc/cli/cli.ts Outdated
Comment threadsrc/cli/commands/invoke/command.tsx Outdated
@github-actionsgithub-actionsBot added size/l PR size: L and removed agentcore-harness-reviewing AgentCore Harness review in progress size/m PR size: M labels May 22, 2026
@HweinstockHweinstock reopened this May 22, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 22, 2026
@github-actionsgithub-actionsBot added size/l PR size: L agentcore-harness-reviewing AgentCore Harness review in progress and removed size/l PR size: L labels May 22, 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 22, 2026

@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 cleanup — the unified renderTUI() entrypoint is a nice improvement and the recent commits address most of the earlier review feedback (double init, isInteractive threading, render-tui module extraction).

Flagging two new issues below; both involve information being silently dropped on the way through renderTUI. The other pre-existing review comments still appear open but their underlying concerns look addressed in the latest commits — happy to leave those for the original reviewer to confirm.

Comment threadsrc/cli/commands/deploy/command.tsx
Comment threadsrc/cli/tui/screens/invoke/useInvokeFlow.ts Outdated
@github-actionsgithub-actionsBot removed the agentcore-harness-reviewing AgentCore Harness review in progress label May 22, 2026
@Hweinstock
Hweinstockforce-pushed the fix/tui-telemetry-init branch from 9456078 to bce4cf9CompareMay 22, 2026 03:32
@github-actionsgithub-actionsBot added size/l PR size: L and removed size/l PR size: L labels May 22, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 22, 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 22, 2026
@Hweinstock
Hweinstockforce-pushed the fix/tui-telemetry-init branch from bce4cf9 to 8d0f3b3CompareMay 22, 2026 03:33
Comment threadsrc/cli/commands/remove/command.tsx Outdated
const project = await configIO.readProjectSpec().catch(() => undefined);
const firstProtocol = project?.runtimes?.[0]?.protocol ?? 'unknown';

const result = await withCommandRunTelemetry(

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.

When i tested this PR locally, i noticed that the value in the emitted telemetry was always 1 (likely how long it took to load the config).

Before this change, the value was the actual duration of my entire invoke session.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Discussed in person, this is to be consistent with dev, and to avoid tying the duration to arbitrary user sessions.

Comment threadsrc/cli/tui/render.ts Outdated
process.stdout.write(SHOW_CURSOR);
}

await TelemetryClientAccessor.shutdown();

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.

wouldnt shutting down TelemetryClientAccessor here make it so if telemetry is emitted during a post-exit action (i.e. launchBrowserDev) then it would stop working?

@HweinstockHweinstockMay 26, 2026

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Yes, good call, this is important for when we add telemetry for the browser actions. I've swapped it to flush before the blocking process, then the shutdown should be handled by the global one in the finally block, if it reaches there. Shutdown is "best-effort" so its fine if it doesn't.

@Hweinstock
Hweinstockforce-pushed the fix/tui-telemetry-init branch from 7f6cf1a to 9bad38dCompareMay 26, 2026 17:36
@github-actionsgithub-actionsBot added size/l PR size: L and removed size/l PR size: L labels May 26, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 26, 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 26, 2026
Comment threadsrc/cli/commands/invoke/command.tsx Outdated
}
enterAltScreen: false,
actionOnBack: 'exit',
isInteractive: false,

@avi-alpertavi-alpertMay 26, 2026

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.

why was isInteractive switched to false here?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

good question. I made this change thinking that isInteractive=true meant exit after first success (as it does for other commands), and I didn't want the tui to exit early. However, turns out its just dead code:

isInteractive: _isInteractive,
. I think we can remove it as a followup.

@github-actionsgithub-actionsBot removed the size/l PR size: L label May 26, 2026
@github-actionsgithub-actionsBot added the size/l PR size: L label May 26, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 26, 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 26, 2026
@github-actionsgithub-actionsBot added size/l PR size: L and removed size/l PR size: L labels May 26, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 26, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

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

avi-alpert
avi-alpert previously approved these changes May 26, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

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

# Conflicts:
#	src/cli/cli.ts
#	src/cli/commands/invoke/command.tsx
#	src/cli/tui/screens/invoke/useInvokeFlow.ts
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

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

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

Labels

size/lPR size: L

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: duplicate header on entrypoint

3 participants

@Hweinstock@avi-alpert@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

fix: create global entrypoint for tui - #1365

Merged
Hweinstock merged 12 commits into
aws:mainfrom
Hweinstock:fix/tui-telemetry-init
May 27, 2026
Merged

fix: create global entrypoint for tui#1365
Hweinstock merged 12 commits into
aws:mainfrom
Hweinstock:fix/tui-telemetry-init

Conversation

@Hweinstock

@HweinstockHweinstock commented May 22, 2026

Copy link
Copy Markdown
Contributor

Description

Previously, commands like agentcore add, deploy, etc. rendered TUI screens inline via direct render() calls. This made it difficult to distinguish CLI vs TUI code paths, caused telemetry to be mislabeled as mode="cli" for interactive flows, and meant bare agentcore never initialized or shut down the telemetry client.

This PR creates a unified renderTUI() entrypoint that owns the telemetry lifecycle and replaces all inline render() calls. As part of this:

  • Extracted rendering logic into src/cli/tui/render.ts and shared notices into src/cli/notices.ts to break circular imports
  • Migrated add, deploy, create, remove, invoke to use renderTUI()
  • Made TelemetryClientAccessor.init() handle re-initialization safely
  • Preserved existing behavior (diffMode, isInteractive, actionOnBack)

This refactor fixes#895.

Type of Change

  • Bug fix
  • New feature

Testing

  • Typecheck, lint, unit tests pass
  • Manual E2E testing of affected commands

basic-flows.mov

Manually tested flows:

  • back on inline tui exists instead of going to help.
  • successful flow on tui exists instead of going back to help.
  • root level agentcore works as expected.
  • errors still propagate correctly in inline TUI.
  • telemetry is now emitted for both inline TUI and full screen.

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

@github-actionsgithub-actionsBot added the size/m PR size: M label May 22, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 22, 2026
@github-actionsgithub-actionsBot added the agentcore-harness-reviewing AgentCore Harness review in progress label May 22, 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 22, 2026
@github-actions

github-actionsBot commented May 22, 2026

Copy link
Copy Markdown
Contributor

Package Tarball

aws-agentcore-0.15.0.tgz

How to install

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

@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 unifying the TUI entrypoint — the RenderTUIOptions shape and InitialRoute typing are nice. I have a few concerns that I think need to be addressed before this merges.

Summary of issues

  1. Telemetry regression for invoke TUI: The PR removes the withCommandRunTelemetry('invoke', …) wrapper that previously wrapped the TUI invoke flow. No replacement was added, so cli.command_run (with has_stream/has_session_id/auth_type/agent_protocol/duration/exit_reason) is no longer emitted for agentcore invoke interactive mode. See inline comment.

  2. Behavior change: TUI flows no longer auto-exit on success when launched from CLI commands. Previously agentcore add, agentcore remove, etc. rendered their flows with isInteractive={false}, which makes screens like AddFlow, RemoveAllScreen, RemoveFlow, AddSuccessScreen, AddMemoryFlow, AddEvaluatorFlow, AddOnlineEvalFlow, AddIdentityFlow auto-exit after success (see e.g. AddFlow.tsx:206, RemoveAllScreen.tsx:30). Going through App now means they always receive isInteractive={true}, so the user has to press escape to leave after every successful run. See inline comment.

  3. Double TelemetryClientAccessor.init() for command-routed paths. When agentcore add (or any of the migrated commands) runs, main() calls TelemetryClientAccessor.init(args[0], 'cli') (cli.ts:271) and then the action handler calls renderTUI() which calls TelemetryClientAccessor.init(initialRoute.name, 'tui') (cli.ts:127). The first client (mode=cli) is overwritten in the singleton and never shutdown()'d. See inline comment.

  4. Circular import.src/cli/cli.ts imports registerInvoke/registerAdd/etc. from ./commands/*, and those command files now import renderTUI from '../../cli'. It works today because renderTUI is only referenced inside async action handlers, but it's fragile. See inline comment.

Items 1 and 3 are the most important — 1 is a clear telemetry regression and 3 partially undoes the telemetry-mode fix this PR is trying to land.

Comment threadsrc/cli/commands/invoke/command.tsx Outdated
Comment threadsrc/cli/tui/App.tsx Outdated
Comment threadsrc/cli/cli.ts Outdated
Comment threadsrc/cli/commands/invoke/command.tsx Outdated
@github-actionsgithub-actionsBot added size/l PR size: L and removed agentcore-harness-reviewing AgentCore Harness review in progress size/m PR size: M labels May 22, 2026
@HweinstockHweinstock reopened this May 22, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 22, 2026
@github-actionsgithub-actionsBot added size/l PR size: L agentcore-harness-reviewing AgentCore Harness review in progress and removed size/l PR size: L labels May 22, 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 22, 2026

@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 cleanup — the unified renderTUI() entrypoint is a nice improvement and the recent commits address most of the earlier review feedback (double init, isInteractive threading, render-tui module extraction).

Flagging two new issues below; both involve information being silently dropped on the way through renderTUI. The other pre-existing review comments still appear open but their underlying concerns look addressed in the latest commits — happy to leave those for the original reviewer to confirm.

Comment threadsrc/cli/commands/deploy/command.tsx
Comment threadsrc/cli/tui/screens/invoke/useInvokeFlow.ts Outdated
@github-actionsgithub-actionsBot removed the agentcore-harness-reviewing AgentCore Harness review in progress label May 22, 2026
@Hweinstock
Hweinstockforce-pushed the fix/tui-telemetry-init branch from 9456078 to bce4cf9CompareMay 22, 2026 03:32
@github-actionsgithub-actionsBot added size/l PR size: L and removed size/l PR size: L labels May 22, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 22, 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 22, 2026
@Hweinstock
Hweinstockforce-pushed the fix/tui-telemetry-init branch from bce4cf9 to 8d0f3b3CompareMay 22, 2026 03:33
Comment threadsrc/cli/commands/remove/command.tsx Outdated
const project = await configIO.readProjectSpec().catch(() => undefined);
const firstProtocol = project?.runtimes?.[0]?.protocol ?? 'unknown';

const result = await withCommandRunTelemetry(

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.

When i tested this PR locally, i noticed that the value in the emitted telemetry was always 1 (likely how long it took to load the config).

Before this change, the value was the actual duration of my entire invoke session.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Discussed in person, this is to be consistent with dev, and to avoid tying the duration to arbitrary user sessions.

Comment threadsrc/cli/tui/render.ts Outdated
process.stdout.write(SHOW_CURSOR);
}

await TelemetryClientAccessor.shutdown();

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.

wouldnt shutting down TelemetryClientAccessor here make it so if telemetry is emitted during a post-exit action (i.e. launchBrowserDev) then it would stop working?

@HweinstockHweinstockMay 26, 2026

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Yes, good call, this is important for when we add telemetry for the browser actions. I've swapped it to flush before the blocking process, then the shutdown should be handled by the global one in the finally block, if it reaches there. Shutdown is "best-effort" so its fine if it doesn't.

@Hweinstock
Hweinstockforce-pushed the fix/tui-telemetry-init branch from 7f6cf1a to 9bad38dCompareMay 26, 2026 17:36
@github-actionsgithub-actionsBot added size/l PR size: L and removed size/l PR size: L labels May 26, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 26, 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 26, 2026
Comment threadsrc/cli/commands/invoke/command.tsx Outdated
}
enterAltScreen: false,
actionOnBack: 'exit',
isInteractive: false,

@avi-alpertavi-alpertMay 26, 2026

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.

why was isInteractive switched to false here?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

good question. I made this change thinking that isInteractive=true meant exit after first success (as it does for other commands), and I didn't want the tui to exit early. However, turns out its just dead code:

isInteractive: _isInteractive,
. I think we can remove it as a followup.

@github-actionsgithub-actionsBot removed the size/l PR size: L label May 26, 2026
@github-actionsgithub-actionsBot added the size/l PR size: L label May 26, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 26, 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 26, 2026
@github-actionsgithub-actionsBot added size/l PR size: L and removed size/l PR size: L labels May 26, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 26, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

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

avi-alpert
avi-alpert previously approved these changes May 26, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

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

# Conflicts:
#	src/cli/cli.ts
#	src/cli/commands/invoke/command.tsx
#	src/cli/tui/screens/invoke/useInvokeFlow.ts
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

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

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

Labels

size/lPR size: L

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: duplicate header on entrypoint

3 participants

@Hweinstock@avi-alpert@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

fix: create global entrypoint for tui - #1365

Merged
Hweinstock merged 12 commits into
aws:mainfrom
Hweinstock:fix/tui-telemetry-init
May 27, 2026
Merged

fix: create global entrypoint for tui#1365
Hweinstock merged 12 commits into
aws:mainfrom
Hweinstock:fix/tui-telemetry-init

Conversation

@Hweinstock

@HweinstockHweinstock commented May 22, 2026

Copy link
Copy Markdown
Contributor

Description

Previously, commands like agentcore add, deploy, etc. rendered TUI screens inline via direct render() calls. This made it difficult to distinguish CLI vs TUI code paths, caused telemetry to be mislabeled as mode="cli" for interactive flows, and meant bare agentcore never initialized or shut down the telemetry client.

This PR creates a unified renderTUI() entrypoint that owns the telemetry lifecycle and replaces all inline render() calls. As part of this:

  • Extracted rendering logic into src/cli/tui/render.ts and shared notices into src/cli/notices.ts to break circular imports
  • Migrated add, deploy, create, remove, invoke to use renderTUI()
  • Made TelemetryClientAccessor.init() handle re-initialization safely
  • Preserved existing behavior (diffMode, isInteractive, actionOnBack)

This refactor fixes#895.

Type of Change

  • Bug fix
  • New feature

Testing

  • Typecheck, lint, unit tests pass
  • Manual E2E testing of affected commands

basic-flows.mov

Manually tested flows:

  • back on inline tui exists instead of going to help.
  • successful flow on tui exists instead of going back to help.
  • root level agentcore works as expected.
  • errors still propagate correctly in inline TUI.
  • telemetry is now emitted for both inline TUI and full screen.

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

@github-actionsgithub-actionsBot added the size/m PR size: M label May 22, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 22, 2026
@github-actionsgithub-actionsBot added the agentcore-harness-reviewing AgentCore Harness review in progress label May 22, 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 22, 2026
@github-actions

github-actionsBot commented May 22, 2026

Copy link
Copy Markdown
Contributor

Package Tarball

aws-agentcore-0.15.0.tgz

How to install

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

@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 unifying the TUI entrypoint — the RenderTUIOptions shape and InitialRoute typing are nice. I have a few concerns that I think need to be addressed before this merges.

Summary of issues

  1. Telemetry regression for invoke TUI: The PR removes the withCommandRunTelemetry('invoke', …) wrapper that previously wrapped the TUI invoke flow. No replacement was added, so cli.command_run (with has_stream/has_session_id/auth_type/agent_protocol/duration/exit_reason) is no longer emitted for agentcore invoke interactive mode. See inline comment.

  2. Behavior change: TUI flows no longer auto-exit on success when launched from CLI commands. Previously agentcore add, agentcore remove, etc. rendered their flows with isInteractive={false}, which makes screens like AddFlow, RemoveAllScreen, RemoveFlow, AddSuccessScreen, AddMemoryFlow, AddEvaluatorFlow, AddOnlineEvalFlow, AddIdentityFlow auto-exit after success (see e.g. AddFlow.tsx:206, RemoveAllScreen.tsx:30). Going through App now means they always receive isInteractive={true}, so the user has to press escape to leave after every successful run. See inline comment.

  3. Double TelemetryClientAccessor.init() for command-routed paths. When agentcore add (or any of the migrated commands) runs, main() calls TelemetryClientAccessor.init(args[0], 'cli') (cli.ts:271) and then the action handler calls renderTUI() which calls TelemetryClientAccessor.init(initialRoute.name, 'tui') (cli.ts:127). The first client (mode=cli) is overwritten in the singleton and never shutdown()'d. See inline comment.

  4. Circular import.src/cli/cli.ts imports registerInvoke/registerAdd/etc. from ./commands/*, and those command files now import renderTUI from '../../cli'. It works today because renderTUI is only referenced inside async action handlers, but it's fragile. See inline comment.

Items 1 and 3 are the most important — 1 is a clear telemetry regression and 3 partially undoes the telemetry-mode fix this PR is trying to land.

Comment threadsrc/cli/commands/invoke/command.tsx Outdated
Comment threadsrc/cli/tui/App.tsx Outdated
Comment threadsrc/cli/cli.ts Outdated
Comment threadsrc/cli/commands/invoke/command.tsx Outdated
@github-actionsgithub-actionsBot added size/l PR size: L and removed agentcore-harness-reviewing AgentCore Harness review in progress size/m PR size: M labels May 22, 2026
@HweinstockHweinstock reopened this May 22, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 22, 2026
@github-actionsgithub-actionsBot added size/l PR size: L agentcore-harness-reviewing AgentCore Harness review in progress and removed size/l PR size: L labels May 22, 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 22, 2026

@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 cleanup — the unified renderTUI() entrypoint is a nice improvement and the recent commits address most of the earlier review feedback (double init, isInteractive threading, render-tui module extraction).

Flagging two new issues below; both involve information being silently dropped on the way through renderTUI. The other pre-existing review comments still appear open but their underlying concerns look addressed in the latest commits — happy to leave those for the original reviewer to confirm.

Comment threadsrc/cli/commands/deploy/command.tsx
Comment threadsrc/cli/tui/screens/invoke/useInvokeFlow.ts Outdated
@github-actionsgithub-actionsBot removed the agentcore-harness-reviewing AgentCore Harness review in progress label May 22, 2026
@Hweinstock
Hweinstockforce-pushed the fix/tui-telemetry-init branch from 9456078 to bce4cf9CompareMay 22, 2026 03:32
@github-actionsgithub-actionsBot added size/l PR size: L and removed size/l PR size: L labels May 22, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 22, 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 22, 2026
@Hweinstock
Hweinstockforce-pushed the fix/tui-telemetry-init branch from bce4cf9 to 8d0f3b3CompareMay 22, 2026 03:33
Comment threadsrc/cli/commands/remove/command.tsx Outdated
const project = await configIO.readProjectSpec().catch(() => undefined);
const firstProtocol = project?.runtimes?.[0]?.protocol ?? 'unknown';

const result = await withCommandRunTelemetry(

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.

When i tested this PR locally, i noticed that the value in the emitted telemetry was always 1 (likely how long it took to load the config).

Before this change, the value was the actual duration of my entire invoke session.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Discussed in person, this is to be consistent with dev, and to avoid tying the duration to arbitrary user sessions.

Comment threadsrc/cli/tui/render.ts Outdated
process.stdout.write(SHOW_CURSOR);
}

await TelemetryClientAccessor.shutdown();

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.

wouldnt shutting down TelemetryClientAccessor here make it so if telemetry is emitted during a post-exit action (i.e. launchBrowserDev) then it would stop working?

@HweinstockHweinstockMay 26, 2026

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Yes, good call, this is important for when we add telemetry for the browser actions. I've swapped it to flush before the blocking process, then the shutdown should be handled by the global one in the finally block, if it reaches there. Shutdown is "best-effort" so its fine if it doesn't.

@Hweinstock
Hweinstockforce-pushed the fix/tui-telemetry-init branch from 7f6cf1a to 9bad38dCompareMay 26, 2026 17:36
@github-actionsgithub-actionsBot added size/l PR size: L and removed size/l PR size: L labels May 26, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 26, 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 26, 2026
Comment threadsrc/cli/commands/invoke/command.tsx Outdated
}
enterAltScreen: false,
actionOnBack: 'exit',
isInteractive: false,

@avi-alpertavi-alpertMay 26, 2026

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.

why was isInteractive switched to false here?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

good question. I made this change thinking that isInteractive=true meant exit after first success (as it does for other commands), and I didn't want the tui to exit early. However, turns out its just dead code:

isInteractive: _isInteractive,
. I think we can remove it as a followup.

@github-actionsgithub-actionsBot removed the size/l PR size: L label May 26, 2026
@github-actionsgithub-actionsBot added the size/l PR size: L label May 26, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 26, 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 26, 2026
@github-actionsgithub-actionsBot added size/l PR size: L and removed size/l PR size: L labels May 26, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label May 26, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

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

avi-alpert
avi-alpert previously approved these changes May 26, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

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

# Conflicts:
#	src/cli/cli.ts
#	src/cli/commands/invoke/command.tsx
#	src/cli/tui/screens/invoke/useInvokeFlow.ts
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

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

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

Labels

size/lPR size: L

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: duplicate header on entrypoint

3 participants

@Hweinstock@avi-alpert@agentcore-cli-automation