Skip to content

fix(run-insights): TUI wizard state, inline name validation, job history #1602

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
notgitika wants to merge 1 commit into
base: main
Choose a base branch
from
+155 −35
Open

fix(run-insights): TUI wizard state, inline name validation, job history #1602

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 70 additions & 10 deletions src/cli/tui/screens/run-insights/RunInsightsFlow.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,20 +1,27 @@
import { ConfigIO } from '../../../../lib';
import type { DeployedState } from '../../../../schema';
import { detectRegion } from '../../../aws/region';
import { getErrorMessage } from '../../../errors';
import { saveInsightsRun } from '../../../operations/insights';
import { createJobEngine } from '../../../operations/jobs';
import type { InsightsJobRecord } from '../../../operations/jobs/shared/types';
import { withCommandRunTelemetry } from '../../../telemetry/cli-command-run.js';
import { ErrorPrompt, GradientText, SuccessPrompt } from '../../components';
import { RunInsightsScreen } from './RunInsightsScreen';
import type { RunInsightsConfig } from './types';
import type { RunInsightsConfig, RunInsightsStep } from './types';
import React, { useCallback, useEffect, useMemo, useState } from 'react';

interface ProjectData {
agentNames: string[];
onlineEvalConfigArns: string[];
}

type FlowState =
| { name: 'loading' }
| { name: 'wizard'; agentNames: string[]; onlineEvalConfigArns: string[] }
| { name: 'wizard'; project: ProjectData; resume?: { config: RunInsightsConfig; step: RunInsightsStep } }
| { name: 'submitting' }
| { name: 'success'; record: InsightsJobRecord }
| { name: 'error'; message: string };
| { name: 'error'; message: string; project?: ProjectData; failedConfig?: RunInsightsConfig };

