refactor(api): vendor tinyhumans-sdk and route the backend client through it - #5232
Conversation
Add https://github.com/tinyhumansai/sdk as a submodule under vendor/, beside the other tiny* crates, and wire it as a path dependency of the core crate. The crate is not published to crates.io, so it is consumed directly by path rather than through a [patch.crates-io] entry like tinyagents / tinyflows / tinycortex / tinyjuice / tinychannels / tinyplace. It shares reqwest 0.12, serde, serde_json, thiserror 2, url, and percent-encoding with the core crate, so the lockfile gains exactly one package and no new transitive dependencies.
Picks up the three upstream SDK changes this migration depends on: injectable reqwest client + default headers, the webhooks namespace, and unsuccessful-envelope errors. NOTE: these commits live on the local submodule branch `feat/openhuman-host-integration` and must be pushed and merged into tinyhumansai/sdk before this pointer resolves for anyone else.
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 21 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. 📝 WalkthroughWalkthroughThe PR vendors ChangesTinyHumans SDK integration
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
The SDK excluded DELETE /teams/{teamId}/members/{userId} and its two
siblings as platform-admin routes because their OpenAPI summaries read
"(admin only)". That "admin" is the team-admin role, not platform
administration, and blocking them broke team_remove_member — caught by
json_rpc_e2e once authed_json started routing through the SDK.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
senamakel has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
There was a problem hiding this comment.
senamakel has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d2f1d24c9a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
senamakel has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: efb0dd6b1a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
senamakel has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
There was a problem hiding this comment.
senamakel has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
coderabbitai
Bot
added
feature
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
senamakel has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 461bad8fa2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if let Some(user) = object.get("user").filter(|user| !user.is_null()) { | ||
| return Ok(user.clone()); | ||
| } |
There was a problem hiding this comment.
success before unwrapping user
When a 2xx backend payload contains both success: false and a non-null user—for example, an auth endpoint returning user context alongside a logical failure—this early return accepts it before inspecting success. The previous parse_api_response_json checked success first, so validate_session_token can now accept a logically rejected profile and other callers receive user data instead of an error; keep the user fallback after the false-success check.
AGENTS.md reference: AGENTS.md:L189-L193
Useful? React with 👍 / 👎.
| `x-tauri-version` headers. Bind a session token with `sdk_for(bearer_jwt)`. | ||
|
|
||
| **Every SDK-backed call must map its error through `classify_sdk_error`.** That |
There was a problem hiding this comment.
Neither sdk_for nor classify_sdk_error exists anywhere in the target tree: BackendOAuthClient binds tokens inline with with_token, while error classification remains embedded in authed_json. Because this authoritative guidance explicitly directs future typed-route migrations through those names, following it produces uncompilable code and incorrectly suggests there is already a shared classifier; either add the promised helpers or document the actual integration path.
AGENTS.md reference: AGENTS.md:L195-L207
Useful? React with 👍 / 👎.
1 participant
Summary
tinyhumans-sdkas a git submodule atvendor/tinyhumans-sdkand make it the source of truth for TinyHumans backend routes.src/api/rest.rsaround the SDK: it now owns aTinyHumansClientthat shares this crate'sreqwest::Client, so the SDK inherits platform TLS, timeouts, and version headers.authed_json— the generic helper behind every remaining backend call — on the SDK transport, so the cutover is complete rather than partial.classify_sdk_erroras the single classifier shared by both paths, so moving a route onto a typed method cannot change its Sentry or session-expiry behaviour.## Related).Problem
src/api/was a second, hand-rolled implementation of the TinyHumans backend contract — ~3.8k lines duplicating routes, URL building, percent-encoding, and envelope handling that the SDK already defines. Route knowledge lived in two places and drifted: the audit found OpenHuman calling one route the backend does not implement, and sending a field nobody honours.Solution
The split. The SDK owns the contract — routes, URL building, encoding, credential headers,
{success,data}envelopes, and the admin/webhook-receiver gate.src/api/keeps what is genuinely OpenHuman's: session-token retrieval (jwt.rs), base-URL/env resolution (config.rs), and the error-classification + Sentry policy (rest.rs).config.rsdeliberately stays — it is env/keyring concern, not contract.Shared transport, not a second client.
BackendOAuthClientbuilds the SDK withwith_http_client(self.client.clone()). This matters: the SDK's default is a barereqwest::Client::new(), and adopting it naively would have dropped schannel on Windows (corporate TLS-inspection proxies present a cert the bundled rustls roots reject), the 120s/15s timeouts,http1_only, and thex-core-version/x-tauri-versionattribution headers. Requires tinyhumansai/sdk#3.One classification, two paths.
classify_sdk_errormirrorsauthed_jsonexactly — 401 →Unauthorized/SESSION_EXPIRED, channel-message 404 →MessageNotFound, announcements 404 →AnnouncementNotFound, transient statuses logged not reported. Without this, migrating a route onto a typed SDK method would silently change its Sentry behaviour.rest_tests.rspins the equivalence for both paths.The cutover is complete, not partial. Rather than migrate call sites one at a time,
authed_jsonitself now runs on the SDK: it sends throughsdk.raw().send(...)and maps failures throughclassify_sdk_error. Every remaining backend call therefore moved at once, with no call-site churn and no risk of a hand-copied path/method drifting from the route it describes.send_channel_typingandsend_channel_deleteadditionally use the SDK's typedchannels()methods.Envelope semantics are preserved deliberately. The response is fetched with
unwrap = falseand parsed by this crate'sparse_api_response_value, not the SDK'sunwrap_envelope. The SDK's version lacks theuser-key fallback thatGET /auth/medepends on, so letting it unwrap would change that route's shape.What this caught. Routing everything through the SDK immediately surfaced a real defect: the SDK classified
DELETE /teams/{teamId}/members/{userId}as platform-admin (its OpenAPI summary says "(admin only)") and blocked it, breakingteam_remove_member. That "admin" is the team-admin role, not platform administration. Fixed upstream and pinned by tests in tinyhumansai/sdk#3. This is exactly the kind of drift the migration exists to eliminate.Submission Checklist
src/api/rest_tests.rscovering the SDK-backed 404 (MessageNotFound), 401 (Unauthorized+SESSION_EXPIREDsentinel), and version-header propagation. All 53 pre-existingapi::restclassification tests pass unchanged against the reimplementedauthed_json— they are the primary safety net for this change, andjson_rpc_e2e(115/115) is the integration net.src/api/rest.rs(client wiring +classify_sdk_error) andsrc/api/jwt.rs, both directly exercised by the tests above.N/A: behaviour-only change; no feature rows added, removed, or renamed.## Related—N/A: no new user-facing feature; this is an internal transport refactor.axumservers on ephemeral ports, no real network.N/A: no release-cut surface changes; existing client signatures and behaviour are preserved.Closes #NNN— see## Related. Note the two filed issues are not closed by this PR; both need a backend decision.Impact
Runtime/platform. Desktop (Windows/macOS/Linux) and CLI. Behaviour is intended to be unchanged: same transport policy, same error classification, same Sentry grouping. The Windows schannel path is specifically preserved — this is the highest-risk area and the reason the SDK gained
with_http_client.Dependencies. One package added to
Cargo.lock. The SDK shares reqwest 0.12, serde, serde_json, thiserror 2, url, and percent-encoding with the core crate, so no new transitive deps.Compatibility. No RPC surface change, no config change, no migration.
Build.
vendor/tinyhumans-sdkis a new submodule —git submodule update --init vendor/tinyhumans-sdkafter checkout. Unlike the othervendor/crates it has no[patch.crates-io]entry, because it is not published to crates.io; it is a direct path dependency.Blocking dependency. Requires tinyhumansai/sdk#3 to merge first. Until then the submodule pointer references commits on the
feat/openhuman-host-integrationbranch.Build prerequisites on Linux. Unchanged for CI, but a bare dev box needs the usual Tauri set (
libasound2-dev,libxdo-dev,libgtk-3-dev,libwebkit2gtk-4.1-dev,libwayland-dev,libxkbcommon-dev,libsoup-3.0-dev,librsvg2-dev) — pre-existing, not introduced here.Related
PATCHroute the backend never implemented; the Telegram/Discord "thinking" indicator has never updated, and the resulting 404 is misread as "message deleted provider-side"idempotency_keyis sent on every channel message but honoured by neither backend nor SDKauthed_jsononto the SDK's typed namespace methods where a 1:1 model exists (now a readability change, not a transport change, since both share one transport and one classifier); adopt the typedSendMessageRequestonce Channel message idempotency_key is sent but honoured by nobody #5231 is resolved.AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
vendor-tinyhumans-sdkValidation Run
pnpm --filter openhuman-app format:check— N/A: no frontend changes.pnpm typecheck— N/A: no TypeScript changes.cargo test --lib api::— 105 passed, 0 failed. Dependent domains (team,billing,announcements,referral,webhooks,channels,orchestration,credentials) all green.scripts/test-rust-with-mock.sh --test json_rpc_e2e— 115 passed, 0 failed.pnpm rust:checkon both crates + lint + format) passed.cargo fmtclean;GGML_NATIVE=OFF cargo check --manifest-path Cargo.tomlexit 0.app/src-tauri/untouched.Vendored SDK, verified independently:
cargo test142 passed / 0 failed,cargo clippy --all-targets -- -D warningsclean,node scripts/sync-openapi.mjs --checkclean against the deployed spec.Validation Blocked
command:cargo test --lib(full suite) andpnpm test:coverageerror:the full lib suite is not deterministic in this environment. It fails on cleanmaintoo, with a different set each run (main: 3 failures; this branch: 7) — all in tests sharing global state (agent registry, provider mocks, event busLagged). Every failure on this branch passes in isolation, and none touch the backend HTTP path. Two agent-harness tests also overflow the default stack onmainand here alike (RUST_MIN_STACK=33554432works around it). This environment additionally lackedlibasound2-dev,libxdo-dev, and the GTK/wayland Tauri prerequisites; all installed.impact:diff coverage unconfirmed locally; relying on CI for the gate. Focusedapi::tests and the fulljson_rpc_e2esuite pass.Behavior Changes
Parity Contract
BackendOAuthClient's public method signatures are unchanged, so all existing call sites are untouched.authed_jsonstill backs every route not yet migrated.classify_sdk_errorreproducesauthed_json's 401/404/transient/budget branches;rest_tests.rsasserts the SDK-backed path yields the same typedBackendApiErrorvariants and the sameSESSION_EXPIREDsentinel.Duplicate / Superseded PR Handling
Summary by CodeRabbit
Bug Fixes
Reliability
Documentation