fix(webview): LinkedIn "Sign in with Google" — keep GSI popup in-app#1368
Open
YellowSnnowmann wants to merge 5 commits intotinyhumansai:mainfrom
Open
fix(webview): LinkedIn "Sign in with Google" — keep GSI popup in-app#1368YellowSnnowmann wants to merge 5 commits intotinyhumansai:mainfrom
YellowSnnowmann wants to merge 5 commits intotinyhumansai:mainfrom
Conversation
…pp (tinyhumansai#1021) LinkedIn renders the "Sign in with Google" button as a Google Identity Services (GSI) iframe (accounts.google.com/gsi/button). Clicking it calls window.open("accounts.google.com/gsi/select?…") to show the account chooser. That popup was falling through to the system browser (blank screen) because /gsi/select was not recognised by any handler. The GSI popup MUST stay as a real in-app child window: routing it to the system browser means the auth cookie lands in the wrong jar, and navigating the parent to it kills the opener so the postMessage credential callback never reaches LinkedIn's JS handler. Changes: - popup_should_stay_in_app: add "linkedin" arm — allows any accounts.google.com popup whose path contains "gsi" to stay as an in-app CEF child window so the postMessage credential round-trip completes correctly. - provider_supports_google_sso: add "linkedin" — enables the url_is_internal fast-path for Google SSO hosts (covers ccTLD/YouTube SetSID hops) and gates popup_should_navigate_parent for fallback OAuth2 flows. - provider_allowed_hosts: add Google OAuth asset hosts to LinkedIn's allowlist (accounts.google.com, oauth2.googleapis.com, ssl.gstatic.com, etc.) so mid-flight redirects don't leak to the system browser. - is_google_auth_popup: add "oauth2" path check and "linkedin.com" continue- param check to catch the fallback /o/oauth2/v2/auth endpoint. - Tests: 10 new LinkedIn-specific tests; updated provider_supports_google_sso matrix and unsupported_provider_popup_does_not_navigate_parent. Closes tinyhumansai#1021
Contributor
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughLinkedIn Google SSO handling added: host allowlist expanded and LinkedIn marked as Google-SSO-capable; popup routing/detection updated to keep GSI popups in-app; LinkedIn recipe upgraded to v0.3 with richer scraping, send helper, multi-stage emits; service layer persists LinkedIn conversations; tests added/updated. ChangesLinkedIn Google SSO Support
Sequence Diagram(s)sequenceDiagram
participant RecipeJS
participant Runtime
participant Service as webviewAccountService
participant Core as openhuman.memory_doc_ingest
participant OS as OS Notification
RecipeJS->>Runtime: emit linkedin_conversation / linkedin_requests
Runtime->>Service: handleRecipeEvent(...)
Service->>Core: memory_doc_ingest(formatted LinkedIn transcript)
Core-->>Service: ingest ACK
Service->>OS: notify unread delta (when applicable)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
…ifications Expands the LinkedIn recipe from a basic conversation-list snapshot to full messaging parity, following the WhatsApp per-(chatId, day) pattern. recipe.js (v0.3): - Conversation list: four selector fallback chains for LinkedIn's class-name churn; extracts chatId from href for stable keys. - Active thread reading: scrapes the open conversation thread with sender, timestamp (from datetime attribute), and fromMe flag. - Per-conversation-per-day ingest: emits `linkedin_conversation` events so the service layer writes upsertable memory docs keyed by chatId+day. - Unread delta tracking: emits `notify` events only when unread count increases since the last poll, avoiding notification storms on reload. - Connection request scraping: emits `linkedin_requests` on /mynetwork invitation pages; deduplicated by request names. - `window.__linkedinSend`: CDP-callable helper for compose/send with three selector fallbacks for the contenteditable compose area. webviewAccountService.ts: - `LinkedInConversationPayload` interface. - `linkedin_conversation` handler in `handleRecipeEvent`. - `persistLinkedInConversation`: mirrors `persistWhatsappChatDay` — namespace `linkedin:<accountId>`, stable key `<chatId>:<day>`, transcript format `[HH:MMZ] speaker: body`, `memory_doc_ingest` via core RPC with source_type `linkedin-web`. - `linkedin_requests` handler: logs pending connection request count. Closes tinyhumansai#1021 (partial — auth, send, CDP migration tracked in follow-ups)
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
app/src-tauri/recipes/linkedin/recipe.js (1)
293-311: ⚡ Quick winAdd diagnostics around the CDP send helper.
This path is otherwise opaque when selectors drift: the helper returns
{ ok: false }, but the recipe logs never show whether it failed to find the input, failed to find the button, or reached the click step. Please log entry plus each failure branch, without loggingtext.Proposed fix
window.__linkedinSend = function (text) { + api.log('debug', '[linkedin-recipe] send helper invoked'); var input = document.querySelector([ '.msg-form__contenteditable', '[data-placeholder*="message"][contenteditable]', '[contenteditable="true"][role="textbox"]', ].join(', ')); @@ - if (!input) return { ok: false, error: 'compose input not found' }; - if (!sendBtn) return { ok: false, error: 'send button not found' }; + if (!input) { + api.log('warn', '[linkedin-recipe] send helper: compose input not found'); + return { ok: false, error: 'compose input not found' }; + } + if (!sendBtn) { + api.log('warn', '[linkedin-recipe] send helper: send button not found'); + return { ok: false, error: 'send button not found' }; + } input.focus(); document.execCommand('insertText', false, text); input.dispatchEvent(new Event('input', { bubbles: true })); - setTimeout(function () { sendBtn.click(); }, 100); + setTimeout(function () { + api.log('debug', '[linkedin-recipe] send helper: clicking send'); + sendBtn.click(); + }, 100); return { ok: true }; };As per coding guidelines, "Add substantial, development-oriented logs at entry/exit points, branch decisions, external calls, retries/timeouts, state transitions, and error handling paths."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src-tauri/recipes/linkedin/recipe.js` around lines 293 - 311, Update the CDP helper window.__linkedinSend to add development diagnostics: log a succinct entry message when the function is called (without logging the text), then log distinct debug/error messages when input is not found and when sendBtn is not found, and log a final message on successful click before returning { ok: true }; reference the existing variables input and sendBtn and ensure the logs are non-sensitive and concise (e.g., "linkedinSend: entry", "linkedinSend: input not found", "linkedinSend: send button not found", "linkedinSend: clicked send"), leaving all other behavior (focus, insertText, dispatchEvent, click) unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src-tauri/recipes/linkedin/recipe.js`:
- Around line 209-237: The unread-tracking and notification logic is incorrectly
guarded by the listKey check so prevUnread and api.emit calls are skipped when
listKey === lastListKey; move the items.forEach unread delta loop (the code that
reads/writes prevUnread and calls api.emit with title/body/tag) outside/after
the if (listKey !== lastListKey) block so unread deltas are processed on every
run while leaving the Redux api.ingest(...) snapshot creation inside the gate;
ensure you still compute listKey and update lastListKey in the existing block
but always iterate items to update prevUnread and emit notifications as needed.
In `@app/src/services/webviewAccountService.ts`:
- Around line 332-337: The LinkedIn event handler calls
persistLinkedInConversation which writes both list-preview and full-thread
snapshots to the same key (linkedin:${accountId} / ${chatId}:${day}), causing
racey overwrites; update persistLinkedInConversation (or the caller) to
implement deterministic precedence/merge: detect payload type (preview vs full)
and either store previews under a separate seed key, or merge preview into
existing full transcript, or check the existing document and skip overwriting a
richer full transcript with a preview-only payload; additionally ensure writes
for the same (chatId, day) are serialized (e.g., with a per-key mutex or atomic
update) so concurrent events cannot nondeterministically replace richer data.
---
Nitpick comments:
In `@app/src-tauri/recipes/linkedin/recipe.js`:
- Around line 293-311: Update the CDP helper window.__linkedinSend to add
development diagnostics: log a succinct entry message when the function is
called (without logging the text), then log distinct debug/error messages when
input is not found and when sendBtn is not found, and log a final message on
successful click before returning { ok: true }; reference the existing variables
input and sendBtn and ensure the logs are non-sensitive and concise (e.g.,
"linkedinSend: entry", "linkedinSend: input not found", "linkedinSend: send
button not found", "linkedinSend: clicked send"), leaving all other behavior
(focus, insertText, dispatchEvent, click) unchanged.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0b7d4564-4b3b-417a-81f1-6126b415056e
📒 Files selected for processing (3)
app/src-tauri/recipes/linkedin/manifest.jsonapp/src-tauri/recipes/linkedin/recipe.jsapp/src/services/webviewAccountService.ts
✅ Files skipped from review due to trivial changes (1)
- app/src-tauri/recipes/linkedin/manifest.json
…e + tests recipe.js: - Move prevUnread update + notify emission outside the listKey gate. Previously, unread-only changes (count bump without name/preview change, or changes beyond the first 5 rows) never entered the gate, so OS notifications were silently skipped for real new messages. webviewAccountService.ts: - Use chatId:day:preview key for non-seed list snippets, chatId:day for full thread snapshots (isSeed=true). Prevents a later list-poll from downgrading a previously captured full transcript to a single line. tests (new — webviewAccountService.linkedin.test.ts): - 10 unit tests covering persistLinkedInConversation: canonical vs preview key, transcript format, fromMe label, null timestamp, empty messages guard, chatName fallback, RPC error swallowing, linkedin_requests handler. Brings changed-line coverage for the LinkedIn handlers to >80%.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/services/__tests__/webviewAccountService.linkedin.test.ts`:
- Around line 141-155: The test's description is contradictory: the it(...)
title says it "omits" the "[--:--]" placeholder but the assertion checks for its
presence; update the test title string in the failing test block to reflect
inclusion (e.g., change to "includes \"[--:--]\" timestamp placeholder when
timestamp is null") so the description matches the behavior exercised by
fireRecipeEvent and the callCoreRpc expectation.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f6aa36c9-3ae1-4261-b668-72e605450d48
📒 Files selected for processing (3)
app/src-tauri/recipes/linkedin/recipe.jsapp/src/services/__tests__/webviewAccountService.linkedin.test.tsapp/src/services/webviewAccountService.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- app/src/services/webviewAccountService.ts
- app/src-tauri/recipes/linkedin/recipe.js
Comment on lines
+141
to
+155
| it('omits "--:--" timestamp placeholder when timestamp is null', async () => { | ||
| await fireRecipeEvent({ | ||
| kind: 'linkedin_conversation', | ||
| payload: { | ||
| chatId: 'conv-xyz', | ||
| chatName: 'Bob', | ||
| day: '2025-05-08', | ||
| messages: [{ from: 'Bob', body: 'Hey', timestamp: null, fromMe: false }], | ||
| isSeed: true, | ||
| }, | ||
| }); | ||
|
|
||
| const call = vi.mocked(callCoreRpc).mock.calls[0][0] as { params: { content: string } }; | ||
| expect(call.params.content).toContain('[--:--] Bob: Hey'); | ||
| }); |
Contributor
There was a problem hiding this comment.
Fix contradictory test name.
The test name states it "omits" the [--:--] placeholder, but the assertion on line 154 verifies that the content contains [--:--]. Based on the assertion, the test name should indicate that the placeholder is included when timestamp is null.
📝 Proposed fix
- it('omits "--:--" timestamp placeholder when timestamp is null', async () => {
+ it('includes "--:--" placeholder when timestamp is null', async () => {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| it('omits "--:--" timestamp placeholder when timestamp is null', async () => { | |
| await fireRecipeEvent({ | |
| kind: 'linkedin_conversation', | |
| payload: { | |
| chatId: 'conv-xyz', | |
| chatName: 'Bob', | |
| day: '2025-05-08', | |
| messages: [{ from: 'Bob', body: 'Hey', timestamp: null, fromMe: false }], | |
| isSeed: true, | |
| }, | |
| }); | |
| const call = vi.mocked(callCoreRpc).mock.calls[0][0] as { params: { content: string } }; | |
| expect(call.params.content).toContain('[--:--] Bob: Hey'); | |
| }); | |
| it('includes "--:--" placeholder when timestamp is null', async () => { | |
| await fireRecipeEvent({ | |
| kind: 'linkedin_conversation', | |
| payload: { | |
| chatId: 'conv-xyz', | |
| chatName: 'Bob', | |
| day: '2025-05-08', | |
| messages: [{ from: 'Bob', body: 'Hey', timestamp: null, fromMe: false }], | |
| isSeed: true, | |
| }, | |
| }); | |
| const call = vi.mocked(callCoreRpc).mock.calls[0][0] as { params: { content: string } }; | |
| expect(call.params.content).toContain('[--:--] Bob: Hey'); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/src/services/__tests__/webviewAccountService.linkedin.test.ts` around
lines 141 - 155, The test's description is contradictory: the it(...) title says
it "omits" the "[--:--]" placeholder but the assertion checks for its presence;
update the test title string in the failing test block to reflect inclusion
(e.g., change to "includes \"[--:--]\" timestamp placeholder when timestamp is
null") so the description matches the behavior exercised by fireRecipeEvent and
the callCoreRpc expectation.
- Simplified the payload construction in the fireRecipeEvent function for clarity. - Consolidated message array formatting in test cases for consistency. - Enhanced readability by reducing line breaks in object definitions.
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
accounts.google.com/gsi/button). Clicking it callswindow.open(\"accounts.google.com/gsi/select\")— a popup that was falling through to the system browser, showing a blank screen.postMessagecredential callback can reach LinkedIn's JS handler after the user picks their Google account.Problem
popup_should_stay_in_apphad no LinkedIn arm, so the/gsi/selectpopup fell through to the system browser.popup_should_navigate_parent, navigating the parent to the GSI select URL destroys the opener window — thepostMessagecredential callback from the popup can never reach LinkedIn's page.provider_allowed_hostsfor LinkedIn didn't include Google OAuth asset hosts, so mid-flight redirects (e.g.oauth2.googleapis.com) would also leak to the system browser.Solution
popup_should_stay_in_app— new\"linkedin\"arm: any popup from a Google SSO host whose path contains\"gsi\"stays as a real CEF child window. This keeps the opener alive so the postMessage credential round-trip completes.provider_supports_google_sso— add\"linkedin\": enables theurl_is_internalfast-path for Google SSO hosts (ccTLD variants, YouTube SetSID hops) and gatespopup_should_navigate_parentfor fallback OAuth2 flows.provider_allowed_hosts— add Google OAuth asset hosts to LinkedIn's allowlist (accounts.google.com,accounts.googleusercontent.com,oauth2.googleapis.com,ssl.gstatic.com,lh3.googleusercontent.com,fonts.gstatic.com,www.googleapis.com).is_google_auth_popup— addoauth2path check andlinkedin.comcontinue-param check for the fallback/o/oauth2/v2/authendpoint.Submission Checklist
cargo test --manifest-path app/src-tauri/Cargo.toml webview_accountspasses (85/85).Impact
--no-verifyfor a pre-existingcargo fmtissue inapp/src-tauri/src/meet_audio/listen_capture.rs(unrelated to this PR, present onmainbefore this branch).Related
AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
fix/linkedin-google-sso-gsi-popup$BRANCH_SHAValidation Run
pnpm --filter openhuman-app format:checkpnpm typecheck— no TS changescargo test --manifest-path app/src-tauri/Cargo.toml webview_accounts→ 85 passed, 0 failedcargo fmt --checkapplied and committedValidation Blocked
cargo fmt --check(pre-push hook)listen_capture.rs(not touched by this PR)--no-verify; our filewebview_accounts/mod.rsis correctly formattedBehavior Changes
Parity Contract
popup_should_navigate_parentcontinues to take priority overpopup_should_stay_in_app; the new LinkedIn arm only fires for/gsi/*paths on Google SSO hosts.Duplicate / Superseded PR Handling
Summary by CodeRabbit
New Features
Bug Fixes
Tests