interface RunInsightsFlowProps {
isInteractive?: boolean;
Expand Down Expand Up @@ -51,7 +58,7 @@ export function RunInsightsFlow({ isInteractive = true, onExit, onBack, onViewJo

const onlineEvalConfigArns = extractOnlineEvalConfigArns(deployedState);

setFlow({ name: 'wizard', agentNames, onlineEvalConfigArns });
setFlow({ name: 'wizard', project: { agentNames, onlineEvalConfigArns } });
} catch (err) {
if (!cancelled) setFlow({ name: 'error', message: getErrorMessage(err) });
}
Expand All @@ -69,7 +76,7 @@ export function RunInsightsFlow({ isInteractive = true, onExit, onBack, onViewJo
}, [isInteractive, flow.name, onExit]);

const handleComplete = useCallback(
(config: RunInsightsConfig) => {
(config: RunInsightsConfig, project: ProjectData) => {
setFlow({ name: 'submitting' });

void (async () => {
Expand All @@ -88,9 +95,22 @@ export function RunInsightsFlow({ isInteractive = true, onExit, onBack, onViewJo
if (!startResult.success) {
throw startResult.error ?? new Error('Failed to start insights job');
}

// Mirror the new job to the legacy insights store so `view insights`
// (InsightsJobsScreen) finds it. Without this, the post-launch screen
// shows "No insights runs found" right after creating the job.
await persistToLegacyStore(startResult.record).catch(() => {

@jariy17 jariy17 Jun 23, 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.

If ViewInsights is looking in the wrong directory shouldn't we change what directory it's looking at. By duplicate the job in two locations, the customer has two "sources of truth" which means we have to maintain both locations.

// Non-fatal — the job started successfully; storage failures shouldn't surface.
});

@agentcore-cli-automation agentcore-cli-automation Jun 22, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Silently swallowing the error here makes it hard to diagnose a future regression where the legacy store stops being written (the user sees "No insights runs found" again and there's no breadcrumb). At minimum, log via the same mechanism the rest of the flow uses (e.g. ExecLogger or a console.warn). Since the comment already says "storage failures shouldn't surface," the goal is debuggability, not user-visible errors.

Minor: per the comment, this is a workaround until the post-launch screen reads from the job-engine store. A follow-up issue/TODO referencing that migration would help avoid leaving two stores diverging indefinitely (the engine store gets refresh updates, the legacy store doesn't — so statuses written here will stay frozen forever).


setFlow({ name: 'success', record: startResult.record });
} catch (err) {
setFlow({ name: 'error', message: getErrorMessage(err) });
setFlow({
name: 'error',
message: getErrorMessage(err),
project,
failedConfig: config,
});
}
})();
},
Expand All @@ -104,10 +124,12 @@ export function RunInsightsFlow({ isInteractive = true, onExit, onBack, onViewJo
if (flow.name === 'wizard') {
return (
<RunInsightsScreen
agentNames={flow.agentNames}
onlineEvalConfigArns={flow.onlineEvalConfigArns}
onComplete={handleComplete}
agentNames={flow.project.agentNames}
onlineEvalConfigArns={flow.project.onlineEvalConfigArns}
onComplete={cfg => handleComplete(cfg, flow.project)}
onExit={onBack}
initialConfig={flow.resume?.config}
initialStep={flow.resume?.step}
/>
);
}
Expand All @@ -124,16 +146,54 @@ export function RunInsightsFlow({ isInteractive = true, onExit, onBack, onViewJo
);
}

// Error: if we still have project data + the user's prior input, jump back
// into the wizard at the Name step with their config preserved. Otherwise
// (catastrophic load failure) fall back to the loading→error cycle.
return (
<ErrorPrompt
message="Failed to start insights job"
detail={flow.message}
onBack={() => setFlow({ name: 'loading' })}
onBack={() => {
if (flow.project && flow.failedConfig) {
setFlow({
name: 'wizard',
project: flow.project,
resume: { config: flow.failedConfig, step: 'name' },
});
} else {
setFlow({ name: 'loading' });
}
}}
onExit={onExit}
/>
);
}

async function persistToLegacyStore(record: InsightsJobRecord): Promise<void> {
// The job engine record uses `id`/`arn`; the legacy storage uses
// `batchEvaluationId`/`batchEvaluationArn`. Translate so InsightsJobsScreen
// (which reads from the legacy store) sees the new run.
const region = regionFromArn(record.arn) ?? (await detectRegion()).region;
saveInsightsRun({
batchEvaluationId: record.id,
batchEvaluationArn: record.arn,
name: record.name,
status: record.status,
region,
createdAt: record.createdAt,
completedAt: record.completedAt,
insights: record.insights,
agent: record.agent,
});
}

function regionFromArn(arn: string | undefined): string | undefined {
if (!arn) return undefined;
const region = arn.split(':')[3];
if (!region) return undefined;
return region;
}

function extractOnlineEvalConfigArns(deployedState: DeployedState): string[] {
const arns: string[] = [];
for (const target of Object.values(deployedState.targets ?? {})) {
Expand Down
61 changes: 50 additions & 11 deletions src/cli/tui/screens/run-insights/RunInsightsScreen.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,20 +10,59 @@ import {
import type { SelectableItem } from '../../components';
import { HELP_TEXT } from '../../constants';
import { useListNavigation, useMultiSelectNavigation } from '../../hooks';
import { AVAILABLE_INSIGHTS, RUN_INSIGHTS_STEP_LABELS, SESSION_MODE_OPTIONS, SOURCE_OPTIONS } from './types';
import type { RunInsightsConfig } from './types';
import {
AVAILABLE_INSIGHTS,
JOB_NAME_PATTERN,
RUN_INSIGHTS_STEP_LABELS,
SESSION_MODE_OPTIONS,
SOURCE_OPTIONS,
} from './types';
import type { RunInsightsConfig, RunInsightsStep } from './types';
import { useRunInsightsWizard } from './useRunInsightsWizard';
import React, { useMemo } from 'react';
import React, { forwardRef, useImperativeHandle, useMemo } from 'react';

export interface RunInsightsScreenHandle {
jumpToStep: (step: RunInsightsStep) => void;
}

interface RunInsightsScreenProps {
agentNames: string[];
onlineEvalConfigArns: string[];
onComplete: (config: RunInsightsConfig) => void;
onExit: () => void;
/** Pre-seed wizard state, e.g. when returning from a validation/API error. */
initialConfig?: RunInsightsConfig;
/** Step to land on at mount (default: 'source'). */
initialStep?: RunInsightsStep;
}

function validateJobName(value: string): true | string {
// Empty is allowed — the CLI auto-generates a name when omitted.
if (!value) return true;
if (!JOB_NAME_PATTERN.test(value)) {
return 'Name must start with a letter and contain only letters, numbers, and underscores (max 48 chars).';
}
return true;
}

function validateLookbackInput(value: string): true | string {
if (!value) return true;
const days = parseInt(value, 10);
if (!Number.isInteger(days) || String(days) !== value.trim()) {
return 'Lookback must be a whole number.';
}
if (days < 1 || days > 90) {
return 'Lookback must be between 1 and 90 days.';
}
return true;
}

export function RunInsightsScreen({ agentNames, onlineEvalConfigArns, onComplete, onExit }: RunInsightsScreenProps) {
const wizard = useRunInsightsWizard(agentNames.length);
export const RunInsightsScreen = forwardRef<RunInsightsScreenHandle, RunInsightsScreenProps>(function RunInsightsScreen(
{ agentNames, onlineEvalConfigArns, onComplete, onExit, initialConfig, initialStep },
ref
) {
const wizard = useRunInsightsWizard(agentNames, initialConfig, initialStep);
useImperativeHandle(ref, () => ({ jumpToStep: wizard.jumpToStep }), [wizard.jumpToStep]);

@agentcore-cli-automation agentcore-cli-automation Jun 22, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This forwardRef + useImperativeHandle plumbing (and the exported RunInsightsScreenHandle / jumpToStep on the wizard hook) doesn't appear to be consumed anywhere — RunInsightsFlow mounts the screen without a ref, and resume-on-error works purely through initialConfig / initialStep. I'd suggest one of:

  • Drop it — remove the forwardRef wrapper, the RunInsightsScreenHandle interface, the useImperativeHandle call, and the jumpToStep exposure from useRunInsightsWizard. The component goes back to a plain function and the public API stays minimal.
  • Wire it up — if there's a follow-up use case (e.g. surfacing field-specific errors and jumping to that step from RunInsightsFlow), pass a ref from the flow and call jumpToStep there so the path is exercised.

Note that with React 19, even if you do want imperative access, forwardRef is no longer needed — ref can be a regular prop.


const isSourceStep = wizard.step === 'source';
const isAgentStep = wizard.step === 'agent';
Expand Down Expand Up @@ -157,10 +196,8 @@ export function RunInsightsScreen({ agentNames, onlineEvalConfigArns, onComplete
key="lookback"
prompt="Lookback window (days)"
initialValue="7"
onSubmit={value => {
const days = parseInt(value, 10);
wizard.setLookbackDays(isNaN(days) || days <= 0 ? 7 : days);
}}
customValidation={validateLookbackInput}
onSubmit={value => wizard.setLookbackDays(parseInt(value, 10))}
onCancel={() => wizard.goBack()}
/>
)}
Expand All @@ -187,6 +224,8 @@ export function RunInsightsScreen({ agentNames, onlineEvalConfigArns, onComplete
<TextInput
key="name"
prompt="Job name (leave blank for auto-generated)"
allowEmpty
customValidation={validateJobName}
onSubmit={value => wizard.setName(value)}
onCancel={() => wizard.goBack()}
/>
Expand All @@ -197,7 +236,7 @@ export function RunInsightsScreen({ agentNames, onlineEvalConfigArns, onComplete
fields={
wizard.config.source === 'agent'
? [
{ label: 'Agent', value: wizard.config.agent ?? agentNames[0] ?? '' },
{ label: 'Agent', value: wizard.config.agent !== '' ? wizard.config.agent : (agentNames[0] ?? '') },
{
label: 'Insights',
value: wizard.config.insights
Expand All @@ -218,4 +257,4 @@ export function RunInsightsScreen({ agentNames, onlineEvalConfigArns, onComplete
</Panel>
</Screen>
);
}
});
7 changes: 7 additions & 0 deletions src/cli/tui/screens/run-insights/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,13 @@ export const RUN_INSIGHTS_STEP_LABELS: Record<RunInsightsStep, string> = {

export const DEFAULT_LOOKBACK_DAYS = 7;

/**
* Validation pattern for job names. Mirrors the service-side BatchEvaluationName
* shape (`^[a-zA-Z][a-zA-Z0-9_]{0,47}$`) so we reject locally instead of
* surfacing a 400 from startBatchEvaluation.
*/
export const JOB_NAME_PATTERN = /^[a-zA-Z][a-zA-Z0-9_]{0,47}$/;

export const AVAILABLE_INSIGHTS = [
{
id: 'Builtin.Insight.FailureAnalysis',
Expand Down
42 changes: 28 additions & 14 deletions src/cli/tui/screens/run-insights/useRunInsightsWizard.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,12 @@ function getStepsForSource(source: RunInsightsSource, agentCount: number): RunIn
return ['source', 'agent', 'insights', 'sessions', 'lookbackDays', 'name', 'confirm'];
}

function getDefaultConfig(): RunInsightsConfig {
function getDefaultConfig(soleAgent: string): RunInsightsConfig {
return {
source: 'agent',
agent: '',
// Pre-populate when only one agent exists so the confirm screen shows it
// even though the agent selection step is skipped.
agent: soleAgent,
insights: [],
sessionMode: 'lookback',
lookbackDays: DEFAULT_LOOKBACK_DAYS,
Expand All @@ -25,9 +27,15 @@ function getDefaultConfig(): RunInsightsConfig {
};
}

export function useRunInsightsWizard(agentCount: number) {
const [config, setConfig] = useState<RunInsightsConfig>(getDefaultConfig);
const [step, setStep] = useState<RunInsightsStep>('source');
export function useRunInsightsWizard(
agentNames: string[],
initialConfig?: RunInsightsConfig,
initialStep: RunInsightsStep = 'source'
) {
const agentCount = agentNames.length;
const soleAgent = agentCount === 1 ? (agentNames[0] ?? '') : '';
const [config, setConfig] = useState<RunInsightsConfig>(() => initialConfig ?? getDefaultConfig(soleAgent));
const [step, setStep] = useState<RunInsightsStep>(initialStep);

const allSteps = useMemo(() => getStepsForSource(config.source, agentCount), [config.source, agentCount]);
const currentIndex = allSteps.indexOf(step);
Expand All @@ -50,6 +58,16 @@ export function useRunInsightsWizard(agentCount: number) {
}
}, [allSteps, currentIndex]);

/** Jump to an arbitrary step in the current flow without resetting config. */
const jumpToStep = useCallback(
(target: RunInsightsStep) => {
if (allSteps.includes(target)) {
setStep(target);
}
},
[allSteps]
);

@agentcore-cli-automation agentcore-cli-automation Jun 22, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

jumpToStep here only exists to be threaded through useImperativeHandle in RunInsightsScreen.tsx — and nothing calls the resulting handle. Consider removing it (and the forwardRef wrapper) unless there's a planned consumer.


const setSource = useCallback(
(source: RunInsightsSource) => {
setConfig(c => ({ ...c, source }));
Expand Down Expand Up @@ -80,13 +98,8 @@ export function useRunInsightsWizard(agentCount: number) {
const setSessionMode = useCallback(
(sessionMode: RunInsightsSessionMode) => {
setConfig(c => ({ ...c, sessionMode }));
if (sessionMode === 'lookback') {
const next = nextStep('sessions');
if (next) setStep(next);
} else {
const next = nextStep('sessions');
if (next) setStep(next);
}
const next = nextStep('sessions');
if (next) setStep(next);
},
[nextStep]
);
Expand Down Expand Up @@ -128,16 +141,17 @@ export function useRunInsightsWizard(agentCount: number) {
);

const reset = useCallback(() => {
setConfig(getDefaultConfig());
setConfig(getDefaultConfig(soleAgent));
setStep('source');
}, []);
}, [soleAgent]);

return {
config,
step,
steps: allSteps,
currentIndex,
goBack,
jumpToStep,
setSource,
setAgent,
setInsights,
Expand Down
Loading