Skip to content

Commit 4bb73c3

Browse files
committed
docs: add agents.md with lessons learned from quality audit
Captures all 19 issues found across 4 SDKs, organized by severity, with process takeaways for preventing recurrence. https://claude.ai/code/session_01MqxtVDf8nA4wPaPZD56zCM
1 parent 5795fa1 commit 4bb73c3

1 file changed

Lines changed: 76 additions & 0 deletions

File tree

agents.md

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
# Agents — Lessons Learned
2+
3+
Quality audit of the HAIAI SDK (Python, Node, Go, Rust) — March 2026.
4+
5+
## Critical Findings
6+
7+
### Wrong endpoints (Go)
8+
- `ProRun()` called `/api/benchmark/subscribe` instead of `/api/benchmark/purchase`.
9+
- `GetAgentAttestation()` used singular `/attestation` instead of plural `/attestations`.
10+
- Key lookup functions (`FetchKeyByEmail`, etc.) defaulted to a separate `keys.hai.ai` host instead of the main HAI endpoint.
11+
- **Lesson:** Endpoint paths drift silently across SDKs. The `fixtures/contract_endpoints.json` fixture now exists to catch this — keep it updated when adding new endpoints.
12+
13+
### Missing payload fields (Python, Go)
14+
- Python `hello_world()` omitted `agent_id` from the POST body (present in the other 3 SDKs).
15+
- `benchmark()` in Python and Go omitted `"transport": "sse"`.
16+
- **Lesson:** When one SDK adds a field, grep all SDKs for the same call and confirm parity.
17+
18+
## High-Priority Patterns
19+
20+
### No retry / no backoff
21+
- Go and Rust had zero retry logic. Node threw immediately on 429.
22+
- **Fix:** All SDKs now retry on `[429, 500, 502, 503, 504]` with exponential backoff (default 3 attempts). The status-code list is validated by a cross-SDK contract test (`test_retryable_status_codes_match_python`).
23+
- **Lesson:** Retry policy is a cross-cutting concern — treat it like a shared spec, not per-SDK discretion.
24+
25+
### Unbounded reads (Go)
26+
- `io.ReadAll()` without a size cap could exhaust memory on malicious responses.
27+
- **Fix:** `limitedReadAll()` helper caps at 10 MB.
28+
- **Lesson:** Any `ReadAll`/`.text()`/`.read()` on an HTTP response should have a size limit.
29+
30+
## Medium-Priority Patterns
31+
32+
### Private key exposure (Node)
33+
- Private key was stored as an enumerable property, visible in `JSON.stringify()` and property iteration.
34+
- **Fix:** Moved to a module-level `WeakMap`, non-enumerable and non-serializable.
35+
- **Lesson:** Secrets should never be plain object properties. Use `WeakMap`, closures, or `Object.defineProperty` with `enumerable: false`.
36+
37+
### Hardcoded passphrase (Node)
38+
- `register()` used `"register-temp"` as a temp key passphrase.
39+
- **Fix:** `crypto.randomBytes()`.
40+
- **Lesson:** Grep for string literals that look like passphrases/tokens — they're easy to miss in review.
41+
42+
### Base URL validation
43+
- Python, Node, and Rust accepted arbitrary URL schemes (`ftp://`, `file://`, `javascript://`).
44+
- **Fix:** All SDKs now reject non-`http(s)://` schemes at construction time.
45+
- **Lesson:** Validate inputs at the boundary, not deep in the call chain.
46+
47+
### Unbounded reconnection (Node, Rust)
48+
- SSE/WebSocket reconnection loops had no cap — could retry forever.
49+
- **Fix:** `maxReconnectAttempts` (default 10) with exponential backoff.
50+
- **Lesson:** Every retry loop needs a ceiling.
51+
52+
### Missing SSE timeout (Go)
53+
- SSE HTTP client had no `ResponseHeaderTimeout` — could hang indefinitely.
54+
- **Fix:** 30-second timeout on response headers.
55+
56+
### Race conditions (Go)
57+
- Mutable fields (`haiAgentID`, `agentEmail`) accessed without synchronization.
58+
- **Fix:** `sync.RWMutex` around mutable state.
59+
- **Lesson:** Any field written after construction needs a mutex in Go.
60+
61+
### Missing query parameters (Python, Go, Node)
62+
- `list_messages()` and `search_messages()` were missing `has_attachments`, `since`, `until` filters that Rust already had.
63+
- **Lesson:** When Rust (the reference implementation) has a feature, all SDKs should too.
64+
65+
### Reply endpoint undocumented (Rust)
66+
- Rust uses a server-side `/email/reply` endpoint; other SDKs compose client-side. Neither approach was documented.
67+
- **Fix:** Added to `fixtures/contract_endpoints.json` with a migration note.
68+
- **Lesson:** Architectural divergences between SDKs need explicit documentation, not just code.
69+
70+
## Process Takeaways
71+
72+
1. **Contract fixtures prevent drift.** `fixtures/contract_endpoints.json` is the source of truth for endpoints. Add new entries there first.
73+
2. **Cross-SDK grep is mandatory.** Before merging a behavior change in one SDK, grep the same function/field name across all four.
74+
3. **Test the sad path.** Most bugs were in error handling, retry logic, and edge cases — not the happy path.
75+
4. **Security is a default, not a feature.** Key exposure, hardcoded secrets, and unbounded reads were all "it works" code that was silently dangerous.
76+
5. **Rust is the reference.** When in doubt about what a function should do, check the Rust implementation first — it tends to be the most complete.

0 commit comments

Comments
 (0)