|
| 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