diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 448e8438..a9b0aab2 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -37,7 +37,10 @@ repos: priority: 30 # ── JS SDK (js/) ──────────────────────────────────────────────────── - # Mirrors release-npm.yml: lint → type-check → test (build is covered by CI). + # Mirrors release-npm.yml: lint → type-check → build → test. The build + # step is required because CLI tests consume @phala/cloud via workspace + # link to js/dist; a stale or missing js/dist masks real failures and + # also breaks downstream cli tests when js code is the thing that changed. - id: biome-js name: Biome (JS SDK) entry: bash -c 'cd js && bun run lint' @@ -54,6 +57,15 @@ repos: types_or: [javascript, ts, tsx, jsx] pass_filenames: false priority: 20 + - id: build-js + name: Build (JS SDK) + entry: bash -c 'cd js && bun run build' + language: system + files: ^js/ + types_or: [javascript, ts, tsx, jsx] + pass_filenames: false + require_serial: true + priority: 25 - id: test-js name: JS SDK tests entry: bash -c 'cd js && bun run test' @@ -65,9 +77,10 @@ repos: priority: 30 # ── CLI (cli/) ────────────────────────────────────────────────────── - # Mirrors release-npm.yml: lint → type-check → test. CLI consumes - # @phala/cloud via workspace link to js/dist, so a stale js/dist can - # mask real issues; rebuild js before committing if you've changed it. + # Mirrors release-npm.yml: lint → type-check → build → test. CLI tests + # spawn `bun cli/dist/index.js --help` for interface-compat checks, so + # the build step is mandatory — without it dist/index.js is missing and + # all the spawn-based tests silently fail with empty stdout. - id: biome-cli name: Biome (CLI) entry: bash -c 'cd cli && bun run lint' @@ -84,6 +97,15 @@ repos: types_or: [javascript, ts, tsx, jsx] pass_filenames: false priority: 20 + - id: build-cli + name: Build (CLI) + entry: bash -c 'cd cli && bun run build' + language: system + files: ^cli/ + types_or: [javascript, ts, tsx, jsx] + pass_filenames: false + require_serial: true + priority: 25 - id: test-cli name: CLI tests entry: bash -c 'cd cli && bun run test' diff --git a/cli/src/commands/cvms/replicate/index.ts b/cli/src/commands/cvms/replicate/index.ts index ba127768..a751d13e 100644 --- a/cli/src/commands/cvms/replicate/index.ts +++ b/cli/src/commands/cvms/replicate/index.ts @@ -6,6 +6,7 @@ import { type Client, type ErrorLink, type EnvVar, + SUPPORTED_CHAINS, safeAddComposeHash, safeAddDevice, safeCheckOnChainPrerequisites, @@ -410,17 +411,18 @@ async function runCvmsReplicateCommand( "Replica prepare response did not include a commit token", ); } - const chain = ( - sourceCvm as { - kms_info?: { - chain?: Parameters< - typeof safeCheckOnChainPrerequisites - >[0]["chain"]; - }; - } - ).kms_info?.chain; + // The SDK's CvmKmsInfo zod transform only injects `chain` when chain_id is in + // SUPPORTED_CHAINS, so unsupported chains would silently produce undefined. + // Resolve from chain_id directly for a clearer error. + const chainId = (sourceCvm as { kms_info?: { chain_id?: number } }) + .kms_info?.chain_id; + const chain = chainId != null ? SUPPORTED_CHAINS[chainId] : undefined; if (!chain) { - throw new Error("Source CVM kms_info is missing chain configuration"); + throw new Error( + chainId != null + ? `Source CVM chain id ${chainId} is not supported by this CLI build` + : "Source CVM kms_info is missing chain_id", + ); } if (!sourceCvm.app_id) { throw new Error( diff --git a/cli/src/commands/deploy/handler.ts b/cli/src/commands/deploy/handler.ts index dda91265..574f0a3f 100644 --- a/cli/src/commands/deploy/handler.ts +++ b/cli/src/commands/deploy/handler.ts @@ -21,6 +21,7 @@ import { CvmIdSchema, MAX_COMPOSE_PAYLOAD_BYTES, ResourceError, + SUPPORTED_CHAINS, createClient, encryptEnvVars, formatStructuredError, @@ -1127,9 +1128,23 @@ const updateCvm = async ( console.log("[DEBUG] patchCvm.deviceId:", result.deviceId); } + // Resolve viem chain from chain_id. The SDK schema only injects `chain` + // when chain_id is in SUPPORTED_CHAINS, so resolving from chain_id directly + // gives a clearer error for unsupported chains. + const cvmChainId = cvm.kms_info?.chain_id; + const cvmChain = + cvmChainId != null ? SUPPORTED_CHAINS[cvmChainId] : undefined; + if (!cvmChain) { + throw new Error( + cvmChainId != null + ? `CVM chain id ${cvmChainId} is not supported by this CLI build` + : "CVM kms_info is missing chain_id", + ); + } + // Check on-chain prerequisites (device + compose hash status) const prereqs = await safeCheckOnChainPrerequisites({ - chain: cvm.kms_info?.chain, + chain: cvmChain, rpcUrl: validatedOptions.rpcUrl, appAddress: cvm.app_id as `0x${string}`, deviceId: result.deviceId, @@ -1152,7 +1167,7 @@ const updateCvm = async ( if (!prereqs.data.deviceAllowed) { logger.info("Device not registered on-chain, adding..."); const deviceResult = await safeAddDevice({ - chain: cvm.kms_info?.chain, + chain: cvmChain, rpcUrl: validatedOptions.rpcUrl, appAddress: cvm.app_id as `0x${string}`, deviceId: result.deviceId, @@ -1169,7 +1184,7 @@ const updateCvm = async ( // Register compose hash on-chain (idempotent — always send to get a real tx hash) const receipt_result = await safeAddComposeHash({ - chain: cvm.kms_info?.chain, + chain: cvmChain, rpcUrl: validatedOptions.rpcUrl, appId: cvm.app_id as `0x${string}`, composeHash: result.composeHash, diff --git a/cli/src/commands/instances/add/index.ts b/cli/src/commands/instances/add/index.ts index ebf1ff61..67870d55 100644 --- a/cli/src/commands/instances/add/index.ts +++ b/cli/src/commands/instances/add/index.ts @@ -6,6 +6,7 @@ import { type Client, type ErrorLink, type EnvVar, + SUPPORTED_CHAINS, safeAddComposeHash, safeAddDevice, safeCheckOnChainPrerequisites, @@ -367,17 +368,19 @@ async function runInstancesAddCommand( throw new Error("Prepare response did not include a commit token"); } - const chain = ( - preparePayload.kmsInfo as - | { - chain?: Parameters< - typeof safeCheckOnChainPrerequisites - >[0]["chain"]; - } - | undefined - )?.chain; + // The prepare payload comes from raw HTTP 465 error.structuredDetails, so it + // does NOT pass through the SDK's KmsInfoSchema zod transform that injects + // `chain`. Resolve from chain_id directly. + const chainId = ( + preparePayload.kmsInfo as { chain_id?: number } | undefined + )?.chain_id; + const chain = chainId != null ? SUPPORTED_CHAINS[chainId] : undefined; if (!chain) { - throw new Error("App KMS info is missing chain configuration"); + throw new Error( + chainId != null + ? `Chain id ${chainId} is not supported by this CLI build` + : "App KMS info is missing chain_id", + ); } const prereqs = await safeCheckOnChainPrerequisites({ diff --git a/js/src/utils/errors.test.ts b/js/src/utils/errors.test.ts index 2c0dfb00..c3a786a5 100644 --- a/js/src/utils/errors.test.ts +++ b/js/src/utils/errors.test.ts @@ -12,6 +12,8 @@ import { getValidationFields, formatValidationErrors, formatErrorMessage, + formatStructuredError, + formatStructuredErrorDetailValue, } from "./errors"; describe("parseApiError", () => { @@ -528,3 +530,55 @@ describe("RequestError.fromFetchError with StructuredError responses", () => { expect(Array.isArray(detail.details)).toBe(true); }); }); + +describe("formatStructuredErrorDetailValue", () => { + it("returns strings unchanged", () => { + expect(formatStructuredErrorDetailValue("hello")).toBe("hello"); + }); + + it("stringifies numbers and booleans", () => { + expect(formatStructuredErrorDetailValue(7)).toBe("7"); + expect(formatStructuredErrorDetailValue(true)).toBe("true"); + expect(formatStructuredErrorDetailValue(false)).toBe("false"); + }); + + it("returns empty string for null/undefined", () => { + expect(formatStructuredErrorDetailValue(null)).toBe(""); + expect(formatStructuredErrorDetailValue(undefined)).toBe(""); + }); + + it("serializes objects to JSON", () => { + expect(formatStructuredErrorDetailValue({ chain_id: 1, slug: "kms-eth" })).toBe( + '{"chain_id":1,"slug":"kms-eth"}', + ); + }); + + it("serializes arrays to JSON", () => { + expect(formatStructuredErrorDetailValue([1, 2, 3])).toBe("[1,2,3]"); + }); +}); + +describe("formatStructuredError details rendering", () => { + it("renders dict and array values via JSON instead of [object Object]", () => { + const error = new ResourceError("Compose hash registration required", { + status: 465, + statusText: "Compose hash registration required", + detail: undefined, + errorCode: "ERR-01-005", + structuredDetails: [ + { field: "compose_hash", value: "428faa5" }, + { + field: "kms_info", + value: { chain_id: 1, slug: "kms-eth-prod7" }, + }, + { field: "extras", value: [1, 2, 3] }, + ], + }); + + const formatted = formatStructuredError(error); + expect(formatted).toContain("compose_hash: 428faa5"); + expect(formatted).toContain('kms_info: {"chain_id":1,"slug":"kms-eth-prod7"}'); + expect(formatted).toContain("extras: [1,2,3]"); + expect(formatted).not.toContain("[object Object]"); + }); +}); diff --git a/js/src/utils/errors.ts b/js/src/utils/errors.ts index 6176706d..f3077a15 100644 --- a/js/src/utils/errors.ts +++ b/js/src/utils/errors.ts @@ -620,6 +620,13 @@ export function getErrorMessage(error: ApiError): string { /** * Structured error detail from new error format (ERR-xxxx codes) + * + * `value` is typed as `unknown` because the backend contract says it should be + * a primitive (string / number / boolean) but in practice some errors emit + * dict / array values (e.g. HashRegistrationRequired carries `kms_info` and + * `onchain_status` as objects). Consumers that need a string should use + * {@link formatStructuredErrorDetailValue}; consumers that need the original + * structure (e.g. CLI extracting `commit_token`) should access `value` directly. */ export interface StructuredErrorDetail { field?: string; @@ -627,6 +634,25 @@ export interface StructuredErrorDetail { message?: string; } +/** + * Coerce a {@link StructuredErrorDetail} `value` to a renderable string. + * + * - `string` returned as-is + * - `number` / `boolean` stringified via `String(...)` + * - `null` / `undefined` returned as `""` so callers never interpolate `null` literally + * - anything else (dict, array) serialized via `JSON.stringify` with a fallback to `String(...)` + */ +export function formatStructuredErrorDetailValue(value: unknown): string { + if (value === null || value === undefined) return ""; + if (typeof value === "string") return value; + if (typeof value === "number" || typeof value === "boolean") return String(value); + try { + return JSON.stringify(value); + } catch { + return String(value); + } +} + /** * Error link from structured error response */ @@ -780,7 +806,7 @@ export function formatStructuredError( if (d.message) { parts.push(` - ${d.message}`); } else if (d.field && d.value !== undefined) { - parts.push(` - ${d.field}: ${d.value}`); + parts.push(` - ${d.field}: ${formatStructuredErrorDetailValue(d.value)}`); } }); } diff --git a/js/src/utils/index.ts b/js/src/utils/index.ts index 36283944..088d8e9e 100644 --- a/js/src/utils/index.ts +++ b/js/src/utils/index.ts @@ -37,6 +37,7 @@ export { formatValidationErrors, formatErrorMessage, formatStructuredError, + formatStructuredErrorDetailValue, getErrorMessage, // Conflict helpers isConflictError,