Skip to content

Commit d910454

Browse files
authored
Merge pull request #6 from commandlayer/claude/review-production-readiness-c01t9
Add comprehensive unit tests and improve error handling
2 parents 077d19c + ccb45b1 commit d910454

File tree

8 files changed

+265
-15
lines changed

8 files changed

+265
-15
lines changed
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
name: Python SDK Tests
2+
3+
on:
4+
push:
5+
paths:
6+
- "python-sdk/**"
7+
- ".github/workflows/python-sdk-tests.yml"
8+
pull_request:
9+
paths:
10+
- "python-sdk/**"
11+
- ".github/workflows/python-sdk-tests.yml"
12+
13+
jobs:
14+
test:
15+
runs-on: ubuntu-latest
16+
strategy:
17+
matrix:
18+
python-version: ["3.10", "3.11", "3.12"]
19+
defaults:
20+
run:
21+
working-directory: python-sdk
22+
steps:
23+
- name: Checkout
24+
uses: actions/checkout@v4
25+
26+
- name: Setup Python ${{ matrix.python-version }}
27+
uses: actions/setup-python@v5
28+
with:
29+
python-version: ${{ matrix.python-version }}
30+
31+
- name: Install dependencies
32+
run: pip install -e '.[dev]'
33+
34+
- name: Lint (ruff)
35+
run: python -m ruff check commandlayer/
36+
37+
- name: Type check (mypy)
38+
run: python -m mypy commandlayer/
39+
40+
- name: Tests (pytest)
41+
run: python -m pytest tests/ -v

.github/workflows/typescript-sdk-cli-smoke.yml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,5 +36,8 @@ jobs:
3636
- name: Build
3737
run: npm run build
3838

39+
- name: Unit tests
40+
run: npm run test:unit
41+
3942
- name: CLI smoke tests
4043
run: npm run test:cli-smoke

python-sdk/commandlayer/client.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -238,11 +238,17 @@ def call(self, verb: str, body: dict[str, Any]) -> Receipt:
238238
if not result["ok"]:
239239
raise CommandLayerError("Receipt verification failed", 422, result)
240240

241-
return data
241+
return data # type: ignore[no-any-return]
242242

243243
def close(self):
244244
self._http.close()
245245

246+
def __enter__(self):
247+
return self
248+
249+
def __exit__(self, *args: object):
250+
self.close()
251+
246252

247253
def create_client(**kwargs) -> CommandLayerClient:
248254
return CommandLayerClient(**kwargs)

python-sdk/commandlayer/verify.py

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
import hashlib
44
import json
55
import re
6-
from typing import Any
6+
from typing import Any, Literal
77

