iframe-proxy

aidandaly24 · 2026-06-25T15:07:00Z

Fixes #982

Problem

Running an interactive TUI command in a non-TTY context (piped stdin, CI, backgrounded) crashes with Ink's "Raw mode is not supported on the current process.stdin" stack trace. For agentcore import (no --source) the process then exits 0, so CI/scripts can't detect the failure; agentcore view <type>, agentcore export harness, and the agentcore dev browser-mode harness picker hit the same unguarded launch.

Fix

Guard every interactive launch with requireTTY() (prints a clear "requires an interactive terminal" message and exits non-zero):

Centralized in renderTUI() so any renderTUI-based launch (export, and future commands) is covered by default.
Inline guards on the raw render() sites: import and view.
Guard on the dev browser-mode picker (launchTuiDevScreenWithPicker).
requireProject / requireTTY ordering made consistent across sites.

Validation

All four interactive paths (import, view, export, dev picker) now exit 1 with the plain-text error and no Ink stack trace in a non-TTY context; the TTY path is unchanged. Tests drive the real requireTTY (toggling process.stdin/stdout.isTTY, spying on process.exit/console.error), verified non-tautological by mutation (remove the guard → test fails). Unit suite green.

) Non-TTY interactive TUI commands crashed with Ink's 'Raw mode is not supported on the current process.stdin' and, for import (no --source), exited 0 so CI/scripts could not detect the failure. deploy was already guarded by requireTTY (PR aws#949); import/view/export were not. Add requireTTY() before each unguarded interactive launch: - export/index.ts: before renderTUI for export-harness - import/command.ts: in the no-source branch before render(ImportFlow) - view/command.tsx: at the top of launchTuiList and launchTuiDetail requireProject()/--json paths untouched. Adds unit tests covering the three command suites. Refs aws#982

agentcore-devx-automation · 2026-06-25T15:07:53Z

github-actions · 2026-06-25T15:08:03Z

Package Tarball

aws-agentcore-0.21.0.tgz

How to install

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

agentcore-cli-automation

Thanks for tackling this — the fix targets the correct three unguarded sites (import no-source, view list/detail, export harness) and the symptom is real. A few things to address before merge:

PR title fails validate-pr-title CI check — the subject must start with a lowercase letter. The current title fix(cli): Add requireTTY() ... needs to become fix(cli): add requireTTY() .... This is currently a failing required check on the PR.
Excessive mocking in all three new test files — each test mocks requireTTY itself (the very thing this PR adds), so the assertions effectively just check that the new line of source code exists before render/renderTUI. That's a tautology against the source; the tests would still pass if requireTTY were called after render, since mockRequireTTY only throws and render is also mocked. See inline comments for the suggested approach (drive the real requireTTY by setting process.stdin.isTTY/process.stdout.isTTY and spying on process.exit + console.error, mirroring src/cli/commands/feedback/__tests__/consent-prompt.test.ts).
Consider the centralized fix path the PR description recommends — the description itself notes that a more robust approach is to call requireTTY() at the top of renderTUI() in src/cli/tui/render.ts:35, which would cover export (and any future TUI launch via that entrypoint) by default, leaving only the two raw render() sites (import, view) needing inline guards. As written, this PR fixes the three known sites but the next interactive command added via renderTUI will hit the same bug. Worth either doing now or filing a follow-up.

Minor/non-blocking: the ordering of requireTTY vs requireProject is inconsistent between import (TTY first) and view (project first). Probably fine, but pick one for consistency.

agentcore-cli-automation · 2026-06-25T15:09:58Z

+vi.mock('../../../tui/guards', () => ({
+  requireTTY: () => mockRequireTTY(),
+}));
+


Mocking requireTTY itself means this test isn't actually verifying the guard's behavior — it only verifies that something called mockRequireTTY runs before something that calls mockRenderTUI. If a future refactor moved requireTTY() after renderTUI(), this test would still pass because mockRequireTTY only throws and mockRenderTUI is a no-op that's then never reached anyway via the rejected promise.

Suggest exercising the real requireTTY:

const origStdinIsTTY = process.stdin.isTTY; const origStdoutIsTTY = process.stdout.isTTY; // Per test, set them to false and spy on process.exit / console.error.

Then mock only renderTUI (true I/O boundary) and assert that with non-TTY, process.exit(1) was called and renderTUI was not; with TTY, renderTUI was called. This is the pattern used in src/cli/commands/feedback/__tests__/consent-prompt.test.ts.

agentcore-cli-automation · 2026-06-25T15:09:58Z

+const mockRequireProject = vi.fn();
+const mockRender = vi.fn();
+
+vi.mock('../../../tui/guards/tty', () => ({


Same excessive-mocking concern as the export test: requireTTY itself is mocked, so the test is a tautology against the source line ordering. Please drive the real guard by toggling process.stdin.isTTY / process.stdout.isTTY and spying on process.exit/console.error, and only mock the I/O boundaries (ink.render, react, and the ImportFlow import).

With the current mock setup the test also wouldn't catch a regression where requireProject() is moved before requireTTY() — both are mocked to never throw, so the test happy-path passes even if the guard never runs in a non-TTY context.

agentcore-cli-automation · 2026-06-25T15:09:58Z

+
+vi.mock('../../../tui/guards', () => ({
+  requireProject: () => mockRequireProject(),
+  requireTTY: () => mockRequireTTY(),


Same excessive-mocking concern. Two additional notes specific to view:

launchTuiList branches on type (recommendation / batch-evaluation / ab-test / insights). Only recommendation is exercised here. The guard is at the top of the function so it's fine in practice, but a single parameterized test over all four types would prevent a future refactor from accidentally moving the guard into a per-type branch.

In production requireProject() runs at line 57 (action handler) before launchTuiList/launchTuiDetail, so non-TTY + no-project users will see the no-project error rather than the TTY error. That's probably the right ordering, but it's the opposite of import/command.ts:28 where the PR puts requireTTY before requireProject. Worth picking one ordering and applying consistently.

agentcore-cli-automation · 2026-06-25T15:09:58Z

      if (!cliOptions.source) {
        // No --source and no subcommand — launch interactive TUI
+        const { requireTTY } = await import('../../tui/guards/tty');
+        requireTTY();


Two small things on this site:

Inconsistent ordering: requireTTY runs before requireProject here, but in view/command.tsx requireProject runs first (at the action-handler level, line 57). Either is defensible — pick one. Putting requireProject first arguably gives a more actionable error to users in CI who just haven't cd'd to the right place.

Style consistency: most call sites do import { requireTTY } from '../../tui/guards' (the barrel) — only run/command.tsx and now import/command.ts use the deep import '../../tui/guards/tty'. Not a blocker, just inconsistent.

agentcore-cli-automation · 2026-06-25T15:09:58Z

          console.log(JSON.stringify({ success: false, error: '--name is required in non-interactive mode' }));
          process.exit(1);
        }
+        requireTTY();


Consider moving this guard into renderTUI() itself (at the top of src/cli/tui/render.ts:35) rather than at each call site. Your own PR description identifies this as the more robust approach: it would cover this site automatically and protect any future renderTUI() caller by default. The raw render() sites in import and view would still need inline guards, but those are the minority. As written, the next interactive command added via renderTUI() (e.g., another export subcommand) will silently regress to the same bug.

- centralize the interactive-terminal guard at the top of renderTUI() so every TUI entrypoint (export-harness and any future renderTUI caller) is covered by default; keep inline guards only on the raw render() sites (import, view) - rewrite the three tty-guard tests to exercise the real requireTTY by toggling process.stdin.isTTY/process.stdout.isTTY and spying on process.exit + console.error, mocking only the I/O boundaries (mirrors feedback/consent-prompt.test.ts) instead of mocking requireTTY - parameterize the view list guard over all four job types - make requireTTY-vs-requireProject ordering consistent: requireProject() first in import (matches view)

agentcore-devx-automation · 2026-06-25T15:29:01Z

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

agentcore-cli-automation

Re-reviewed after commit 5edc308 (test(cli): drive real requireTTY guard; centralize in renderTUI). All serious issues from the prior CHANGES_REQUESTED review have been addressed:

PR title — now lowercase (fix(cli): add requireTTY()...).
Excessive mocking — the three new test files now drive the real requireTTY by toggling process.stdin.isTTY/process.stdout.isTTY and spying on process.exit + console.error. Only true I/O boundaries (ink.render, screen modules, telemetry, notices) are mocked. This is the pattern the prior review asked for.
Centralized guard — requireTTY() is now called at the top of renderTUI() (src/cli/tui/render.ts:39), so export and any future TUI launched through renderTUI is covered by default; only the two raw render() sites (import, view) need inline guards.
Ordering consistency — import/command.ts now runs requireProject() before requireTTY(), matching view, with a comment explaining why.
view parameterization — the new view test parameterizes over all four job types (recommendation, batch-evaluation, ab-test, insights).
Barrel import — import/command.ts now imports from '../../tui/guards' rather than the deep tty path.

No new blocking issues found. The redundancy of requireTTY() being called both at existing call sites (e.g. cli.ts:158, deploy/command.tsx:222,225, etc.) and now inside renderTUI() is harmless — the guard is idempotent and the duplicate calls can be cleaned up in a follow-up if desired.

LGTM.

agentcore-devx-automation · 2026-06-25T15:33:04Z

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

agentcore-cli-automation · 2026-06-25T15:34:12Z

LGTM. Reviewed after commit 5edc308. All serious issues raised in the prior automation review have been addressed:

Excessive mocking — the three new tests now drive the real requireTTY by toggling process.stdin.isTTY/process.stdout.isTTY and spying on process.exit/console.error, mocking only true I/O boundaries (Ink renderer, screen modules, telemetry, notices). The TTY-true paths in each test also assert mockRender is reached, so a regression that moved the guard after render would now actually be caught.
Centralized guard — requireTTY() is at the top of renderTUI() (src/cli/tui/render.ts:39), so the export harness site (and any future TUI launched via renderTUI) is covered by default. Only the two raw render() call sites (import, view) need inline guards, and both have them.
PR title — lowercased, validate-pr-title passing.
Ordering consistency — import now runs requireProject() before requireTTY(), matching view, with a comment explaining the rationale.
View parameterization — all four job types (recommendation, batch-evaluation, ab-test, insights) are exercised, so moving the guard into a per-type branch would be caught.
Barrel import — import/command.ts uses '../../tui/guards' rather than the deep tty path.

The duplication between the new renderTUI-level guard and the existing inline requireTTY() calls at the various command sites (cli.ts:158, deploy/command.tsx:222,225, etc.) is harmless — the guard is idempotent — and can be cleaned up in a follow-up.

No new blocking issues found.

launchTuiDevScreenWithPicker called render() without a requireTTY() guard, so in a non-TTY context it threw Ink's "Raw mode is not supported" stack trace plus a partial alt-screen paint — a counterexample to the centralized guard PR aws#1640 added. Call requireTTY() at the top of the picker (before the alt-screen write and render) and add a tty-guard test that drives the real guard for this path.

agentcore-devx-automation · 2026-06-25T16:13:05Z

agentcore-cli-automation

LGTM. The prior automation reviews already flagged the substantive issues (excessive mocking, centralized guard, view parameterization, requireProject/requireTTY ordering, barrel import) and the current revision addresses all of them. I verified:

requireTTY is now at the top of renderTUI() in src/cli/tui/render.ts:37, so export harness and any future renderTUI caller are covered by default.
The three raw render() call sites that bypass renderTUI are guarded inline: import/command.ts:31, view/command.tsx:97 (launchTuiList) and :118 (launchTuiDetail).
launchTuiDevScreenWithPicker in dev/browser-mode.ts:332 — the browser-mode picker — is also guarded. This site is not mentioned in the PR description but is a real previously-unguarded path (dev/command.tsx:473's requireTTY only covers the --no-browser branch), so the addition is correct.
All four new test files drive the real requireTTY by toggling process.stdin.isTTY / process.stdout.isTTY and spying on process.exit / console.error, mocking only true I/O boundaries (Ink renderer, screen modules). Each test also asserts mockRender is not called on the non-TTY paths and is called on the TTY path, so a regression that moved the guard after render() would actually be caught.
View tests are parameterized over all four job types (recommendation, batch-evaluation, ab-test, insights), preventing a future per-type refactor from moving the guard.
import/command.ts runs requireProject() before requireTTY(), matching view, with a comment explaining the rationale.
Telemetry: not applicable here. The guard short-circuits with process.exit(1) before command logic; existing runCliCommand / withCommandRunTelemetry wrappers continue to cover the underlying commands and no new metric semantics are introduced.

No new blocking issues.

aidandaly24 temporarily deployed to e2e-testing June 25, 2026 15:07 — with GitHub Actions Inactive

github-actions Bot added the size/m PR size: M label Jun 25, 2026

agentcore-devx-automation Bot added the claude-security-reviewing Claude Code /security-review in progress label Jun 25, 2026

github-actions Bot added the agentcore-harness-reviewing AgentCore Harness review in progress label Jun 25, 2026

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

agentcore-cli-automation suggested changes Jun 25, 2026

View reviewed changes

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

aidandaly24 changed the title ~~fix(cli): Add requireTTY() before each unguarded interactive TUI la... (#982)~~ fix(cli): add requireTTY() before each unguarded interactive TUI launch (#982) Jun 25, 2026

github-actions Bot added size/m PR size: M and removed size/m PR size: M labels Jun 25, 2026

aidandaly24 temporarily deployed to e2e-testing June 25, 2026 15:28 — with GitHub Actions Inactive

github-actions Bot added the agentcore-harness-reviewing AgentCore Harness review in progress label Jun 25, 2026

agentcore-devx-automation Bot added the claude-security-reviewing Claude Code /security-review in progress label Jun 25, 2026

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

agentcore-cli-automation approved these changes Jun 25, 2026

View reviewed changes

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

fix(test): type ink render mocks to satisfy tsc --noEmit

379e02d

github-actions Bot added size/m PR size: M and removed size/m PR size: M labels Jun 25, 2026

agentcore-devx-automation Bot added the claude-security-reviewing Claude Code /security-review in progress label Jun 25, 2026

github-actions Bot added the agentcore-harness-reviewing AgentCore Harness review in progress label Jun 25, 2026

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

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

aidandaly24 temporarily deployed to e2e-testing June 25, 2026 15:42 — with GitHub Actions Inactive

github-actions Bot added size/m PR size: M and removed size/m PR size: M labels Jun 25, 2026

aidandaly24 temporarily deployed to e2e-testing June 25, 2026 16:12 — with GitHub Actions Inactive

github-actions Bot added the agentcore-harness-reviewing AgentCore Harness review in progress label Jun 25, 2026

agentcore-devx-automation Bot added the claude-security-reviewing Claude Code /security-review in progress label Jun 25, 2026

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

agentcore-cli-automation approved these changes Jun 25, 2026

View reviewed changes

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

aidandaly24 marked this pull request as ready for review June 25, 2026 16:42

aidandaly24 requested a review from a team June 25, 2026 16:42

aidandaly24 mentioned this pull request Jun 25, 2026

fix(import): guard interactive TUI with requireTTY() so non-TTY exits 1 cleanly (#982) #1170

Closed

15 tasks

Sunbelt Computer Software

PL/B Language Development and Support

Uh oh!

Conversation

aidandaly24 commented Jun 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Fix

Validation

Uh oh!

agentcore-devx-automation Bot commented Jun 25, 2026

Uh oh!

github-actions Bot commented Jun 25, 2026

Package Tarball

How to install

Uh oh!

agentcore-cli-automation left a comment

Choose a reason for hiding this comment

Uh oh!

agentcore-cli-automation Jun 25, 2026

Choose a reason for hiding this comment

Uh oh!

agentcore-cli-automation Jun 25, 2026

Choose a reason for hiding this comment

Uh oh!

agentcore-cli-automation Jun 25, 2026

Choose a reason for hiding this comment

Uh oh!

agentcore-cli-automation Jun 25, 2026

Choose a reason for hiding this comment

Uh oh!

agentcore-cli-automation Jun 25, 2026

Choose a reason for hiding this comment

Uh oh!

agentcore-devx-automation Bot commented Jun 25, 2026

Uh oh!

agentcore-cli-automation left a comment

Choose a reason for hiding this comment

Uh oh!

agentcore-devx-automation Bot commented Jun 25, 2026

Uh oh!

agentcore-cli-automation commented Jun 25, 2026

Uh oh!

agentcore-devx-automation Bot commented Jun 25, 2026

Uh oh!

agentcore-cli-automation left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

aidandaly24 commented Jun 25, 2026 •

edited

Loading