mirror of
https://github.com/Kaelio/ktx.git
synced 2026-06-10 08:05:14 +02:00
fix(cli): classify ktx setup abandonment as aborted, not a blank error (#278)
* fix(cli): classify ktx setup abandonment as aborted, not a blank error ktx setup returned a non-zero exit code without throwing when a user abandoned the interactive wizard, so the command telemetry recorded outcome=error with no errorClass/errorDetail — an unactionable blank in the errors dashboard, where most ktx setup "errors" were really people backing out of the wizard. Add annotateCommandOutcome() to the command span so the setup flow (the decision-maker) records the true outcome: genuine step failures and --no-input missing input become outcome=error with a self-diagnosing reason, while interactive abandonment and project cancellation become outcome=aborted and drop out of the error view. Unify the exit code and telemetry through setupTerminalOutcome() so they can never diverge: aborts now exit 0 (matching the entry-menu Exit, project cancel, and a confirmed Ctrl+C), while failures and automation errors still exit 1. * fix(cli): treat non-TTY setup missing-input as an error, not an abort setupTerminalOutcome classified `missing-input` by `args.inputMode`, but `auto` only means "interactive if a TTY is attached". A piped/CI `ktx setup` without `--no-input` and without `--yes` is still `auto`, yet the project and agents steps return `missing-input` there without ever prompting (e.g. "pass --yes to create a project outside an interactive terminal"). Classifying that as `aborted` made a broken automation run exit 0 — a silent failure. Key the classification off actual interactivity instead: input enabled AND `io.stdout.isTTY === true`. Non-interactive missing-input now exits 1 with a `KtxSetupMissingInput` reason; only a genuine interactive abort exits 0. Adds a non-TTY regression test and fixes the abandonment test to use a real TTY.
This commit is contained in:
parent
66517fc320
commit
470802e58e
6 changed files with 493 additions and 14 deletions
|
|
@ -12,6 +12,7 @@ import { runtimeInstallPolicyFromFlags } from './managed-python-command.js';
|
|||
import { readManagedPythonRuntimeStatus } from './managed-python-runtime.js';
|
||||
import { resolveProjectRuntimeRequirements } from './runtime-requirements.js';
|
||||
import { isKtxSetupExitError } from './setup-interrupt.js';
|
||||
import type { CommandOutcome } from './telemetry/index.js';
|
||||
import {
|
||||
type KtxAgentScope,
|
||||
type KtxAgentTarget,
|
||||
|
|
@ -211,6 +212,80 @@ function setupTelemetryOutcome(
|
|||
return 'abandoned';
|
||||
}
|
||||
|
||||
interface SetupCommandAnnotation {
|
||||
outcome: CommandOutcome;
|
||||
errorClass?: string;
|
||||
errorDetail?: string;
|
||||
}
|
||||
|
||||
/**
|
||||
* Classify a terminal non-ready setup status into the `command` telemetry
|
||||
* outcome. The setup flow is the decision-maker and knows the difference:
|
||||
* - `failed` is a genuine error; attach a step-scoped reason so the dashboard
|
||||
* shows an actionable signature instead of a blank.
|
||||
* - `missing-input` from a *non-interactive* run is an automation error
|
||||
* (required flags absent and no prompt was possible); attach a reason too.
|
||||
* - `missing-input` from an interactive prompt, or a project `cancelled`, is the
|
||||
* user backing out of the wizard — an abort, not a failure. Keep it out of
|
||||
* error telemetry so it stops inflating the error count.
|
||||
*
|
||||
* `interactive` must reflect whether a prompt could actually be shown — input
|
||||
* is enabled AND a TTY is attached. `inputMode: 'auto'` alone is not enough: a
|
||||
* piped/CI run without `--no-input` is still non-interactive, and steps such as
|
||||
* the project step return `missing-input` ("pass --yes …") there without ever
|
||||
* prompting. Treating that as an abort would make a broken automation run exit
|
||||
* 0, so it must classify as an error.
|
||||
*
|
||||
* Reasons are synthetic, step-scoped strings (no user input), so they satisfy
|
||||
* the telemetry privacy rules. The step's own `errorDetail`, when present, has
|
||||
* already been vetted for the `setup_step` event and is safe to reuse.
|
||||
*/
|
||||
function setupCommandOutcomeAnnotation(input: {
|
||||
status: 'failed' | 'missing-input' | 'cancelled';
|
||||
step: TelemetrySetupStep;
|
||||
interactive: boolean;
|
||||
errorDetail?: string;
|
||||
}): SetupCommandAnnotation {
|
||||
if (input.status === 'failed') {
|
||||
return {
|
||||
outcome: 'error',
|
||||
errorClass: 'KtxSetupStepFailed',
|
||||
errorDetail: input.errorDetail ?? `${input.step} setup step failed`,
|
||||
};
|
||||
}
|
||||
if (input.status === 'missing-input' && !input.interactive) {
|
||||
return {
|
||||
outcome: 'error',
|
||||
errorClass: 'KtxSetupMissingInput',
|
||||
errorDetail: `${input.step} setup step requires input not provided in a non-interactive run`,
|
||||
};
|
||||
}
|
||||
return { outcome: 'aborted' };
|
||||
}
|
||||
|
||||
/**
|
||||
* Single source of truth for how a non-ready setup step ends: the process exit
|
||||
* code and the telemetry annotation are both derived from one classification,
|
||||
* so they can never disagree. A genuine failure (`error`) exits non-zero; an
|
||||
* abort — the user leaving an interactive wizard — exits 0, matching the entry
|
||||
* menu's "Exit", a project cancellation, and a confirmed Ctrl+C.
|
||||
*/
|
||||
/** @internal */
|
||||
export function setupTerminalOutcome(input: {
|
||||
status: 'failed' | 'missing-input' | 'cancelled';
|
||||
step: TelemetrySetupStep;
|
||||
interactive: boolean;
|
||||
errorDetail?: string;
|
||||
}): { exitCode: number; annotation: SetupCommandAnnotation } {
|
||||
const annotation = setupCommandOutcomeAnnotation(input);
|
||||
return { exitCode: annotation.outcome === 'error' ? 1 : 0, annotation };
|
||||
}
|
||||
|
||||
async function annotateSetupCommandOutcome(annotation: SetupCommandAnnotation): Promise<void> {
|
||||
const { annotateCommandOutcome } = await import('./telemetry/index.js');
|
||||
annotateCommandOutcome(annotation);
|
||||
}
|
||||
|
||||
async function recordSetupStep(input: {
|
||||
projectDir: string;
|
||||
step: TelemetrySetupStep;
|
||||
|
|
@ -573,6 +648,10 @@ async function runKtxSetupInner(args: KtxSetupArgs, io: KtxCliIo, deps: KtxSetup
|
|||
args.inputMode !== 'disabled' &&
|
||||
!args.agents &&
|
||||
(io.stdout.isTTY === true || deps.entryMenuDeps?.prompts !== undefined);
|
||||
// A prompt is only possible when input is enabled AND a TTY is attached. A
|
||||
// piped/CI `ktx setup` without `--no-input` is still `inputMode: 'auto'` but
|
||||
// cannot prompt, so its `missing-input` is an automation error, not an abort.
|
||||
const interactive = args.inputMode !== 'disabled' && io.stdout.isTTY === true;
|
||||
|
||||
setupLoop: while (true) {
|
||||
entryAction = undefined;
|
||||
|
|
@ -619,7 +698,13 @@ async function runKtxSetupInner(args: KtxSetupArgs, io: KtxCliIo, deps: KtxSetup
|
|||
}
|
||||
|
||||
if (projectResult.status !== 'ready') {
|
||||
return projectResult.status === 'cancelled' ? 0 : 1;
|
||||
const terminal = setupTerminalOutcome({
|
||||
status: projectResult.status,
|
||||
step: 'project',
|
||||
interactive,
|
||||
});
|
||||
await annotateSetupCommandOutcome(terminal.annotation);
|
||||
return terminal.exitCode;
|
||||
}
|
||||
|
||||
const agentsRequested = args.agents || entryAction === 'agents';
|
||||
|
|
@ -856,11 +941,15 @@ async function runKtxSetupInner(args: KtxSetupArgs, io: KtxCliIo, deps: KtxSetup
|
|||
...(stepResult.errorDetail ? { errorDetail: stepResult.errorDetail } : {}),
|
||||
});
|
||||
|
||||
if (stepResult.status === 'failed') {
|
||||
return 1;
|
||||
}
|
||||
if (stepResult.status === 'missing-input') {
|
||||
return 1;
|
||||
if (stepResult.status === 'failed' || stepResult.status === 'missing-input') {
|
||||
const terminal = setupTerminalOutcome({
|
||||
status: stepResult.status,
|
||||
step,
|
||||
interactive,
|
||||
...(stepResult.errorDetail ? { errorDetail: stepResult.errorDetail } : {}),
|
||||
});
|
||||
await annotateSetupCommandOutcome(terminal.annotation);
|
||||
return terminal.exitCode;
|
||||
}
|
||||
if (stepResult.status === 'back') {
|
||||
const previousIndex = previousNavigableStepIndex(stepIndex);
|
||||
|
|
|
|||
|
|
@ -9,6 +9,9 @@ interface CommandSpan {
|
|||
hasProject: boolean;
|
||||
attachProjectGroup: boolean;
|
||||
startedAt: number;
|
||||
annotatedOutcome?: CommandOutcome;
|
||||
annotatedErrorClass?: string;
|
||||
annotatedErrorDetail?: string;
|
||||
}
|
||||
|
||||
export interface CompletedCommandSpan {
|
||||
|
|
@ -29,6 +32,41 @@ export function beginCommandSpan(input: CommandSpan): void {
|
|||
activeCommandSpan = input;
|
||||
}
|
||||
|
||||
/**
|
||||
* Let a command action record the true outcome and reason on the active span.
|
||||
*
|
||||
* The Commander wrapper can only derive an outcome from a thrown error or the
|
||||
* process exit code, so a command that exits non-zero *without throwing* (e.g.
|
||||
* `ktx setup` when the user abandons the wizard) lands as `outcome: 'error'`
|
||||
* with no `errorClass`/`errorDetail` — an unactionable blank in the dashboard.
|
||||
* The action is the decision-maker: it can mark the run `aborted`, or attach a
|
||||
* scrubbed reason so the next occurrence is self-diagnosing. A later thrown
|
||||
* error still wins (see {@link completeCommandSpan}), since that is the most
|
||||
* authoritative signal and also feeds the `$exception` stream. No-ops when no
|
||||
* span is active so call sites stay safe in tests and bare-help paths.
|
||||
*
|
||||
* Values are emitted verbatim and must already satisfy the telemetry privacy
|
||||
* rules — pass synthetic or already-scrubbed strings, never raw user input.
|
||||
*/
|
||||
export function annotateCommandOutcome(input: {
|
||||
outcome?: CommandOutcome;
|
||||
errorClass?: string;
|
||||
errorDetail?: string;
|
||||
}): void {
|
||||
if (!activeCommandSpan) {
|
||||
return;
|
||||
}
|
||||
if (input.outcome !== undefined) {
|
||||
activeCommandSpan.annotatedOutcome = input.outcome;
|
||||
}
|
||||
if (input.errorClass !== undefined) {
|
||||
activeCommandSpan.annotatedErrorClass = input.errorClass;
|
||||
}
|
||||
if (input.errorDetail !== undefined) {
|
||||
activeCommandSpan.annotatedErrorDetail = input.errorDetail;
|
||||
}
|
||||
}
|
||||
|
||||
export function completeCommandSpan(input: {
|
||||
completedAt: number;
|
||||
outcome: CommandOutcome;
|
||||
|
|
@ -40,13 +78,17 @@ export function completeCommandSpan(input: {
|
|||
return undefined;
|
||||
}
|
||||
|
||||
const errorClass = input.error ? scrubErrorClass(input.error) : undefined;
|
||||
const errorDetail = input.error ? formatErrorDetail(input.error) : undefined;
|
||||
// Precedence: a thrown error is authoritative; otherwise an action's own
|
||||
// annotation; otherwise the wrapper's exit-code-derived outcome.
|
||||
const thrown = Boolean(input.error);
|
||||
const outcome = thrown ? input.outcome : (span.annotatedOutcome ?? input.outcome);
|
||||
const errorClass = thrown ? scrubErrorClass(input.error) : span.annotatedErrorClass;
|
||||
const errorDetail = thrown ? formatErrorDetail(input.error) : span.annotatedErrorDetail;
|
||||
|
||||
return {
|
||||
commandPath: span.commandPath,
|
||||
durationMs: Math.max(0, input.completedAt - span.startedAt),
|
||||
outcome: input.outcome,
|
||||
outcome,
|
||||
...(errorClass ? { errorClass } : {}),
|
||||
...(errorDetail ? { errorDetail } : {}),
|
||||
flagsPresent: span.flagsPresent,
|
||||
|
|
|
|||
|
|
@ -1,6 +1,7 @@
|
|||
import { getKtxCliPackageInfo, type KtxCliIo, type KtxCliPackageInfo } from '../cli-runtime.js';
|
||||
import { loadKtxProject } from '../context/project/project.js';
|
||||
import {
|
||||
annotateCommandOutcome,
|
||||
beginCommandSpan,
|
||||
completeCommandSpan,
|
||||
type CommandOutcome,
|
||||
|
|
@ -18,7 +19,7 @@ import {
|
|||
import { computeTelemetryProjectId, loadTelemetryIdentity } from './identity.js';
|
||||
import { buildProjectStackSnapshotFields } from './project-snapshot.js';
|
||||
|
||||
export { beginCommandSpan, completeCommandSpan, reportException, shutdownTelemetryEmitter };
|
||||
export { annotateCommandOutcome, beginCommandSpan, completeCommandSpan, reportException, shutdownTelemetryEmitter };
|
||||
export type { CommandOutcome, CompletedCommandSpan, ExceptionContext };
|
||||
|
||||
export async function showTelemetryNoticeIfNeeded(io: KtxCliIo, packageInfo: KtxCliPackageInfo): Promise<void> {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue