Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
54 changes: 54 additions & 0 deletions apps/cli/src/shared/cli/invalid-value-message.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
// Workaround for a doubled "Expected: Expected ..." prefix in
// effect@4.0.0-beta.93's own primitive parsers. Several `Primitive`s under
// `effect/unstable/cli` (`choice` — used by `Flag.choice`/
// `Flag.choiceWithValue` — plus the schema-backed `integer`, `float`,
// `boolean`, and `date`) fail with a raw message that already starts with
// the word "Expected" (e.g. `Expected "micro" | "small", got "nano"` or
// `Expected a valid date, got Invalid Date`), and `CliError.InvalidValue`'s
// own `message` getter independently prepends its own `"Expected: "` label
// on top of that — so any flag or argument backed by one of these
// primitives renders "Expected: Expected ...". Detect this from
// `error.expected` (the field the buggy primitives actually populate)
// rather than searching the fully composed `error.message`: `error.value`
// is user-controlled and interpolated into that same message (including a
// second time inside `expected` itself, via `choice`'s "got <value>"
// suffix), so a message-wide, first-occurrence string replace can target
// the wrong spot if the value itself happens to contain the literal text
// "Expected: Expected ". Anchoring on `error.expected` and rebuilding the
// message from the same template `CliError.InvalidValue` uses avoids ever
// scanning `error.value`. Remove once upstream `effect` fixes this (see
// CLI-1898).
//
// Shared by two call sites that each see `InvalidValue` failures at a
// different point in `effect`'s CLI runtime:
// - `subcommand-flag-suggestions.ts` formats errors that reach the
// `CliOutput.Formatter` via the `ShowHelp` envelope — i.e. ordinary
// subcommand/argument flags, validated while `Command.runWith` parses the
// command tree.
// - `normalize-error.ts` formats errors from `GlobalFlag.setting` flags
// (`--output-format`, and the legacy `--output`/`-o`, `--dns-resolver`,
// `--agent`), which `Command.runWith` validates in a later step that runs
// *outside* the `ShowHelp` path and therefore never reaches the
// formatter — it surfaces as a raw failure through `runCli`'s catch-all
// instead.
const EXPECTED_PREFIX = "Expected ";

export interface InvalidValueMessageFields {
readonly option: string;
readonly value: string;
readonly expected: string;
readonly kind: "flag" | "argument";
}

/**
* Rebuilds a `CliError.InvalidValue` message from its own template when
* `expected` carries the doubled "Expected" prefix. Returns `undefined` when
* `expected` is unaffected, so callers can fall back to the error's own
* untouched `message`.
*/
export function formatInvalidValueMessage(error: InvalidValueMessageFields): string | undefined {
if (!error.expected.startsWith(EXPECTED_PREFIX)) return undefined;
return error.kind === "argument"
? `Invalid value for argument <${error.option}>: "${error.value}". ${error.expected}`
: `Invalid value for flag --${error.option}: "${error.value}". ${error.expected}`;
}
15 changes: 15 additions & 0 deletions apps/cli/src/shared/cli/subcommand-flag-suggestions.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import type { CliError, Command, HelpDoc } from "effect/unstable/cli";
import { formatInvalidValueMessage } from "./invalid-value-message.ts";

export interface CliErrorSuggestionContext {
readonly rootCommand: Command.Command.Any;
Expand Down Expand Up @@ -228,6 +229,20 @@ export function formatCliErrorsForDisplay(
continue;
}

if (error._tag === "InvalidValue") {
const message = formatInvalidValueMessage(error);
if (message !== undefined) {
changed = true;
formatted.push({
_tag: error._tag,
message,
source: error,
changed: true,
});
continue;
}
}

formatted.push({ _tag: error._tag, message: error.message, source: error, changed: false });
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,4 +122,101 @@ describe("subcommand flag placement suggestions", () => {
);
expect(errors.errors[0]?.message).not.toContain("--project-ref=jacraenyzrorgjhsdvvf <value>");
});

it("collapses the doubled 'Expected: Expected' prefix for an invalid choice flag value", () => {
const errors = formatCliErrorsForDisplay([
new CliError.InvalidValue({
option: "size",
value: "nano",
expected: 'Expected "micro" | "small" | "medium", got "nano"',
kind: "flag",
}),
]);

expect(errors.changed).toBe(true);
expect(errors.errors).toHaveLength(1);
expect(errors.errors[0]?.message).toBe(
'Invalid value for flag --size: "nano". Expected "micro" | "small" | "medium", got "nano"',
);
expect(errors.errors[0]?.message).not.toMatch(/Expected:\s*Expected/);
});

it("collapses the doubled 'Expected: Expected' prefix for an invalid choice argument value", () => {
const errors = formatCliErrorsForDisplay([
new CliError.InvalidValue({
option: "level",
value: "bogus",
expected: 'Expected "debug" | "info", got "bogus"',
kind: "argument",
}),
]);

expect(errors.changed).toBe(true);
expect(errors.errors[0]?.message).toBe(
'Invalid value for argument <level>: "bogus". Expected "debug" | "info", got "bogus"',
);
expect(errors.errors[0]?.message).not.toMatch(/Expected:\s*Expected/);
});

