Skip to content

fix(eval): keep CLI-only commands out of the TUI menu - #2191

Merged
nborges-aws merged 1 commit into
refactorfrom
fix/eval-tui-allowlist
Sep 3, 2026
Merged

fix(eval): keep CLI-only commands out of the TUI menu#2191
nborges-aws merged 1 commit into
refactorfrom
fix/eval-tui-allowlist

Conversation

@jariy17

@jariy17 jariy17 commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Problem

The eval router and its batch-evaluation subgroup open an interactive TUI on a bare invocation (.default(renderTui(...))) but never called .supportedTuiCommands(...), so the menu (RouterScreenisTuiCommandSupported) offered commands that have no interactive screen:

  • eval menu listed ondemand (help-only default) and recommendation (no screen).
  • eval batch-evaluation menu listed evaluate and simulate (imperative job-starting commands with required flags).

Selecting any of them navigated to a route with no screen. Every sibling with the same shape (batch-insights, ab-test, online-eval, dataset, config-bundle, …) already restricts the menu to ("get", "list"); these two were the outliers.

Fix

Add the missing allowlists, matching the existing batch-insights / gateway pattern:

  • eval → the 8 groups that actually have an interactive screen (drops ondemand + recommendation).
  • eval batch-evaluation("get", "list") (drops evaluate + simulate).

Behaviour of the commands themselves is unchanged — agentcore eval ondemand still prints help, batch-evaluation simulate still runs from the CLI. This only gates TUI-menu membership.

Tests

Mirror the gateway command-hierarchy suite (supportsTui helper + test.each for supported vs CLI-only):

  • New src/handlers/eval/eval.test.tsx — the 8 screen-backed groups are offered; ondemand + recommendation are hidden.
  • batch-evaluation.test.tsxget/list (and bare group) offered; evaluate/simulate hidden.

bun test src/handlers/eval/377 pass, 0 fail. oxlint + prettier clean on changed files.

Note: tsc --noEmit surfaces a pre-existing duplicate-identifier error in src/handlers/project/buildDeploy.screen.test.tsx (present on refactor before this branch; unrelated to these files).

@github-actions github-actions Bot added the size/m PR size: M label Sep 3, 2026
@agentcore-devx-automation agentcore-devx-automation Bot added agentcore-harness-reviewing AgentCore Harness review in progress claude-security-reviewing Claude Code /security-review in progress labels Sep 3, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

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

@agentcore-devx-automation agentcore-devx-automation Bot removed claude-security-reviewing Claude Code /security-review in progress agentcore-harness-reviewing AgentCore Harness review in progress labels Sep 3, 2026

@agentcore-devx-automation agentcore-devx-automation Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

AgentCore Harness Review

Verdict: Looks good

Small, focused fix that adds supportedTuiCommands to the eval and eval batch-evaluation routers so CLI-only commands (ondemand, recommendation, evaluate, simulate) stop showing up in the interactive menu. The categorization matches each subhandler's actual state (either no .default(renderTui...) or an explicit empty supportedTuiCommands()), and the pattern mirrors what's already in place for gateway, runtime, identity, etc.

The new tests validate menu membership through the real compiled router with TestCoreClient, consistent with gateway.test.tsx. No excessive mocking, no telemetry surface changed. LGTM.

@codecov-commenter

codecov-commenter commented Sep 3, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.15%. Comparing base (a665955) to head (a3cc129).

Additional details and impacted files
@@            Coverage Diff            @@
##           refactor    #2191   +/-   ##
=========================================
  Coverage     97.15%   97.15%           
=========================================
  Files           542      542           
  Lines         37728    37740   +12     
=========================================
+ Hits          36653    36665   +12     
  Misses         1075     1075           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

The eval router and its batch-evaluation subgroup open an interactive
TUI on a bare invocation but never restricted which children the menu
offers via supportedTuiCommands(), so the menu surfaced commands that
have no interactive screen:

- eval menu listed ondemand (help-only) and recommendation (no screen)
- batch-evaluation menu listed evaluate and simulate (imperative jobs)

Selecting one navigated to a route with no screen. Add the allowlists,
matching the existing batch-insights / gateway pattern, so only groups
and leaves with a real screen appear. Behaviour of the commands
themselves is unchanged; this only gates TUI-menu membership.
@jariy17
jariy17 force-pushed the fix/eval-tui-allowlist branch from d55c9b5 to a3cc129 Compare September 3, 2026 19:29
@github-actions github-actions Bot added size/s PR size: S and removed size/m PR size: M labels Sep 3, 2026
@agentcore-devx-automation agentcore-devx-automation Bot added the claude-security-reviewing Claude Code /security-review in progress label Sep 3, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

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

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

@nborges-aws nborges-aws left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@nborges-aws
nborges-aws merged commit b57f237 into refactor Sep 3, 2026
21 of 22 checks passed
@nborges-aws
nborges-aws deleted the fix/eval-tui-allowlist branch September 3, 2026 20:24
This was referenced Sep 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/s PR size: S

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants