Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 26 additions & 4 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand All @@ -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'
Expand All @@ -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'
Expand All @@ -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'
Expand Down
22 changes: 12 additions & 10 deletions cli/src/commands/cvms/replicate/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
type Client,
type ErrorLink,
type EnvVar,
SUPPORTED_CHAINS,
safeAddComposeHash,
safeAddDevice,
safeCheckOnChainPrerequisites,
Expand Down Expand Up @@ -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(
Expand Down
21 changes: 18 additions & 3 deletions cli/src/commands/deploy/handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import {
CvmIdSchema,
MAX_COMPOSE_PAYLOAD_BYTES,
ResourceError,
SUPPORTED_CHAINS,
createClient,
encryptEnvVars,
formatStructuredError,
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand Down
23 changes: 13 additions & 10 deletions cli/src/commands/instances/add/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
type Client,
type ErrorLink,
type EnvVar,
SUPPORTED_CHAINS,
safeAddComposeHash,
safeAddDevice,
safeCheckOnChainPrerequisites,
Expand Down Expand Up @@ -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({
Expand Down
54 changes: 54 additions & 0 deletions js/src/utils/errors.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import {
getValidationFields,
formatValidationErrors,
formatErrorMessage,
formatStructuredError,
formatStructuredErrorDetailValue,
} from "./errors";

describe("parseApiError", () => {
Expand Down Expand Up @@ -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]");
});
});
28 changes: 27 additions & 1 deletion js/src/utils/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -620,13 +620,39 @@ 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;
value?: unknown;
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
*/
Expand Down Expand Up @@ -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)}`);
}
});
}
Expand Down
1 change: 1 addition & 0 deletions js/src/utils/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ export {
formatValidationErrors,
formatErrorMessage,
formatStructuredError,
formatStructuredErrorDetailValue,
getErrorMessage,
// Conflict helpers
isConflictError,
Expand Down
Loading