it("also collapses the doubled prefix for a non-choice primitive whose failure text starts with 'Expected' (e.g. an invalid integer flag value)", () => {
// Real failure text from effect@4.0.0-beta.93's schema-backed `Primitive.integer`
// (also affects `float`, `boolean`, and `date` — every primitive whose parse
// failure happens to start with the word "Expected" hits the same doubling).
const errors = formatCliErrorsForDisplay([
new CliError.InvalidValue({
option: "port",
value: "abc",
expected: 'Expected a string representing a finite number, got "abc"',
kind: "flag",
}),
]);

expect(errors.changed).toBe(true);
expect(errors.errors[0]?.message).toBe(
'Invalid value for flag --port: "abc". Expected a string representing a finite number, got "abc"',
);
expect(errors.errors[0]?.message).not.toMatch(/Expected:\s*Expected/);
});

it("leaves invalid-value errors whose expected text does not start with 'Expected' unchanged", () => {
// Real failure text from effect@4.0.0-beta.93's `Primitive.keyValuePair` —
// it never starts with the word "Expected", so it isn't doubled by
// `CliError.InvalidValue`'s own "Expected: " prefix and needs no rewriting.
const errors = formatCliErrorsForDisplay([
new CliError.InvalidValue({
option: "define",
value: "bogus",
expected: "Invalid key=value format. Expected format: key=value, got: bogus",
kind: "flag",
}),
]);

expect(errors.changed).toBe(false);
expect(errors.errors[0]?.changed).toBe(false);
expect(errors.errors[0]?.message).toBe(
'Invalid value for flag --define: "bogus". Expected: Invalid key=value format. Expected format: key=value, got: bogus',
);
});

it("does not corrupt a value that itself contains the literal 'Expected: Expected' text", () => {
// Regression test: the fix must anchor on `error.expected` (the field the
// buggy primitive actually populates) rather than searching the fully
// composed `error.message`, since `error.value` is user-controlled and is
// interpolated into that same message twice (once directly, once again
// inside `expected`'s "got <value>" suffix). A value that happens to
// contain the literal doubled-prefix text must be left untouched.
const errors = formatCliErrorsForDisplay([
new CliError.InvalidValue({
option: "env",
value: "Expected: Expected nano",
expected: 'Expected "dev" | "staging" | "prod", got "Expected: Expected nano"',
kind: "flag",
}),
]);

expect(errors.changed).toBe(true);
expect(errors.errors[0]?.message).toBe(
'Invalid value for flag --env: "Expected: Expected nano". Expected "dev" | "staging" | "prod", got "Expected: Expected nano"',
);
});
});
40 changes: 40 additions & 0 deletions apps/cli/src/shared/output/normalize-error.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { Cause, Option } from "effect";
import { formatInvalidValueMessage } from "../cli/invalid-value-message.ts";

