Uh oh!
There was an error while loading. Please reload this page.
Carry the backend base URL into the module, so proxied Composio can address it - #103
Conversation
…ddress it v1.8.0 gave the module a way to obtain a session bearer, which was necessary for proxied Composio and not sufficient: `effective_backend_api_url` still answered an empty string, so the request built against an empty base and failed inside the HTTP client with `builder error` — a message that names neither the cause nor the field. Found by routing OpenHuman's Composio sync onto the driver and watching the run get three provider actions in before dying on the transport. The bearer had reached it; the address had not. ## A field, not a seam The bearer is a seam because it is a credential that expires and gets refreshed, so any snapshot of it goes stale. A base URL is the opposite kind of value: routing configuration that changes when an operator points the host at a different backend, which is a restart rather than a mid-session event. Carrying it in `ModuleConfig` is both simpler and more honest about what it is. ## No default is substituted An empty URL stays empty and fails in the HTTP client. Guessing one here would send a user's memory at whichever backend this crate happened to hard-code — including, for a self-hosted operator, a backend they do not control. A bad error message is a much smaller problem than that, so the field is documented as required for proxied mode rather than defaulted into looking optional. `#[serde(default)]`, so a host that predates the field still loads.
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: Warning Your free Security trial is over. An organization admin can activate billing to continue. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Uh oh!
There was an error while loading. Please reload this page.
How this change flows4 changed behaviours across 7 relationships. 4 surrounding behaviours are shown (60 graph nodes walked). 45 further behaviours left out to keep the diagram readable. flowchart LR
n0["ModuleConfig<br/>changed"]:::changed
n1["EngineRuntimeConfig<br/>changed"]:::changed
n2["runtime_config<br/>changed"]:::changed
n3["provider_config<br/>changed"]:::changed
n4["install"]:::impacted
n5["test_config"]:::impacted
n6["test_opener"]:::impacted
n7["join"]:::impacted
n2 -->|uses| n1
n3 -->|uses| n1
n3 -->|calls| n7
n3 -->|tests| n7
n4 -->|uses| n0
n5 -->|uses| n0
n6 -->|uses| n0
classDef changed fill:#0d4429,stroke:#238636,color:#e6edf3
classDef impacted fill:#161b22,stroke:#6e7681,color:#c9d1d9
classDef flagged fill:#5a1e02,stroke:#d93f0b,color:#ffffff
classDef blocking fill:#67060c,stroke:#f85149,color:#ffffff
Green: changed behaviour. Grey: surrounding behaviour. Arrows name the call, use, implementation, or test relationship. Orange: has findings. Red: has a finding that blocks the merge. |
Follow-up to #102, and the last thing openhuman#5560 needs.
What #102 missed
#102 gave the module a session bearer so proxied Composio could authenticate. Necessary, not sufficient:
EngineRuntimeConfig::effective_backend_api_urlstill answeredString::new(), so the request built against an empty base and died inside the HTTP client with:Found by routing OpenHuman's Composio sync onto the driver and watching a real run — three provider actions in before the transport failed. The bearer had reached it; the address had not.
actions_called=3is what said the credential path was fixed and something after it wasn't.Why a field and not another seam member
The bearer is a seam because it is a credential that expires and gets refreshed — any snapshot goes stale, which is why #102 argued for asking per call.
A base URL is the opposite kind of value: routing configuration that changes when an operator points the host at a different backend. That is a restart, not a mid-session event.
ModuleConfigis the right place for it, and treating the two identically would have been cargo-culting the shape of the previous fix rather than reasoning about the value.No default is substituted
An empty URL stays empty and fails in the HTTP client rather than falling back to a hard-coded backend. Guessing would send a user's memory at whichever host this crate happened to name — including, for a self-hosted operator, one they do not control. A poor error message is a far smaller problem than a wrong destination, so the field is documented as required for proxied mode rather than defaulted into looking optional.
#[serde(default)], so a host predating the field still loads and simply cannot use proxied mode — which is exactly where it was before.Validation
cargo check --workspace --all-targetsclean in both workspaces;cargo test --workspace2034 passing, module lane 70 passing; clippy-D warningsclean on root and module;cargo fmtclean on both;cargo doc --no-deps --all-featuresclean under-D warnings.