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/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/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/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..12c4ae8 100644 --- a/cli/scripts/generate-command-reference.ts +++ b/cli/scripts/generate-command-reference.ts @@ -1,11 +1,20 @@ -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 { + parseGeneratedArtifactMode, + updateOrCheckGeneratedArtifact, +} 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 = parseGeneratedArtifactMode(process.argv.slice(2)); +updateOrCheckGeneratedArtifact({ + 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..654bc48 100644 --- a/cli/scripts/generate-openapi-index.ts +++ b/cli/scripts/generate-openapi-index.ts @@ -1,5 +1,9 @@ -import { readFileSync, writeFileSync } from "node:fs"; +import { readFileSync } from "node:fs"; import { join } from "node:path"; +import { + parseGeneratedArtifactMode, + updateOrCheckGeneratedArtifact, +} from "@/../scripts/generated-artifact.ts"; const HTTP_METHODS = ["get", "post", "patch", "delete", "put"] as const; @@ -70,5 +74,8 @@ ${lines.join("\n")} ]; `; -writeFileSync(outputPath, output); -console.log(`Wrote ${outputPath} (${operations.length} operations)`); +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 2191fff..4b8b60b 100644 --- a/cli/scripts/generate-openapi-types.ts +++ b/cli/scripts/generate-openapi-types.ts @@ -1,6 +1,10 @@ import openapiTS, { astToString } from "openapi-typescript"; -import { readFileSync, writeFileSync } from "node:fs"; +import { readFileSync } from "node:fs"; import { join } from "node:path"; +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"); @@ -8,8 +12,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 = parseGeneratedArtifactMode(process.argv.slice(2)); +updateOrCheckGeneratedArtifact({ 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..4937dac --- /dev/null +++ b/cli/scripts/generated-artifact.test.ts @@ -0,0 +1,68 @@ +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 { + parseGeneratedArtifactMode, + updateOrCheckGeneratedArtifact, +} from "@/../scripts/generated-artifact.ts"; + +const testDirectories: string[] = []; + +function createTestArtifactPath(): 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(parseGeneratedArtifactMode([])).toBe("write"); + expect(parseGeneratedArtifactMode(["--check"])).toBe("check"); + }); + + test("writes generated content", () => { + const outputPath = createTestArtifactPath(); + + updateOrCheckGeneratedArtifact({ outputPath, content: "generated\n", mode: "write" }); + + expect(readFileSync(outputPath, "utf8")).toBe("generated\n"); + }); + + test("checks current content without writing", () => { + const outputPath = createTestArtifactPath(); + writeFileSync(outputPath, "generated\n"); + + 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 = createTestArtifactPath(); + writeFileSync(stalePath, "old\n"); + + expect(() => + updateOrCheckGeneratedArtifact({ + outputPath: stalePath, + content: "new\n", + mode: "check", + generateCommand: "bun run generate:commands", + }), + ).toThrow("Run: bun run generate:commands"); + + expect(() => + updateOrCheckGeneratedArtifact({ + outputPath: createTestArtifactPath(), + 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..c6f88c3 --- /dev/null +++ b/cli/scripts/generated-artifact.ts @@ -0,0 +1,36 @@ +import { readFileSync, writeFileSync } from "node:fs"; + +export type GeneratedArtifactMode = "write" | "check"; + +type GeneratedArtifactOptions = { + outputPath: string; + content: string; + mode: GeneratedArtifactMode; + generateCommand?: string; +}; + +export function parseGeneratedArtifactMode(argv: readonly string[]): GeneratedArtifactMode { + return argv.includes("--check") ? "check" : "write"; +} + +export function updateOrCheckGeneratedArtifact({ + outputPath, + content, + mode, + generateCommand = "bun run generate", +}: GeneratedArtifactOptions): 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/cli/src/commands/completion/index.test.ts b/cli/src/commands/completion/index.test.ts index fbeabad..95b5699 100644 --- a/cli/src/commands/completion/index.test.ts +++ b/cli/src/commands/completion/index.test.ts @@ -6,7 +6,6 @@ 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 { buildCompletionSpec } from "@/commands/completion/lib/spec.ts"; import { createCliRuntime } from "@/lib/runtime.ts"; import { runWithCliRuntime } from "@/test-utils/runtime.ts"; @@ -256,14 +255,4 @@ describe("completion command", () => { expect(existsSync(join(home, ".bashrc"))).toBe(false); expect(visibleTerminalText(output)).toContain("Startup: left unchanged (--no-rc)"); }); - - test("integration root command top-level count matches registry", async () => { - const spec = await buildCompletionSpec(buildMainCommand()); - const output = await runCompletion(buildMainCommand, "bash"); - const topLevelCount = spec.subcommands.length; - expect(topLevelCount).toBe(14); - expect(output).toContain("completion"); - 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..1c070ee --- /dev/null +++ b/cli/src/commands/doctor/index.test.ts @@ -0,0 +1,203 @@ +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +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 = ""; + +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"); + 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: JSON.stringify(VALID_WHOAMI), + }, + { urlPattern: "/query", method: "POST", body: VALID_LAKEHOUSE_PROBE }, + ]), + ); + + 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("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 () => { + 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, + 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..40b320b --- /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 { createDiagnosticExecutionContext } 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: createDiagnosticExecutionContext(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..700bed4 --- /dev/null +++ b/cli/src/commands/doctor/lib/checks.ts @@ -0,0 +1,196 @@ +import { VERSION } from "@/version.ts"; +import { + detectConfiguredProfileAuth, + readProfileConfiguration, + 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"; +import { secretStoreDisplay } from "@/lib/secrets.ts"; +import { sendHttp } from "@/lib/http-request.ts"; +import { buildLakehouseVerifyRequest } from "@/lib/lakehouse/query.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, + DoctorCheckOutcome, +} from "@/commands/doctor/lib/model.ts"; + +function passOutcome(message: string, details?: Record): DoctorCheckOutcome { + return { status: "pass", message, details }; +} + +function failOutcome( + 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 formatManagementIdentity(body: string): string { + return formatWhoamiPrincipalLine(parseWhoamiResponse(body)); +} + +function rowContainsProbeValue(row: LakehouseRow): boolean { + return Array.isArray(row) ? row.includes(1) : Object.values(row).includes(1); +} + +function validateLakehouseProbeResponse(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.", + }); + } +} + +function checkManagementCredentialPresence(auth: ProfileAuth): DoctorCheckOutcome { + if (auth.management === "none") { + return failOutcome("No management credentials configured.", "configuration_error", [ + "Run: altertable login", + "Or run: altertable profile configure --scope management", + ]); + } + return passOutcome(`Configured (${auth.management}).`, { auth: auth.management }); +} + +function checkLakehouseCredentialPresence(auth: ProfileAuth): DoctorCheckOutcome { + if (auth.lakehouse === "none") { + return failOutcome("No lakehouse credentials configured.", "configuration_error", [ + "Run: altertable login", + "Or run: altertable profile configure --scope lakehouse", + ]); + } + return passOutcome(`Configured (${auth.lakehouse}).`, { auth: auth.lakehouse }); +} + +function offlineSkipReason(context: DoctorCheckContext): string | undefined { + return context.offline ? "Offline mode." : undefined; +} + +export function createDoctorChecks(): DoctorCheck[] { + let profileAuth: ProfileAuth | undefined; + + function requireProfileAuth(): ProfileAuth { + if (!profileAuth) { + throw new Error("Secret store check did not inspect profile authentication."); + } + return profileAuth; + } + + return [ + { + id: "cli.runtime", + label: "CLI", + run: () => + passOutcome(`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 = readProfileConfiguration(context.execution.profile); + const source = profileSource(context); + return passOutcome(`${current.name} (${source})`, { + name: current.name, + source, + environment: current.environment, + config_file: current.config_file, + timestamps: current.timestamps, + endpoints: { + data_plane: resolveApiBase(current.name), + control_plane: resolveManagementApiBase(current.name), + }, + }); + }, + remediation: () => ["Run: altertable profile show"], + }, + { + id: "credentials.store", + label: "Secret store", + requires: ["profile.configuration"], + run: (context) => { + profileAuth = detectConfiguredProfileAuth(context.execution.profile); + if (envConfigMode()) { + return passOutcome("Credentials supplied by environment variables."); + } + const store = secretStoreDisplay(); + return passOutcome(`${store} accessible.`, { store }); + }, + remediation: () => ["Run: altertable profile show"], + }, + { + id: "management.credentials", + label: "Management auth", + requires: ["credentials.store"], + run: () => checkManagementCredentialPresence(requireProfileAuth()), + }, + { + id: "management.api", + label: "Management API", + requires: ["management.credentials"], + skip: offlineSkipReason, + 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 = formatManagementIdentity(body); + return passOutcome(`${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: ["credentials.store"], + run: () => checkLakehouseCredentialPresence(requireProfileAuth()), + }, + { + id: "lakehouse.api", + label: "Lakehouse API", + requires: ["lakehouse.credentials"], + skip: offlineSkipReason, + async run(context) { + const endpoint = resolveApiBase(context.execution.profile); + const body = await sendHttp( + { ...buildLakehouseVerifyRequest(), authRecovery: false }, + context.execution, + ); + validateLakehouseProbeResponse(body); + return passOutcome(`${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..0166012 --- /dev/null +++ b/cli/src/commands/doctor/lib/model.ts @@ -0,0 +1,45 @@ +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; + http_status?: number; + details?: string | 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.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 new file mode 100644 index 0000000..35eea9f --- /dev/null +++ b/cli/src/commands/doctor/lib/render.ts @@ -0,0 +1,67 @@ +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 formatCheckResult(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), + ]), + ]; + 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")])); + } + 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) => 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 new file mode 100644 index 0000000..20de479 --- /dev/null +++ b/cli/src/commands/doctor/lib/runner.test.ts @@ -0,0 +1,220 @@ +import { describe, expect, test } from "bun:test"; +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 createDoctorContext(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, createDoctorContext()); + + 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." }), + }, + ], + createDoctorContext(true), + ); + + expect(report.healthy).toBe(true); + expect(report.summary.skipped).toBe(1); + 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." }; + }, + }, + ], + createDoctorContext(), + ); + + 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( + [ + { + 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.", + }); + }, + }, + ], + createDoctorContext(), + ); + + 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", + label: "Same", + run: () => ({ status: "pass", message: "Ready." }), + }; + await expectRejection( + runDoctorChecks([duplicate, duplicate], createDoctorContext()), + "Duplicate doctor check id", + ); + await expectRejection( + runDoctorChecks( + [ + { + id: "first", + label: "First", + requires: ["later"], + run: () => ({ status: "pass", message: "Ready." }), + }, + { ...duplicate, id: "later" }, + ], + createDoctorContext(), + ), + "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..300fa7c --- /dev/null +++ b/cli/src/commands/doctor/lib/runner.ts @@ -0,0 +1,111 @@ +import { serializeCliError } from "@/lib/errors.ts"; +import type { + DoctorCheck, + DoctorCheckContext, + DoctorCheckResult, + DoctorReport, + DoctorSummary, +} from "@/commands/doctor/lib/model.ts"; + +function summarizeCheckResults(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 validateCheckSequence(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}`); + } + 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 runDoctorCheck( + 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), + }; + } +} + +export async function runDoctorChecks( + checks: readonly DoctorCheck[], + context: DoctorCheckContext, +): Promise { + 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) => runDoctorCheck(check, context, results)), + ); + } + + const checkResults = await Promise.all(pendingResults.values()); + const summary = summarizeCheckResults(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/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.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..309ea31 100644 --- a/cli/src/lib/management/model.ts +++ b/cli/src/lib/management/model.ts @@ -1,17 +1,51 @@ -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 { isRecord } from "@/lib/object.ts"; +import { parseApiJson } from "@/lib/parse-api-json.ts"; + +export type WhoamiResponse = components["schemas"]["WhoamiResponse"]; + +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/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/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> { diff --git a/cli/src/lib/profile-store.test.ts b/cli/src/lib/profile-store.test.ts index e91103e..71fb878 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, + resolveProfileReference, 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(resolveProfileReference()).toBe("missing"); + }); }); diff --git a/cli/src/lib/profile-store.ts b/cli/src/lib/profile-store.ts index fb07bf9..5df3573 100644 --- a/cli/src/lib/profile-store.ts +++ b/cli/src/lib/profile-store.ts @@ -159,6 +159,28 @@ export function resolveWorkingProfile(override?: string): string { return getActiveProfileName(); } +export function resolveProfileReference(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) { + assertSafeProfileName(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..b1c109b 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"; @@ -47,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; }; @@ -78,7 +80,7 @@ export type ProfileUpdate = { controlPlane?: string; }; -export type ProfileInspect = { +export type ProfileConfiguration = { name: string; active: boolean; config_file: string; @@ -97,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; @@ -108,6 +108,11 @@ export type ProfileInspect = { }; }; +export type ProfileInspect = ProfileConfiguration & { + auth: ProfileAuth; + status: "configured" | "partial" | "empty"; +}; + type ProfileSnapshot = { config: Record; auth: ProfileAuth; @@ -178,7 +183,13 @@ function writeProfileUpdate(name: string, update: ProfileUpdate): void { } } -function profileAuth(name: string): ProfileAuth { +export function detectConfiguredProfileAuth(name: string): ProfileAuth { + if (isFromEnvProfile(name)) { + return { + management: hasEnvManagementCredentials() ? "api_key" : "none", + lakehouse: envLakehouseAuthKind(), + }; + } return { management: secretExists("oauth/access-token", name) ? "oauth" @@ -194,14 +205,14 @@ function profileAuth(name: string): ProfileAuth { } export function profileHasAnyAuthConfigured(name: string): boolean { - const auth = profileAuth(name); + const auth = detectConfiguredProfileAuth(name); return auth.management !== "none" || auth.lakehouse !== "none"; } function readProfileSnapshot(name: string): ProfileSnapshot { return { config: readProfileConfigRecord(name), - auth: profileAuth(name), + auth: detectConfiguredProfileAuth(name), }; } @@ -279,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 readEnvironmentProfileConfiguration(): ProfileConfiguration { return { name: FROM_ENV_PSEUDOPROFILE_NAME, active: true, @@ -295,28 +302,18 @@ 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: {}, }; } -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); - const { config, auth } = snapshot; - +function buildProfileConfiguration( + name: string, + active: boolean, + config: Record, +): ProfileConfiguration { return { name, - active: getActiveProfileName() === name, + active, config_file: profileConfigFile(name), organization: { slug: config.organization_slug, @@ -333,8 +330,6 @@ export function inspectProfile(name: string): ProfileInspect { 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, @@ -345,6 +340,50 @@ export function inspectProfile(name: string): ProfileInspect { }; } +function buildProfileInspection( + name: string, + active: boolean, + snapshot: ProfileSnapshot, +): ProfileInspect { + return { + ...buildProfileConfiguration(name, active, snapshot.config), + auth: snapshot.auth, + status: profileStatus(snapshot), + }; +} + +export function inspectProfile(name: string): ProfileInspect { + if (isFromEnvProfile(name)) { + const configuration = readEnvironmentProfileConfiguration(); + const auth = detectConfiguredProfileAuth(name); + return { + ...configuration, + auth, + status: auth.management !== "none" || auth.lakehouse !== "none" ? "configured" : "empty", + }; + } + assertSafeProfileName(name); + ensureProfilesLayout(); + if (!profileExists(name)) { + throw new ConfigurationError(`Profile not found: ${name}`); + } + + return buildProfileInspection(name, getActiveProfileName() === name, readProfileSnapshot(name)); +} + +export function readProfileConfiguration(name: string): ProfileConfiguration { + if (isFromEnvProfile(name)) { + return readEnvironmentProfileConfiguration(); + } + 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 buildProfileConfiguration(name, activeProfile === name, readProfileConfigRecord(name)); +} + export function createEmptyProfile(name: string): ProfileInspect { assertSafeProfileName(name); ensureProfilesLayout(); 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/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/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 }; } 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 diff --git a/tests/doctor.test.ts b/tests/doctor.test.ts new file mode 100644 index 0000000..0911ad0 --- /dev/null +++ b/tests/doctor.test.ts @@ -0,0 +1,116 @@ +import { beforeAll, beforeEach, describe, expect, test } from "bun:test"; +import { createTestWorkspace, type TestWorkspace } from "./helpers.ts"; +import { jsonMock, textMock } 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: { + id: "user-1", + type: "User", + name: "Jane", + email: "jane@example.com", + }, + organization: { id: "org-1", name: "Acme", slug: "acme" }, + authentication_scope: "user", + }), + textMock( + "POST", + "/query", + ['{"statement":"SELECT 1"}', '["result"]', "[1]"].join("\n"), + ), + ]); + 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"); + }); +});