feat(welcome): split onboarding tool + detect gmail/webview logins#894
Merged
senamakel merged 4 commits intotinyhumansai:mainfrom Apr 25, 2026
Merged
Conversation
Split `complete_onboarding` (previously gated on an `action` param) into
two single-purpose tools so the welcome agent can reason about them
independently:
- `check_onboarding_status` — read-only JSON snapshot, no args.
- `complete_onboarding` — finalizer only, no args. Still enforces the
engagement criteria (>=3 exchanges OR >=1 Composio connection) and
rejects premature calls.
Shared state and snapshot helpers moved to a new `onboarding_status`
module so both tools (and `welcome_proactive`) agree on one code path.
Extended the snapshot so the agent can actually see what's connected:
- `composio_connected_toolkits: [..]` — the list of OAuth-authorised
toolkits (was computed for the ready-gate and thrown away).
- `webview_logins: {gmail, whatsapp, telegram, slack, discord, linkedin,
zoom, google_messages}` — per-provider bool derived from the shared
CEF cookie store.
Webview login detection is a new `openhuman::webview_accounts` module:
Tauri exposes the shared CEF cookies SQLite via `OPENHUMAN_CEF_COOKIES_DB`,
core opens it read-only (`mode=ro&immutable=1&nolock=1` so CEF's lock
doesn't block us) and probes for known session-cookie names under the
expected host suffix. All failure modes (missing env, locked DB, schema
drift) degrade silently to "all false" — the snapshot must always build.
Welcome `prompt.md` updated to reference both tools by name, read the
new snapshot fields before pitching, and explicitly forbid re-offering
a toolkit that's already in `composio_connected_toolkits` or logged in
per `webview_logins`.
Contributor
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 3 minutes and 36 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a read-only Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Agent as Welcome Agent
participant Check as check_onboarding_status
participant Status as onboarding_status
participant Webview as webview_accounts
participant Config as Config/State
participant Complete as complete_onboarding
Agent->>Check: request snapshot
Check->>Config: load config/state
Check->>Status: compute_state(config)
Status->>Config: read exchange count, composio connections, JWT
Check->>Webview: detect_webview_logins(env: OPENHUMAN_CEF_COOKIES_DB)
Webview->>Webview: open cookies DB (readonly) & query providers
Webview-->>Check: webview_logins JSON
Status->>Status: build_status_snapshot(..., webview_logins)
Status-->>Check: snapshot JSON
Check-->>Agent: return snapshot
Agent->>Agent: decide actions/pitches
Agent->>Complete: call when snapshot.ready_to_complete == true
Complete->>Status: compute_state(config)
Status-->>Complete: onboarding state (ready/not-ready)
Complete-->>Agent: success / not-ready error
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/openhuman/agent/agents/welcome/prompt.md (1)
39-39: Minor: Consider capitalizing "LinkedIn".Static analysis flagged that
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/openhuman/agent/agents/welcome/prompt.md` at line 39, Minor documentation polish: in the description of the webview_logins field, update the provider name "linkedin" to the proper-cased "LinkedIn" (inside the providers list for webview_logins) so the line reads: **`webview_logins`** — bools per provider (`gmail`, `whatsapp`, `telegram`, `slack`, `discord`, `LinkedIn`, `zoom`, `google_messages`).src/openhuman/tools/impl/agent/onboarding_status.rs (1)
135-177: Consider extracting channel detection into a helper.The channel detection logic (14
ifblocks checkingis_some()) is straightforward but verbose. As a future refactor, consider a macro or a struct-to-channel-list helper to reduce repetition. This is purely optional and doesn't affect correctness.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/openhuman/tools/impl/agent/onboarding_status.rs` around lines 135 - 177, The repeated is_some() checks populating channels_connected from config.channels_config are verbose; extract this into a helper to iterate known channel fields and push their string names when present. Create a function (e.g., collect_connected_channels or impl ChannelsConfig::connected_names) that takes &config.channels_config and returns Vec<&'static str>, move the series of if checks into that helper, and replace the inlined block in onboarding_status.rs to call that function and assign channels_connected to its result (referencing channels_connected and config.channels_config to find the call site).src/openhuman/webview_accounts/mod.rs (1)
276-294: Consider test isolation for env var manipulation.The tests manipulate
OPENHUMAN_CEF_COOKIES_DBviastd::env::set_var/remove_var. In Rust, environment variable access is not thread-safe, so these tests could interfere with each other if run in parallel. Consider using#[serial]from theserial_testcrate if you observe flaky test failures.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/openhuman/webview_accounts/mod.rs` around lines 276 - 294, The tests manipulate the environment variable COOKIES_DB_ENV (via std::env::set_var/remove_var) which is not thread-safe; annotate the tests (e.g., missing_env_returns_all_false and detects_gmail_via_sid_cookie) with a serialization attribute such as #[serial] from the serial_test crate (add the crate to dev-dependencies if missing and import serial_test::serial) so detect_webview_logins-based tests run one at a time and avoid cross-test env var interference.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src-tauri/src/lib.rs`:
- Around line 697-709: The code currently logs the full cookies DB path and
leaves OPENHUMAN_CEF_COOKIES_DB unchanged on cache-dir resolution failure;
update the app.path().app_cache_dir() branch so the success path does NOT log
the absolute cookies_db (remove cookies_db.display() from log::info or redact
it) and the failure branch unsets/clears the OPENHUMAN_CEF_COOKIES_DB
environment variable (e.g., remove the var or set to empty) instead of leaving a
stale value; refer to the cookies_db variable, the OPENHUMAN_CEF_COOKIES_DB env
var, and the log::info / log::warn calls to locate where to change this.
---
Nitpick comments:
In `@src/openhuman/agent/agents/welcome/prompt.md`:
- Line 39: Minor documentation polish: in the description of the webview_logins
field, update the provider name "linkedin" to the proper-cased "LinkedIn"
(inside the providers list for webview_logins) so the line reads:
**`webview_logins`** — bools per provider (`gmail`, `whatsapp`, `telegram`,
`slack`, `discord`, `LinkedIn`, `zoom`, `google_messages`).
In `@src/openhuman/tools/impl/agent/onboarding_status.rs`:
- Around line 135-177: The repeated is_some() checks populating
channels_connected from config.channels_config are verbose; extract this into a
helper to iterate known channel fields and push their string names when present.
Create a function (e.g., collect_connected_channels or impl
ChannelsConfig::connected_names) that takes &config.channels_config and returns
Vec<&'static str>, move the series of if checks into that helper, and replace
the inlined block in onboarding_status.rs to call that function and assign
channels_connected to its result (referencing channels_connected and
config.channels_config to find the call site).
In `@src/openhuman/webview_accounts/mod.rs`:
- Around line 276-294: The tests manipulate the environment variable
COOKIES_DB_ENV (via std::env::set_var/remove_var) which is not thread-safe;
annotate the tests (e.g., missing_env_returns_all_false and
detects_gmail_via_sid_cookie) with a serialization attribute such as #[serial]
from the serial_test crate (add the crate to dev-dependencies if missing and
import serial_test::serial) so detect_webview_logins-based tests run one at a
time and avoid cross-test env var interference.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 61dbdcfd-eaae-4e02-9626-bb3f381a390d
📒 Files selected for processing (16)
app/src-tauri/src/lib.rssrc/openhuman/agent/agents/loader.rssrc/openhuman/agent/agents/welcome/agent.tomlsrc/openhuman/agent/agents/welcome/prompt.mdsrc/openhuman/agent/welcome_proactive.rssrc/openhuman/channels/providers/web.rssrc/openhuman/channels/runtime/dispatch.rssrc/openhuman/config/ops.rssrc/openhuman/config/schema/types.rssrc/openhuman/mod.rssrc/openhuman/tools/impl/agent/check_onboarding_status.rssrc/openhuman/tools/impl/agent/complete_onboarding.rssrc/openhuman/tools/impl/agent/mod.rssrc/openhuman/tools/impl/agent/onboarding_status.rssrc/openhuman/tools/ops.rssrc/openhuman/webview_accounts/mod.rs
- Don't log the absolute CEF cookies DB path; demote to debug with no path interpolation. Avoids leaking user-path PII. - Clear `OPENHUMAN_CEF_COOKIES_DB` when `app_cache_dir()` resolution fails so a stale value (from a previous run or the parent shell) can't make core probe an unrelated DB. - Serialise the env-mutating webview_accounts tests behind a module- local mutex; `std::env::set_var` is process-global and parallel tests can race on it. - Capitalise "LinkedIn" in the welcome prompt's webview_logins list.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/openhuman/agent/agents/welcome/prompt.md`:
- Around line 39-41: The prompt text uses the wrong casing for the LinkedIn
snapshot key; update the prompt fragment that mentions webview_logins and
composio_connected_toolkits to use the actual key "linkedin" (lowercase) instead
of "LinkedIn" so it matches the webview_logins schema and allows the agent to
detect an existing LinkedIn session (look for the webview_logins,
composio_connected_toolkits, and the Gmail/LinkedIn pairing logic in the welcome
prompt).
In `@src/openhuman/webview_accounts/mod.rs`:
- Around line 124-246: Extract the operational implementation from this mod.rs
into a new sibling file (e.g., ops.rs) and re-export the public API from mod.rs;
specifically move the functions detect_webview_logins,
provider_has_session_cookie, escape_like and any helper logic that touches
Connection/OpenFlags/cookies_db_path and PROVIDERS usage into the new file, keep
mod.rs only for exports (pub use
crate::openhuman::webview_accounts::ops::{detect_webview_logins,
provider_has_session_cookie, escape_like} or similar), update module
declarations (mod ops;) and adjust visibility (pub(crate) / pub) so tests and
callers still compile, and run cargo build/tests to ensure no missing imports or
visibility issues.
- Around line 137-160: The tracing::debug call inside the
Connection::open_with_flags Err branch currently logs the absolute cookies DB
path via path.display(), which can expose PII; change that debug call to remove
path.display() and instead log a non-sensitive identifier such as the
environment key or a constant like "cookies_db" and the error class (err) only —
update the tracing::debug invocation in the error arm of
Connection::open_with_flags (the block that currently returns
Value::Object(out)) to omit the raw path field and replace it with a redacted
identifier while keeping the existing error = %err and message.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2c169439-e75b-43c8-9ff4-1a038fb4e37c
📒 Files selected for processing (3)
app/src-tauri/src/lib.rssrc/openhuman/agent/agents/welcome/prompt.mdsrc/openhuman/webview_accounts/mod.rs
- Move webview_accounts detector + tests into a sibling `ops.rs` so `mod.rs` stays the export-focused file the repo conventions ask for. - Drop `path.display()` from the two debug logs in the cookies-DB open/exists branches; log the env key instead so usernames in /Users/<name>/... paths can't end up in trace output. - Restore lowercase `linkedin` in the welcome prompt — the JSON key exported by `webview_logins` is lowercase, capitalising it would desync the agent's pattern match from the actual snapshot field.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/openhuman/webview_accounts/ops.rs (1)
106-160: Add explicit start/end probe logs for the detector.You already log error paths and per-provider outcomes; adding one entry log and one completion summary (e.g., providers checked, logged-in count) would better satisfy the repo’s logging policy for external DB probes.
As per coding guidelines: "Add substantial development-oriented logging at entry/exit points, branch decisions, external calls, retries/timeouts, state transitions, and error paths" and "Rust debug logging must use stable grep-friendly prefixes (e.g.,
[domain],[rpc],[ui-flow]) and correlation fields (request IDs, method names, entity IDs)."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/openhuman/webview_accounts/ops.rs` around lines 106 - 160, Add a debug entry log at the start of detect_webview_logins (include env = COOKIES_DB_ENV and a stable prefix like "[webview_accounts] detect_start") and an exit/summary debug log before returning Value::Object(out) that reports how many providers from PROVIDERS were checked and the count of logged-in providers (include a stable prefix like "[webview_accounts] detect_complete" and correlation fields such as the number of providers and logged_in_count). Place the entry log immediately after creating the out map and the summary log after the loop that calls provider_has_session_cookie(&conn, p); ensure both logs follow the project's grep-friendly prefix style and include the COOKIES_DB_ENV field where appropriate.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/openhuman/webview_accounts/ops.rs`:
- Around line 134-138: The SQLite file URI currently embeds path.display() raw
into uri, which breaks on spaces and reserved chars; update the code that builds
uri (the let uri = ... line used before Connection::open_with_flags) to first
convert backslashes to forward slashes and percent-encode the filesystem path
using the urlencoding crate (e.g., call urlencoding::encode on the normalized
path string) and then interpolate that encoded path into the
"file:{encoded}?mode=ro&immutable=1&nolock=1" URI before calling
Connection::open_with_flags with OpenFlags::SQLITE_OPEN_READ_ONLY |
OpenFlags::SQLITE_OPEN_URI so SQLite parses it correctly.
---
Nitpick comments:
In `@src/openhuman/webview_accounts/ops.rs`:
- Around line 106-160: Add a debug entry log at the start of
detect_webview_logins (include env = COOKIES_DB_ENV and a stable prefix like
"[webview_accounts] detect_start") and an exit/summary debug log before
returning Value::Object(out) that reports how many providers from PROVIDERS were
checked and the count of logged-in providers (include a stable prefix like
"[webview_accounts] detect_complete" and correlation fields such as the number
of providers and logged_in_count). Place the entry log immediately after
creating the out map and the summary log after the loop that calls
provider_has_session_cookie(&conn, p); ensure both logs follow the project's
grep-friendly prefix style and include the COOKIES_DB_ENV field where
appropriate.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 78289c64-1929-4b6e-a476-7bdadad55a01
📒 Files selected for processing (3)
src/openhuman/agent/agents/welcome/prompt.mdsrc/openhuman/webview_accounts/mod.rssrc/openhuman/webview_accounts/ops.rs
macOS usernames often contain a space (`/Users/John Doe/...`) and Windows paths use backslashes — without encoding, SQLite's URI parser rejects the `file:` URI and the open silently fails, so the welcome snapshot would report every webview as logged_out even when cookies exist. Added `sqlite_uri_path()` that swaps `\` for `/` and percent-encodes the path via the existing `urlencoding` dep, leaving `/` separators literal (SQLite expects them unencoded). New regression test stages a cookies DB under a directory with a space and asserts gmail still detects.
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
complete_onboardinginto two single-purpose tools:check_onboarding_status(read-only, no args) andcomplete_onboarding(finalizer, no args). Shared helpers live in a newonboarding_statusmodule so both tools andwelcome_proactiveuse one code path.composio_connected_toolkits(which OAuth toolkits are actually authorised — e.g.["gmail"]) andwebview_logins(per-provider bool for gmail, whatsapp, telegram, slack, discord, linkedin, zoom, google_messages).openhuman::webview_accounts: Tauri exports the shared CEF cookies SQLite viaOPENHUMAN_CEF_COOKIES_DB, core opens it read-only (mode=ro&immutable=1&nolock=1) and probes for known session-cookie names per host suffix. Fails closed (all-false) on any error — the welcome snapshot must always build.agent.toml+prompt.mdto use both tools by name, read the new snapshot fields before pitching, and never re-offer an already-connected toolkit.Why
The old single tool gated two distinct operations (inspect + finalize) on an
actionparam, which the model occasionally confused. More importantly the snapshot only exposedintegrations.composio: bool(the enabled flag), not which toolkits were actually connected, so the welcome agent would re-pitch Gmail even when the user had just authorised it. Webview logins (the user being signed in tomail.google.comorweb.whatsapp.cominside the embedded browser) were invisible to the agent entirely.Test plan
cargo check— core + Tauri shell clean.cargo test --libforwebview_accounts,onboarding_status,check_onboarding_status,complete_onboarding,welcome— all pass (36 tests across these modules).chat_onboarding_completed=false, sign in, chat with welcome agent, confirm it callscheck_onboarding_statusfirst and reflects gmail connection (Composio and/or webview) before offering.complete_onboardingrejects whenready_to_complete=falsewith the existing error wording.Summary by CodeRabbit
New Features
Refactor