diff --git a/CHANGELOG.md b/CHANGELOG.md index ff29e86..452829b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -68,6 +68,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- Make first-run authentication guidance safe for agents and non-interactive shells, and keep configuration errors concise and readable. - Simplify `altertable update` to install by default, add `--check` mode, preserve inherited global flags, resolve compiled self-update paths safely, and verify source-checkout updates through the package manager's global binary. - Return exit code `1` for unhealthy `doctor` and `profile status` reports while preserving complete stdout output, and add actionable next steps to empty or partial `profile show` results. diff --git a/cli/src/commands/doctor/index.test.ts b/cli/src/commands/doctor/index.test.ts index 54d5d20..f234216 100644 --- a/cli/src/commands/doctor/index.test.ts +++ b/cli/src/commands/doctor/index.test.ts @@ -10,6 +10,7 @@ import { configFile, credentialsFile, kvSet } from "@/lib/config.ts"; let testHome = ""; let mockFile = ""; +let stdinIsTty: PropertyDescriptor | undefined; const VALID_WHOAMI = { principal: { @@ -34,9 +35,15 @@ beforeEach(() => { process.env.ALTERTABLE_CONFIG_HOME = testHome; process.env.ALTERTABLE_SECRET_BACKEND = "file"; process.env.ALTERTABLE_MOCK_HTTP_FILE = mockFile; + stdinIsTty = Object.getOwnPropertyDescriptor(process.stdin, "isTTY"); }); afterEach(() => { + if (stdinIsTty) { + Object.defineProperty(process.stdin, "isTTY", stdinIsTty); + } else { + Reflect.deleteProperty(process.stdin, "isTTY"); + } rmSync(testHome, { recursive: true, force: true }); delete process.env.ALTERTABLE_CONFIG_HOME; delete process.env.ALTERTABLE_SECRET_BACKEND; @@ -188,9 +195,21 @@ describe("doctor command", () => { }), ]), ); + const remediation = report.checks.flatMap( + (check: { remediation?: string[] }) => check.remediation ?? [], + ); + expect(remediation).toContain( + `Run: printf '%s' "$KEY" | altertable profile configure --api-key-stdin --env `, + ); + expect(remediation).toContain( + `Run: printf '%s' "$PASSWORD" | altertable profile configure --user --password-stdin`, + ); + expect(remediation).not.toContain("Run: altertable login"); + expect(remediation.every((line: string) => !line.includes("--scope"))).toBe(true); }); - test("renders remediation in human output", async () => { + test("renders non-interactive remediation when stdin is not a TTY", async () => { + Object.defineProperty(process.stdin, "isTTY", { value: false, configurable: true }); const harness = await runCommandWithTestRuntime(["doctor", "--offline"], { debug: false, json: false, @@ -199,7 +218,39 @@ describe("doctor command", () => { }); expect(harness.stdout[0]).toContain("ALTERTABLE CLI DOCTOR"); - expect(harness.stdout[0]).toContain("altertable profile configure --scope management"); + expect(harness.stdout[0]).toContain("--api-key-stdin --env "); + expect(harness.stdout[0]).toContain("--user --password-stdin"); + expect(harness.stdout[0]).toContain("ALTERTABLE_API_KEY and ALTERTABLE_ENV"); + expect(harness.stdout[0]).not.toContain("altertable login"); + expect(harness.stdout[0]).not.toContain("--scope"); expect(harness.stdout[0]).toContain("Result: unhealthy"); }); + + test("preserves login-first remediation for an interactive human terminal", async () => { + Object.defineProperty(process.stdin, "isTTY", { value: true, configurable: true }); + const harness = await runCommandWithTestRuntime(["doctor", "--offline"], { + debug: false, + json: false, + agent: false, + noColor: true, + }); + + expect(harness.stdout[0]).toContain("Run: altertable login"); + expect(harness.stdout[0]).toContain("altertable profile configure --scope management"); + expect(harness.stdout[0]).toContain("altertable profile configure --scope lakehouse"); + }); + + test("uses non-interactive remediation for agent output even with a TTY", async () => { + Object.defineProperty(process.stdin, "isTTY", { value: true, configurable: true }); + const harness = await runCommandWithTestRuntime(["--agent", "doctor", "--offline"]); + const report = JSON.parse(harness.stdout[0] ?? ""); + const remediation = report.checks.flatMap( + (check: { remediation?: string[] }) => check.remediation ?? [], + ); + + expect(remediation).toContain( + `Run: printf '%s' "$KEY" | altertable profile configure --api-key-stdin --env `, + ); + expect(remediation).not.toContain("Run: altertable login"); + }); }); diff --git a/cli/src/commands/doctor/index.ts b/cli/src/commands/doctor/index.ts index 7f8c0c5..d50584e 100644 --- a/cli/src/commands/doctor/index.ts +++ b/cli/src/commands/doctor/index.ts @@ -23,6 +23,7 @@ export const doctorCommand = defineCommand({ const report = await runDoctorChecks(createDoctorChecks(), { execution: createDiagnosticExecutionContext(runtime), offline: args.offline === true, + interactive: !runtime.context.json && !runtime.context.agent && process.stdin.isTTY === true, }); await writeCommandOutput( { diff --git a/cli/src/commands/doctor/lib/checks.ts b/cli/src/commands/doctor/lib/checks.ts index 700bed4..46a7049 100644 --- a/cli/src/commands/doctor/lib/checks.ts +++ b/cli/src/commands/doctor/lib/checks.ts @@ -57,22 +57,50 @@ function validateLakehouseProbeResponse(body: string): void { } } -function checkManagementCredentialPresence(auth: ProfileAuth): DoctorCheckOutcome { +function managementCredentialRemediation(context: DoctorCheckContext): string[] { + if (context.interactive) { + return ["Run: altertable login", "Or run: altertable profile configure --scope management"]; + } + return [ + `Run: printf '%s' "$KEY" | altertable profile configure --api-key-stdin --env `, + "Or set: ALTERTABLE_API_KEY and ALTERTABLE_ENV", + ]; +} + +function lakehouseCredentialRemediation(context: DoctorCheckContext): string[] { + if (context.interactive) { + return ["Run: altertable login", "Or run: altertable profile configure --scope lakehouse"]; + } + return [ + `Run: printf '%s' "$PASSWORD" | altertable profile configure --user --password-stdin`, + "Or set: ALTERTABLE_BASIC_AUTH_TOKEN or ALTERTABLE_LAKEHOUSE_USERNAME and ALTERTABLE_LAKEHOUSE_PASSWORD", + ]; +} + +function checkManagementCredentialPresence( + auth: ProfileAuth, + context: DoctorCheckContext, +): DoctorCheckOutcome { if (auth.management === "none") { - return failOutcome("No management credentials configured.", "configuration_error", [ - "Run: altertable login", - "Or run: altertable profile configure --scope management", - ]); + return failOutcome( + "No management credentials configured.", + "configuration_error", + managementCredentialRemediation(context), + ); } return passOutcome(`Configured (${auth.management}).`, { auth: auth.management }); } -function checkLakehouseCredentialPresence(auth: ProfileAuth): DoctorCheckOutcome { +function checkLakehouseCredentialPresence( + auth: ProfileAuth, + context: DoctorCheckContext, +): DoctorCheckOutcome { if (auth.lakehouse === "none") { - return failOutcome("No lakehouse credentials configured.", "configuration_error", [ - "Run: altertable login", - "Or run: altertable profile configure --scope lakehouse", - ]); + return failOutcome( + "No lakehouse credentials configured.", + "configuration_error", + lakehouseCredentialRemediation(context), + ); } return passOutcome(`Configured (${auth.lakehouse}).`, { auth: auth.lakehouse }); } @@ -141,7 +169,7 @@ export function createDoctorChecks(): DoctorCheck[] { id: "management.credentials", label: "Management auth", requires: ["credentials.store"], - run: () => checkManagementCredentialPresence(requireProfileAuth()), + run: (context) => checkManagementCredentialPresence(requireProfileAuth(), context), }, { id: "management.api", @@ -162,16 +190,16 @@ export function createDoctorChecks(): DoctorCheck[] { const identity = formatManagementIdentity(body); return passOutcome(`${endpoint} · ${identity}`, { endpoint, identity }); }, - remediation: () => [ + remediation: (context) => [ "Check the control-plane URL and management credentials.", - "Run: altertable profile configure --scope management", + ...managementCredentialRemediation(context), ], }, { id: "lakehouse.credentials", label: "Lakehouse auth", requires: ["credentials.store"], - run: () => checkLakehouseCredentialPresence(requireProfileAuth()), + run: (context) => checkLakehouseCredentialPresence(requireProfileAuth(), context), }, { id: "lakehouse.api", @@ -187,9 +215,9 @@ export function createDoctorChecks(): DoctorCheck[] { validateLakehouseProbeResponse(body); return passOutcome(`${endpoint} · SELECT 1 succeeded.`, { endpoint }); }, - remediation: () => [ + remediation: (context) => [ "Check the data-plane URL and lakehouse credentials.", - "Run: altertable profile configure --scope lakehouse", + ...lakehouseCredentialRemediation(context), ], }, ]; diff --git a/cli/src/commands/doctor/lib/model.ts b/cli/src/commands/doctor/lib/model.ts index 0166012..4246287 100644 --- a/cli/src/commands/doctor/lib/model.ts +++ b/cli/src/commands/doctor/lib/model.ts @@ -31,6 +31,7 @@ export type DoctorReport = { export type DoctorCheckContext = { execution: ExecutionContext; offline: boolean; + interactive: boolean; }; export type DoctorCheckOutcome = Omit; @@ -41,5 +42,5 @@ export type DoctorCheck = { requires?: readonly string[]; skip?: (context: DoctorCheckContext) => string | undefined; run: (context: DoctorCheckContext) => DoctorCheckOutcome | Promise; - remediation?: (error: unknown, context: DoctorCheckContext) => string[]; + remediation?: (context: DoctorCheckContext) => string[]; }; diff --git a/cli/src/commands/doctor/lib/runner.test.ts b/cli/src/commands/doctor/lib/runner.test.ts index 5a39768..1e0d6a3 100644 --- a/cli/src/commands/doctor/lib/runner.test.ts +++ b/cli/src/commands/doctor/lib/runner.test.ts @@ -6,6 +6,7 @@ import { runDoctorChecks } from "@/commands/doctor/lib/runner.ts"; function createDoctorContext(offline = false): DoctorCheckContext { return { offline, + interactive: false, execution: { profile: "test", cli: { debug: false, json: true, agent: false }, @@ -46,7 +47,10 @@ describe("runDoctorChecks", () => { run() { throw new ConfigurationError("Missing."); }, - remediation: () => ["Configure it."], + remediation: (context) => { + expect(context.execution.profile).toBe("test"); + return ["Configure it."]; + }, }, { id: "dependent", diff --git a/cli/src/commands/doctor/lib/runner.ts b/cli/src/commands/doctor/lib/runner.ts index 300fa7c..629250a 100644 --- a/cli/src/commands/doctor/lib/runner.ts +++ b/cli/src/commands/doctor/lib/runner.ts @@ -79,7 +79,7 @@ async function runDoctorCheck( code: serialized.code, http_status: serialized.status, details: serialized.details, - remediation: check.remediation?.(error, context), + remediation: check.remediation?.(context), duration_ms: Math.round(performance.now() - startedAt), }; } diff --git a/cli/src/commands/login/index.ts b/cli/src/commands/login/index.ts index c47944b..ca4affc 100644 --- a/cli/src/commands/login/index.ts +++ b/cli/src/commands/login/index.ts @@ -59,7 +59,10 @@ function isInteractiveTerminal(): boolean { function assertInteractiveLogin(): void { if (isJsonOutput(getCliContext()) || !isInteractiveTerminal()) { throw new ConfigurationError( - "altertable login needs an interactive terminal with a browser and does not support --json or --agent.\nFor headless setups use 'altertable profile configure --api-key atm_xxx --env '.", + "altertable login needs an interactive terminal with a browser and does not support --json or --agent.\n" + + "For headless setups, pipe a management key:\n" + + ` printf '%s' "$KEY" | altertable profile configure --api-key-stdin --env \n` + + "Or set ALTERTABLE_API_KEY and ALTERTABLE_ENV.", ); } } diff --git a/cli/src/lib/auth.test.ts b/cli/src/lib/auth.test.ts index 83aee36..74ba596 100644 --- a/cli/src/lib/auth.test.ts +++ b/cli/src/lib/auth.test.ts @@ -37,7 +37,7 @@ describe("auth", () => { test("getLakehouseAuthHeader throws ConfigurationError when credentials are missing", () => { expect(() => getLakehouseAuthHeader(profileName)).toThrow(ConfigurationError); expect(() => getLakehouseAuthHeader(profileName)).toThrow( - "No credentials. Run 'altertable login', 'altertable profile configure', or set ALTERTABLE_LAKEHOUSE_USERNAME/PASSWORD (or ALTERTABLE_BASIC_AUTH_TOKEN).", + "No lakehouse credentials. Run 'altertable login', 'altertable profile configure', or set ALTERTABLE_LAKEHOUSE_USERNAME/PASSWORD (or ALTERTABLE_BASIC_AUTH_TOKEN).", ); }); diff --git a/cli/src/lib/auth.ts b/cli/src/lib/auth.ts index e6f0be5..5055b01 100644 --- a/cli/src/lib/auth.ts +++ b/cli/src/lib/auth.ts @@ -90,7 +90,7 @@ export function resolveLakehouseCredential(profileName: string): LakehouseCreden return credential; } throw new ConfigurationError( - "No credentials. Run 'altertable login', 'altertable profile configure', or set ALTERTABLE_LAKEHOUSE_USERNAME/PASSWORD (or ALTERTABLE_BASIC_AUTH_TOKEN).", + "No lakehouse credentials. Run 'altertable login', 'altertable profile configure', or set ALTERTABLE_LAKEHOUSE_USERNAME/PASSWORD (or ALTERTABLE_BASIC_AUTH_TOKEN).", ); } diff --git a/cli/src/lib/errors.test.ts b/cli/src/lib/errors.test.ts index 04befd2..43f7431 100644 --- a/cli/src/lib/errors.test.ts +++ b/cli/src/lib/errors.test.ts @@ -40,18 +40,26 @@ afterEach(() => { describe("errors", () => { test("renderCliError formats CliError", () => { - expect(renderCliError(new CliError("x"))).toBe("ERROR x"); + expect(renderCliError(new CliError("x"))).toBe("[ERROR] x"); }); - test("renderCliErrorDetails preserves trusted line structure and sanitizes each line", () => { - const rendered = renderCliErrorDetails("First line\nRun this\u001b]0;spoofed\u0007"); + test("renderCliError preserves trusted line structure and sanitizes each line", () => { + const rendered = renderCliError( + new ConfigurationError("First line\nRun this\u001b]0;spoofed\u0007"), + ); - expect(rendered).toBe("ERROR First line\nRun this\\x1b]0;spoofed\\x07"); + expect(rendered).toBe("[ERROR] First line\nRun this\\x1b]0;spoofed\\x07"); expect(rendered).not.toContain("\\x0a"); expect(rendered).not.toContain("\u001b"); expect(rendered).not.toContain("\u0007"); }); + test("renderCliErrorDetails prefixes the first line with the documented marker", () => { + expect(renderCliErrorDetails("First detail\nSecond detail")).toBe( + "[ERROR] First detail\nSecond detail", + ); + }); + test("ConfigurationError uses EXIT_CONFIG", () => { const error = new ConfigurationError("missing config"); expect(error.exitCode).toBe(EXIT_CONFIG); @@ -60,14 +68,14 @@ describe("errors", () => { test("unknown errors render without stack traces", () => { const rendered = renderCliError(new TypeError("secret internal detail")); - expect(rendered).toBe("ERROR Unexpected error."); + expect(rendered).toBe("[ERROR] Unexpected error."); expect(rendered).not.toContain("secret internal detail"); expect(rendered).not.toContain("TypeError"); }); - test("shouldShowCommandExamplesOnError is true for usage CliErrors", () => { + test("shouldShowCommandExamplesOnError is true only for usage CliErrors", () => { expect(shouldShowCommandExamplesOnError(new CliError("Endpoint path is required."))).toBe(true); - expect(shouldShowCommandExamplesOnError(new ConfigurationError("Not configured."))).toBe(true); + expect(shouldShowCommandExamplesOnError(new ConfigurationError("Not configured."))).toBe(false); }); test("shouldShowCommandExamplesOnError is false for transport and HTTP errors", () => { diff --git a/cli/src/lib/errors.ts b/cli/src/lib/errors.ts index f78593c..1add3f0 100644 --- a/cli/src/lib/errors.ts +++ b/cli/src/lib/errors.ts @@ -290,5 +290,5 @@ export function shouldShowCommandExamplesOnError(error: unknown): boolean { ) { return false; } - return error.exitCode === EXIT_GENERIC || error.exitCode === EXIT_CONFIG; + return error.exitCode === EXIT_GENERIC; } diff --git a/cli/src/lib/profile-configure.ts b/cli/src/lib/profile-configure.ts index cc5671d..e76a0f7 100644 --- a/cli/src/lib/profile-configure.ts +++ b/cli/src/lib/profile-configure.ts @@ -84,7 +84,7 @@ function writeOutro( export function configureNonTtyErrorMessage(): string { return ( "Interactive configure requires a TTY. Examples:\n" + - " altertable profile configure --api-key atm_xxx --env production\n" + + ` printf '%s' "$KEY" | altertable profile configure --api-key-stdin --env production\n` + " printf '%s' \"$PASS\" | altertable profile configure --user alice --password-stdin" ); } diff --git a/cli/src/ui/error.ts b/cli/src/ui/error.ts index 8e579f8..f9e9e45 100644 --- a/cli/src/ui/error.ts +++ b/cli/src/ui/error.ts @@ -8,9 +8,9 @@ export function renderCliErrorJson(error: unknown): string { export function renderCliError(error: unknown): string { if (error instanceof CliError || (error instanceof Error && error.name === "CLIError")) { - return renderDisplayText([span("ERROR", "error"), span(` ${error.message}`)]); + return renderCliErrorDetails(error.message); } - return renderDisplayText([span("ERROR", "error"), span(" Unexpected error.")]); + return renderCliErrorDetails("Unexpected error."); } export function renderCliErrorDetails(details: string): string { @@ -18,7 +18,7 @@ export function renderCliErrorDetails(details: string): string { .split(/\r\n|\r|\n/) .map((line, index) => index === 0 - ? renderDisplayText([span("ERROR", "error"), span(` ${line}`)]) + ? renderDisplayText([span("[ERROR]", "error"), span(` ${line}`)]) : renderDisplayText(line), ) .join("\n"); diff --git a/tests/doctor.test.ts b/tests/doctor.test.ts index a406653..01163b6 100644 --- a/tests/doctor.test.ts +++ b/tests/doctor.test.ts @@ -39,11 +39,19 @@ describe("altertable doctor", () => { id: "management.credentials", status: "fail", code: "configuration_error", + remediation: [ + `Run: printf '%s' "$KEY" | altertable profile configure --api-key-stdin --env `, + "Or set: ALTERTABLE_API_KEY and ALTERTABLE_ENV", + ], }), expect.objectContaining({ id: "lakehouse.credentials", status: "fail", code: "configuration_error", + remediation: [ + `Run: printf '%s' "$PASSWORD" | altertable profile configure --user --password-stdin`, + "Or set: ALTERTABLE_BASIC_AUTH_TOKEN or ALTERTABLE_LAKEHOUSE_USERNAME and ALTERTABLE_LAKEHOUSE_PASSWORD", + ], }), ]), ); @@ -110,7 +118,10 @@ describe("altertable doctor", () => { expect(result.stderr).toBe(""); expect(result.stdout).toContain("ALTERTABLE CLI DOCTOR"); expect(result.stdout).toContain("Management auth"); - expect(result.stdout).toContain("altertable profile configure --scope management"); + expect(result.stdout).toContain("--api-key-stdin --env "); + expect(result.stdout).toContain("--user --password-stdin"); + expect(result.stdout).not.toContain("altertable login"); + expect(result.stdout).not.toContain("--scope"); expect(result.stdout).toContain("Result: unhealthy"); expect(result.stdout).not.toContain("undefined"); }); diff --git a/tests/scripting.test.ts b/tests/scripting.test.ts index 62f952f..c05a248 100644 --- a/tests/scripting.test.ts +++ b/tests/scripting.test.ts @@ -89,6 +89,25 @@ describe("scriptable exit codes and JSON errors", () => { } }); + test("missing query credentials identify the lakehouse plane", async () => { + const isolated = await createTestWorkspace({ + ALTERTABLE_API_KEY: undefined, + ALTERTABLE_ENV: undefined, + }); + try { + const result = await isolated.runCommand(`altertable --json query "SELECT 1"`); + const error = JSON.parse(result.stderr); + + expect(result.exitCode).toBe(10); + expect(error).toMatchObject({ + code: "configuration_error", + message: expect.stringContaining("No lakehouse credentials"), + }); + } finally { + await isolated.cleanup(); + } + }); + test("network errors exit 9 with network_error", async () => { const result = await workspace.runCommand("altertable api /whoami --json", { env: { ALTERTABLE_MANAGEMENT_API_BASE: "http://127.0.0.1:1", ALTERTABLE_MOCK_HTTP_FILE: undefined }, @@ -111,6 +130,31 @@ describe("scriptable exit codes and JSON errors", () => { expect(result.exitCode).toBe(1); }); + test.each(["login", "profile configure"])( + "%s renders multiline configuration guidance without command examples", + async (command) => { + const isolated = await createTestWorkspace({ + ALTERTABLE_API_KEY: undefined, + ALTERTABLE_ENV: undefined, + }); + try { + const result = await isolated.runCommand(`altertable ${command}`); + + expect(result.exitCode).toBe(10); + expect(result.stdout).toBe(""); + expect(result.stderr).toStartWith("[ERROR] "); + expect(result.stderr).toContain("altertable profile configure"); + expect(result.stderr).toContain("--api-key-stdin"); + expect(result.stderr).not.toContain("--api-key atm_"); + expect(result.stderr.trimEnd().split("\n").length).toBeGreaterThan(1); + expect(result.stderr).not.toContain("\\x0a"); + expect(result.stderr).not.toContain("\nEXAMPLES\n"); + } finally { + await isolated.cleanup(); + } + }, + ); + test("invalid trailing timeouts use the JSON error envelope", async () => { const result = await workspace.runCommand( 'altertable query "SELECT 1" --connect-timeout nope --json',