88
from nacl.signing import VerifyKey
99
from nacl.exceptions import BadSignatureError
@@ -83,7 +83,10 @@ def resolve_ens_ed25519_pubkey(name: str, rpc_url: str, pubkey_text_key: str = "
8383
return {"pubkey": None, "source": None, "error": "Unable to connect to RPC", "txt_key": pubkey_text_key}
8484

8585
try:
86-
txt = w3.ens.get_text(name, pubkey_text_key) # type: ignore[attr-defined]
86+
ens_module = w3.ens # type: ignore[attr-defined]
87+
if ens_module is None:
88+
return {"pubkey": None, "source": None, "error": "ENS module not available", "txt_key": pubkey_text_key}
89+
txt = ens_module.get_text(name, pubkey_text_key) # type: ignore[union-attr]
8790
except Exception as err:
8891
return {"pubkey": None, "source": None, "error": f"ENS TXT lookup failed: {err}", "txt_key": pubkey_text_key}
8992

@@ -142,22 +145,24 @@ def verify_receipt(receipt: Receipt, public_key: str | None = None, ens: dict[st
142145
hash_matches = bool(claimed_hash and claimed_hash == recomputed_hash)
143146

144147
metadata = receipt.get("metadata") if isinstance(receipt.get("metadata"), dict) else {}
148+
assert isinstance(metadata, dict) # narrowing for mypy; always true given the guard above
145149
receipt_id = metadata.get("receipt_id") or receipt.get("receipt_id")
146150
receipt_id = receipt_id if isinstance(receipt_id, str) else None
147151
receipt_id_matches = bool(claimed_hash and receipt_id == claimed_hash)
148152

149-
pubkey = None
150-
pubkey_source = None
151-
ens_error = None
152-
ens_txt_key = None
153+
pubkey: bytes | None = None
154+
pubkey_source: Literal["explicit", "ens"] | None = None
155+
ens_error: str | None = None
156+
ens_txt_key: str | None = None
153157

154158
if public_key:
155159
pubkey = parse_ed25519_pubkey(public_key)
156160
pubkey_source = "explicit"
157161
elif ens:
162+
ens_rpc_url = ens.get("rpcUrl") or ens.get("rpc_url") or ""
158163
res = resolve_ens_ed25519_pubkey(
159164
name=ens["name"],
160-
rpc_url=ens.get("rpcUrl") or ens.get("rpc_url"),
165+
rpc_url=str(ens_rpc_url),
161166
pubkey_text_key=ens.get("pubkeyTextKey") or ens.get("pubkey_text_key") or "cl.pubkey",
162167
)
163168
ens_txt_key = res.get("txt_key")

typescript-sdk/package.json

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,9 @@
2828
"dev": "tsup --watch",
2929
"typecheck": "tsc -p tsconfig.json --noEmit",
3030
"prepack": "npm run build",
31-
"test:cli-smoke": "node scripts/cli-smoke.mjs"
31+
"test:cli-smoke": "node scripts/cli-smoke.mjs",
32+
"test:unit": "node scripts/unit-tests.mjs",
33+
"test": "npm run test:unit && npm run test:cli-smoke"
3234
},
3335
"dependencies": {
3436
"commander": "^12.1.0",
Lines changed: 176 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,176 @@
1+
/**
2+
* Unit tests for core SDK logic (canonicalization, hashing, verification).
3+
* Uses tweetnacl for Ed25519 key generation — no external test framework needed.
4+
*/
5+
import { createRequire } from "node:module";
6+
const require = createRequire(import.meta.url);
7+
8+
const {
9+
canonicalizeStableJsonV1,
10+
sha256HexUtf8,
11+
parseEd25519Pubkey,
12+
verifyEd25519SignatureOverUtf8HashString,
13+
recomputeReceiptHashSha256,
14+
verifyReceipt,
15+
CommandLayerError,
16+
CommandLayerClient,
17+
} = require("../dist/index.cjs");
18+
19+
const nacl = require("tweetnacl");
20+
21+
let passed = 0;
22+
let failed = 0;
23+
24+
function assert(condition, name) {
25+
if (!condition) {
26+
failed++;
27+
console.error(`FAIL: ${name}`);
28+
} else {
29+
passed++;
30+
console.log(`PASS: ${name}`);
31+
}
32+
}
33+
34+
function assertThrows(fn, name) {
35+
try {
36+
fn();
37+
failed++;
38+
console.error(`FAIL: ${name} (did not throw)`);
39+
} catch {
40+
passed++;
41+
console.log(`PASS: ${name}`);
42+
}
43+
}
44+
45+
// ---- Canonicalization ----
46+
47+
assert(canonicalizeStableJsonV1(null) === "null", "canonicalize null");
48+
assert(canonicalizeStableJsonV1(true) === "true", "canonicalize true");
49+
assert(canonicalizeStableJsonV1(false) === "false", "canonicalize false");
50+
assert(canonicalizeStableJsonV1(42) === "42", "canonicalize int");
51+
assert(canonicalizeStableJsonV1(3.14) === "3.14", "canonicalize float");
52+
assert(canonicalizeStableJsonV1("hello") === '"hello"', "canonicalize string");
53+
assert(canonicalizeStableJsonV1([1, 2, 3]) === "[1,2,3]", "canonicalize array");
54+
assert(
55+
canonicalizeStableJsonV1({ b: 2, a: 1 }) === '{"a":1,"b":2}',
56+
"canonicalize sorts keys"
57+
);
58+
assert(
59+
canonicalizeStableJsonV1({ z: { b: 1, a: 2 } }) === '{"z":{"a":2,"b":1}}',
60+
"canonicalize nested sorted"
61+
);
62+
assertThrows(
63+
() => canonicalizeStableJsonV1(BigInt(1)),
64+
"canonicalize rejects bigint"
65+
);
66+
assertThrows(
67+
() => canonicalizeStableJsonV1(Infinity),
68+
"canonicalize rejects Infinity"
69+
);
70+
assertThrows(
71+
() => canonicalizeStableJsonV1({ a: undefined }),
72+
"canonicalize rejects undefined value"
73+
);
74+
75+
// Negative zero
76+
assert(canonicalizeStableJsonV1(-0) === "0", "canonicalize -0 => 0");
77+
78+
// ---- SHA-256 ----
79+
80+
const knownHash = sha256HexUtf8("hello");
81+
assert(knownHash.length === 64, "sha256 returns 64 hex chars");
82+
assert(sha256HexUtf8("hello") === knownHash, "sha256 deterministic");
83+
assert(sha256HexUtf8("hello") !== sha256HexUtf8("world"), "sha256 differs for different inputs");
84+
85+
// ---- Ed25519 pubkey parsing ----
86+
87+
const kp = nacl.sign.keyPair();
88+
const b64Key = Buffer.from(kp.publicKey).toString("base64");
89+
const hexKey = Buffer.from(kp.publicKey).toString("hex");
90+
91+
const pk1 = parseEd25519Pubkey(b64Key);
92+
assert(pk1.length === 32, "parse base64 pubkey");
93+
94+
const pk2 = parseEd25519Pubkey(`ed25519:${b64Key}`);
95+
assert(pk2.length === 32, "parse ed25519: prefixed pubkey");
96+
97+
const pk3 = parseEd25519Pubkey(hexKey);
98+
assert(pk3.length === 32, "parse hex pubkey");
99+
100+
const pk4 = parseEd25519Pubkey(`0x${hexKey}`);
101+
assert(pk4.length === 32, "parse 0x-prefixed hex pubkey");
102+
103+
assertThrows(
104+
() => parseEd25519Pubkey("not_valid_key!!"),
105+
"rejects invalid pubkey"
106+
);
107+
108+
// ---- Signature verification ----
109+
110+
const hashHex = sha256HexUtf8('{"test":true}');
111+
const msg = Buffer.from(hashHex, "utf8");
112+
const sig = nacl.sign.detached(new Uint8Array(msg), kp.secretKey);
113+
const sigB64 = Buffer.from(sig).toString("base64");
114+
115+
assert(
116+
verifyEd25519SignatureOverUtf8HashString(hashHex, sigB64, kp.publicKey) === true,
117+
"valid signature verifies"
118+
);
119+
120+
const badKp = nacl.sign.keyPair();
121+
assert(
122+
verifyEd25519SignatureOverUtf8HashString(hashHex, sigB64, badKp.publicKey) === false,
123+
"wrong key rejects"
124+
);
125+
126+
// ---- Receipt verification (end-to-end) ----
127+
128+
const receipt = {
129+
status: "success",
130+
x402: { verb: "summarize", version: "1.0.0", entry: "x402://summarizeagent.eth/summarize/v1.0.0" },
131+
result: { summary: "test" },
132+
metadata: {
133+
proof: {
134+
alg: "ed25519-sha256",
135+
canonical: "cl-stable-json-v1",
136+
signer_id: "runtime.commandlayer.eth",
137+
},
138+
},
139+
};
140+
141+
const { hash_sha256 } = recomputeReceiptHashSha256(receipt);
142+
const receiptMsg = Buffer.from(hash_sha256, "utf8");
143+
const receiptSig = nacl.sign.detached(new Uint8Array(receiptMsg), kp.secretKey);
144+
145+
receipt.metadata.proof.hash_sha256 = hash_sha256;
146+
receipt.metadata.proof.signature_b64 = Buffer.from(receiptSig).toString("base64");
147+
receipt.metadata.receipt_id = hash_sha256;
148+
149+
const vr = await verifyReceipt(receipt, { publicKey: `ed25519:${b64Key}` });
150+
assert(vr.ok === true, "verifyReceipt ok for valid receipt");
151+
assert(vr.checks.hash_matches === true, "verifyReceipt hash matches");
152+
assert(vr.checks.signature_valid === true, "verifyReceipt signature valid");
153+
assert(vr.checks.receipt_id_matches === true, "verifyReceipt receipt_id matches");
154+
155+
// Tampered receipt
156+
const tamperedReceipt = JSON.parse(JSON.stringify(receipt));
157+
tamperedReceipt.result.summary = "tampered";
158+
const vr2 = await verifyReceipt(tamperedReceipt, { publicKey: `ed25519:${b64Key}` });
159+
assert(vr2.ok === false, "verifyReceipt rejects tampered receipt");
160+
assert(vr2.checks.hash_matches === false, "tampered receipt hash mismatch");
161+
162+
// ---- Client verb validation ----
163+
164+
const client = new CommandLayerClient();
165+
try {
166+
await client.call("nonexistent", {});
167+
failed++;
168+
console.error("FAIL: client.call accepts unknown verb");
169+
} catch (err) {
170+
assert(err instanceof CommandLayerError, "client.call rejects unknown verb with CommandLayerError");
171+
}
172+
173+
// ---- Summary ----
174+
175+
console.log(`\n${passed} passed, ${failed} failed`);
176+
if (failed > 0) process.exit(1);

typescript-sdk/src/cli.ts

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,11 @@
22
import { Command } from "commander";
33
import { createClient, type Receipt } from "./index";
44

5+
function parseIntSafe(value: string, fallback: number): number {
6+
const n = Number(value);
7+
return Number.isFinite(n) && n > 0 ? Math.floor(n) : fallback;
8+
}
9+
510
const program = new Command();
611

712
program
@@ -41,14 +46,14 @@ withCommonOptions(
4146
const client = createClient({
4247
runtime: root.runtime,
4348
actor: root.actor,
44-
timeoutMs: Number(root.timeoutMs)
49+
timeoutMs: parseIntSafe(root.timeoutMs, 30_000)
4550
});
4651

4752
const receipt = await client.summarize({
4853
content: opts.content,
4954
style: opts.style,
5055
format: opts.format,
51-
maxTokens: Number(opts.maxTokens)
56+
maxTokens: parseIntSafe(opts.maxTokens, 1000)
5257
});
5358

5459
printResult(receipt, !!root.json);
@@ -60,13 +65,13 @@ withCommonOptions(program.command("analyze").description("Analyze content").opti
6065
const client = createClient({
6166
runtime: root.runtime,
6267
actor: root.actor,
63-
timeoutMs: Number(root.timeoutMs)
68+
timeoutMs: parseIntSafe(root.timeoutMs, 30_000)
6469
});
6570

6671
const receipt = await client.analyze({
6772
content: opts.content,
6873
goal: opts.goal,
69-
maxTokens: Number(opts.maxTokens)
74+
maxTokens: parseIntSafe(opts.maxTokens, 1000)
7075
});
7176

7277
printResult(receipt, !!root.json);
@@ -80,7 +85,7 @@ program.command("call").description("Call a verb with a raw JSON payload").requi
8085
const client = createClient({
8186
runtime: root.runtime,
8287
actor: root.actor,
83-
timeoutMs: Number(root.timeoutMs)
88+
timeoutMs: parseIntSafe(root.timeoutMs, 30_000)
8489
});
8590

8691
const body = JSON.parse(opts.body) as Record<string, unknown>;

typescript-sdk/src/index.ts

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -614,6 +614,10 @@ export class CommandLayerClient {
614614
}
615615

616616
async call(verb: Verb, body: Record<string, any>): Promise<Receipt> {
617+
if (!VERBS.includes(verb as any)) {
618+
throw new CommandLayerError(`Unsupported verb: ${verb}`, 400);
619+
}
620+
617621
const url = `${this.runtime}/${verb}/v1.0.0`;
618622

619623
this.ensureVerifyConfigIfEnabled();
@@ -642,7 +646,15 @@ export class CommandLayerClient {
642646
signal: controller.signal
643647
});
644648

645-
const data: any = await resp.json().catch(() => ({}));
649+
let data: any;
650+
try {
651+
data = await resp.json();
652+
} catch {
653+
if (!resp.ok) {
654+
throw new CommandLayerError(`HTTP ${resp.status} (non-JSON response)`, resp.status);
655+
}
656+
throw new CommandLayerError("Runtime returned non-JSON response", resp.status);
657+
}
646658

647659
if (!resp.ok) {
648660
throw new CommandLayerError(

0 commit comments

Comments
 (0)