From 6d25013f4705bfc4d8b089c220f4eb9c9d5c237d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Chalifour?= Date: Mon, 20 Jul 2026 11:47:00 +0200 Subject: [PATCH 1/6] feat(diagnostics): add read-only doctor command --- COMMANDS.md | 22 +++ README.md | 18 ++ cli/src/commands/completion/index.test.ts | 3 +- cli/src/commands/doctor/index.test.ts | 91 ++++++++++ cli/src/commands/doctor/index.ts | 35 ++++ cli/src/commands/doctor/lib/checks.ts | 187 +++++++++++++++++++++ cli/src/commands/doctor/lib/model.ts | 44 +++++ cli/src/commands/doctor/lib/render.ts | 55 ++++++ cli/src/commands/doctor/lib/runner.test.ts | 119 +++++++++++++ cli/src/commands/doctor/lib/runner.ts | 105 ++++++++++++ cli/src/commands/index.ts | 2 + cli/src/lib/execution-context.ts | 10 +- cli/src/lib/http-request.ts | 10 +- cli/src/lib/lakehouse-provision.test.ts | 18 +- cli/src/lib/oauth-profile.test.ts | 35 ++++ cli/src/lib/profile-store.ts | 21 +++ cli/src/lib/profile/model.ts | 51 ++++-- tests/doctor.test.ts | 106 ++++++++++++ 18 files changed, 913 insertions(+), 19 deletions(-) create mode 100644 cli/src/commands/doctor/index.test.ts create mode 100644 cli/src/commands/doctor/index.ts create mode 100644 cli/src/commands/doctor/lib/checks.ts create mode 100644 cli/src/commands/doctor/lib/model.ts create mode 100644 cli/src/commands/doctor/lib/render.ts create mode 100644 cli/src/commands/doctor/lib/runner.test.ts create mode 100644 cli/src/commands/doctor/lib/runner.ts create mode 100644 tests/doctor.test.ts diff --git a/COMMANDS.md b/COMMANDS.md index c17f003..56dde18 100644 --- a/COMMANDS.md +++ b/COMMANDS.md @@ -338,6 +338,28 @@ altertable api spec altertable api spec --json ``` +### `altertable doctor` + +Diagnose local configuration and Altertable connectivity. + +**Usage** + +`altertable doctor [options]` + +**Options** + +| Option | Description | +| --- | --- | +| `--offline` | Inspect local configuration without contacting Altertable APIs. | + +**Examples** + +```bash +altertable doctor +altertable doctor --offline +altertable --json doctor +``` + ### `altertable update` Update Altertable CLI to the latest release. diff --git a/README.md b/README.md index 86afd4a..bf9ecf2 100644 --- a/README.md +++ b/README.md @@ -24,6 +24,7 @@ Query and manage your Altertable data platform from the terminal. - [Commands](#commands) - [Lakehouse](#lakehouse) - [Management](#management) + - [Diagnostics](#diagnostics) - [Shell completion](#shell-completion) - [Global flags](#global-flags) - [Scripting](#scripting) @@ -415,6 +416,23 @@ altertable api /environments/production/connections --input postgres-connection. printf '%s' '{"name":"Analytics"}' | altertable api /environments/production/databases --input - ``` +### Diagnostics + +Run read-only checks against the selected profile, credential store, and both API +planes: + +```bash +altertable doctor +altertable doctor --offline +altertable --json doctor +``` + +`--offline` validates only local configuration and credential presence. Network +checks use the global `--connect-timeout` and `--read-timeout` values. Doctor +findings do not refresh OAuth tokens, provision lakehouse credentials, or modify +profile files. A completed diagnostic exits successfully even when its report is +unhealthy; scripts should inspect the JSON `healthy` field. + ### Shell completion Install completion for bash, zsh, or fish: diff --git a/cli/src/commands/completion/index.test.ts b/cli/src/commands/completion/index.test.ts index fbeabad..4f28006 100644 --- a/cli/src/commands/completion/index.test.ts +++ b/cli/src/commands/completion/index.test.ts @@ -261,8 +261,9 @@ describe("completion command", () => { const spec = await buildCompletionSpec(buildMainCommand()); const output = await runCompletion(buildMainCommand, "bash"); const topLevelCount = spec.subcommands.length; - expect(topLevelCount).toBe(14); + expect(topLevelCount).toBe(15); expect(output).toContain("completion"); + expect(output).toContain("doctor"); expect(output).toContain("upgrade"); expect(output).not.toContain(" GET "); }); diff --git a/cli/src/commands/doctor/index.test.ts b/cli/src/commands/doctor/index.test.ts new file mode 100644 index 0000000..06f9536 --- /dev/null +++ b/cli/src/commands/doctor/index.test.ts @@ -0,0 +1,91 @@ +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { join } from "node:path"; +import { tmpdir } from "node:os"; +import { configureRunSet } from "@/lib/profile-configure-core.ts"; +import { createCliRuntime } from "@/lib/runtime.ts"; +import { runCommandWithTestRuntime } from "@/test-utils/cli.ts"; +import { runWithCliRuntime } from "@/test-utils/runtime.ts"; + +let testHome = ""; +let mockFile = ""; + +beforeEach(() => { + testHome = mkdtempSync(join(tmpdir(), "altertable-doctor-test-")); + mockFile = join(testHome, "mocks.json"); + process.env.ALTERTABLE_CONFIG_HOME = testHome; + process.env.ALTERTABLE_SECRET_BACKEND = "file"; + process.env.ALTERTABLE_MOCK_HTTP_FILE = mockFile; +}); + +afterEach(() => { + rmSync(testHome, { recursive: true, force: true }); + delete process.env.ALTERTABLE_CONFIG_HOME; + delete process.env.ALTERTABLE_SECRET_BACKEND; + delete process.env.ALTERTABLE_MOCK_HTTP_FILE; +}); + +describe("doctor command", () => { + test("reports local findings without materializing the default profile", async () => { + const harness = await runCommandWithTestRuntime(["doctor", "--offline"]); + const report = JSON.parse(harness.stdout[0] ?? ""); + + expect(report).toMatchObject({ + healthy: false, + profile: "default", + summary: { passed: 3, failed: 2, skipped: 2 }, + }); + expect(await Bun.file(join(testHome, "config")).exists()).toBe(false); + expect(await Bun.file(join(testHome, "profiles", "default", "config")).exists()).toBe(false); + }); + + test("runs both read-only API probes for a configured profile", async () => { + const runtime = createCliRuntime({ debug: false, json: true, agent: false }); + await runWithCliRuntime(runtime, async () => { + await configureRunSet({ apiKey: "atm_test", env: "production" }); + await configureRunSet({ user: "alice", password: "secret" }); + }); + writeFileSync( + mockFile, + JSON.stringify([ + { + urlPattern: "/whoami", + method: "GET", + body: '{"principal":{"type":"User","name":"Jane","email":"jane@example.com"},"organization":{"name":"Acme","slug":"acme"}}', + }, + { urlPattern: "/query", method: "POST", body: "{}" }, + ]), + ); + + const harness = await runCommandWithTestRuntime(["doctor"]); + const report = JSON.parse(harness.stdout[0] ?? ""); + + expect(report).toMatchObject({ + healthy: true, + summary: { passed: 7, failed: 0, skipped: 0 }, + }); + expect(report.checks).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + id: "management.api", + status: "pass", + message: expect.stringContaining("Jane "), + }), + expect.objectContaining({ id: "lakehouse.api", status: "pass" }), + ]), + ); + }); + + test("renders remediation in human output", async () => { + const harness = await runCommandWithTestRuntime(["doctor", "--offline"], { + debug: false, + json: false, + agent: false, + noColor: true, + }); + + expect(harness.stdout[0]).toContain("ALTERTABLE CLI DOCTOR"); + expect(harness.stdout[0]).toContain("altertable profile configure --scope management"); + expect(harness.stdout[0]).toContain("Result: unhealthy"); + }); +}); diff --git a/cli/src/commands/doctor/index.ts b/cli/src/commands/doctor/index.ts new file mode 100644 index 0000000..0395df9 --- /dev/null +++ b/cli/src/commands/doctor/index.ts @@ -0,0 +1,35 @@ +import { defineCommand } from "@/lib/command.ts"; +import { writeCommandOutput } from "@/lib/command-output.ts"; +import { createDoctorChecks } from "@/commands/doctor/lib/checks.ts"; +import { formatDoctorReport } from "@/commands/doctor/lib/render.ts"; +import { runDoctorChecks } from "@/commands/doctor/lib/runner.ts"; +import { createReadOnlyExecutionContext } from "@/lib/execution-context.ts"; + +export const doctorCommand = defineCommand({ + metadata: { + name: "doctor", + commandGroup: "platform", + description: "Diagnose local configuration and Altertable connectivity.", + examples: ["altertable doctor", "altertable doctor --offline", "altertable --json doctor"], + }, + args: { + offline: { + type: "boolean", + description: "Inspect local configuration without contacting Altertable APIs.", + }, + }, + async run({ args, runtime, sink }) { + const report = await runDoctorChecks(createDoctorChecks(), { + execution: createReadOnlyExecutionContext(runtime), + offline: args.offline === true, + }); + await writeCommandOutput( + { + kind: "normalized", + data: report, + humanText: formatDoctorReport(report), + }, + sink, + ); + }, +}); diff --git a/cli/src/commands/doctor/lib/checks.ts b/cli/src/commands/doctor/lib/checks.ts new file mode 100644 index 0000000..667f7e7 --- /dev/null +++ b/cli/src/commands/doctor/lib/checks.ts @@ -0,0 +1,187 @@ +import { VERSION } from "@/version.ts"; +import { inspectProfileReadOnly, type ProfileInspect } from "@/lib/profile/model.ts"; +import { envConfigMode } from "@/lib/profile-store.ts"; +import { readEnv } from "@/lib/env.ts"; +import { configGetGlobal, resolveApiBase, resolveManagementApiBase } from "@/lib/config.ts"; +import { secretStoreDisplay } from "@/lib/secrets.ts"; +import { sendHttp } from "@/lib/http-request.ts"; +import { buildLakehouseVerifyRequest } from "@/lib/lakehouse/query.ts"; +import type { WhoamiResponse } from "@/lib/management/model.ts"; +import { formatWhoamiPrincipalLine } from "@/lib/management/render.ts"; +import type { + DoctorCheck, + DoctorCheckContext, + DoctorCheckOutcome, +} from "@/commands/doctor/lib/model.ts"; + +function pass(message: string, details?: Record): DoctorCheckOutcome { + return { status: "pass", message, details }; +} + +function fail( + message: string, + code: string, + remediation: string[], + details?: Record, +): DoctorCheckOutcome { + return { status: "fail", message, code, remediation, details }; +} + +function profileSource(context: DoctorCheckContext): string { + if (envConfigMode()) return "environment configuration"; + if (context.execution.cli.profile) return "--profile"; + if (readEnv("ALTERTABLE_PROFILE")) return "ALTERTABLE_PROFILE"; + return configGetGlobal("active_profile") ? "active profile" : "default profile"; +} + +function managementIdentity(body: string): string { + try { + return formatWhoamiPrincipalLine(JSON.parse(body) as WhoamiResponse); + } catch { + return "authenticated"; + } +} + +function managementCredentialCheck(profile: ProfileInspect): DoctorCheckOutcome { + if (profile.auth.management === "none") { + return fail("No management credentials configured.", "configuration_error", [ + "Run: altertable login", + "Or run: altertable profile configure --scope management", + ]); + } + return pass(`Configured (${profile.auth.management}).`, { + auth: profile.auth.management, + expires_at: profile.timestamps.oauth_expires_at, + }); +} + +function lakehouseCredentialCheck(profile: ProfileInspect): DoctorCheckOutcome { + if (profile.auth.lakehouse === "none") { + return fail("No lakehouse credentials configured.", "configuration_error", [ + "Run: altertable login", + "Or run: altertable profile configure --scope lakehouse", + ]); + } + return pass(`Configured (${profile.auth.lakehouse}).`, { + auth: profile.auth.lakehouse, + expires_at: profile.timestamps.lakehouse_expires_at, + }); +} + +function networkSkip(context: DoctorCheckContext): string | undefined { + return context.offline ? "Offline mode." : undefined; +} + +export function createDoctorChecks(): DoctorCheck[] { + let inspectedProfile: ProfileInspect | undefined; + const profile = (context: DoctorCheckContext): ProfileInspect => { + inspectedProfile ??= inspectProfileReadOnly(context.execution.profile); + return inspectedProfile; + }; + + return [ + { + id: "cli.runtime", + label: "CLI", + run: () => + pass(`v${VERSION} · ${process.platform} ${process.arch}`, { + version: VERSION, + platform: process.platform, + arch: process.arch, + runtime: `Bun ${process.versions.bun ?? Bun.version}`, + }), + }, + { + id: "profile.configuration", + label: "Profile", + run: (context) => { + const current = profile(context); + const dataPlane = resolveApiBase(current.name); + const controlPlane = resolveManagementApiBase(current.name); + return pass(`${current.name} (${profileSource(context)})`, { + name: current.name, + source: profileSource(context), + status: current.status, + environment: current.environment, + config_file: current.config_file, + endpoints: { + data_plane: dataPlane, + control_plane: controlPlane, + }, + }); + }, + remediation: () => ["Run: altertable profile show"], + }, + { + id: "credentials.store", + label: "Secret store", + requires: ["profile.configuration"], + run: (context) => { + if (envConfigMode()) { + return { + status: "skipped", + message: "Credentials supplied by environment variables.", + }; + } + profile(context); + const store = secretStoreDisplay(); + return pass(`${store} accessible.`, { store }); + }, + remediation: () => ["Run: altertable profile show"], + }, + { + id: "management.credentials", + label: "Management auth", + requires: ["profile.configuration"], + run: (context) => managementCredentialCheck(profile(context)), + }, + { + id: "management.api", + label: "Management API", + requires: ["management.credentials"], + skip: networkSkip, + async run(context) { + const endpoint = resolveManagementApiBase(context.execution.profile); + const body = await sendHttp( + { + plane: "management", + method: "GET", + endpoint: "/whoami", + authRecovery: false, + }, + context.execution, + ); + const identity = managementIdentity(body); + return pass(`${endpoint} · ${identity}`, { endpoint, identity }); + }, + remediation: () => [ + "Check the control-plane URL and management credentials.", + "Run: altertable profile configure --scope management", + ], + }, + { + id: "lakehouse.credentials", + label: "Lakehouse auth", + requires: ["profile.configuration"], + run: (context) => lakehouseCredentialCheck(profile(context)), + }, + { + id: "lakehouse.api", + label: "Lakehouse API", + requires: ["lakehouse.credentials"], + skip: networkSkip, + async run(context) { + const endpoint = resolveApiBase(context.execution.profile); + await sendHttp( + { ...buildLakehouseVerifyRequest(), authRecovery: false }, + context.execution, + ); + return pass(`${endpoint} · SELECT 1 succeeded.`, { endpoint }); + }, + remediation: () => [ + "Check the data-plane URL and lakehouse credentials.", + "Run: altertable profile configure --scope lakehouse", + ], + }, + ]; +} diff --git a/cli/src/commands/doctor/lib/model.ts b/cli/src/commands/doctor/lib/model.ts new file mode 100644 index 0000000..8d67d23 --- /dev/null +++ b/cli/src/commands/doctor/lib/model.ts @@ -0,0 +1,44 @@ +import type { ExecutionContext } from "@/lib/execution-context.ts"; + +export type DoctorCheckStatus = "pass" | "warn" | "fail" | "skipped"; + +export type DoctorCheckResult = { + id: string; + label: string; + status: DoctorCheckStatus; + message: string; + code?: string; + details?: Record; + remediation?: string[]; + duration_ms?: number; +}; + +export type DoctorSummary = { + passed: number; + warnings: number; + failed: number; + skipped: number; +}; + +export type DoctorReport = { + healthy: boolean; + profile: string; + summary: DoctorSummary; + checks: DoctorCheckResult[]; +}; + +export type DoctorCheckContext = { + execution: ExecutionContext; + offline: boolean; +}; + +export type DoctorCheckOutcome = Omit; + +export type DoctorCheck = { + id: string; + label: string; + requires?: readonly string[]; + skip?: (context: DoctorCheckContext) => string | undefined; + run: (context: DoctorCheckContext) => DoctorCheckOutcome | Promise; + remediation?: (error: unknown, context: DoctorCheckContext) => string[]; +}; diff --git a/cli/src/commands/doctor/lib/render.ts b/cli/src/commands/doctor/lib/render.ts new file mode 100644 index 0000000..c98d3af --- /dev/null +++ b/cli/src/commands/doctor/lib/render.ts @@ -0,0 +1,55 @@ +import type { + DoctorCheckResult, + DoctorCheckStatus, + DoctorReport, +} from "@/commands/doctor/lib/model.ts"; +import { span, type DisplayTextStyle } from "@/ui/document.ts"; +import { renderDisplayText } from "@/ui/terminal/styles.ts"; + +const STATUS_DISPLAY: Record = { + pass: { icon: "✓", style: "success" }, + warn: { icon: "!", style: "warning" }, + fail: { icon: "✗", style: "error" }, + skipped: { icon: "-", style: "muted" }, +}; + +function formatCheck(check: DoctorCheckResult, labelWidth: number): string[] { + const display = STATUS_DISPLAY[check.status]; + const lines = [ + renderDisplayText([ + span(display.icon, display.style), + span(` ${check.label.padEnd(labelWidth)} `, "strong"), + span(check.message, check.status === "skipped" ? "muted" : undefined), + ]), + ]; + for (const remediation of check.remediation ?? []) { + lines.push(renderDisplayText([span(`${" ".repeat(labelWidth + 4)}${remediation}`, "muted")])); + } + return lines; +} + +function formatSummary(report: DoctorReport): string { + const values = [ + `${report.summary.passed} passed`, + report.summary.warnings > 0 ? `${report.summary.warnings} warned` : undefined, + report.summary.failed > 0 ? `${report.summary.failed} failed` : undefined, + report.summary.skipped > 0 ? `${report.summary.skipped} skipped` : undefined, + ].filter((value): value is string => value !== undefined); + const style: DisplayTextStyle = report.healthy ? "success" : "error"; + return renderDisplayText([ + span("Result: ", "strong"), + span(report.healthy ? "healthy" : "unhealthy", style), + span(` · ${values.join(", ")}`, "muted"), + ]); +} + +export function formatDoctorReport(report: DoctorReport): string { + const labelWidth = Math.max(...report.checks.map((check) => check.label.length)); + return [ + renderDisplayText([span("Altertable CLI doctor", "heading")]), + "", + ...report.checks.flatMap((check) => formatCheck(check, labelWidth)), + "", + formatSummary(report), + ].join("\n"); +} diff --git a/cli/src/commands/doctor/lib/runner.test.ts b/cli/src/commands/doctor/lib/runner.test.ts new file mode 100644 index 0000000..68d60fa --- /dev/null +++ b/cli/src/commands/doctor/lib/runner.test.ts @@ -0,0 +1,119 @@ +import { describe, expect, test } from "bun:test"; +import { ConfigurationError } from "@/lib/errors.ts"; +import type { DoctorCheck, DoctorCheckContext } from "@/commands/doctor/lib/model.ts"; +import { runDoctorChecks } from "@/commands/doctor/lib/runner.ts"; + +function context(offline = false): DoctorCheckContext { + return { + offline, + execution: { + profile: "test", + cli: { debug: false, json: true, agent: false }, + output: { + json: true, + debug: false, + writeStderr() {}, + writeJson() {}, + writeRaw() {}, + writeHuman() {}, + writeMetadata() {}, + }, + }, + }; +} + +async function expectRejection(promise: Promise, message: string): Promise { + const error = await promise.then( + () => undefined, + (thrown: unknown) => thrown, + ); + expect(error).toBeInstanceOf(Error); + expect((error as Error).message).toContain(message); +} + +describe("runDoctorChecks", () => { + test("aggregates outcomes and isolates thrown check errors", async () => { + const checks: DoctorCheck[] = [ + { + id: "passing", + label: "Passing", + run: () => ({ status: "pass", message: "Ready." }), + }, + { + id: "failing", + label: "Failing", + run() { + throw new ConfigurationError("Missing."); + }, + remediation: () => ["Configure it."], + }, + { + id: "dependent", + label: "Dependent", + requires: ["failing"], + run: () => ({ status: "pass", message: "Should not run." }), + }, + ]; + + const report = await runDoctorChecks(checks, context()); + + expect(report.healthy).toBe(false); + expect(report.summary).toEqual({ passed: 1, warnings: 0, failed: 1, skipped: 1 }); + expect(report.checks[1]).toMatchObject({ + id: "failing", + status: "fail", + code: "configuration_error", + message: "Missing.", + remediation: ["Configure it."], + }); + expect(report.checks[2]).toMatchObject({ + status: "skipped", + message: "Blocked by failing.", + }); + }); + + test("supports intentional skips without making the report unhealthy", async () => { + const report = await runDoctorChecks( + [ + { + id: "network", + label: "Network", + skip: ({ offline }) => (offline ? "Offline mode." : undefined), + run: () => ({ status: "pass", message: "Connected." }), + }, + ], + context(true), + ); + + expect(report.healthy).toBe(true); + expect(report.summary.skipped).toBe(1); + expect(report.checks[0]).toMatchObject({ status: "skipped", message: "Offline mode." }); + }); + + test("rejects duplicate and forward dependency ids", async () => { + const duplicate: DoctorCheck = { + id: "same", + label: "Same", + run: () => ({ status: "pass", message: "Ready." }), + }; + await expectRejection( + runDoctorChecks([duplicate, duplicate], context()), + "Duplicate doctor check id", + ); + await expectRejection( + runDoctorChecks( + [ + { + id: "first", + label: "First", + requires: ["later"], + run: () => ({ status: "pass", message: "Ready." }), + }, + { ...duplicate, id: "later" }, + ], + context(), + ), + "requires unknown or later check", + ); + }); +}); diff --git a/cli/src/commands/doctor/lib/runner.ts b/cli/src/commands/doctor/lib/runner.ts new file mode 100644 index 0000000..3a30716 --- /dev/null +++ b/cli/src/commands/doctor/lib/runner.ts @@ -0,0 +1,105 @@ +import { errorCodeFromError } from "@/lib/errors.ts"; +import type { + DoctorCheck, + DoctorCheckContext, + DoctorCheckResult, + DoctorReport, + DoctorSummary, +} from "@/commands/doctor/lib/model.ts"; + +function errorMessage(error: unknown): string { + if (error instanceof Error) { + return error.message; + } + return "Check failed."; +} + +function summarize(checks: readonly DoctorCheckResult[]): DoctorSummary { + return { + passed: checks.filter((check) => check.status === "pass").length, + warnings: checks.filter((check) => check.status === "warn").length, + failed: checks.filter((check) => check.status === "fail").length, + skipped: checks.filter((check) => check.status === "skipped").length, + }; +} + +function blockedBy( + check: DoctorCheck, + results: ReadonlyMap, +): DoctorCheckResult | undefined { + for (const dependencyId of check.requires ?? []) { + const dependency = results.get(dependencyId); + if (!dependency) { + throw new Error(`Doctor check ${check.id} requires unknown or later check ${dependencyId}.`); + } + if (dependency.status !== "pass" && dependency.status !== "warn") { + return dependency; + } + } + return undefined; +} + +export async function runDoctorChecks( + checks: readonly DoctorCheck[], + context: DoctorCheckContext, +): Promise { + const results = new Map(); + + for (const check of checks) { + if (results.has(check.id)) { + throw new Error(`Duplicate doctor check id: ${check.id}`); + } + + const dependency = blockedBy(check, results); + if (dependency) { + results.set(check.id, { + id: check.id, + label: check.label, + status: "skipped", + message: `Blocked by ${dependency.label.toLowerCase()}.`, + }); + continue; + } + + const skipReason = check.skip?.(context); + if (skipReason) { + results.set(check.id, { + id: check.id, + label: check.label, + status: "skipped", + message: skipReason, + }); + continue; + } + + const startedAt = performance.now(); + try { + const outcome = await check.run(context); + results.set(check.id, { + ...outcome, + id: check.id, + label: check.label, + duration_ms: Math.round(performance.now() - startedAt), + }); + } catch (error) { + results.set(check.id, { + id: check.id, + label: check.label, + status: "fail", + message: errorMessage(error), + code: errorCodeFromError(error), + remediation: check.remediation?.(error, context), + duration_ms: Math.round(performance.now() - startedAt), + }); + } + } + + const checkResults = [...results.values()]; + const summary = summarize(checkResults); + return { + healthy: summary.failed === 0, + profile: context.execution.profile, + summary, + checks: checkResults, + }; +} diff --git a/cli/src/commands/index.ts b/cli/src/commands/index.ts index 06c1f0d..3855ca5 100644 --- a/cli/src/commands/index.ts +++ b/cli/src/commands/index.ts @@ -12,6 +12,7 @@ import { upsertCommand } from "@/commands/upsert/index.ts"; import { apiCommand } from "@/commands/api/index.ts"; import { createCompletionCommand } from "@/commands/completion/index.ts"; import { updateCommand } from "@/commands/update/index.ts"; +import { doctorCommand } from "@/commands/doctor/index.ts"; export function buildTopLevelCommands(getMainCommand: () => Command): Record { return { @@ -26,6 +27,7 @@ export function buildTopLevelCommands(getMainCommand: () => Command): Record { if (request.plane === "management") { - if (hasOAuthSession(context.profile)) { + if (request.authRecovery !== false && hasOAuthSession(context.profile)) { await ensureFreshAccessToken(context.profile); } return getManagementAuthHeader(context.profile); @@ -50,7 +55,7 @@ async function resolveRequestAuthHeader( if (existing) { return existing; } - if (hasManagementCredentials(context.profile)) { + if (request.authRecovery !== false && hasManagementCredentials(context.profile)) { return provisionLakehouseCredential(context); } return getLakehouseAuthHeader(context.profile); @@ -84,6 +89,7 @@ function isRecoverableLakehouseAuthFailure( error instanceof HttpError && error.status === 401 && request.plane === "lakehouse" && + request.authRecovery !== false && // A stream body was consumed by the failed attempt and cannot be resent. !(request.body instanceof ReadableStream) && canRecoverLakehouseAuth(profileName) diff --git a/cli/src/lib/lakehouse-provision.test.ts b/cli/src/lib/lakehouse-provision.test.ts index 3e75b8d..ef41013 100644 --- a/cli/src/lib/lakehouse-provision.test.ts +++ b/cli/src/lib/lakehouse-provision.test.ts @@ -65,9 +65,9 @@ function writeMocks(credentialBody: string = CREDENTIAL_BODY): void { ); } -async function sendLakehouseRequest(): Promise { +async function sendLakehouseRequest(authRecovery?: boolean): Promise { return sendHttp( - { plane: "lakehouse", method: "GET", endpoint: "/tables" }, + { plane: "lakehouse", method: "GET", endpoint: "/tables", authRecovery }, createExecutionContext(getCliRuntime()), ); } @@ -250,4 +250,18 @@ describe("lakehouse credential recovery after 401", () => { await expectRejection(sendLakehouseRequest(), "Authentication failed (401)"); expect(readFileSync(logFile, "utf8")).not.toContain("/whoami"); }); + + test("does not re-provision a managed credential when auth recovery is disabled", async () => { + storeProvisionedCredential(); + writeFileSync( + mockFile, + JSON.stringify([ + { urlPattern: "api.example.com/tables", method: "GET", status: 401, body: "" }, + ]), + ); + + await expectRejection(sendLakehouseRequest(false), "Authentication failed (401)"); + expect(secretGet("lakehouse/basic-token", profileName)).toBe(OLD_TOKEN); + expect(readFileSync(logFile, "utf8")).not.toContain("/whoami"); + }); }); diff --git a/cli/src/lib/oauth-profile.test.ts b/cli/src/lib/oauth-profile.test.ts index c1bb703..61841da 100644 --- a/cli/src/lib/oauth-profile.test.ts +++ b/cli/src/lib/oauth-profile.test.ts @@ -215,6 +215,41 @@ describe("management request auth resolution", () => { expect(Number.parseInt(configGet("oauth_expiry", profileName), 10)).toBeGreaterThan(Date.now()); }); + test("does not refresh an expired OAuth token when auth recovery is disabled", async () => { + writeFileSync( + mockFile, + JSON.stringify([ + { + urlPattern: "/whoami", + method: "GET", + authPattern: "stale", + body: '{"principal":{},"organization":{}}', + }, + ]), + ); + secretSet("oauth/access-token", "stale", profileName); + secretSet("oauth/refresh-token", "ref", profileName); + const expiredAt = String(Date.now() - 1000); + configSet("oauth_expiry", expiredAt, profileName); + + const runtime = createCliRuntime({ debug: false, json: false, agent: false }); + await runWithCliRuntime(runtime, async () => { + refreshCliRuntimeContext(getCliContext()); + await sendHttp( + { + plane: "management", + method: "GET", + endpoint: "/whoami", + authRecovery: false, + }, + createExecutionContext(runtime), + ); + }); + + expect(storedAccessToken()).toBe("stale"); + expect(configGet("oauth_expiry", profileName)).toBe(expiredAt); + }); + test("resolves the header live from the store, not a bootstrap cache", async () => { storeOAuthTokens({ access_token: "tok_a", refresh_token: "r", expires_in: 3600 }, profileName); diff --git a/cli/src/lib/profile-store.ts b/cli/src/lib/profile-store.ts index fb07bf9..93012e4 100644 --- a/cli/src/lib/profile-store.ts +++ b/cli/src/lib/profile-store.ts @@ -159,6 +159,27 @@ export function resolveWorkingProfile(override?: string): string { return getActiveProfileName(); } +export function resolveWorkingProfileReadOnly(override?: string): string { + if (envConfigMode()) { + return FROM_ENV_PSEUDOPROFILE_NAME; + } + + const explicit = override ?? readEnv("ALTERTABLE_PROFILE") ?? ""; + if (explicit.length > 0) { + assertSafeProfileName(explicit); + if (explicit !== DEFAULT_PROFILE_NAME && !profileExists(explicit)) { + throw new ConfigurationError(`Profile not found: ${explicit}`); + } + return explicit; + } + + const active = kvGet(configFile(), "active_profile"); + if (active.length > 0 && profileExists(active)) { + return active; + } + return DEFAULT_PROFILE_NAME; +} + export function listProfileNames(): string[] { ensureProfilesLayout(); if (!existsSync(profilesDir())) { diff --git a/cli/src/lib/profile/model.ts b/cli/src/lib/profile/model.ts index 844476c..b16a7d9 100644 --- a/cli/src/lib/profile/model.ts +++ b/cli/src/lib/profile/model.ts @@ -1,8 +1,10 @@ import { renameSync, rmSync } from "node:fs"; import { configDir, + configFile, configGet, configSet, + kvGet, resolveApiBase, resolveManagementApiBase, } from "@/lib/config.ts"; @@ -301,22 +303,15 @@ function inspectFromEnvProfile(): ProfileInspect { }; } -export function inspectProfile(name: string): ProfileInspect { - if (isFromEnvProfile(name)) { - return inspectFromEnvProfile(); - } - assertSafeProfileName(name); - ensureProfilesLayout(); - if (!profileExists(name)) { - throw new ConfigurationError(`Profile not found: ${name}`); - } - - const snapshot = readProfileSnapshot(name); +function profileInspectFromSnapshot( + name: string, + active: boolean, + snapshot: ProfileSnapshot, +): ProfileInspect { const { config, auth } = snapshot; - return { name, - active: getActiveProfileName() === name, + active, config_file: profileConfigFile(name), organization: { slug: config.organization_slug, @@ -345,6 +340,36 @@ export function inspectProfile(name: string): ProfileInspect { }; } +export function inspectProfile(name: string): ProfileInspect { + if (isFromEnvProfile(name)) { + return inspectFromEnvProfile(); + } + assertSafeProfileName(name); + ensureProfilesLayout(); + if (!profileExists(name)) { + throw new ConfigurationError(`Profile not found: ${name}`); + } + + return profileInspectFromSnapshot( + name, + getActiveProfileName() === name, + readProfileSnapshot(name), + ); +} + +export function inspectProfileReadOnly(name: string): ProfileInspect { + if (isFromEnvProfile(name)) { + return inspectFromEnvProfile(); + } + assertSafeProfileName(name); + if (name !== DEFAULT_PROFILE_NAME && !profileExists(name)) { + throw new ConfigurationError(`Profile not found: ${name}`); + } + + const activeProfile = kvGet(configFile(), "active_profile") || DEFAULT_PROFILE_NAME; + return profileInspectFromSnapshot(name, activeProfile === name, readProfileSnapshot(name)); +} + export function createEmptyProfile(name: string): ProfileInspect { assertSafeProfileName(name); ensureProfilesLayout(); diff --git a/tests/doctor.test.ts b/tests/doctor.test.ts new file mode 100644 index 0000000..cc6994e --- /dev/null +++ b/tests/doctor.test.ts @@ -0,0 +1,106 @@ +import { beforeAll, beforeEach, describe, expect, test } from "bun:test"; +import { createTestWorkspace, type TestWorkspace } from "./helpers.ts"; +import { jsonMock } from "./mock-http.ts"; + +describe("altertable doctor", () => { + let workspace: TestWorkspace; + + beforeAll(async () => { + workspace = await createTestWorkspace({ + ALTERTABLE_API_BASE: undefined, + ALTERTABLE_API_KEY: undefined, + ALTERTABLE_BASIC_AUTH_TOKEN: undefined, + ALTERTABLE_ENV: undefined, + ALTERTABLE_LAKEHOUSE_PASSWORD: undefined, + ALTERTABLE_LAKEHOUSE_USERNAME: undefined, + ALTERTABLE_MANAGEMENT_API_BASE: undefined, + }); + }); + + beforeEach(async () => { + await workspace.resetConfig(); + await workspace.resetNetwork(); + }); + + test("reports missing credentials as findings instead of a command error", async () => { + const result = await workspace.runCommand("altertable --json doctor --offline"); + + expect(result.exitCode).toBe(0); + expect(result.stderr).toBe(""); + const report = JSON.parse(result.stdout); + expect(report).toMatchObject({ + healthy: false, + profile: "default", + summary: { passed: 3, warnings: 0, failed: 2, skipped: 2 }, + }); + expect(report.checks).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + id: "management.credentials", + status: "fail", + code: "configuration_error", + }), + expect.objectContaining({ + id: "lakehouse.credentials", + status: "fail", + code: "configuration_error", + }), + ]), + ); + expect(await workspace.fileExists(workspace.configFile)).toBe(false); + expect(await workspace.fileExists(workspace.defaultProfileConfig)).toBe(false); + expect(await workspace.fileExists(workspace.credentialsFile)).toBe(false); + }); + + test("verifies both API planes with read-only probes", async () => { + expect( + ( + await workspace.runCommand( + "altertable profile configure --api-key atm_test --env production", + ) + ).exitCode, + ).toBe(0); + expect( + ( + await workspace.runCommand( + "altertable profile configure --user alice --password secret", + ) + ).exitCode, + ).toBe(0); + await workspace.setupMockHttp([ + jsonMock("GET", "/whoami", { + principal: { type: "User", name: "Jane", email: "jane@example.com" }, + organization: { name: "Acme", slug: "acme" }, + }), + jsonMock("POST", "/query", {}), + ]); + await workspace.setupHttpLog(); + + const credentialsBefore = await workspace.readFile(workspace.credentialsFile); + const result = await workspace.runCommand("altertable --json doctor"); + + expect(result.exitCode).toBe(0); + expect(result.stderr).toBe(""); + expect(JSON.parse(result.stdout)).toMatchObject({ + healthy: true, + profile: "default", + summary: { passed: 7, warnings: 0, failed: 0, skipped: 0 }, + }); + expect(await workspace.readFile(workspace.credentialsFile)).toBe(credentialsBefore); + expect(await workspace.httpLogValues("METHOD")).toEqual(["GET", "POST"]); + expect(await workspace.httpLogValues("URL")).toEqual( + expect.arrayContaining([expect.stringContaining("/whoami"), expect.stringContaining("/query")]), + ); + }); + + test("renders an actionable human report", async () => { + const result = await workspace.runCommand("altertable doctor --offline"); + + expect(result.exitCode).toBe(0); + 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("Result: unhealthy"); + expect(result.stdout).not.toContain("undefined"); + }); +}); From b9b77f6b3259dfe7f8b24b0fb125bd1db39745b3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Chalifour?= Date: Mon, 20 Jul 2026 11:59:05 +0200 Subject: [PATCH 2/6] refactor(cli): centralize generated artifact drift checks --- DEVELOPMENT.md | 6 ++- cli/package.json | 2 + cli/scripts/generate-command-reference.ts | 12 +++-- cli/scripts/generate-openapi-index.ts | 10 ++-- cli/scripts/generate-openapi-types.ts | 13 +++-- cli/scripts/generated-artifact.test.ts | 65 +++++++++++++++++++++++ cli/scripts/generated-artifact.ts | 36 +++++++++++++ scripts/verify.sh | 3 +- 8 files changed, 132 insertions(+), 15 deletions(-) create mode 100644 cli/scripts/generated-artifact.test.ts create mode 100644 cli/scripts/generated-artifact.ts diff --git a/DEVELOPMENT.md b/DEVELOPMENT.md index 70e111a..5c2dea4 100644 --- a/DEVELOPMENT.md +++ b/DEVELOPMENT.md @@ -18,7 +18,9 @@ bun run lint:fix # oxlint --fix bun run format # oxfmt bun run format:check # CI formatting check bun run knip # required dead-code/unused-export check -bun run generate # regenerate OpenAPI types +bun run generate # regenerate OpenAPI types, operation index, and COMMANDS.md +bun run generate:commands # regenerate only COMMANDS.md after command changes +bun run generate:check # non-mutating generated-artifact drift check bun run spec:refresh # fetch hosted OpenAPI spec (see specs/rest/SPEC.md) + generate bun run build # bundle to cli/dist/cli.js bun run pack:check # build + dry-run pack (verify publish contents) @@ -167,7 +169,7 @@ NODE_TLS_REJECT_UNAUTHORIZED=0 altertable login From repo root, one script mirrors CI (minus native binary compile): ```bash -./scripts/verify.sh --quick # typecheck, lint, format, knip, coverage, openapi drift +./scripts/verify.sh --quick # typecheck, lint, format, knip, coverage, generated-artifact drift ./scripts/verify.sh # + build, pack:check, top-level black-box tests ./scripts/verify.sh --integration # + tests/integration.e2e.ts (requires mock at :15000) ``` diff --git a/cli/package.json b/cli/package.json index f22915b..697aafb 100644 --- a/cli/package.json +++ b/cli/package.json @@ -29,6 +29,8 @@ "test": "bun test", "test:coverage": "bun test --coverage", "generate": "bun run scripts/generate-openapi-types.ts && bun run scripts/generate-openapi-index.ts && bun run scripts/generate-command-reference.ts", + "generate:check": "bun run scripts/generate-openapi-types.ts --check && bun run scripts/generate-openapi-index.ts --check && bun run scripts/generate-command-reference.ts --check", + "generate:commands": "bun run scripts/generate-command-reference.ts", "spec:refresh": "curl -fsSL https://app.altertable.ai/rest/v1/openapi.yaml -o openapi/openapi.yaml && bun run generate && oxfmt openapi/openapi.yaml", "build": "bun run scripts/npm-bundle.ts", "release:build": "bun run scripts/release.ts build", diff --git a/cli/scripts/generate-command-reference.ts b/cli/scripts/generate-command-reference.ts index 576d763..a146401 100644 --- a/cli/scripts/generate-command-reference.ts +++ b/cli/scripts/generate-command-reference.ts @@ -1,11 +1,17 @@ -import { writeFileSync } from "node:fs"; import { join } from "node:path"; import { buildMainCommand } from "@/cli.ts"; import { resolveCommandDescriptor, validateCommandDescriptor } from "@/lib/command-descriptor.ts"; import { renderCommandReference } from "@/lib/command-reference.ts"; +import { generatedArtifactMode, syncGeneratedArtifact } from "@/../scripts/generated-artifact.ts"; const outputPath = join(import.meta.dir, "../../COMMANDS.md"); const descriptor = await resolveCommandDescriptor(buildMainCommand()); validateCommandDescriptor(descriptor); -writeFileSync(outputPath, renderCommandReference(descriptor)); -console.log(`Wrote ${outputPath}`); +const mode = generatedArtifactMode(process.argv.slice(2)); +syncGeneratedArtifact({ + outputPath, + content: renderCommandReference(descriptor), + mode, + generateCommand: "bun run generate:commands", +}); +console.log(`${mode === "check" ? "Checked" : "Wrote"} ${outputPath}`); diff --git a/cli/scripts/generate-openapi-index.ts b/cli/scripts/generate-openapi-index.ts index 46b6949..dd525e1 100644 --- a/cli/scripts/generate-openapi-index.ts +++ b/cli/scripts/generate-openapi-index.ts @@ -1,5 +1,6 @@ -import { readFileSync, writeFileSync } from "node:fs"; +import { readFileSync } from "node:fs"; import { join } from "node:path"; +import { generatedArtifactMode, syncGeneratedArtifact } from "@/../scripts/generated-artifact.ts"; const HTTP_METHODS = ["get", "post", "patch", "delete", "put"] as const; @@ -70,5 +71,8 @@ ${lines.join("\n")} ]; `; -writeFileSync(outputPath, output); -console.log(`Wrote ${outputPath} (${operations.length} operations)`); +const mode = generatedArtifactMode(process.argv.slice(2)); +syncGeneratedArtifact({ outputPath, content: output, mode }); +console.log( + `${mode === "check" ? "Checked" : "Wrote"} ${outputPath} (${operations.length} operations)`, +); diff --git a/cli/scripts/generate-openapi-types.ts b/cli/scripts/generate-openapi-types.ts index 2191fff..e7625cc 100644 --- a/cli/scripts/generate-openapi-types.ts +++ b/cli/scripts/generate-openapi-types.ts @@ -1,6 +1,7 @@ import openapiTS, { astToString } from "openapi-typescript"; -import { readFileSync, writeFileSync } from "node:fs"; +import { readFileSync } from "node:fs"; import { join } from "node:path"; +import { generatedArtifactMode, syncGeneratedArtifact } from "@/../scripts/generated-artifact.ts"; const specPath = join(import.meta.dir, "../openapi/openapi.yaml"); const outputPath = join(import.meta.dir, "../src/generated/openapi-types.ts"); @@ -8,8 +9,10 @@ const outputPath = join(import.meta.dir, "../src/generated/openapi-types.ts"); const spec = readFileSync(specPath, "utf8"); const ast = await openapiTS(spec); const types = astToString(ast); -writeFileSync( +const mode = generatedArtifactMode(process.argv.slice(2)); +syncGeneratedArtifact({ outputPath, - `/* AUTO-GENERATED by scripts/generate-openapi-types.ts — do not edit */\n\n${types}\n`, -); -console.log(`Wrote ${outputPath}`); + content: `/* AUTO-GENERATED by scripts/generate-openapi-types.ts — do not edit */\n\n${types}\n`, + mode, +}); +console.log(`${mode === "check" ? "Checked" : "Wrote"} ${outputPath}`); diff --git a/cli/scripts/generated-artifact.test.ts b/cli/scripts/generated-artifact.test.ts new file mode 100644 index 0000000..de11be0 --- /dev/null +++ b/cli/scripts/generated-artifact.test.ts @@ -0,0 +1,65 @@ +import { afterEach, describe, expect, test } from "bun:test"; +import { mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { join } from "node:path"; +import { tmpdir } from "node:os"; +import { generatedArtifactMode, syncGeneratedArtifact } from "@/../scripts/generated-artifact.ts"; + +const testDirectories: string[] = []; + +function artifactPath(): string { + const directory = mkdtempSync(join(tmpdir(), "altertable-generated-artifact-")); + testDirectories.push(directory); + return join(directory, "artifact.txt"); +} + +afterEach(() => { + for (const directory of testDirectories.splice(0)) { + rmSync(directory, { recursive: true, force: true }); + } +}); + +describe("generated artifact synchronization", () => { + test("selects check mode explicitly", () => { + expect(generatedArtifactMode([])).toBe("write"); + expect(generatedArtifactMode(["--check"])).toBe("check"); + }); + + test("writes generated content", () => { + const outputPath = artifactPath(); + + syncGeneratedArtifact({ outputPath, content: "generated\n", mode: "write" }); + + expect(readFileSync(outputPath, "utf8")).toBe("generated\n"); + }); + + test("checks current content without writing", () => { + const outputPath = artifactPath(); + writeFileSync(outputPath, "generated\n"); + + syncGeneratedArtifact({ outputPath, content: "generated\n", mode: "check" }); + + expect(readFileSync(outputPath, "utf8")).toBe("generated\n"); + }); + + test("reports stale and missing output with regeneration guidance", () => { + const stalePath = artifactPath(); + writeFileSync(stalePath, "old\n"); + + expect(() => + syncGeneratedArtifact({ + outputPath: stalePath, + content: "new\n", + mode: "check", + generateCommand: "bun run generate:commands", + }), + ).toThrow("Run: bun run generate:commands"); + + expect(() => + syncGeneratedArtifact({ + outputPath: artifactPath(), + content: "new\n", + mode: "check", + }), + ).toThrow("Generated artifact is stale"); + }); +}); diff --git a/cli/scripts/generated-artifact.ts b/cli/scripts/generated-artifact.ts new file mode 100644 index 0000000..354f4c4 --- /dev/null +++ b/cli/scripts/generated-artifact.ts @@ -0,0 +1,36 @@ +import { readFileSync, writeFileSync } from "node:fs"; + +export type GeneratedArtifactMode = "write" | "check"; + +type SyncGeneratedArtifactOptions = { + outputPath: string; + content: string; + mode: GeneratedArtifactMode; + generateCommand?: string; +}; + +export function generatedArtifactMode(argv: readonly string[]): GeneratedArtifactMode { + return argv.includes("--check") ? "check" : "write"; +} + +export function syncGeneratedArtifact({ + outputPath, + content, + mode, + generateCommand = "bun run generate", +}: SyncGeneratedArtifactOptions): void { + if (mode === "write") { + writeFileSync(outputPath, content); + return; + } + + let current: string | undefined; + try { + current = readFileSync(outputPath, "utf8"); + } catch { + // Missing output is reported as stale with the same remediation. + } + if (current !== content) { + throw new Error(`Generated artifact is stale: ${outputPath}\nRun: ${generateCommand}`); + } +} diff --git a/scripts/verify.sh b/scripts/verify.sh index 2737c3a..abecd21 100755 --- a/scripts/verify.sh +++ b/scripts/verify.sh @@ -50,8 +50,7 @@ run_step "typecheck" bun run typecheck run_step "top-level test typecheck" ./node_modules/.bin/tsc -p "${REPO_ROOT}/tsconfig.tests.json" run_step "lint" bun run lint run_step "format:check" bun run format:check -run_step "generate artifacts" bun run generate -run_step "generated artifact drift check" git diff --exit-code src/generated/openapi-types.ts src/generated/openapi-operations.ts ../COMMANDS.md +run_step "generated artifact drift check" bun run generate:check run_step "unit tests with coverage" bun run test:coverage run_step "knip" bun run knip run_step "knip (production)" bun run knip:production From 1b56385a355b012c06016951e9b117a836e19902 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Chalifour?= Date: Mon, 20 Jul 2026 12:11:50 +0200 Subject: [PATCH 3/6] fix(doctor): validate diagnostic API responses --- cli/src/commands/completion/index.test.ts | 29 ++++++++-- cli/src/commands/doctor/index.test.ts | 60 +++++++++++++++++++- cli/src/commands/doctor/lib/checks.ts | 24 ++++++-- cli/src/commands/doctor/lib/model.ts | 3 +- cli/src/commands/doctor/lib/render.test.ts | 26 +++++++++ cli/src/commands/doctor/lib/render.ts | 12 ++++ cli/src/commands/doctor/lib/runner.test.ts | 45 ++++++++++++++- cli/src/commands/doctor/lib/runner.ts | 16 ++---- cli/src/lib/management/model.test.ts | 40 +++++++++++++ cli/src/lib/management/model.ts | 65 +++++++++++++++++----- cli/src/lib/updater-config.ts | 2 +- cli/src/lib/updater.test.ts | 1 + tests/doctor.test.ts | 18 ++++-- 13 files changed, 298 insertions(+), 43 deletions(-) create mode 100644 cli/src/commands/doctor/lib/render.test.ts create mode 100644 cli/src/lib/management/model.test.ts diff --git a/cli/src/commands/completion/index.test.ts b/cli/src/commands/completion/index.test.ts index 4f28006..41c9629 100644 --- a/cli/src/commands/completion/index.test.ts +++ b/cli/src/commands/completion/index.test.ts @@ -2,10 +2,11 @@ import { afterEach, describe, expect, test } from "bun:test"; import { existsSync, mkdtempSync, readFileSync, rmSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; -import { defineCommand, type Command } from "@/lib/command.ts"; +import { defineCommand, resolveCommandValue, type Command } from "@/lib/command.ts"; import { executeCommand } from "@/lib/command-parser.ts"; import { buildMainCommand } from "@/cli.ts"; import { createCompletionCommand } from "@/commands/completion/index.ts"; +import { buildTopLevelCommands } from "@/commands/index.ts"; import { buildCompletionSpec } from "@/commands/completion/lib/spec.ts"; import { createCliRuntime } from "@/lib/runtime.ts"; import { runWithCliRuntime } from "@/test-utils/runtime.ts"; @@ -257,11 +258,31 @@ describe("completion command", () => { expect(visibleTerminalText(output)).toContain("Startup: left unchanged (--no-rc)"); }); - test("integration root command top-level count matches registry", async () => { + test("integration root commands and aliases match the registry", async () => { const spec = await buildCompletionSpec(buildMainCommand()); const output = await runCompletion(buildMainCommand, "bash"); - const topLevelCount = spec.subcommands.length; - expect(topLevelCount).toBe(15); + const registry = buildTopLevelCommands(buildMainCommand); + const registeredNames = ( + await Promise.all( + Object.values(registry).map(async (command) => { + const metadata = await resolveCommandValue(command.metadata); + const aliases = Array.isArray(metadata?.alias) + ? metadata.alias + : metadata?.alias + ? [metadata.alias] + : []; + return metadata?.name ? [metadata.name, ...aliases] : aliases; + }), + ) + ) + .flat() + .sort((left, right) => left.localeCompare(right)); + + expect( + spec.subcommands + .map((command) => command.name) + .sort((left, right) => left.localeCompare(right)), + ).toEqual(registeredNames); expect(output).toContain("completion"); expect(output).toContain("doctor"); expect(output).toContain("upgrade"); diff --git a/cli/src/commands/doctor/index.test.ts b/cli/src/commands/doctor/index.test.ts index 06f9536..99be392 100644 --- a/cli/src/commands/doctor/index.test.ts +++ b/cli/src/commands/doctor/index.test.ts @@ -10,6 +10,23 @@ import { runWithCliRuntime } from "@/test-utils/runtime.ts"; let testHome = ""; let mockFile = ""; +const VALID_WHOAMI = { + principal: { + id: "user-1", + type: "User", + name: "Jane", + email: "jane@example.com", + }, + organization: { + id: "org-1", + name: "Acme", + slug: "acme", + }, + authentication_scope: "user", +}; + +const VALID_LAKEHOUSE_PROBE = ['{"statement":"SELECT 1"}', '["result"]', "[1]"].join("\n"); + beforeEach(() => { testHome = mkdtempSync(join(tmpdir(), "altertable-doctor-test-")); mockFile = join(testHome, "mocks.json"); @@ -51,9 +68,9 @@ describe("doctor command", () => { { urlPattern: "/whoami", method: "GET", - body: '{"principal":{"type":"User","name":"Jane","email":"jane@example.com"},"organization":{"name":"Acme","slug":"acme"}}', + body: JSON.stringify(VALID_WHOAMI), }, - { urlPattern: "/query", method: "POST", body: "{}" }, + { urlPattern: "/query", method: "POST", body: VALID_LAKEHOUSE_PROBE }, ]), ); @@ -76,6 +93,45 @@ describe("doctor command", () => { ); }); + test("rejects malformed successful API responses", async () => { + const runtime = createCliRuntime({ debug: false, json: true, agent: false }); + await runWithCliRuntime(runtime, async () => { + await configureRunSet({ apiKey: "atm_test", env: "production" }); + await configureRunSet({ user: "alice", password: "secret" }); + }); + writeFileSync( + mockFile, + JSON.stringify([ + { urlPattern: "/whoami", method: "GET", body: "{}" }, + { urlPattern: "/query", method: "POST", body: "{}" }, + ]), + ); + + const harness = await runCommandWithTestRuntime(["doctor"]); + const report = JSON.parse(harness.stdout[0] ?? ""); + + expect(report).toMatchObject({ + healthy: false, + summary: { passed: 5, failed: 2, skipped: 0 }, + }); + expect(report.checks).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + id: "management.api", + status: "fail", + code: "parse_error", + details: "Expected principal and organization objects.", + }), + expect.objectContaining({ + id: "lakehouse.api", + status: "fail", + code: "parse_error", + details: "Expected SELECT 1 to return the numeric value 1.", + }), + ]), + ); + }); + test("renders remediation in human output", async () => { const harness = await runCommandWithTestRuntime(["doctor", "--offline"], { debug: false, diff --git a/cli/src/commands/doctor/lib/checks.ts b/cli/src/commands/doctor/lib/checks.ts index 667f7e7..4228970 100644 --- a/cli/src/commands/doctor/lib/checks.ts +++ b/cli/src/commands/doctor/lib/checks.ts @@ -6,8 +6,10 @@ import { configGetGlobal, resolveApiBase, resolveManagementApiBase } from "@/lib import { secretStoreDisplay } from "@/lib/secrets.ts"; import { sendHttp } from "@/lib/http-request.ts"; import { buildLakehouseVerifyRequest } from "@/lib/lakehouse/query.ts"; -import type { WhoamiResponse } from "@/lib/management/model.ts"; +import { parseWhoamiResponse } from "@/lib/management/model.ts"; import { formatWhoamiPrincipalLine } from "@/lib/management/render.ts"; +import { parseLakehouseQueryResponse, type LakehouseRow } from "@/lib/lakehouse-ndjson.ts"; +import { ParseError } from "@/lib/errors.ts"; import type { DoctorCheck, DoctorCheckContext, @@ -35,10 +37,19 @@ function profileSource(context: DoctorCheckContext): string { } function managementIdentity(body: string): string { - try { - return formatWhoamiPrincipalLine(JSON.parse(body) as WhoamiResponse); - } catch { - return "authenticated"; + return formatWhoamiPrincipalLine(parseWhoamiResponse(body)); +} + +function rowContainsProbeValue(row: LakehouseRow): boolean { + return Array.isArray(row) ? row.includes(1) : Object.values(row).includes(1); +} + +function validateLakehouseProbe(body: string): void { + const result = parseLakehouseQueryResponse(body); + if (!result.rows.some(rowContainsProbeValue)) { + throw new ParseError("Lakehouse probe returned an unexpected result.", { + details: "Expected SELECT 1 to return the numeric value 1.", + }); } } @@ -172,10 +183,11 @@ export function createDoctorChecks(): DoctorCheck[] { skip: networkSkip, async run(context) { const endpoint = resolveApiBase(context.execution.profile); - await sendHttp( + const body = await sendHttp( { ...buildLakehouseVerifyRequest(), authRecovery: false }, context.execution, ); + validateLakehouseProbe(body); return pass(`${endpoint} · SELECT 1 succeeded.`, { endpoint }); }, remediation: () => [ diff --git a/cli/src/commands/doctor/lib/model.ts b/cli/src/commands/doctor/lib/model.ts index 8d67d23..0166012 100644 --- a/cli/src/commands/doctor/lib/model.ts +++ b/cli/src/commands/doctor/lib/model.ts @@ -8,7 +8,8 @@ export type DoctorCheckResult = { status: DoctorCheckStatus; message: string; code?: string; - details?: Record; + http_status?: number; + details?: string | Record; remediation?: string[]; duration_ms?: number; }; diff --git a/cli/src/commands/doctor/lib/render.test.ts b/cli/src/commands/doctor/lib/render.test.ts new file mode 100644 index 0000000..3ea0b99 --- /dev/null +++ b/cli/src/commands/doctor/lib/render.test.ts @@ -0,0 +1,26 @@ +import { describe, expect, test } from "bun:test"; +import { formatDoctorReport } from "@/commands/doctor/lib/render.ts"; + +describe("formatDoctorReport", () => { + test("renders structured failure status and details", () => { + const output = formatDoctorReport({ + healthy: false, + profile: "default", + summary: { passed: 0, warnings: 0, failed: 1, skipped: 0 }, + checks: [ + { + id: "management.api", + label: "Management API", + status: "fail", + message: "Authentication failed.", + code: "auth_failed", + http_status: 401, + details: "Token expired", + }, + ], + }); + + expect(output).toContain("HTTP status: 401"); + expect(output).toContain("Details: Token expired"); + }); +}); diff --git a/cli/src/commands/doctor/lib/render.ts b/cli/src/commands/doctor/lib/render.ts index c98d3af..22cbe9b 100644 --- a/cli/src/commands/doctor/lib/render.ts +++ b/cli/src/commands/doctor/lib/render.ts @@ -22,6 +22,18 @@ function formatCheck(check: DoctorCheckResult, labelWidth: number): string[] { span(check.message, check.status === "skipped" ? "muted" : undefined), ]), ]; + if (check.http_status !== undefined) { + lines.push( + renderDisplayText([ + span(`${" ".repeat(labelWidth + 4)}HTTP status: ${check.http_status}`, "muted"), + ]), + ); + } + if (typeof check.details === "string") { + lines.push( + renderDisplayText([span(`${" ".repeat(labelWidth + 4)}Details: ${check.details}`, "muted")]), + ); + } for (const remediation of check.remediation ?? []) { lines.push(renderDisplayText([span(`${" ".repeat(labelWidth + 4)}${remediation}`, "muted")])); } diff --git a/cli/src/commands/doctor/lib/runner.test.ts b/cli/src/commands/doctor/lib/runner.test.ts index 68d60fa..cd78c50 100644 --- a/cli/src/commands/doctor/lib/runner.test.ts +++ b/cli/src/commands/doctor/lib/runner.test.ts @@ -1,5 +1,5 @@ import { describe, expect, test } from "bun:test"; -import { ConfigurationError } from "@/lib/errors.ts"; +import { ConfigurationError, HttpError } from "@/lib/errors.ts"; import type { DoctorCheck, DoctorCheckContext } from "@/commands/doctor/lib/model.ts"; import { runDoctorChecks } from "@/commands/doctor/lib/runner.ts"; @@ -90,6 +90,49 @@ describe("runDoctorChecks", () => { expect(report.checks[0]).toMatchObject({ status: "skipped", message: "Offline mode." }); }); + test("preserves HTTP status and CLI error details", async () => { + const report = await runDoctorChecks( + [ + { + id: "http", + label: "HTTP", + run() { + throw new HttpError({ + status: 401, + body: '{"message":"Token expired"}', + method: "GET", + url: "https://example.com/whoami", + parsedDetail: "Token expired", + authPlane: "management", + }); + }, + }, + { + id: "config", + label: "Config", + run() { + throw new ConfigurationError("Invalid configuration.", { + details: "The profile file is unreadable.", + }); + }, + }, + ], + context(), + ); + + expect(report.checks[0]).toMatchObject({ + status: "fail", + code: "auth_failed", + http_status: 401, + details: "Token expired", + }); + expect(report.checks[1]).toMatchObject({ + status: "fail", + code: "configuration_error", + details: "The profile file is unreadable.", + }); + }); + test("rejects duplicate and forward dependency ids", async () => { const duplicate: DoctorCheck = { id: "same", diff --git a/cli/src/commands/doctor/lib/runner.ts b/cli/src/commands/doctor/lib/runner.ts index 3a30716..78cc2c7 100644 --- a/cli/src/commands/doctor/lib/runner.ts +++ b/cli/src/commands/doctor/lib/runner.ts @@ -1,4 +1,4 @@ -import { errorCodeFromError } from "@/lib/errors.ts"; +import { serializeCliError } from "@/lib/errors.ts"; import type { DoctorCheck, DoctorCheckContext, @@ -7,13 +7,6 @@ import type { DoctorSummary, } from "@/commands/doctor/lib/model.ts"; -function errorMessage(error: unknown): string { - if (error instanceof Error) { - return error.message; - } - return "Check failed."; -} - function summarize(checks: readonly DoctorCheckResult[]): DoctorSummary { return { passed: checks.filter((check) => check.status === "pass").length, @@ -82,12 +75,15 @@ export async function runDoctorChecks( duration_ms: Math.round(performance.now() - startedAt), }); } catch (error) { + const serialized = serializeCliError(error); results.set(check.id, { id: check.id, label: check.label, status: "fail", - message: errorMessage(error), - code: errorCodeFromError(error), + message: serialized.message, + code: serialized.code, + http_status: serialized.status, + details: serialized.details, remediation: check.remediation?.(error, context), duration_ms: Math.round(performance.now() - startedAt), }); diff --git a/cli/src/lib/management/model.test.ts b/cli/src/lib/management/model.test.ts new file mode 100644 index 0000000..be4fc9b --- /dev/null +++ b/cli/src/lib/management/model.test.ts @@ -0,0 +1,40 @@ +import { describe, expect, test } from "bun:test"; +import { ParseError } from "@/lib/errors.ts"; +import { parseWhoamiResponse } from "@/lib/management/model.ts"; + +const VALID_WHOAMI = { + principal: { + id: "user-1", + type: "User" as const, + name: "Jane", + email: "jane@example.com", + }, + organization: { + id: "org-1", + name: "Acme", + slug: "acme", + }, + authentication_scope: "user", + environment_slug: "production", +}; + +describe("parseWhoamiResponse", () => { + test("parses the required management identity shape", () => { + expect(parseWhoamiResponse(JSON.stringify(VALID_WHOAMI))).toEqual(VALID_WHOAMI); + }); + + test("rejects invalid JSON and missing required fields", () => { + expect(() => parseWhoamiResponse("not-json")).toThrow(ParseError); + expect(() => parseWhoamiResponse("{}")).toThrow( + "Management identity response has an invalid shape", + ); + expect(() => + parseWhoamiResponse( + JSON.stringify({ + ...VALID_WHOAMI, + principal: { type: "User", name: "Jane" }, + }), + ), + ).toThrow("Management identity response has an invalid shape"); + }); +}); diff --git a/cli/src/lib/management/model.ts b/cli/src/lib/management/model.ts index 577abda..fac5653 100644 --- a/cli/src/lib/management/model.ts +++ b/cli/src/lib/management/model.ts @@ -1,17 +1,54 @@ -export type WhoamiResponse = { - principal: { - type?: string; - name?: string; - email?: string; - slug?: string; - }; - organization: { - name?: string; - slug?: string; - }; - authentication_scope?: string; - environment_slug?: string; -}; +import type { components } from "@/generated/openapi-types.ts"; +import { ParseError } from "@/lib/errors.ts"; +import { parseApiJson } from "@/lib/parse-api-json.ts"; + +export type WhoamiResponse = components["schemas"]["WhoamiResponse"]; + +function isRecord(value: unknown): value is Record { + return typeof value === "object" && value !== null && !Array.isArray(value); +} + +function hasRequiredString(record: Record, key: string): boolean { + return typeof record[key] === "string" && record[key].length > 0; +} + +function hasOptionalString(record: Record, key: string): boolean { + return record[key] === undefined || typeof record[key] === "string"; +} + +export function parseWhoamiResponse(body: string): WhoamiResponse { + const data = parseApiJson(body); + if (!isRecord(data) || !isRecord(data.principal) || !isRecord(data.organization)) { + throw new ParseError("Management identity response has an invalid shape.", { + details: "Expected principal and organization objects.", + }); + } + + const principal = data.principal; + const organization = data.organization; + const validPrincipal = + hasRequiredString(principal, "id") && + (principal.type === "User" || principal.type === "ServiceAccount") && + hasRequiredString(principal, "name") && + hasOptionalString(principal, "email") && + hasOptionalString(principal, "slug"); + const validOrganization = + hasRequiredString(organization, "id") && + hasRequiredString(organization, "name") && + hasRequiredString(organization, "slug"); + const validResponse = + validPrincipal && + validOrganization && + hasRequiredString(data, "authentication_scope") && + hasOptionalString(data, "environment_slug"); + + if (!validResponse) { + throw new ParseError("Management identity response has an invalid shape.", { + details: "Response does not match the required WhoamiResponse fields.", + }); + } + return data as WhoamiResponse; +} export type CatalogRow = { type: string; diff --git a/cli/src/lib/updater-config.ts b/cli/src/lib/updater-config.ts index 9ad92a2..a78f32e 100644 --- a/cli/src/lib/updater-config.ts +++ b/cli/src/lib/updater-config.ts @@ -96,7 +96,7 @@ export const UpdaterConfig = { manual: 10_000, }, intervalsMs: UpdaterIntervalsMs, - automaticCheckSkipCommands: ["completion", "update", "upgrade"], + automaticCheckSkipCommands: ["completion", "doctor", "update", "upgrade"], installCommands: UpdaterInstallCommands, installMethods: UpdaterInstallMethodConfig, releasePlatforms: RELEASE_PLATFORM_CONFIG, diff --git a/cli/src/lib/updater.test.ts b/cli/src/lib/updater.test.ts index bea85aa..1d8d622 100644 --- a/cli/src/lib/updater.test.ts +++ b/cli/src/lib/updater.test.ts @@ -643,6 +643,7 @@ describe("automatic update checks", () => { }); test("skips commands configured for automatic update checks", () => { + expect(UpdaterConfig.automaticCheckSkipCommands).toContain("doctor"); for (const commandName of UpdaterConfig.automaticCheckSkipCommands) { expect( shouldRunAutomaticUpdateCheck({ diff --git a/tests/doctor.test.ts b/tests/doctor.test.ts index cc6994e..0911ad0 100644 --- a/tests/doctor.test.ts +++ b/tests/doctor.test.ts @@ -1,6 +1,6 @@ import { beforeAll, beforeEach, describe, expect, test } from "bun:test"; import { createTestWorkspace, type TestWorkspace } from "./helpers.ts"; -import { jsonMock } from "./mock-http.ts"; +import { jsonMock, textMock } from "./mock-http.ts"; describe("altertable doctor", () => { let workspace: TestWorkspace; @@ -69,10 +69,20 @@ describe("altertable doctor", () => { ).toBe(0); await workspace.setupMockHttp([ jsonMock("GET", "/whoami", { - principal: { type: "User", name: "Jane", email: "jane@example.com" }, - organization: { name: "Acme", slug: "acme" }, + principal: { + id: "user-1", + type: "User", + name: "Jane", + email: "jane@example.com", + }, + organization: { id: "org-1", name: "Acme", slug: "acme" }, + authentication_scope: "user", }), - jsonMock("POST", "/query", {}), + textMock( + "POST", + "/query", + ['{"statement":"SELECT 1"}', '["result"]', "[1]"].join("\n"), + ), ]); await workspace.setupHttpLog(); From f97eeba2067ed563f9e254950482b965d2ccc281 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Chalifour?= Date: Mon, 20 Jul 2026 13:12:48 +0200 Subject: [PATCH 4/6] feat(doctor): run independent checks concurrently and isolate auth state --- cli/src/commands/completion/index.test.ts | 35 +----- cli/src/commands/doctor/index.test.ts | 58 ++++++++- cli/src/commands/doctor/lib/checks.ts | 67 +++++------ cli/src/commands/doctor/lib/runner.test.ts | 58 +++++++++ cli/src/commands/doctor/lib/runner.ts | 134 +++++++++++---------- cli/src/lib/profile-store.test.ts | 8 ++ cli/src/lib/profile-store.ts | 3 +- cli/src/lib/profile/model.ts | 70 +++++++---- cli/src/lib/secrets.test.ts | 12 ++ cli/src/lib/secrets.ts | 10 ++ cli/src/test-utils/keychain.ts | 7 +- 11 files changed, 303 insertions(+), 159 deletions(-) diff --git a/cli/src/commands/completion/index.test.ts b/cli/src/commands/completion/index.test.ts index 41c9629..95b5699 100644 --- a/cli/src/commands/completion/index.test.ts +++ b/cli/src/commands/completion/index.test.ts @@ -2,12 +2,10 @@ import { afterEach, describe, expect, test } from "bun:test"; import { existsSync, mkdtempSync, readFileSync, rmSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; -import { defineCommand, resolveCommandValue, type Command } from "@/lib/command.ts"; +import { defineCommand, type Command } from "@/lib/command.ts"; import { executeCommand } from "@/lib/command-parser.ts"; import { buildMainCommand } from "@/cli.ts"; import { createCompletionCommand } from "@/commands/completion/index.ts"; -import { buildTopLevelCommands } from "@/commands/index.ts"; -import { buildCompletionSpec } from "@/commands/completion/lib/spec.ts"; import { createCliRuntime } from "@/lib/runtime.ts"; import { runWithCliRuntime } from "@/test-utils/runtime.ts"; @@ -257,35 +255,4 @@ describe("completion command", () => { expect(existsSync(join(home, ".bashrc"))).toBe(false); expect(visibleTerminalText(output)).toContain("Startup: left unchanged (--no-rc)"); }); - - test("integration root commands and aliases match the registry", async () => { - const spec = await buildCompletionSpec(buildMainCommand()); - const output = await runCompletion(buildMainCommand, "bash"); - const registry = buildTopLevelCommands(buildMainCommand); - const registeredNames = ( - await Promise.all( - Object.values(registry).map(async (command) => { - const metadata = await resolveCommandValue(command.metadata); - const aliases = Array.isArray(metadata?.alias) - ? metadata.alias - : metadata?.alias - ? [metadata.alias] - : []; - return metadata?.name ? [metadata.name, ...aliases] : aliases; - }), - ) - ) - .flat() - .sort((left, right) => left.localeCompare(right)); - - expect( - spec.subcommands - .map((command) => command.name) - .sort((left, right) => left.localeCompare(right)), - ).toEqual(registeredNames); - expect(output).toContain("completion"); - expect(output).toContain("doctor"); - expect(output).toContain("upgrade"); - expect(output).not.toContain(" GET "); - }); }); diff --git a/cli/src/commands/doctor/index.test.ts b/cli/src/commands/doctor/index.test.ts index 99be392..1c070ee 100644 --- a/cli/src/commands/doctor/index.test.ts +++ b/cli/src/commands/doctor/index.test.ts @@ -1,11 +1,12 @@ import { afterEach, beforeEach, describe, expect, test } from "bun:test"; -import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { chmodSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; import { join } from "node:path"; import { tmpdir } from "node:os"; import { configureRunSet } from "@/lib/profile-configure-core.ts"; import { createCliRuntime } from "@/lib/runtime.ts"; import { runCommandWithTestRuntime } from "@/test-utils/cli.ts"; import { runWithCliRuntime } from "@/test-utils/runtime.ts"; +import { configFile, credentialsFile, kvSet } from "@/lib/config.ts"; let testHome = ""; let mockFile = ""; @@ -93,6 +94,61 @@ describe("doctor command", () => { ); }); + test("attributes credential backend failures to the secret store", async () => { + writeFileSync(credentialsFile(), "profile/default/api-key=atm_test\n", { mode: 0o644 }); + chmodSync(credentialsFile(), 0o644); + + const harness = await runCommandWithTestRuntime(["doctor", "--offline"]); + const report = JSON.parse(harness.stdout[0] ?? ""); + + expect(report).toMatchObject({ + healthy: false, + summary: { passed: 2, failed: 1, skipped: 4 }, + }); + expect(report.checks).toEqual( + expect.arrayContaining([ + expect.objectContaining({ id: "profile.configuration", status: "pass" }), + expect.objectContaining({ + id: "credentials.store", + status: "fail", + message: expect.stringContaining("permissions 644 are too open"), + }), + expect.objectContaining({ + id: "management.credentials", + status: "skipped", + message: "Blocked by secret store.", + }), + ]), + ); + }); + + test("reports a stale active profile instead of selecting the default", async () => { + kvSet(configFile(), "active_profile", "missing"); + + const harness = await runCommandWithTestRuntime(["doctor", "--offline"]); + const report = JSON.parse(harness.stdout[0] ?? ""); + + expect(report).toMatchObject({ + healthy: false, + profile: "missing", + summary: { passed: 1, failed: 1, skipped: 5 }, + }); + expect(report.checks).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + id: "profile.configuration", + status: "fail", + message: "Profile not found: missing", + }), + expect.objectContaining({ + id: "credentials.store", + status: "skipped", + message: "Blocked by profile.", + }), + ]), + ); + }); + test("rejects malformed successful API responses", async () => { const runtime = createCliRuntime({ debug: false, json: true, agent: false }); await runWithCliRuntime(runtime, async () => { diff --git a/cli/src/commands/doctor/lib/checks.ts b/cli/src/commands/doctor/lib/checks.ts index 4228970..66c895c 100644 --- a/cli/src/commands/doctor/lib/checks.ts +++ b/cli/src/commands/doctor/lib/checks.ts @@ -1,5 +1,9 @@ import { VERSION } from "@/version.ts"; -import { inspectProfileReadOnly, type ProfileInspect } from "@/lib/profile/model.ts"; +import { + inspectProfileAuth, + inspectProfileConfigurationReadOnly, + type ProfileAuth, +} from "@/lib/profile/model.ts"; import { envConfigMode } from "@/lib/profile-store.ts"; import { readEnv } from "@/lib/env.ts"; import { configGetGlobal, resolveApiBase, resolveManagementApiBase } from "@/lib/config.ts"; @@ -53,30 +57,24 @@ function validateLakehouseProbe(body: string): void { } } -function managementCredentialCheck(profile: ProfileInspect): DoctorCheckOutcome { - if (profile.auth.management === "none") { +function checkManagementCredentials(auth: ProfileAuth): DoctorCheckOutcome { + if (auth.management === "none") { return fail("No management credentials configured.", "configuration_error", [ "Run: altertable login", "Or run: altertable profile configure --scope management", ]); } - return pass(`Configured (${profile.auth.management}).`, { - auth: profile.auth.management, - expires_at: profile.timestamps.oauth_expires_at, - }); + return pass(`Configured (${auth.management}).`, { auth: auth.management }); } -function lakehouseCredentialCheck(profile: ProfileInspect): DoctorCheckOutcome { - if (profile.auth.lakehouse === "none") { +function checkLakehouseCredentials(auth: ProfileAuth): DoctorCheckOutcome { + if (auth.lakehouse === "none") { return fail("No lakehouse credentials configured.", "configuration_error", [ "Run: altertable login", "Or run: altertable profile configure --scope lakehouse", ]); } - return pass(`Configured (${profile.auth.lakehouse}).`, { - auth: profile.auth.lakehouse, - expires_at: profile.timestamps.lakehouse_expires_at, - }); + return pass(`Configured (${auth.lakehouse}).`, { auth: auth.lakehouse }); } function networkSkip(context: DoctorCheckContext): string | undefined { @@ -84,11 +82,14 @@ function networkSkip(context: DoctorCheckContext): string | undefined { } export function createDoctorChecks(): DoctorCheck[] { - let inspectedProfile: ProfileInspect | undefined; - const profile = (context: DoctorCheckContext): ProfileInspect => { - inspectedProfile ??= inspectProfileReadOnly(context.execution.profile); - return inspectedProfile; - }; + let profileAuth: ProfileAuth | undefined; + + function requireProfileAuth(): ProfileAuth { + if (!profileAuth) { + throw new Error("Secret store check did not inspect profile authentication."); + } + return profileAuth; + } return [ { @@ -106,18 +107,17 @@ export function createDoctorChecks(): DoctorCheck[] { id: "profile.configuration", label: "Profile", run: (context) => { - const current = profile(context); - const dataPlane = resolveApiBase(current.name); - const controlPlane = resolveManagementApiBase(current.name); - return pass(`${current.name} (${profileSource(context)})`, { + const current = inspectProfileConfigurationReadOnly(context.execution.profile); + const source = profileSource(context); + return pass(`${current.name} (${source})`, { name: current.name, - source: profileSource(context), - status: current.status, + source, environment: current.environment, config_file: current.config_file, + timestamps: current.timestamps, endpoints: { - data_plane: dataPlane, - control_plane: controlPlane, + data_plane: resolveApiBase(current.name), + control_plane: resolveManagementApiBase(current.name), }, }); }, @@ -128,13 +128,10 @@ export function createDoctorChecks(): DoctorCheck[] { label: "Secret store", requires: ["profile.configuration"], run: (context) => { + profileAuth = inspectProfileAuth(context.execution.profile); if (envConfigMode()) { - return { - status: "skipped", - message: "Credentials supplied by environment variables.", - }; + return pass("Credentials supplied by environment variables."); } - profile(context); const store = secretStoreDisplay(); return pass(`${store} accessible.`, { store }); }, @@ -143,8 +140,8 @@ export function createDoctorChecks(): DoctorCheck[] { { id: "management.credentials", label: "Management auth", - requires: ["profile.configuration"], - run: (context) => managementCredentialCheck(profile(context)), + requires: ["credentials.store"], + run: () => checkManagementCredentials(requireProfileAuth()), }, { id: "management.api", @@ -173,8 +170,8 @@ export function createDoctorChecks(): DoctorCheck[] { { id: "lakehouse.credentials", label: "Lakehouse auth", - requires: ["profile.configuration"], - run: (context) => lakehouseCredentialCheck(profile(context)), + requires: ["credentials.store"], + run: () => checkLakehouseCredentials(requireProfileAuth()), }, { id: "lakehouse.api", diff --git a/cli/src/commands/doctor/lib/runner.test.ts b/cli/src/commands/doctor/lib/runner.test.ts index cd78c50..945f56f 100644 --- a/cli/src/commands/doctor/lib/runner.test.ts +++ b/cli/src/commands/doctor/lib/runner.test.ts @@ -90,6 +90,64 @@ describe("runDoctorChecks", () => { expect(report.checks[0]).toMatchObject({ status: "skipped", message: "Offline mode." }); }); + test("runs independent checks concurrently and preserves report order", async () => { + const started: string[] = []; + let finishFirst!: () => void; + let finishSecond!: () => void; + const firstFinished = new Promise((resolve) => { + finishFirst = resolve; + }); + const secondFinished = new Promise((resolve) => { + finishSecond = resolve; + }); + + const reportPromise = runDoctorChecks( + [ + { + id: "first", + label: "First", + async run() { + started.push("first"); + await firstFinished; + return { status: "pass", message: "First done." }; + }, + }, + { + id: "second", + label: "Second", + async run() { + started.push("second"); + await secondFinished; + return { status: "pass", message: "Second done." }; + }, + }, + { + id: "dependent", + label: "Dependent", + requires: ["first", "second"], + run() { + started.push("dependent"); + return { status: "pass", message: "Dependent done." }; + }, + }, + ], + context(), + ); + + await Promise.resolve(); + expect(started).toEqual(["first", "second"]); + + finishSecond(); + await Promise.resolve(); + expect(started).toEqual(["first", "second"]); + + finishFirst(); + const report = await reportPromise; + + expect(started).toEqual(["first", "second", "dependent"]); + expect(report.checks.map((check) => check.id)).toEqual(["first", "second", "dependent"]); + }); + test("preserves HTTP status and CLI error details", async () => { const report = await runDoctorChecks( [ diff --git a/cli/src/commands/doctor/lib/runner.ts b/cli/src/commands/doctor/lib/runner.ts index 78cc2c7..415bfd1 100644 --- a/cli/src/commands/doctor/lib/runner.ts +++ b/cli/src/commands/doctor/lib/runner.ts @@ -16,81 +16,91 @@ function summarize(checks: readonly DoctorCheckResult[]): DoctorSummary { }; } -function blockedBy( - check: DoctorCheck, - results: ReadonlyMap, -): DoctorCheckResult | undefined { - for (const dependencyId of check.requires ?? []) { - const dependency = results.get(dependencyId); - if (!dependency) { - throw new Error(`Doctor check ${check.id} requires unknown or later check ${dependencyId}.`); +function validateChecks(checks: readonly DoctorCheck[]): void { + const precedingIds = new Set(); + for (const check of checks) { + if (precedingIds.has(check.id)) { + throw new Error(`Duplicate doctor check id: ${check.id}`); } - if (dependency.status !== "pass" && dependency.status !== "warn") { - return dependency; + for (const dependencyId of check.requires ?? []) { + if (!precedingIds.has(dependencyId)) { + throw new Error( + `Doctor check ${check.id} requires unknown or later check ${dependencyId}.`, + ); + } } + precedingIds.add(check.id); + } +} + +async function runCheck( + check: DoctorCheck, + context: DoctorCheckContext, + dependencies: readonly DoctorCheckResult[], +): Promise { + const blocker = dependencies.find( + (dependency) => dependency.status !== "pass" && dependency.status !== "warn", + ); + if (blocker) { + return { + id: check.id, + label: check.label, + status: "skipped", + message: `Blocked by ${blocker.label.toLowerCase()}.`, + }; + } + + const skipReason = check.skip?.(context); + if (skipReason) { + return { + id: check.id, + label: check.label, + status: "skipped", + message: skipReason, + }; + } + + const startedAt = performance.now(); + try { + const outcome = await check.run(context); + return { + ...outcome, + id: check.id, + label: check.label, + duration_ms: Math.round(performance.now() - startedAt), + }; + } catch (error) { + const serialized = serializeCliError(error); + return { + id: check.id, + label: check.label, + status: "fail", + message: serialized.message, + code: serialized.code, + http_status: serialized.status, + details: serialized.details, + remediation: check.remediation?.(error, context), + duration_ms: Math.round(performance.now() - startedAt), + }; } - return undefined; } export async function runDoctorChecks( checks: readonly DoctorCheck[], context: DoctorCheckContext, ): Promise { - const results = new Map(); + validateChecks(checks); + const pendingResults = new Map>(); for (const check of checks) { - if (results.has(check.id)) { - throw new Error(`Duplicate doctor check id: ${check.id}`); - } - - const dependency = blockedBy(check, results); - if (dependency) { - results.set(check.id, { - id: check.id, - label: check.label, - status: "skipped", - message: `Blocked by ${dependency.label.toLowerCase()}.`, - }); - continue; - } - - const skipReason = check.skip?.(context); - if (skipReason) { - results.set(check.id, { - id: check.id, - label: check.label, - status: "skipped", - message: skipReason, - }); - continue; - } - - const startedAt = performance.now(); - try { - const outcome = await check.run(context); - results.set(check.id, { - ...outcome, - id: check.id, - label: check.label, - duration_ms: Math.round(performance.now() - startedAt), - }); - } catch (error) { - const serialized = serializeCliError(error); - results.set(check.id, { - id: check.id, - label: check.label, - status: "fail", - message: serialized.message, - code: serialized.code, - http_status: serialized.status, - details: serialized.details, - remediation: check.remediation?.(error, context), - duration_ms: Math.round(performance.now() - startedAt), - }); - } + const dependencies = (check.requires ?? []).map((id) => pendingResults.get(id)!); + pendingResults.set( + check.id, + Promise.all(dependencies).then((results) => runCheck(check, context, results)), + ); } - const checkResults = [...results.values()]; + const checkResults = await Promise.all(pendingResults.values()); const summary = summarize(checkResults); return { healthy: summary.failed === 0, diff --git a/cli/src/lib/profile-store.test.ts b/cli/src/lib/profile-store.test.ts index e91103e..62ac225 100644 --- a/cli/src/lib/profile-store.test.ts +++ b/cli/src/lib/profile-store.test.ts @@ -11,8 +11,10 @@ import { profileConfigFile, profileExists, resolveWorkingProfile, + resolveWorkingProfileReadOnly, setActiveProfile, } from "@/lib/profile-store.ts"; +import { kvSet } from "@/lib/config.ts"; let testHome = ""; @@ -59,4 +61,10 @@ describe("profile store", () => { expect(profileExists("staging")).toBe(true); expect(profileExists("prod-eu")).toBe(true); }); + + test("preserves a stale active profile reference for read-only diagnostics", () => { + kvSet(join(testHome, "config"), "active_profile", "missing"); + + expect(resolveWorkingProfileReadOnly()).toBe("missing"); + }); }); diff --git a/cli/src/lib/profile-store.ts b/cli/src/lib/profile-store.ts index 93012e4..ec8bc4a 100644 --- a/cli/src/lib/profile-store.ts +++ b/cli/src/lib/profile-store.ts @@ -174,7 +174,8 @@ export function resolveWorkingProfileReadOnly(override?: string): string { } const active = kvGet(configFile(), "active_profile"); - if (active.length > 0 && profileExists(active)) { + if (active.length > 0) { + assertSafeProfileName(active); return active; } return DEFAULT_PROFILE_NAME; diff --git a/cli/src/lib/profile/model.ts b/cli/src/lib/profile/model.ts index b16a7d9..1ab55da 100644 --- a/cli/src/lib/profile/model.ts +++ b/cli/src/lib/profile/model.ts @@ -49,7 +49,7 @@ const PROFILE_SECRET_STORE: ProfileSecretStore = { moveProfileSecrets, secretDel type ProfileManagementAuth = "oauth" | "api_key" | "none"; type ProfileLakehouseAuth = "basic_token" | "username_password" | "none"; -type ProfileAuth = { +export type ProfileAuth = { management: ProfileManagementAuth; lakehouse: ProfileLakehouseAuth; }; @@ -80,7 +80,7 @@ export type ProfileUpdate = { controlPlane?: string; }; -export type ProfileInspect = { +export type ProfileConfiguration = { name: string; active: boolean; config_file: string; @@ -99,8 +99,6 @@ export type ProfileInspect = { data_plane?: string; control_plane?: string; }; - auth: ProfileAuth; - status: "configured" | "partial" | "empty"; timestamps: { created_at?: string; updated_at?: string; @@ -110,6 +108,11 @@ export type ProfileInspect = { }; }; +export type ProfileInspect = ProfileConfiguration & { + auth: ProfileAuth; + status: "configured" | "partial" | "empty"; +}; + type ProfileSnapshot = { config: Record; auth: ProfileAuth; @@ -180,7 +183,13 @@ function writeProfileUpdate(name: string, update: ProfileUpdate): void { } } -function profileAuth(name: string): ProfileAuth { +export function inspectProfileAuth(name: string): ProfileAuth { + if (isFromEnvProfile(name)) { + return { + management: hasEnvManagementCredentials() ? "api_key" : "none", + lakehouse: envLakehouseAuthKind(), + }; + } return { management: secretExists("oauth/access-token", name) ? "oauth" @@ -196,14 +205,14 @@ function profileAuth(name: string): ProfileAuth { } export function profileHasAnyAuthConfigured(name: string): boolean { - const auth = profileAuth(name); + const auth = inspectProfileAuth(name); return auth.management !== "none" || auth.lakehouse !== "none"; } function readProfileSnapshot(name: string): ProfileSnapshot { return { config: readProfileConfigRecord(name), - auth: profileAuth(name), + auth: inspectProfileAuth(name), }; } @@ -281,11 +290,7 @@ function envLakehouseAuthKind(): ProfileLakehouseAuth { return "none"; } -// The `_from_env` identity has no stored directory, so build its inspection view -// from the environment variables in effect rather than reading disk. -function inspectFromEnvProfile(): ProfileInspect { - const management: ProfileManagementAuth = hasEnvManagementCredentials() ? "api_key" : "none"; - const lakehouse = envLakehouseAuthKind(); +function inspectFromEnvConfiguration(): ProfileConfiguration { return { name: FROM_ENV_PSEUDOPROFILE_NAME, active: true, @@ -297,18 +302,15 @@ function inspectFromEnvProfile(): ProfileInspect { data_plane: readEnv("ALTERTABLE_API_BASE"), control_plane: readEnv("ALTERTABLE_MANAGEMENT_API_BASE"), }, - auth: { management, lakehouse }, - status: management !== "none" || lakehouse !== "none" ? "configured" : "empty", timestamps: {}, }; } -function profileInspectFromSnapshot( +function profileConfigurationFromConfig( name: string, active: boolean, - snapshot: ProfileSnapshot, -): ProfileInspect { - const { config, auth } = snapshot; + config: Record, +): ProfileConfiguration { return { name, active, @@ -328,8 +330,6 @@ function profileInspectFromSnapshot( data_plane: config.api_base, control_plane: config.management_api_base, }, - auth, - status: profileStatus(snapshot), timestamps: { created_at: config.created_at, updated_at: config.updated_at, @@ -340,9 +340,27 @@ function profileInspectFromSnapshot( }; } +function profileInspectFromSnapshot( + name: string, + active: boolean, + snapshot: ProfileSnapshot, +): ProfileInspect { + return { + ...profileConfigurationFromConfig(name, active, snapshot.config), + auth: snapshot.auth, + status: profileStatus(snapshot), + }; +} + export function inspectProfile(name: string): ProfileInspect { if (isFromEnvProfile(name)) { - return inspectFromEnvProfile(); + const configuration = inspectFromEnvConfiguration(); + const auth = inspectProfileAuth(name); + return { + ...configuration, + auth, + status: auth.management !== "none" || auth.lakehouse !== "none" ? "configured" : "empty", + }; } assertSafeProfileName(name); ensureProfilesLayout(); @@ -357,9 +375,9 @@ export function inspectProfile(name: string): ProfileInspect { ); } -export function inspectProfileReadOnly(name: string): ProfileInspect { +export function inspectProfileConfigurationReadOnly(name: string): ProfileConfiguration { if (isFromEnvProfile(name)) { - return inspectFromEnvProfile(); + return inspectFromEnvConfiguration(); } assertSafeProfileName(name); if (name !== DEFAULT_PROFILE_NAME && !profileExists(name)) { @@ -367,7 +385,11 @@ export function inspectProfileReadOnly(name: string): ProfileInspect { } const activeProfile = kvGet(configFile(), "active_profile") || DEFAULT_PROFILE_NAME; - return profileInspectFromSnapshot(name, activeProfile === name, readProfileSnapshot(name)); + return profileConfigurationFromConfig( + name, + activeProfile === name, + readProfileConfigRecord(name), + ); } export function createEmptyProfile(name: string): ProfileInspect { diff --git a/cli/src/lib/secrets.test.ts b/cli/src/lib/secrets.test.ts index 991c201..535fb6e 100644 --- a/cli/src/lib/secrets.test.ts +++ b/cli/src/lib/secrets.test.ts @@ -39,6 +39,18 @@ describe("secretSet", () => { expect(keychain.calls.some((args) => args.includes("add-generic-password"))).toBe(false); expect(keychain.store.secretGet("lakehouse/password", "default")).toBe("secret"); }); + + test("distinguishes a Keychain failure from a missing secret", () => { + const keychain = createFakeKeychain(); + keychain.failingReads.add("profile/default/api-key"); + + expect(() => keychain.store.secretExists("api-key", "default")).toThrow( + "Failed to inspect secret in macOS keychain", + ); + expect(() => keychain.store.secretGet("api-key", "default")).toThrow( + "Failed to read secret from macOS keychain", + ); + }); }); describe("secretDelete", () => { diff --git a/cli/src/lib/secrets.ts b/cli/src/lib/secrets.ts index 103e24b..1f1094a 100644 --- a/cli/src/lib/secrets.ts +++ b/cli/src/lib/secrets.ts @@ -18,6 +18,10 @@ type SecretProcessResult = { error?: Error; }; +function keychainLookupFailed(result: SecretProcessResult): boolean { + return result.status !== 0 && result.status !== KEYCHAIN_ITEM_NOT_FOUND_STATUS; +} + type SecretProcess = { platform: NodeJS.Platform; spawnSync(command: string, args: string[], options: SpawnSyncOptions): SecretProcessResult; @@ -152,6 +156,9 @@ export function createSecretStore( ["find-generic-password", "-s", "altertable", "-a", secretKey, "-w"], { encoding: "utf8" }, ); + if (keychainLookupFailed(result)) { + throw new CliError(`Failed to read secret from macOS keychain (${secretKey}).`); + } const keychainValue = result.status === 0 ? String(result.stdout).trim() : ""; if (keychainValue !== "") return keychainValue; requireSafeCredentialsFile(); @@ -174,6 +181,9 @@ export function createSecretStore( { stdio: "ignore" }, ); if (result.status === 0) return true; + if (keychainLookupFailed(result)) { + throw new CliError(`Failed to inspect secret in macOS keychain (${secretKey}).`); + } requireSafeCredentialsFile(); return kvGet(credentialsFile(), secretKey) !== ""; } diff --git a/cli/src/test-utils/keychain.ts b/cli/src/test-utils/keychain.ts index 4ee721c..de28f30 100644 --- a/cli/src/test-utils/keychain.ts +++ b/cli/src/test-utils/keychain.ts @@ -4,6 +4,7 @@ type FakeKeychain = { store: SecretStore; calls: string[][]; failingWrites: Set; + failingReads: Set; failingDeletes: Set; }; @@ -16,6 +17,7 @@ export function createFakeKeychain(): FakeKeychain { const calls: string[][] = []; const values = new Map(); const failingWrites = new Set(); + const failingReads = new Set(); const failingDeletes = new Set(); const store = createSecretStore({ @@ -31,9 +33,10 @@ export function createFakeKeychain(): FakeKeychain { return { status: 0, stdout: Buffer.from("") }; } if (args.includes("find-generic-password")) { + if (failingReads.has(account)) return { status: 1, stdout: Buffer.from("") }; const value = values.get(account); return value === undefined - ? { status: 1, stdout: Buffer.from("") } + ? { status: 44, stdout: Buffer.from("") } : { status: 0, stdout: Buffer.from(value) }; } if (args.includes("delete-generic-password")) { @@ -46,5 +49,5 @@ export function createFakeKeychain(): FakeKeychain { }, }); - return { store, calls, failingWrites, failingDeletes }; + return { store, calls, failingWrites, failingReads, failingDeletes }; } From 8b12c25702f5df3f28ad541ec48b43c51cece95c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Chalifour?= Date: Mon, 20 Jul 2026 13:37:12 +0200 Subject: [PATCH 5/6] refactor(cli): centralize record type guard --- cli/src/lib/management-output.ts | 15 ++++++--------- cli/src/lib/management/model.ts | 5 +---- cli/src/lib/object.ts | 4 ++++ 3 files changed, 11 insertions(+), 13 deletions(-) diff --git a/cli/src/lib/management-output.ts b/cli/src/lib/management-output.ts index e63a19a..14bdde9 100644 --- a/cli/src/lib/management-output.ts +++ b/cli/src/lib/management-output.ts @@ -1,4 +1,5 @@ import { parseApiJson } from "@/lib/parse-api-json.ts"; +import { isRecord } from "@/lib/object.ts"; import { redactSensitiveJsonValue } from "@/lib/redact.ts"; import { renderTabularOutput, @@ -6,12 +7,8 @@ import { type TabularResult, } from "@/lib/tabular-result.ts"; -function isPlainObject(value: unknown): value is Record { - return typeof value === "object" && value !== null && !Array.isArray(value); -} - export function extractManagementRows(data: unknown): Record[] { - if (!isPlainObject(data)) { + if (!isRecord(data)) { return []; } @@ -21,14 +18,14 @@ export function extractManagementRows(data: unknown): Record[] if (rows === undefined) { return []; } - const plainRows = rows.filter(isPlainObject); + const plainRows = rows.filter(isRecord); return plainRows.map(redactSensitiveRow); } - const nestedObjects = Object.entries(data).filter(([, value]) => isPlainObject(value)); + const nestedObjects = Object.entries(data).filter(([, value]) => isRecord(value)); if (nestedObjects.length === 1 && Object.keys(data).length === 1) { const nestedObject = nestedObjects[0]?.[1]; - if (!isPlainObject(nestedObject)) { + if (!isRecord(nestedObject)) { return []; } return [redactSensitiveRow(nestedObject)]; @@ -36,7 +33,7 @@ export function extractManagementRows(data: unknown): Record[] const row: Record = {}; for (const [key, value] of Object.entries(data)) { - if (isPlainObject(value)) { + if (isRecord(value)) { for (const [nestedKey, nestedValue] of Object.entries(value)) { row[nestedKey] = redactSensitiveRowValue(nestedKey, nestedValue); } diff --git a/cli/src/lib/management/model.ts b/cli/src/lib/management/model.ts index fac5653..309ea31 100644 --- a/cli/src/lib/management/model.ts +++ b/cli/src/lib/management/model.ts @@ -1,13 +1,10 @@ import type { components } from "@/generated/openapi-types.ts"; import { ParseError } from "@/lib/errors.ts"; +import { isRecord } from "@/lib/object.ts"; import { parseApiJson } from "@/lib/parse-api-json.ts"; export type WhoamiResponse = components["schemas"]["WhoamiResponse"]; -function isRecord(value: unknown): value is Record { - return typeof value === "object" && value !== null && !Array.isArray(value); -} - function hasRequiredString(record: Record, key: string): boolean { return typeof record[key] === "string" && record[key].length > 0; } diff --git a/cli/src/lib/object.ts b/cli/src/lib/object.ts index c00e1da..25af3ae 100644 --- a/cli/src/lib/object.ts +++ b/cli/src/lib/object.ts @@ -1,3 +1,7 @@ +export function isRecord(value: unknown): value is Record { + return typeof value === "object" && value !== null && !Array.isArray(value); +} + export function objectKeys>( value: TValue, ): Array> { From 75bda0e5bc0fa2d12e2c61339dba7347f6f6641e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Chalifour?= Date: Mon, 20 Jul 2026 15:32:11 +0200 Subject: [PATCH 6/6] refactor(cli): clarify diagnostic and generated artifact helpers --- cli/scripts/generate-command-reference.ts | 9 ++-- cli/scripts/generate-openapi-index.ts | 9 ++-- cli/scripts/generate-openapi-types.ts | 9 ++-- cli/scripts/generated-artifact.test.ts | 27 ++++++----- cli/scripts/generated-artifact.ts | 8 ++-- cli/src/commands/doctor/index.ts | 4 +- cli/src/commands/doctor/lib/checks.ts | 54 +++++++++++----------- cli/src/commands/doctor/lib/render.ts | 4 +- cli/src/commands/doctor/lib/runner.test.ts | 14 +++--- cli/src/commands/doctor/lib/runner.ts | 12 ++--- cli/src/lib/execution-context.ts | 6 +-- cli/src/lib/profile-store.test.ts | 4 +- cli/src/lib/profile-store.ts | 2 +- cli/src/lib/profile/model.ts | 34 ++++++-------- 14 files changed, 100 insertions(+), 96 deletions(-) diff --git a/cli/scripts/generate-command-reference.ts b/cli/scripts/generate-command-reference.ts index a146401..12c4ae8 100644 --- a/cli/scripts/generate-command-reference.ts +++ b/cli/scripts/generate-command-reference.ts @@ -2,13 +2,16 @@ import { join } from "node:path"; import { buildMainCommand } from "@/cli.ts"; import { resolveCommandDescriptor, validateCommandDescriptor } from "@/lib/command-descriptor.ts"; import { renderCommandReference } from "@/lib/command-reference.ts"; -import { generatedArtifactMode, syncGeneratedArtifact } from "@/../scripts/generated-artifact.ts"; +import { + parseGeneratedArtifactMode, + updateOrCheckGeneratedArtifact, +} from "@/../scripts/generated-artifact.ts"; const outputPath = join(import.meta.dir, "../../COMMANDS.md"); const descriptor = await resolveCommandDescriptor(buildMainCommand()); validateCommandDescriptor(descriptor); -const mode = generatedArtifactMode(process.argv.slice(2)); -syncGeneratedArtifact({ +const mode = parseGeneratedArtifactMode(process.argv.slice(2)); +updateOrCheckGeneratedArtifact({ outputPath, content: renderCommandReference(descriptor), mode, diff --git a/cli/scripts/generate-openapi-index.ts b/cli/scripts/generate-openapi-index.ts index dd525e1..654bc48 100644 --- a/cli/scripts/generate-openapi-index.ts +++ b/cli/scripts/generate-openapi-index.ts @@ -1,6 +1,9 @@ import { readFileSync } from "node:fs"; import { join } from "node:path"; -import { generatedArtifactMode, syncGeneratedArtifact } from "@/../scripts/generated-artifact.ts"; +import { + parseGeneratedArtifactMode, + updateOrCheckGeneratedArtifact, +} from "@/../scripts/generated-artifact.ts"; const HTTP_METHODS = ["get", "post", "patch", "delete", "put"] as const; @@ -71,8 +74,8 @@ ${lines.join("\n")} ]; `; -const mode = generatedArtifactMode(process.argv.slice(2)); -syncGeneratedArtifact({ outputPath, content: output, mode }); +const mode = parseGeneratedArtifactMode(process.argv.slice(2)); +updateOrCheckGeneratedArtifact({ outputPath, content: output, mode }); console.log( `${mode === "check" ? "Checked" : "Wrote"} ${outputPath} (${operations.length} operations)`, ); diff --git a/cli/scripts/generate-openapi-types.ts b/cli/scripts/generate-openapi-types.ts index e7625cc..4b8b60b 100644 --- a/cli/scripts/generate-openapi-types.ts +++ b/cli/scripts/generate-openapi-types.ts @@ -1,7 +1,10 @@ import openapiTS, { astToString } from "openapi-typescript"; import { readFileSync } from "node:fs"; import { join } from "node:path"; -import { generatedArtifactMode, syncGeneratedArtifact } from "@/../scripts/generated-artifact.ts"; +import { + parseGeneratedArtifactMode, + updateOrCheckGeneratedArtifact, +} from "@/../scripts/generated-artifact.ts"; const specPath = join(import.meta.dir, "../openapi/openapi.yaml"); const outputPath = join(import.meta.dir, "../src/generated/openapi-types.ts"); @@ -9,8 +12,8 @@ const outputPath = join(import.meta.dir, "../src/generated/openapi-types.ts"); const spec = readFileSync(specPath, "utf8"); const ast = await openapiTS(spec); const types = astToString(ast); -const mode = generatedArtifactMode(process.argv.slice(2)); -syncGeneratedArtifact({ +const mode = parseGeneratedArtifactMode(process.argv.slice(2)); +updateOrCheckGeneratedArtifact({ outputPath, content: `/* AUTO-GENERATED by scripts/generate-openapi-types.ts — do not edit */\n\n${types}\n`, mode, diff --git a/cli/scripts/generated-artifact.test.ts b/cli/scripts/generated-artifact.test.ts index de11be0..4937dac 100644 --- a/cli/scripts/generated-artifact.test.ts +++ b/cli/scripts/generated-artifact.test.ts @@ -2,11 +2,14 @@ import { afterEach, describe, expect, test } from "bun:test"; import { mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; import { join } from "node:path"; import { tmpdir } from "node:os"; -import { generatedArtifactMode, syncGeneratedArtifact } from "@/../scripts/generated-artifact.ts"; +import { + parseGeneratedArtifactMode, + updateOrCheckGeneratedArtifact, +} from "@/../scripts/generated-artifact.ts"; const testDirectories: string[] = []; -function artifactPath(): string { +function createTestArtifactPath(): string { const directory = mkdtempSync(join(tmpdir(), "altertable-generated-artifact-")); testDirectories.push(directory); return join(directory, "artifact.txt"); @@ -20,33 +23,33 @@ afterEach(() => { describe("generated artifact synchronization", () => { test("selects check mode explicitly", () => { - expect(generatedArtifactMode([])).toBe("write"); - expect(generatedArtifactMode(["--check"])).toBe("check"); + expect(parseGeneratedArtifactMode([])).toBe("write"); + expect(parseGeneratedArtifactMode(["--check"])).toBe("check"); }); test("writes generated content", () => { - const outputPath = artifactPath(); + const outputPath = createTestArtifactPath(); - syncGeneratedArtifact({ outputPath, content: "generated\n", mode: "write" }); + updateOrCheckGeneratedArtifact({ outputPath, content: "generated\n", mode: "write" }); expect(readFileSync(outputPath, "utf8")).toBe("generated\n"); }); test("checks current content without writing", () => { - const outputPath = artifactPath(); + const outputPath = createTestArtifactPath(); writeFileSync(outputPath, "generated\n"); - syncGeneratedArtifact({ outputPath, content: "generated\n", mode: "check" }); + updateOrCheckGeneratedArtifact({ outputPath, content: "generated\n", mode: "check" }); expect(readFileSync(outputPath, "utf8")).toBe("generated\n"); }); test("reports stale and missing output with regeneration guidance", () => { - const stalePath = artifactPath(); + const stalePath = createTestArtifactPath(); writeFileSync(stalePath, "old\n"); expect(() => - syncGeneratedArtifact({ + updateOrCheckGeneratedArtifact({ outputPath: stalePath, content: "new\n", mode: "check", @@ -55,8 +58,8 @@ describe("generated artifact synchronization", () => { ).toThrow("Run: bun run generate:commands"); expect(() => - syncGeneratedArtifact({ - outputPath: artifactPath(), + updateOrCheckGeneratedArtifact({ + outputPath: createTestArtifactPath(), content: "new\n", mode: "check", }), diff --git a/cli/scripts/generated-artifact.ts b/cli/scripts/generated-artifact.ts index 354f4c4..c6f88c3 100644 --- a/cli/scripts/generated-artifact.ts +++ b/cli/scripts/generated-artifact.ts @@ -2,23 +2,23 @@ import { readFileSync, writeFileSync } from "node:fs"; export type GeneratedArtifactMode = "write" | "check"; -type SyncGeneratedArtifactOptions = { +type GeneratedArtifactOptions = { outputPath: string; content: string; mode: GeneratedArtifactMode; generateCommand?: string; }; -export function generatedArtifactMode(argv: readonly string[]): GeneratedArtifactMode { +export function parseGeneratedArtifactMode(argv: readonly string[]): GeneratedArtifactMode { return argv.includes("--check") ? "check" : "write"; } -export function syncGeneratedArtifact({ +export function updateOrCheckGeneratedArtifact({ outputPath, content, mode, generateCommand = "bun run generate", -}: SyncGeneratedArtifactOptions): void { +}: GeneratedArtifactOptions): void { if (mode === "write") { writeFileSync(outputPath, content); return; diff --git a/cli/src/commands/doctor/index.ts b/cli/src/commands/doctor/index.ts index 0395df9..40b320b 100644 --- a/cli/src/commands/doctor/index.ts +++ b/cli/src/commands/doctor/index.ts @@ -3,7 +3,7 @@ import { writeCommandOutput } from "@/lib/command-output.ts"; import { createDoctorChecks } from "@/commands/doctor/lib/checks.ts"; import { formatDoctorReport } from "@/commands/doctor/lib/render.ts"; import { runDoctorChecks } from "@/commands/doctor/lib/runner.ts"; -import { createReadOnlyExecutionContext } from "@/lib/execution-context.ts"; +import { createDiagnosticExecutionContext } from "@/lib/execution-context.ts"; export const doctorCommand = defineCommand({ metadata: { @@ -20,7 +20,7 @@ export const doctorCommand = defineCommand({ }, async run({ args, runtime, sink }) { const report = await runDoctorChecks(createDoctorChecks(), { - execution: createReadOnlyExecutionContext(runtime), + execution: createDiagnosticExecutionContext(runtime), offline: args.offline === true, }); await writeCommandOutput( diff --git a/cli/src/commands/doctor/lib/checks.ts b/cli/src/commands/doctor/lib/checks.ts index 66c895c..700bed4 100644 --- a/cli/src/commands/doctor/lib/checks.ts +++ b/cli/src/commands/doctor/lib/checks.ts @@ -1,7 +1,7 @@ import { VERSION } from "@/version.ts"; import { - inspectProfileAuth, - inspectProfileConfigurationReadOnly, + detectConfiguredProfileAuth, + readProfileConfiguration, type ProfileAuth, } from "@/lib/profile/model.ts"; import { envConfigMode } from "@/lib/profile-store.ts"; @@ -20,11 +20,11 @@ import type { DoctorCheckOutcome, } from "@/commands/doctor/lib/model.ts"; -function pass(message: string, details?: Record): DoctorCheckOutcome { +function passOutcome(message: string, details?: Record): DoctorCheckOutcome { return { status: "pass", message, details }; } -function fail( +function failOutcome( message: string, code: string, remediation: string[], @@ -40,7 +40,7 @@ function profileSource(context: DoctorCheckContext): string { return configGetGlobal("active_profile") ? "active profile" : "default profile"; } -function managementIdentity(body: string): string { +function formatManagementIdentity(body: string): string { return formatWhoamiPrincipalLine(parseWhoamiResponse(body)); } @@ -48,7 +48,7 @@ function rowContainsProbeValue(row: LakehouseRow): boolean { return Array.isArray(row) ? row.includes(1) : Object.values(row).includes(1); } -function validateLakehouseProbe(body: string): void { +function validateLakehouseProbeResponse(body: string): void { const result = parseLakehouseQueryResponse(body); if (!result.rows.some(rowContainsProbeValue)) { throw new ParseError("Lakehouse probe returned an unexpected result.", { @@ -57,27 +57,27 @@ function validateLakehouseProbe(body: string): void { } } -function checkManagementCredentials(auth: ProfileAuth): DoctorCheckOutcome { +function checkManagementCredentialPresence(auth: ProfileAuth): DoctorCheckOutcome { if (auth.management === "none") { - return fail("No management credentials configured.", "configuration_error", [ + return failOutcome("No management credentials configured.", "configuration_error", [ "Run: altertable login", "Or run: altertable profile configure --scope management", ]); } - return pass(`Configured (${auth.management}).`, { auth: auth.management }); + return passOutcome(`Configured (${auth.management}).`, { auth: auth.management }); } -function checkLakehouseCredentials(auth: ProfileAuth): DoctorCheckOutcome { +function checkLakehouseCredentialPresence(auth: ProfileAuth): DoctorCheckOutcome { if (auth.lakehouse === "none") { - return fail("No lakehouse credentials configured.", "configuration_error", [ + return failOutcome("No lakehouse credentials configured.", "configuration_error", [ "Run: altertable login", "Or run: altertable profile configure --scope lakehouse", ]); } - return pass(`Configured (${auth.lakehouse}).`, { auth: auth.lakehouse }); + return passOutcome(`Configured (${auth.lakehouse}).`, { auth: auth.lakehouse }); } -function networkSkip(context: DoctorCheckContext): string | undefined { +function offlineSkipReason(context: DoctorCheckContext): string | undefined { return context.offline ? "Offline mode." : undefined; } @@ -96,7 +96,7 @@ export function createDoctorChecks(): DoctorCheck[] { id: "cli.runtime", label: "CLI", run: () => - pass(`v${VERSION} · ${process.platform} ${process.arch}`, { + passOutcome(`v${VERSION} · ${process.platform} ${process.arch}`, { version: VERSION, platform: process.platform, arch: process.arch, @@ -107,9 +107,9 @@ export function createDoctorChecks(): DoctorCheck[] { id: "profile.configuration", label: "Profile", run: (context) => { - const current = inspectProfileConfigurationReadOnly(context.execution.profile); + const current = readProfileConfiguration(context.execution.profile); const source = profileSource(context); - return pass(`${current.name} (${source})`, { + return passOutcome(`${current.name} (${source})`, { name: current.name, source, environment: current.environment, @@ -128,12 +128,12 @@ export function createDoctorChecks(): DoctorCheck[] { label: "Secret store", requires: ["profile.configuration"], run: (context) => { - profileAuth = inspectProfileAuth(context.execution.profile); + profileAuth = detectConfiguredProfileAuth(context.execution.profile); if (envConfigMode()) { - return pass("Credentials supplied by environment variables."); + return passOutcome("Credentials supplied by environment variables."); } const store = secretStoreDisplay(); - return pass(`${store} accessible.`, { store }); + return passOutcome(`${store} accessible.`, { store }); }, remediation: () => ["Run: altertable profile show"], }, @@ -141,13 +141,13 @@ export function createDoctorChecks(): DoctorCheck[] { id: "management.credentials", label: "Management auth", requires: ["credentials.store"], - run: () => checkManagementCredentials(requireProfileAuth()), + run: () => checkManagementCredentialPresence(requireProfileAuth()), }, { id: "management.api", label: "Management API", requires: ["management.credentials"], - skip: networkSkip, + skip: offlineSkipReason, async run(context) { const endpoint = resolveManagementApiBase(context.execution.profile); const body = await sendHttp( @@ -159,8 +159,8 @@ export function createDoctorChecks(): DoctorCheck[] { }, context.execution, ); - const identity = managementIdentity(body); - return pass(`${endpoint} · ${identity}`, { endpoint, identity }); + const identity = formatManagementIdentity(body); + return passOutcome(`${endpoint} · ${identity}`, { endpoint, identity }); }, remediation: () => [ "Check the control-plane URL and management credentials.", @@ -171,21 +171,21 @@ export function createDoctorChecks(): DoctorCheck[] { id: "lakehouse.credentials", label: "Lakehouse auth", requires: ["credentials.store"], - run: () => checkLakehouseCredentials(requireProfileAuth()), + run: () => checkLakehouseCredentialPresence(requireProfileAuth()), }, { id: "lakehouse.api", label: "Lakehouse API", requires: ["lakehouse.credentials"], - skip: networkSkip, + skip: offlineSkipReason, async run(context) { const endpoint = resolveApiBase(context.execution.profile); const body = await sendHttp( { ...buildLakehouseVerifyRequest(), authRecovery: false }, context.execution, ); - validateLakehouseProbe(body); - return pass(`${endpoint} · SELECT 1 succeeded.`, { endpoint }); + validateLakehouseProbeResponse(body); + return passOutcome(`${endpoint} · SELECT 1 succeeded.`, { endpoint }); }, remediation: () => [ "Check the data-plane URL and lakehouse credentials.", diff --git a/cli/src/commands/doctor/lib/render.ts b/cli/src/commands/doctor/lib/render.ts index 22cbe9b..35eea9f 100644 --- a/cli/src/commands/doctor/lib/render.ts +++ b/cli/src/commands/doctor/lib/render.ts @@ -13,7 +13,7 @@ const STATUS_DISPLAY: Record formatCheck(check, labelWidth)), + ...report.checks.flatMap((check) => formatCheckResult(check, labelWidth)), "", formatSummary(report), ].join("\n"); diff --git a/cli/src/commands/doctor/lib/runner.test.ts b/cli/src/commands/doctor/lib/runner.test.ts index 945f56f..20de479 100644 --- a/cli/src/commands/doctor/lib/runner.test.ts +++ b/cli/src/commands/doctor/lib/runner.test.ts @@ -3,7 +3,7 @@ import { ConfigurationError, HttpError } from "@/lib/errors.ts"; import type { DoctorCheck, DoctorCheckContext } from "@/commands/doctor/lib/model.ts"; import { runDoctorChecks } from "@/commands/doctor/lib/runner.ts"; -function context(offline = false): DoctorCheckContext { +function createDoctorContext(offline = false): DoctorCheckContext { return { offline, execution: { @@ -55,7 +55,7 @@ describe("runDoctorChecks", () => { }, ]; - const report = await runDoctorChecks(checks, context()); + const report = await runDoctorChecks(checks, createDoctorContext()); expect(report.healthy).toBe(false); expect(report.summary).toEqual({ passed: 1, warnings: 0, failed: 1, skipped: 1 }); @@ -82,7 +82,7 @@ describe("runDoctorChecks", () => { run: () => ({ status: "pass", message: "Connected." }), }, ], - context(true), + createDoctorContext(true), ); expect(report.healthy).toBe(true); @@ -131,7 +131,7 @@ describe("runDoctorChecks", () => { }, }, ], - context(), + createDoctorContext(), ); await Promise.resolve(); @@ -175,7 +175,7 @@ describe("runDoctorChecks", () => { }, }, ], - context(), + createDoctorContext(), ); expect(report.checks[0]).toMatchObject({ @@ -198,7 +198,7 @@ describe("runDoctorChecks", () => { run: () => ({ status: "pass", message: "Ready." }), }; await expectRejection( - runDoctorChecks([duplicate, duplicate], context()), + runDoctorChecks([duplicate, duplicate], createDoctorContext()), "Duplicate doctor check id", ); await expectRejection( @@ -212,7 +212,7 @@ describe("runDoctorChecks", () => { }, { ...duplicate, id: "later" }, ], - context(), + createDoctorContext(), ), "requires unknown or later check", ); diff --git a/cli/src/commands/doctor/lib/runner.ts b/cli/src/commands/doctor/lib/runner.ts index 415bfd1..300fa7c 100644 --- a/cli/src/commands/doctor/lib/runner.ts +++ b/cli/src/commands/doctor/lib/runner.ts @@ -7,7 +7,7 @@ import type { DoctorSummary, } from "@/commands/doctor/lib/model.ts"; -function summarize(checks: readonly DoctorCheckResult[]): DoctorSummary { +function summarizeCheckResults(checks: readonly DoctorCheckResult[]): DoctorSummary { return { passed: checks.filter((check) => check.status === "pass").length, warnings: checks.filter((check) => check.status === "warn").length, @@ -16,7 +16,7 @@ function summarize(checks: readonly DoctorCheckResult[]): DoctorSummary { }; } -function validateChecks(checks: readonly DoctorCheck[]): void { +function validateCheckSequence(checks: readonly DoctorCheck[]): void { const precedingIds = new Set(); for (const check of checks) { if (precedingIds.has(check.id)) { @@ -33,7 +33,7 @@ function validateChecks(checks: readonly DoctorCheck[]): void { } } -async function runCheck( +async function runDoctorCheck( check: DoctorCheck, context: DoctorCheckContext, dependencies: readonly DoctorCheckResult[], @@ -89,19 +89,19 @@ export async function runDoctorChecks( checks: readonly DoctorCheck[], context: DoctorCheckContext, ): Promise { - validateChecks(checks); + validateCheckSequence(checks); const pendingResults = new Map>(); for (const check of checks) { const dependencies = (check.requires ?? []).map((id) => pendingResults.get(id)!); pendingResults.set( check.id, - Promise.all(dependencies).then((results) => runCheck(check, context, results)), + Promise.all(dependencies).then((results) => runDoctorCheck(check, context, results)), ); } const checkResults = await Promise.all(pendingResults.values()); - const summary = summarize(checkResults); + const summary = summarizeCheckResults(checkResults); return { healthy: summary.failed === 0, profile: context.execution.profile, diff --git a/cli/src/lib/execution-context.ts b/cli/src/lib/execution-context.ts index a91b39c..e2d3aae 100644 --- a/cli/src/lib/execution-context.ts +++ b/cli/src/lib/execution-context.ts @@ -1,5 +1,5 @@ import type { CliContext } from "@/context.ts"; -import { resolveWorkingProfile, resolveWorkingProfileReadOnly } from "@/lib/profile-store.ts"; +import { resolveProfileReference, resolveWorkingProfile } from "@/lib/profile-store.ts"; import type { CliRuntime, OutputSink } from "@/lib/runtime.ts"; export type ExecutionContext = { @@ -27,10 +27,10 @@ export function createExecutionContext(runtime: CliRuntime): ExecutionContext { }; } -export function createReadOnlyExecutionContext(runtime: CliRuntime): ExecutionContext { +export function createDiagnosticExecutionContext(runtime: CliRuntime): ExecutionContext { return { cli: runtime.context, output: runtime.output, - profile: resolveWorkingProfileReadOnly(runtime.context.profile), + profile: resolveProfileReference(runtime.context.profile), }; } diff --git a/cli/src/lib/profile-store.test.ts b/cli/src/lib/profile-store.test.ts index 62ac225..71fb878 100644 --- a/cli/src/lib/profile-store.test.ts +++ b/cli/src/lib/profile-store.test.ts @@ -11,7 +11,7 @@ import { profileConfigFile, profileExists, resolveWorkingProfile, - resolveWorkingProfileReadOnly, + resolveProfileReference, setActiveProfile, } from "@/lib/profile-store.ts"; import { kvSet } from "@/lib/config.ts"; @@ -65,6 +65,6 @@ describe("profile store", () => { test("preserves a stale active profile reference for read-only diagnostics", () => { kvSet(join(testHome, "config"), "active_profile", "missing"); - expect(resolveWorkingProfileReadOnly()).toBe("missing"); + expect(resolveProfileReference()).toBe("missing"); }); }); diff --git a/cli/src/lib/profile-store.ts b/cli/src/lib/profile-store.ts index ec8bc4a..5df3573 100644 --- a/cli/src/lib/profile-store.ts +++ b/cli/src/lib/profile-store.ts @@ -159,7 +159,7 @@ export function resolveWorkingProfile(override?: string): string { return getActiveProfileName(); } -export function resolveWorkingProfileReadOnly(override?: string): string { +export function resolveProfileReference(override?: string): string { if (envConfigMode()) { return FROM_ENV_PSEUDOPROFILE_NAME; } diff --git a/cli/src/lib/profile/model.ts b/cli/src/lib/profile/model.ts index 1ab55da..b1c109b 100644 --- a/cli/src/lib/profile/model.ts +++ b/cli/src/lib/profile/model.ts @@ -183,7 +183,7 @@ function writeProfileUpdate(name: string, update: ProfileUpdate): void { } } -export function inspectProfileAuth(name: string): ProfileAuth { +export function detectConfiguredProfileAuth(name: string): ProfileAuth { if (isFromEnvProfile(name)) { return { management: hasEnvManagementCredentials() ? "api_key" : "none", @@ -205,14 +205,14 @@ export function inspectProfileAuth(name: string): ProfileAuth { } export function profileHasAnyAuthConfigured(name: string): boolean { - const auth = inspectProfileAuth(name); + const auth = detectConfiguredProfileAuth(name); return auth.management !== "none" || auth.lakehouse !== "none"; } function readProfileSnapshot(name: string): ProfileSnapshot { return { config: readProfileConfigRecord(name), - auth: inspectProfileAuth(name), + auth: detectConfiguredProfileAuth(name), }; } @@ -290,7 +290,7 @@ function envLakehouseAuthKind(): ProfileLakehouseAuth { return "none"; } -function inspectFromEnvConfiguration(): ProfileConfiguration { +function readEnvironmentProfileConfiguration(): ProfileConfiguration { return { name: FROM_ENV_PSEUDOPROFILE_NAME, active: true, @@ -306,7 +306,7 @@ function inspectFromEnvConfiguration(): ProfileConfiguration { }; } -function profileConfigurationFromConfig( +function buildProfileConfiguration( name: string, active: boolean, config: Record, @@ -340,13 +340,13 @@ function profileConfigurationFromConfig( }; } -function profileInspectFromSnapshot( +function buildProfileInspection( name: string, active: boolean, snapshot: ProfileSnapshot, ): ProfileInspect { return { - ...profileConfigurationFromConfig(name, active, snapshot.config), + ...buildProfileConfiguration(name, active, snapshot.config), auth: snapshot.auth, status: profileStatus(snapshot), }; @@ -354,8 +354,8 @@ function profileInspectFromSnapshot( export function inspectProfile(name: string): ProfileInspect { if (isFromEnvProfile(name)) { - const configuration = inspectFromEnvConfiguration(); - const auth = inspectProfileAuth(name); + const configuration = readEnvironmentProfileConfiguration(); + const auth = detectConfiguredProfileAuth(name); return { ...configuration, auth, @@ -368,16 +368,12 @@ export function inspectProfile(name: string): ProfileInspect { throw new ConfigurationError(`Profile not found: ${name}`); } - return profileInspectFromSnapshot( - name, - getActiveProfileName() === name, - readProfileSnapshot(name), - ); + return buildProfileInspection(name, getActiveProfileName() === name, readProfileSnapshot(name)); } -export function inspectProfileConfigurationReadOnly(name: string): ProfileConfiguration { +export function readProfileConfiguration(name: string): ProfileConfiguration { if (isFromEnvProfile(name)) { - return inspectFromEnvConfiguration(); + return readEnvironmentProfileConfiguration(); } assertSafeProfileName(name); if (name !== DEFAULT_PROFILE_NAME && !profileExists(name)) { @@ -385,11 +381,7 @@ export function inspectProfileConfigurationReadOnly(name: string): ProfileConfig } const activeProfile = kvGet(configFile(), "active_profile") || DEFAULT_PROFILE_NAME; - return profileConfigurationFromConfig( - name, - activeProfile === name, - readProfileConfigRecord(name), - ); + return buildProfileConfiguration(name, activeProfile === name, readProfileConfigRecord(name)); } export function createEmptyProfile(name: string): ProfileInspect {