type NormalizedCliError = {
readonly code: string;
Expand All @@ -17,6 +18,16 @@ const readString = (value: ErrorRecord, key: string): string | undefined => {
return typeof field === "string" && field.trim().length > 0 ? field.trim() : undefined;
};

// Unlike `readString`, does not trim or reject empty strings. Use this for
// fields that carry raw user input (e.g. `CliError.InvalidValue#value`),
// where an empty string or meaningful surrounding whitespace is a legitimate
// value the user typed (`supabase --output-format ''`) and must be preserved
// and reported verbatim rather than normalized away.
const readRawString = (value: ErrorRecord, key: string): string | undefined => {
const field = value[key];
return typeof field === "string" ? field : undefined;
};

const mappedError = (error: ErrorRecord): NormalizedCliError | undefined => {
const tag = readString(error, "_tag");
switch (tag) {
Expand Down Expand Up @@ -75,6 +86,35 @@ const mappedError = (error: ErrorRecord): NormalizedCliError | undefined => {
: "Error: required flag(s) not set",
};
}
case "InvalidValue": {
// `CliError.InvalidValue` for a `GlobalFlag.setting` flag (e.g.
// `--output-format`, or the legacy `--output`/`-o`, `--dns-resolver`,
// `--agent`) never reaches `CliOutput.Formatter` — `Command.runWith`
// validates those flags in a step that runs outside the `ShowHelp`
// path, so the failure lands here instead. Apply the same
// doubled-"Expected"-prefix workaround `subcommand-flag-suggestions.ts`
// applies for the `ShowHelp`-formatted case (see CLI-1898), so every
// `InvalidValue` failure — whichever path it takes — renders the same
// way.
const option = readString(error, "option");
// Raw read: `value` is the exact argv token the user typed and can
// legitimately be `""` or carry surrounding whitespace — `readString`
// would trim it or drop it entirely, either masking the bug this case
// exists to fix or misreporting what the user actually typed.
const value = readRawString(error, "value");
const expected = readString(error, "expected");
const kind = readString(error, "kind");
if (
option !== undefined &&
value !== undefined &&
expected !== undefined &&
(kind === "flag" || kind === "argument")
) {
const message = formatInvalidValueMessage({ option, value, expected, kind });
if (message !== undefined) return { code: tag, message };
}
return undefined;
}
case "ShowHelp": {
// Effect CLI wraps parse errors in a ShowHelp envelope (`CliError.ts`)
// whose `errors` array holds the underlying causes. If exactly one of
Expand Down
93 changes: 93 additions & 0 deletions apps/cli/src/shared/output/normalize-error.unit.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { describe, expect, test } from "vitest";
import { Cause } from "effect";
import { CliError } from "effect/unstable/cli";
import { formatCliError, normalizeCause, normalizeCliError } from "./normalize-error.ts";

describe("normalizeCliError", () => {
Expand Down Expand Up @@ -50,6 +51,98 @@ describe("normalizeCliError", () => {
});
});

test("InvalidValue collapses the doubled 'Expected: Expected' prefix (e.g. a bad GlobalFlag.setting value)", () => {
// Regression test for CLI-1898: `--output-format`/`--dns-resolver`/`--agent`/
// legacy `--output` are `GlobalFlag.setting` flags backed by `Flag.choice`.
// `Command.runWith` validates their values in a step that runs outside the
// `ShowHelp` path, so a bad value never reaches `CliOutput.Formatter` (and
// `subcommand-flag-suggestions.ts`'s fix) — it surfaces here instead.
const error = new CliError.InvalidValue({
option: "output-format",
value: "bogus",
expected: 'Expected "text" | "json" | "stream-json", got "bogus"',
kind: "flag",
});

expect(normalizeCliError(error)).toEqual({
code: "InvalidValue",
message:
'Invalid value for flag --output-format: "bogus". Expected "text" | "json" | "stream-json", got "bogus"',
});
});

test("InvalidValue preserves an empty invalid value (e.g. `--output-format ''`)", () => {
// Regression test for a Codex review finding on CLI-1898: `value` is raw
// user input read straight off argv, so `''` is a legitimate way to
// trigger this failure. Reading it through the trim-and-reject-empty
// `readString` helper would fail the guard and leak the original
// doubled "Expected: Expected" message instead of fixing it.
const error = new CliError.InvalidValue({
option: "output-format",
value: "",
expected: 'Expected "text" | "json" | "stream-json", got ""',
kind: "flag",
});

expect(normalizeCliError(error)).toEqual({
code: "InvalidValue",
message:
'Invalid value for flag --output-format: "". Expected "text" | "json" | "stream-json", got ""',
});
});

test("InvalidValue preserves surrounding whitespace in the invalid value (e.g. `--output-format ' json'`)", () => {
// Regression test for the same Codex finding: trimming `value` would
// report a different string than what the user actually typed.
const error = new CliError.InvalidValue({
option: "output-format",
value: " json",
expected: 'Expected "text" | "json" | "stream-json", got " json"',
kind: "flag",
});

expect(normalizeCliError(error)).toEqual({
code: "InvalidValue",
message:
'Invalid value for flag --output-format: " json". Expected "text" | "json" | "stream-json", got " json"',
});
});

test("InvalidValue leaves an already-clean 'expected' message untouched", () => {
const error = new CliError.InvalidValue({
option: "define",
value: "bogus",
expected: "Invalid key=value format. Expected format: key=value, got: bogus",
kind: "flag",
});

expect(normalizeCliError(error)).toEqual({
code: "InvalidValue",
message:
'Invalid value for flag --define: "bogus". Expected: Invalid key=value format. Expected format: key=value, got: bogus',
});
});

test("ShowHelp envelope unwraps a single InvalidValue with the same doubled-prefix fix", () => {
const error = {
_tag: "ShowHelp",
commandPath: ["db", "lint"],
errors: [
new CliError.InvalidValue({
option: "level",
value: "bogus",
expected: 'Expected "warning" | "error", got "bogus"',
kind: "flag",
}),
],
};

expect(normalizeCliError(error)).toEqual({
code: "InvalidValue",
message: 'Invalid value for flag --level: "bogus". Expected "warning" | "error", got "bogus"',
});
});

test("ShowHelp envelope unwraps a single MissingOption to Cobra wording", () => {
// Effect CLI raises `ShowHelp` containing the parse error in its `errors`
// array. We unwrap to surface the actionable message instead of "Help requested".
Expand Down
18 changes: 18 additions & 0 deletions apps/cli/src/shared/output/text-formatter.unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,4 +53,22 @@ describe("textCliOutputFormatter", () => {
expect(text).toContain("Did you mean this?");
expect(text).toContain("--plan");
});

it("does not double the 'Expected' prefix for an invalid choice flag value", () => {
const formatter = textCliOutputFormatter();

const text = formatter.formatErrors([
new CliError.InvalidValue({
option: "size",
value: "nano",
expected: 'Expected "micro" | "small" | "medium", got "nano"',
kind: "flag",
}),
]);

expect(text).toContain(
'Invalid value for flag --size: "nano". Expected "micro" | "small" | "medium", got "nano"',
);
expect(text).not.toMatch(/Expected:\s*Expected/);
});
});